Repository navigation
fix(security): a cd in a pipeline or background job does not move the scrub hook into the tracker - #1915
Merged
Conversation
The scrub hook skipped a `cd` for directory tracking only when the separator after it was `|` or `&`, which catches a `cd` that starts a pipeline but not one in a later stage. In `echo x | cd /tracker; gh issue create ...` the `cd` runs in a subshell and the shell stays in the upstream clone, but the hook moved its tracked directory to the tracker and exempted the bare `gh` from the scrub. The same held for a `cd` inside a brace group in a pipeline. Once a `|` or `&` has appeared on the line, any `cd` now marks the line as redirected, so a later bare git or gh command loses the cwd exemption, the same fail-closed rule the hook applies to a `cd` after `&&` or `||`. Generated-by: Claude Opus 5
… redirect
In `{ cd /tracker; true; } | cat` the group is the first pipeline stage, so
its inner `cd` ends with `;` and the `|` sits only on the group's last
command. The splitter now marks every command of a brace group that is
piped or backgrounded, and such a `cd` removes the cwd exemption.
Generated-by: Claude Opus 5
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
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.
The public-surface scrub hook skipped a
cdfor directory tracking only when the separator after it was|or&.That catches a
cdthat starts a pipeline, but not one in a later stage:Bash runs that
cdin a subshell, so the shell stays in the upstream clone,but the hook moved its tracked directory to the tracker and gave the bare
ghthe tracker exemption, skipping the scrub.A
cdinside a brace group in a pipeline did the same, whether the group came after the pipe (echo x | { true; cd /tracker; })or was itself piped or backgrounded (
{ cd /tracker; true; } | cat, where the|sits only on the group's last command).Once a
|or&has appeared on the line, anycdnow marks the line as redirected,and the splitter marks every command of a piped or backgrounded brace group so its
cddoes too;a later bare
gitorghthen loses the cwd exemption.This is the fail-closed rule the hook already applies to a
cdafter&&or||.Tests: a new case in the per-path fixture (start upstream, piped
cdinto the tracker, baregh/git commitmust be scanned, four shapes),and four shapes added to the redirect list.
Both fail on
mainand pass with the fix; the whole mod suite passes (tools/dev/check-claude-mods.sh --local).Found by an automated security review of the hook after #1914.
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Claude Opus 5) following the guidelines
🤖 Generated with Claude Code