From 7a2228156a0c306f3ee8d60e09fe22f19886bd19 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Fri, 25 Sep 2026 13:17:28 +0200 Subject: [PATCH 1/3] fix: narrow bash manifest-write detection to actual write targets looksLikeManifestWrite previously only checked that a known manifest name and a write construct (redirect, tee, sed -i, etc.) both appeared somewhere in the command string, not that the write construct actually targeted the manifest. This false-positived on commands where the manifest name is merely read or mentioned but nothing is written to it (e.g. a read with a stderr redirect elsewhere in the command, or the manifest named in an unrelated later clause). redirectToManifestRE and writeConstructToManifestRE now require the manifest name to immediately follow the write construct's own target position, scoped to the same shell clause (stopped at ;, &&, ||, |, and newlines) so a write in one clause can't match a manifest name that only appears in another. Co-Authored-By: Claude Sonnet 5 --- main.go | 42 +++++++++++++++++++++++++++++++----------- main_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/main.go b/main.go index dea16a14..602a22ef 100644 --- a/main.go +++ b/main.go @@ -77,22 +77,42 @@ type hookInput struct { } `json:"tool_input"` } -// manifestRE matches a known manifest name in a shell command. RE2 has no -// lookahead, so the trailing boundary is a capturing alternative instead. -var manifestRE = regexp.MustCompile(`(^|[/\\ '"=])(pom\.xml|requirements\.txt|pyproject\.toml|package\.json|go\.mod|Cargo\.toml|\.github/workflows/[^\s'"]+\.ya?ml)([/\\ '"]|$)`) - -// writeConstructRE matches shell constructs that mutate a file's content, -// other than `>`/`>>` (handled by redirectRE). -var writeConstructRE = regexp.MustCompile(`\btee\b|\bsed\s+-i|\bperl\s+-i|\bdd\s+of=|\bcp\s|\bmv\s`) +// manifestNamesRE is the shared alternation of known manifest names/paths, +// reused as a suffix by every check below so each one requires the manifest +// to be the actual target of a write, not merely mentioned somewhere else in +// the command (e.g. `git add pyproject.toml && cat > .gitignore </dev/null` where the manifest is just cat's read arg and +// the `>` belongs to an unrelated stderr redirect). +const manifestNamesRE = `(?:pom\.xml|requirements\.txt|pyproject\.toml|package\.json|go\.mod|Cargo\.toml|\.github/workflows/[^\s'"]+\.ya?ml)` + +// clause is a same-pipeline-stage character class: it stops at `;`, `&&`, +// `||`, `|`, and newlines so a write construct in one shell clause can't be +// matched against a manifest name that only appears in a different clause. +const clause = `[^;&|\n]` + +// writeConstructToManifestRE matches shell constructs that mutate a file's +// content, other than `>`/`>>` (handled by redirectToManifestRE), where the +// manifest name is the construct's own target argument. +var writeConstructToManifestRE = regexp.MustCompile( + `\btee\b` + clause + `*` + manifestNamesRE + + `|\b(?:sed|perl)\s+-i\b` + clause + `*` + manifestNamesRE + + `|\bdd\b` + clause + `*?\bof=['"]?(?:[^\s'"]*/)?` + manifestNamesRE + + `|\b(?:cp|mv)\s+` + clause + `*` + manifestNamesRE, +) -// redirectRE matches a `>`/`>>` that writes file content, excluding fd -// duplication like `2>&1`. -var redirectRE = regexp.MustCompile(`>>?[^&]|>>?$`) +// redirectToManifestRE matches a `>`/`>>` whose target is a known manifest +// name, e.g. `cat > pom.xml <> requirements.txt`, or +// `cat > node_modules/pkg/package.json <&1` and unrelated redirects like `2>/dev/null` by requiring the +// manifest name immediately after the operator, rather than just matching +// any `>` present elsewhere in cmd. +var redirectToManifestRE = regexp.MustCompile(`>>?\s*['"]?(?:[^\s'"]*/)?` + manifestNamesRE + `['"]?(\s|;|&|\||$)`) // looksLikeManifestWrite reports whether cmd looks like it rewrites a known // manifest's content directly, bypassing the Write/Edit path runHook checks. func looksLikeManifestWrite(cmd string) bool { - return manifestRE.MatchString(cmd) && (writeConstructRE.MatchString(cmd) || redirectRE.MatchString(cmd)) + return writeConstructToManifestRE.MatchString(cmd) || redirectToManifestRE.MatchString(cmd) } // runHook is a PreToolUse hook for the Write, Edit, and Bash tools. diff --git a/main_test.go b/main_test.go index 19349998..81c834da 100644 --- a/main_test.go +++ b/main_test.go @@ -88,6 +88,36 @@ func TestLooksLikeManifestWrite(t *testing.T) { {"unrelated file redirect", `echo hi > notes.txt`, false}, {"mkdir unrelated", `mkdir -p .github/workflows`, false}, {"ls workflows dir", `ls -la .github/workflows/`, false}, + + // Regression cases: stderr-to-file (not fd dup) or an unrelated `>` + // elsewhere in the command used to false-positive because the old + // check only required the manifest name and *some* `>` to co-occur + // anywhere in cmd, rather than requiring the `>` to actually target + // the manifest. + {"read with stderr to /dev/null", `cat Cargo.toml 2>/dev/null`, false}, + {"read with stderr to /dev/null, compound", `ls -la && cat go.mod 2>/dev/null; go version`, false}, + {"unrelated redirect elsewhere, manifest read in same clause", `npm init -y >/dev/null && cat package.json`, false}, + {"manifest named in different clause than the write", `cat > .gitignore << 'EOF' +ignored +EOF +git add pyproject.toml .gitignore`, false}, + {"manifest mentioned in a URL, no local write", `curl -s "https://example.com/spring-boot/pom.xml" | grep version`, false}, + {"manifest mentioned inside a string literal, unrelated redirect", `python3 -c "print('pyproject.toml')" > /tmp/out.log`, false}, + {"find pattern for manifest name, not a write", `find . -iname "go.mod" 2>/dev/null`, false}, + + {"redirect target is the manifest despite trailing stderr redirect", `cat > pom.xml << 'EOF' + +EOF +` + "true", true}, + {"sed -i with trailing pipe to unrelated command", `sed -i 's/1.0/2.0/' package.json | cat`, true}, + {"append redirect with terminator", `echo pinned >> Cargo.toml; echo done`, true}, + {"heredoc write to manifest in a scratch dir", `cd /tmp/x && cat > go.mod <<'EOF' +module tmp +EOF`, true}, + {"mv with multiple sources including the manifest", `mv a.txt pom.xml src .`, true}, + {"redirect target with a relative directory prefix", `cat > node_modules/pkg-a/package.json <<'EOF' +{} +EOF`, true}, } for _, test := range tests { From a08c593a78d15c9827a05482c815e50026bbd5af Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Fri, 25 Sep 2026 13:27:13 +0200 Subject: [PATCH 2/3] Refactor test cases --- main_test.go | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/main_test.go b/main_test.go index 81c834da..f366f7be 100644 --- a/main_test.go +++ b/main_test.go @@ -79,6 +79,19 @@ func TestLooksLikeManifestWrite(t *testing.T) { {"mv onto manifest", `mv /tmp/new.mod go.mod`, true}, {"github actions workflow redirect", `cat > .github/workflows/ci.yml << 'EOF'`, true}, {"quoted path redirect", `printf '%s' "$content" > "requirements.txt"`, true}, + {"redirect target is the manifest despite trailing stderr redirect", `cat > pom.xml << 'EOF' + +EOF +` + "true", true}, + {"sed -i with trailing pipe to unrelated command", `sed -i 's/1.0/2.0/' package.json | cat`, true}, + {"append redirect with terminator", `echo pinned >> Cargo.toml; echo done`, true}, + {"heredoc write to manifest in a scratch dir", `cd /tmp/x && cat > go.mod <<'EOF' +module tmp +EOF`, true}, + {"mv with multiple sources including the manifest", `mv a.txt pom.xml src .`, true}, + {"redirect target with a relative directory prefix", `cat > node_modules/pkg-a/package.json <<'EOF' +{} +EOF`, true}, {"plain read", `cat requirements.txt`, false}, {"grep manifest", `grep react package.json`, false}, @@ -88,12 +101,6 @@ func TestLooksLikeManifestWrite(t *testing.T) { {"unrelated file redirect", `echo hi > notes.txt`, false}, {"mkdir unrelated", `mkdir -p .github/workflows`, false}, {"ls workflows dir", `ls -la .github/workflows/`, false}, - - // Regression cases: stderr-to-file (not fd dup) or an unrelated `>` - // elsewhere in the command used to false-positive because the old - // check only required the manifest name and *some* `>` to co-occur - // anywhere in cmd, rather than requiring the `>` to actually target - // the manifest. {"read with stderr to /dev/null", `cat Cargo.toml 2>/dev/null`, false}, {"read with stderr to /dev/null, compound", `ls -la && cat go.mod 2>/dev/null; go version`, false}, {"unrelated redirect elsewhere, manifest read in same clause", `npm init -y >/dev/null && cat package.json`, false}, @@ -104,20 +111,6 @@ git add pyproject.toml .gitignore`, false}, {"manifest mentioned in a URL, no local write", `curl -s "https://example.com/spring-boot/pom.xml" | grep version`, false}, {"manifest mentioned inside a string literal, unrelated redirect", `python3 -c "print('pyproject.toml')" > /tmp/out.log`, false}, {"find pattern for manifest name, not a write", `find . -iname "go.mod" 2>/dev/null`, false}, - - {"redirect target is the manifest despite trailing stderr redirect", `cat > pom.xml << 'EOF' - -EOF -` + "true", true}, - {"sed -i with trailing pipe to unrelated command", `sed -i 's/1.0/2.0/' package.json | cat`, true}, - {"append redirect with terminator", `echo pinned >> Cargo.toml; echo done`, true}, - {"heredoc write to manifest in a scratch dir", `cd /tmp/x && cat > go.mod <<'EOF' -module tmp -EOF`, true}, - {"mv with multiple sources including the manifest", `mv a.txt pom.xml src .`, true}, - {"redirect target with a relative directory prefix", `cat > node_modules/pkg-a/package.json <<'EOF' -{} -EOF`, true}, } for _, test := range tests { From d450cb0407540e0a9bfe60c911cb2c89977f04da Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Fri, 25 Sep 2026 13:33:00 +0200 Subject: [PATCH 3/3] docs: make the clause comment concrete with an example Signed-off-by: Aman Sharma --- main.go | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/main.go b/main.go index 602a22ef..79cf0298 100644 --- a/main.go +++ b/main.go @@ -77,17 +77,13 @@ type hookInput struct { } `json:"tool_input"` } -// manifestNamesRE is the shared alternation of known manifest names/paths, -// reused as a suffix by every check below so each one requires the manifest -// to be the actual target of a write, not merely mentioned somewhere else in -// the command (e.g. `git add pyproject.toml && cat > .gitignore </dev/null` where the manifest is just cat's read arg and -// the `>` belongs to an unrelated stderr redirect). const manifestNamesRE = `(?:pom\.xml|requirements\.txt|pyproject\.toml|package\.json|go\.mod|Cargo\.toml|\.github/workflows/[^\s'"]+\.ya?ml)` -// clause is a same-pipeline-stage character class: it stops at `;`, `&&`, -// `||`, `|`, and newlines so a write construct in one shell clause can't be -// matched against a manifest name that only appears in a different clause. +// clause bounds the gap between a write construct's keyword (e.g. `tee`) +// and the manifest name that must be its own target argument, so e.g. +// `tee notes.txt; cat package.json` doesn't match: the `;` before +// package.json stops the gap, since tee's real target is notes.txt, not the +// manifest. const clause = `[^;&|\n]` // writeConstructToManifestRE matches shell constructs that mutate a file's @@ -101,12 +97,10 @@ var writeConstructToManifestRE = regexp.MustCompile( ) // redirectToManifestRE matches a `>`/`>>` whose target is a known manifest -// name, e.g. `cat > pom.xml <> requirements.txt`, or -// `cat > node_modules/pkg/package.json <&1` and unrelated redirects like `2>/dev/null` by requiring the -// manifest name immediately after the operator, rather than just matching -// any `>` present elsewhere in cmd. +// name, e.g. `cat > pom.xml <> requirements.txt`. +// Excludes fd duplication like `2>&1` and unrelated redirects like `2>/dev/null` +// by requiring the manifest name immediately after the operator, rather +// than just matching any `>` present elsewhere in cmd. var redirectToManifestRE = regexp.MustCompile(`>>?\s*['"]?(?:[^\s'"]*/)?` + manifestNamesRE + `['"]?(\s|;|&|\||$)`) // looksLikeManifestWrite reports whether cmd looks like it rewrites a known