Skip to content

fix(security): scope the scrub hook by the live shell directory, not the session root - #1914

Merged
potiuk merged 14 commits into
apache:mainfrom
potiuk:fix/scrub-hook-live-cwd
Oct 30, 2026
Merged

fix(security): scope the scrub hook by the live shell directory, not the session root#1914
potiuk merged 14 commits into
apache:mainfrom
potiuk:fix/scrub-hook-live-cwd

Conversation

@potiuk

@potiuk potiuk commented Oct 30, 2026

Copy link
Copy Markdown
Member

Summary

  • Claude Code's Bash tool.call event gives a mod no working directory. register() therefore fell back to $.session.root(), which is only the directory the session started in, and set the fallback flag. Under that flag the tracker exemption is skipped, so in a session started in the tracker checkout, a plain git commit -F msg.txt for the tracker's own config was blocked whenever the message used embargo terms. The block message's --repo hint did not help, because git commit/tag/merge have no such flag.
  • The mod API also has $.session.cwd(), the session's current directory. Claude Code updates it when a Bash call changes directory, and for a subagent it is that agent's own directory. register() now reads it first, after an explicit event cwd, and before the root. A bare command is then scoped by where the shell really is. A commit in the tracker checkout is exempt. A commit made after an earlier call moved the shell into an upstream worktree is scanned. A cd on the same line is followed as before.
  • The session-root fallback rule itself is unchanged and still applies when the current directory is unavailable. Its block message no longer suggests --repo for a git command.
  • Because a known directory is now the normal case, this PR also closes the ways an earlier segment on the same line can repoint a later git or gh command without changing directory, with an allow-list that fails closed. An earlier segment keeps the later command's directory-based exemption only when its command is git with add / status / diff / log / show, gh <noun> with view / list / status / diff, cd (already tracked), or one of echo, test, [, true, false, :, pwd, ls, cat, with no leading variable assignment, no output redirection, no $ expansion, no command or process substitution, not followed by ||, and (for git) no --output / -o; brace-group braces are ignored. Anything else — export GIT_DIR=... in any spelling, export GH_REPO=..., eval, source, control words, function or alias definitions, PATH=..., git remote set-url, git config, gh repo set-default, untracked pushd / popd — sends the later command through the term scan. A leading assignment or an env wrapper on the git or gh command itself, after any prefix, does the same (GIT_COMMON_DIR=... git tag; the parser recognised only GIT_DIR / GIT_WORK_TREE there). A deny-list was tried first and each review pass found another bypass. Claude Code does not carry variables across Bash calls, so only the same line matters — except variables the session itself inherited: if Claude was started with GH_REPO, GH_HOST or a repository-selecting GIT_* variable set, register() reads it and every bare git / gh command loses the directory-based exemption.
  • The same fail-closed rule covers the git or gh command itself and the cd tracking: a $ expansion, backtick, <( or >( inside the git / gh command, a cd that runs only conditionally (after && / ||), any cd other than a plain cd <dir> (cd -, cd -P, extra arguments), a relative cd that an inherited CDPATH can redirect, and bash -c '...' / eval '...' invoking git or gh all remove the directory-based exemption.

Type of change

  • Skill change (.claude/skills/<name>/) — eval fixtures updated below
  • Tool / bridge contract (tools/<system>/*.md)
  • Python package (tools/*/ with pyproject.toml)
  • Groovy reference impl
  • Cross-cutting (RFC, AGENTS.md, sandbox, privacy-LLM)
  • Documentation (docs/, README.md, CONTRIBUTING.md)
  • Project template (projects/_template/)
  • CI / dev loop (prek, workflows, validators)
  • Other: Claude Code mod (plugins/magpie-security/hooks/public-surface-scrub.ts) and its tests

Test plan

  • prek run --all-files passes (ran prek run --files on the two changed files: all hooks pass, including check-claude-mods-local and pytest (workspace))
  • For Python packages touched: uv run pytest / ruff check / mypy passes
  • For Groovy bridges touched: command-line invocation tested end-to-end
  • For skill changes: eval suite passes for the affected skill
  • For skill behaviour changes: a new or updated eval fixture is included in this PR
  • Other: ran tools/dev/check-claude-mods.sh --local with Claude Code 2.1.312. claude plugin validate --strict passes with the new $.session.cwd() call, and public-surface-scrub.test.ts has 158 pass / 0 fail.
    • New register() tests, with the session root always the tracker checkout:
      • session.cwd is the tracker: a bare git commit -F passes.
      • session.cwd is an upstream worktree, as after a cd in an earlier call: the commit is blocked.
      • A cd into upstream on the same line: blocked.
      • An explicit event cwd takes precedence over session.cwd.
      • gh pr create with a verified tracker cwd is exempt.
      • Under the root-only fallback, a git block does not mention --repo.
    • The feat(security): add deterministic public-surface scrub Claude Code mod (part of #1497) #1676 fallback tests are unchanged and pass. When session.cwd has no answer, the hook still falls back to the root.
    • Regression check: with the session.cwd() read removed, the two tests for the tracker case fail.
    • New tests for an earlier environment change, all with a verified tracker cwd:
      • Blocked: every GIT_* declaration form (export, declare -x, typeset -x, readonly, local, bare, builtin / command export, +=, scoped GIT_DIR=x ls), eval, source, ., set -a, printf -v, export "$V", export FOO=1, if source ...; then, a leading GIT_COMMON_DIR= on the git command, git remote set-url, git config, pushd / popd, a function definition, PATH=..., env GIT_COMMON_DIR=... git tag, then GIT_COMMON_DIR=... git tag, >> .git/config, echo $(...), backticks, <(...), git diff --output=...; and before a bare gh: export GH_REPO, GH_HOST=, a leading GH_REPO=, git remote set-url, gh repo set-default, printf ... > .git/config, printf -vGH_REPO ..., cd /upstream || cd /tracker;, echo "${GH_REPO:=...}";; and any bare git / gh command when the session environment carries GH_REPO or GIT_DIR.
      • Still exempt: echo ready && git commit, git add file && git commit, git status && git commit, and a bare commit; the existing { cd ../tracker; gh issue view 1; }; git commit test still passes.
      • Also blocked: conditional cd inside if / while / for / case or after && / ||, cd -, cd -P, cd dir extra, $(...) / backticks / ${GH_REPO:=...} / >(...) inside the git or gh command, cd child and cd .hidden under an inherited CDPATH (cd ./child and absolute paths stay exempt), CDPATH=... / export CDPATH=... earlier on the line, and bash -c / sh -c / eval invoking git or gh.
      • A neutral message after a GIT_* export still passes (the rule removes the exemption; it does not block by itself).
      • With the new condition disabled, the blocked-cases test fails.

RFC-AI-0004 compliance

  • Write-access discipline — the guard now scopes each command by the directory the shell is actually in. The fallback (no exemption) still applies whenever that directory is unknown.

Linked issues

Refs #1676

Notes for reviewers (optional)

  • Where $.session.cwd() comes from: in Claude Code 2.1.312, the session.cwd call is answered from the call context's cwd, else from the session's current cwd, and only then from the original cwd. That current cwd is updated from pwd -P after each Bash command (setCwd, telemetry tengu_shell_set_cwd). session.root answers the original cwd. Claude Code's own built-in mods read both, and resolve relative paths against session.cwd. There is no public .d.ts for the mod API, so this comes from the shipped engine, not from documentation.
  • Trusting the session root for bare git commands was considered and rejected: it cannot see a cd made in an earlier Bash call, nor GIT_DIR/GIT_WORK_TREE set by an earlier export. The latter is now covered for every directory by the allow-list above.
  • The allow-list grew out of several adversarial review rounds: each pass over a deny-list found another shell construct that changes the environment or the repository before a later command (control words, env wrappers, printf -v, ${VAR:=...}, cd after && or ||, redirections into .git/config, git remote set-url, gh repo set-default, inherited GH_REPO, inherited CDPATH, bash -c / eval). The hook still parses shell text statically, so treat the exemption as best-effort; when in doubt it now blocks, and the blocked path stays the term scan, not a hard refusal.

🤖 Generated with Claude Code

@potiuk potiuk added family:security security-* skills capability:fix Implement code change to resolve an issue labels Oct 30, 2026
@potiuk
potiuk enabled auto-merge (squash) October 30, 2026 15:57
potiuk added 14 commits October 30, 2026 17:50
@potiuk
fix(security): scope the scrub hook by the live shell directory, not …
e9df9a3 
…the session root

The Bash tool event that Claude Code hands a mod carries no working
directory, so register() fell back to $.session.root() and flagged the
scope as a fallback. The root is only where the session started; with
the fallback flag set, the tracker exemption is skipped, so in a
session started in the tracker checkout a plain `git commit -F msg.txt`
for the tracker's own config was blocked whenever the message used
embargo terms. git has no repository flag, so the `--repo` hint in the
block message offered no way through.

The mod API also exposes $.session.cwd(): the session's current
directory, which Claude Code moves when a Bash call changes directory
(and which is the calling agent's own directory for a subagent).
register() now reads it before falling back to the root. The hook then
scopes a bare command by the directory the shell is really in: a commit
in the tracker checkout takes the tracker exemption, and one made after
an earlier call moved the shell into an upstream worktree is scanned.
A cd on the same line is still followed as before. An explicit event
cwd still wins.

The session-root fallback and its rule are unchanged for harnesses
where the current directory is unavailable. Its block message no
longer suggests `--repo` for a git command, which has no such flag.

Generated-by: Claude Opus 5
@potiuk
fix(security): treat an earlier GIT_* assignment on the line as a git…
fc4b74d 
… redirect

The scrub hook already treats GIT_DIR / GIT_WORK_TREE leading the git
command itself (`GIT_DIR=x git commit`) as a repository redirect that
the cwd-based tracker exemption cannot cover. The same variables set
by an earlier segment of the line slipped past it: with a verified
tracker cwd, `export GIT_DIR=../upstream/.git && git commit -m ...`
took the tracker exemption although the commit lands in the other
repository.

A git public-surface subcommand now loses the cwd-based exemptions
(tracker and known non-upstream remote) and falls through to the term
scan when an earlier segment on the same line sets or exports a GIT_*
variable through export, declare, typeset, readonly or local
(including `export GIT_DIR` of an earlier assignment), or through a
bare `GIT_*=...` segment with no command after it. Any GIT_* name
counts, not only the repository-selection ones: the list grows with
git releases and a tracker commit has no reason to set any of them
earlier on the same line. An assignment scoped to another command
(`GIT_DIR=x ls`) does not persist and is not flagged. `set -a` style
auto-export is out of scope. Claude Code does not carry variables
across Bash calls, so the same line is the only place this can happen.

Generated-by: Claude Opus 5
@potiuk
fix(security): fail closed on any earlier environment change before g…
33fe733 
…it or gh

The GIT_* rule only recognised plain declaration builtins and git
commands, so `builtin export GIT_DIR=...`, `GIT_DIR+=...`, an `eval` or
`source`, or `export GH_REPO=...` before a bare `gh` command still let
the later command take the directory-based exemption while acting on
another repository.

Fail closed instead: once an earlier segment on the line mentions any
GIT_* or GH_* variable, runs an environment-opaque builtin (eval,
source, ., set, read, mapfile, printf -v) after any builtin / command /
control-word prefix, or declares variables with expansions, later git
and gh commands lose the directory-based exemption and go through the
term scan. A tracker commit has no reason to do any of this on the same
line.

Generated-by: Claude Opus 5
@potiuk
fix(security): allow-list what may precede a cwd-exempt git or gh com…
df6755f 
…mand

Each pass over the deny-list found another way to change the
environment or the repository before a later git or gh command:
control words (`if source ...; then`), `GIT_COMMON_DIR=...` leading the
git command itself, `git remote set-url` / `git config`, `gh repo
set-default`, untracked `pushd` / `popd`, function and alias
definitions, `PATH=...`.

Replace it with an allow-list. An earlier segment keeps a later git or
gh command's directory-based exemption only when its command is `git`
with add / status / diff / log / show, `gh <noun>` with view / list /
status / diff, `cd` (tracked by the loop), or one of echo, printf
(without -v), test, [, true, false, :, pwd, ls, cat, with no leading
variable assignment; brace-group braces are ignored. Any leading
assignment on the git or gh command itself also removes its exemption.

Generated-by: Claude Opus 5
@potiuk
fix(security): fail closed on side effects inside allow-listed commands
815b377 
An allow-listed command could still change repository state before a
later git or gh command ran, after remote inspection had read the old
state: through an output redirection (`printf ... > .git/config`), a
command or process substitution (`echo $(git remote set-url ...)`), or
`git diff --output=.git/config`. And an own-environment git command
(`GIT_COMMON_DIR=... git tag`, `env ... git tag`, also after a prefix
such as `then`) passed the first-word assignment check.

An earlier segment with `>`, a backtick, `$(` or `<(` now counts as a
redirect for later git and gh commands, as does an allow-listed git
command with an `--output` / `-o` option. parseSubcommand records
whether it unwrapped a leading assignment or an `env` wrapper, after any
prefix, and such a git or gh command gets no directory-based exemption.

Generated-by: Claude Opus 5
@potiuk
fix(security): honour inherited repository variables; drop printf fro…
1155e1d 
…m the allow-list

A session started with GH_REPO, GIT_DIR or another repository-selecting
variable in its environment repoints every bare git or gh command, but
the directory-based exemption only looked at the directory. register()
now reads those variables (literal $.env.get names, as the mod API
requires) and any of them removes the exemption for the whole line.

`printf -vNAME` assigns a variable (bash accepts the option glued to the
name), so printf can change the environment and leaves the allow-list;
echo covers the harmless case.

Generated-by: Claude Opus 5
@potiuk
fix(security): treat || and any expansion before git / gh as a redi…
c7ba61b 
…rect

The directory tracking follows every `cd`, but `||` runs what follows
only on failure, so `cd /upstream || cd /tracker; gh ...` left the
tracked directory at the tracker while the shell stayed upstream. And
an allow-listed echo could assign through `${GH_REPO:=...}`.

A segment followed by `||`, or containing any `$` expansion, now counts
as a redirect for later git and gh commands.

Generated-by: Claude Opus 5
@potiuk
fix(security): treat a conditional cd before git / gh as a redirect
fc539d6 
A `cd` after `&&` or `||` runs only conditionally (`false && cd
/tracker; gh ...`), but the directory tracking followed it anyway, so a
later command could take the tracker exemption from a directory the
shell never entered. Such a `cd` now counts as a redirect for later git
and gh commands; a leading or `;`-separated `cd` is still tracked.

Generated-by: Claude Opus 5
@potiuk
fix(security): treat any cd after an earlier && or || as a redirect
3176aa5 
Checking only the separator right before a `cd` missed a conditional
group: in `false && { true; cd /tracker; }`, the `cd` follows `;` inside
the braces but still runs only if `false` succeeds. Once an `&&` or `||`
has appeared on the line, every later `cd` now counts as a redirect for
later git and gh commands.

Generated-by: Claude Opus 5
@potiuk
test(security): cover cd inside if / while / for / case before git or gh
12e032c 
A `cd` inside a compound command runs only conditionally. The
allow-list already treats a segment starting with `if`, `while`, `for`,
`case` or another compound keyword as a redirect for every later
command, so these lines go through the term scan; pin that with tests.

Generated-by: Claude Opus 5
@potiuk
fix(security): no exemption for in-command substitution or a malforme…
56e3647 
…d cd

A command substitution inside the git or gh command itself
(`gh issue create -t "$(git remote set-url ...)"`) runs before the
command resolves its repository, but only earlier segments were
checked. Such a command now gets no directory-based exemption and goes
through the dynamic-input check.

Only a plain `cd <dir>` is tracked: `cd a b` fails and leaves the shell
where it was, and options or `cd -` move it somewhere the tracking does
not model, so any other cd form counts as a redirect.

Generated-by: Claude Opus 5
@potiuk
fix(security): same fail-closed expansion check inside the git or gh …
4ba2254 
…command

The in-command check caught `$(`, backticks and `<(` but not a plain
`$` expansion (`"${GH_REPO:=...}"` assigns before gh resolves its
repository) or `>(`. Use the same set as the earlier-segment check:
any `$`, backtick, `<(` or `>(` removes the directory-based exemption.

Generated-by: Claude Opus 5
@potiuk
fix(security): no cwd exemption under CDPATH or for bash -c / eval
bdba949 
With CDPATH inherited, `cd name` searches the CDPATH entries before
the current directory, so the tracked directory can name a tracker
while the shell lands in an upstream checkout. A relative `cd` that
CDPATH can redirect (not `/...` or `./...`) now counts as a redirect,
and the hook reads CDPATH from the session environment.

`bash -c '...'` and `eval '...'` invoking git or gh can cd or set
GIT_DIR inside the string, but their command type did not match the
step-5 redirect check, so the tracker exemption skipped the
dynamic-input block. They now get no directory-based exemption.

Generated-by: Claude Opus 5
@potiuk
fix(security): match bash's rule for when cd skips CDPATH
91bbe4e 
Bash skips the CDPATH search only for `/...`, `.`, `..`, `./...` and
`../...`; a name such as `.hidden` is still searched, so it counts as a
CDPATH-redirectable cd too. Tests also pin that an earlier
`CDPATH=...` or `export CDPATH=...` on the line already removes the
exemption through the allow-list.

Generated-by: Claude Opus 5
@potiuk
potiuk force-pushed the fix/scrub-hook-live-cwd branch from 6ce188d to 91bbe4e Compare October 30, 2026 17:11
@potiuk
potiuk merged commit 9c86967 into apache:main Oct 30, 2026
9 checks passed
@potiuk potiuk mentioned this pull request Oct 30, 2026
Merged
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

capability:fix Implement code change to resolve an issue family:security security-* skills

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@potiuk