diff --git a/main.go b/main.go index dea16a14..79cf0298 100644 --- a/main.go +++ b/main.go @@ -77,22 +77,36 @@ 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`) +const manifestNamesRE = `(?:pom\.xml|requirements\.txt|pyproject\.toml|package\.json|go\.mod|Cargo\.toml|\.github/workflows/[^\s'"]+\.ya?ml)` + +// 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 +// 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`. +// 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 // 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..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,6 +101,16 @@ 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}, + {"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}, } for _, test := range tests {