feat(rfc-008): P4d ESC — gates block ONLY repo-source writes (R1/R2/R3)#409
Merged
Merged
Conversation
…eckpoint-gate parity Extract the 'is this a gated repo-source write?' predicate out of checkpoint-gate.sh into a new shared lib plugins/claude-code/hooks/lib/repo-source.sh (Rule 14: ONE definition, to be adopted by plan-gate.sh in ESC-S2). checkpoint-gate's _tool_call_targets_repo_source() becomes a thin wrapper over the lib's _tool_targets_repo_source_shared(), retaining only the checkpoint-gate-LOCAL _path_verdict_downgrades arming bypass (deliberately NOT shared). The lib adds two carve-outs vs the original: explicit .episodic-memory/** (was caught only by gitignore) and the new docs/plans/** allow (intended, REQ-10) — so plan files are never gated (locked R1). Fail-closed preserved: missing lib now in the lib-presence guard; empty path -> gate. Tests: new tests/test-enforcement-scope.mjs t_checkpoint_parity 3/3 (real deployed gate via mock install); checkpoint-gate regression 373/0; plan-gate 36/0 (untouched). Code review (negative-scenario-reviewer): ACCEPT, no blocker; 9/9 canonicalizer differential match, F1 fail-closed scenario run. Reply episode 20260620-074612. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… classifier redirect target plan-gate.sh: stop blanket-blocking every Write/Bash while a plan-approval marker is pending. Delegate to ESC-S1's shared predicate (lib/repo-source.sh) so ONLY a repo-source write gates — episodes, reads, NEW plan files (docs/plans), carve-outs, and off-repo writes are never blocked (locked R1/R2/R3). read_only Bash and own-session plan-marker rm/touch still exit 0 early (BLOCKER-B1 cross-session narrowing preserved). Relative Bash redirect TARGET normalized vs CWD; fail-closed lib guard adds repo-source.sh; block message rewritten. command-classifier.sh: the 4 shell-redirect emit arms now populate the TARGET field (REQ-5) instead of an empty middle field, so plan-gate can localize the redirect destination. F2: >1 non-marker redirect clears the (ambiguous) target so plan-gate gates conservatively rather than last-target-wins. lib/repo-source.sh: F1 fix (from S2 code review) — a '..'-traversal path skips the raw-prefix short-circuit so off-repo '..'-relative writes are permitted (R3), matching the absolute-collapsed verdict; the symlink-OUT class-3 defense (no-'..' paths) is preserved. checkpoint-gate parity intact (373/0). Tests: tests/test-enforcement-scope.mjs 16/16 on the real deployed gates (mock install + runHook) — R1/R2/R3 end-to-end incl. F1 regression + traversal-back-into -repo still blocks. Regression: checkpoint-gate 373/0, plan-gate 36/0, classifier 430/0. Review (negative-scenario-reviewer): ACCEPT-WITH-MODIFICATION, no fail-OPEN; F1 fixed inline, F2 deferred to #405 (OD-3/NF-2 residual), F3 NIT. Reply 20260620-080240. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…_global_touch (REQ-11) Step 3.1: add `node tests/test-enforcement-scope.mjs` to plan-marker-validate.yml (additive, grouped after the P12 global-clean guardrail). Add t_no_global_touch (REQ-11 / P12): installs via --install-enforcement (same path the suite drives) and asserts hookCodeFilesInGlobalScope(home) === [] — the substrate stays hook-free in global scope; enforcement lands per-project only. Suite: 17 passed, 0 failed against the real deployed gate hooks (runHook). Step 3.2 global deploy + 3.3 deploy-audit deferred to the post-merge moment per the deploy-after-merge discipline (ESC still on its feature branch). Step 3.4: OD-1 stop-gate stale-marker hygiene filed as #408 (5-field defer). Classifier-marker friction tracked separately as #407 (off ESC critical path). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 tasks
lantiscooperdev
left a comment
lantiscooperdev
left a comment
Collaborator
There was a problem hiding this comment.
Bot review — PR-level adversarial pass (not an approval)
Automated pre-merge review via negative-scenario-reviewer on the combined main...HEAD diff (the PR-level layer, distinct from the per-slice S1/S2 reviews). Every claim was reproduced against the REAL deployed gate through the mock-project harness (runHook), not code-reading. Baseline node tests/test-enforcement-scope.mjs = 17/17 green.
Verdict: ACCEPT-WITH-FU — no blockers
What was confirmed to hold (no fail-open found)
Walked every classifier label arm (read_only, nonsrc_write, shared_write, marker_write, unsafe_complex, push_or_pr_create) through both gates:
read_only→ exit 0;nonsrc_write→ allow (writes no tracked content);shared_writeredirect → target localized, repo → BLOCK / off-repo → ALLOW / empty-or-ambiguous → BLOCK;marker_write→ own-session narrowing preserved;unsafe_complex/push_or_pr_createempty-target → BLOCK.- All 4 S3 redirect arms (readonly_cmd / echo / pkg_install / dir_cmd) integrate correctly with the S1/S2 shared predicate: repo → BLOCK, off-repo → ALLOW.
- Fail-closed on missing
repo-source.sh(both gates), garbageenforce-config.json, and empty path.active:falsecannot silence the structural lib-missing block.set -esafe at every call site. - R3: symlink-IN → BLOCK,
..-back-into-repo → BLOCK,..-escape //tmp//var/tmp→ ALLOW. - P12: global-clean intact (
t_no_global_touch).
The combined diff correctly inverts the gate default to "allow unless repo-source" per R1/R2/R3 and keeps the substrate hook-free per P12.
Findings — 3 residuals, none blocking, all dispositioned
| Finding | Severity | Verdict | Disposition |
|---|---|---|---|
F-MULTIREDIR — all-off-repo multi-redirect (echo x > /tmp/a > /tmp/b) over-blocks under pending plan |
MINOR (R3, fail-CLOSED direction) | known residual | already tracked → #405 (§17 OD-3/NF-2) |
F-CLASS — : > scripts/x.mjs and sed -i '' … scripts/x.mjs mislabeled read_only → repo-source write allowed |
NIT (R2) | pre-existing, reproduced identically on main — NOT an ESC regression |
→ #410 (classifier fix) |
F-CWDNORM — plan-gate (.cwd) vs checkpoint-gate (tool_input.cwd) relative-path normalizer divergence |
NIT (Rule-14) | NEEDS-EVIDENCE — could not repro (Claude Code sends absolute paths) | → #411 (resolve evidence before touching) |
Why this is mergeable
No BLOCKER/MAJOR. The two non-residual findings are either pre-existing (F-CLASS repros on main, so it's not introduced here) or unreproducible (F-CWDNORM). The one R3 residual (F-MULTIREDIR) was accepted at plan time and is fail-closed/recoverable. All three carry tracking issues with runHook-based acceptance criteria.
Full transcripts + invariant table: reply episode 20260620-085519-accept-with-fu-rfc-008-p4d-esc-pr-409-pr-8cb9.
Recommend merge after human approval.
…e probe targets (CI regression) ESC-S2 made repo-source.sh a REQUIRED lib for plan-gate.sh, but test-plan-marker-cross-session.mjs staged a hardcoded lib list that omitted it. plan-gate.sh's lib-missing guard then blocked EVERY call — flipping X1/X12 (expect-allow) red while X2/X3 (expect-block) passed spuriously off the block. Two-part fix: - mkTmpRepo now stages the WHOLE lib dir (readdir, not a hardcoded list) so a gate gaining a new required lib can't silently re-break this harness. - X1/X2/X3 use a repo-source target (scripts/x.mjs) instead of off-repo /tmp/x: under ESC R3 an off-repo write is correctly ALLOWED, so the block/allow probes must target repo source. X9 keeps /tmp/x on purpose (F14 fail-closed fires before the path check — off-repo makes it a stronger test). Local: node tests/test-plan-marker-cross-session.mjs → 30 passed, 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…synchronize) No code change — the prior push's PR-synchronize webhook was dropped, so the pull_request merge ref (refs/pull/409/merge) was stale and CI ran the pre-fix tree. This empty commit forces a fresh synchronize + merge-ref recompute so CI executes the harness fix in 8be1085. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lantiscooperdev
approved these changes
Jun 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RFC-008 P4d enforcement-scope correction (ESC): the PreToolUse gates (
plan-gate.sh,checkpoint-gate.sh) now block only repo-source writes while a plan/checkpoint is pending. Episodes, plain reads, new plan files, and any write outside the repo are no longer gated.Closes the locked requirement set (inviolable — every reviewer confirms these hold):
Plus Principle 12: the global memory substrate stays hook-free; enforcement is per-project.
Slices
89854balib/repo-source.sh;checkpoint-gate.shadopts it as a thin wrapper (full parity preserved)1bf968aplan-gate.shgates only repo-source writes;command-classifier.shemits the redirect/write TARGET so off-repo redirects are localized and allowed4b235cft_no_global_touch(REQ-11)Design
_path_is_repo_source+_tool_targets_repo_source_sharedlive once inlib/repo-source.sh, sourced by both gates. Carve-outs:.episodic-memory/,.checkpoints/,.review-store/,.git/,docs/plans/, and any gitignored path → ALLOW. Off-repo → ALLOW. Empty path → GATE (fail-closed). Else → GATE...-escape fix (F1, caught in S2 review): relative..-traversal paths skip the raw-prefix short-circuit and canonicalize first, soecho hi > ../escape.txtis correctly allowed (off-repo) whileecho x > docs/../scripts/evil.mjsstill blocks (resolves back into repo) — no fail-open.shared_writeredirect arms now emit the redirect target; the gate runs the repo-source predicate on it. Multi-redirect / ambiguous-target → cleared → fail-closed gate (F2 / §17 OD-3 / NF-2 residual).Test evidence
node tests/test-enforcement-scope.mjs→ 17 passed, 0 failed, driving the real deployed gate hooks (runHook) installed per-project into isolated temp HOMEs — not stubs.Coverage: checkpoint parity, em-store allowed in all marker states, plan-file allowed, non-src carve-outs, empty-path fail-closed, reads always allowed, repo-source write blocked, garbage-config fail-closed, off-repo write/redirect allowed, relative
..-escape allowed (F1 regression), per-project isolation, missing-lib fail-closed, multi-redirect mixed (F2), andt_no_global_touch(REQ-11:hookCodeFilesInGlobalScope(home) === []after--install-enforcement).Now wired into
plan-marker-validate.yml.Deferred (tracked, off critical path)
runHooksuite; deploying unmerged gates into live global hooks is what that rule guards against).Files
plugins/claude-code/hooks/lib/repo-source.sh(new shared predicate)plugins/claude-code/hooks/plan-gate.sh,checkpoint-gate.sh(consumers)plugins/claude-code/hooks/lib/command-classifier.sh(redirect TARGET emission)tests/test-enforcement-scope.mjs(new E2E suite).github/workflows/plan-marker-validate.yml(CI wire)🤖 Generated with Claude Code