fix(guardrails): boundary-anchor _py_write path( indicator (#1178)

[kyle-sexton]

Summary

block-hook-bypass's python-write rule false-positived on read-only path arithmetic. The
_py_write write indicator's path[[:space:]]*\( was an unanchored substring — it matched path(
as the suffix of a longer identifier, so python3 -c "…os.path.normpath(os.path.join(a,b))…" (and every
other os.path.*path( helper: abspath, realpath, relpath, commonpath) was blocked as a
file-write bypass despite writing nothing.

This anchors the pathlib / path( indicators to an identifier boundary
((^|[^[:alnum:]_])), so they still catch the write-capable pathlib.Path( producer while clearing the
read-only helpers. Real writes remain blocked via the independent .write( / open( indicators. Patch
release 0.13.1.

New instance of the guardrails false-positive class tracked by #547; per its convention the false
positive lands as a regression fixture.

Test plan

• Reproduced end-to-end through the actual hook (guardrails@0.13.0, Claude Code 2.1.218):
  • Before: os.path.normpath/abspath/realpath/relpath/join → exit 2 (blocked ❌); real
    open('x','w').write / pathlib.Path().write_text → exit 2 (blocked ✓).
  • After: the *path( helpers → exit 0 (allowed ✓); real writes → exit 2 (still blocked ✓).
• block-hook-bypass.test.sh extended with a python-write false-positive regression block (four
  MUST-stay-quiet os.path.*path( cases + one MUST-block pathlib.Path().write_text). Full suite:
  PASS=169 FAIL=0.
• shellcheck -S error clean on block-hook-bypass.sh.

Fresh-docs basis (CLAUDE.md mandate): code.claude.com/docs/en/hooks and .../plugins-reference
fetched this session; the fix (a hook regex + a semver/manifest bump) was additionally verified by live
reproduction through the actual installed hook, so behavior is empirically grounded, not recalled.

Related

Closes #1178.
Class umbrella: #547 (guardrails false-positive over-fire prevention).

[claude]

Claude finished @kyle-sexton's task in 2m 16s —— View job

---

Code Review

• Gather context and understand the request
• Read changed files and diff
• Review correctness of regex anchor fix
• Review test coverage
• Review CHANGELOG and version bump
• Post final review findings

---

The fix is correct and precisely targeted. Here's a full breakdown:

Regex correctness

Old pattern:

open[[:space:]]*\(|\.write[[:space:]]*\(|pathlib|path[[:space:]]*\(

New pattern:

open[[:space:]]*\(|\.write[[:space:]]*\(|(^|[^[:alnum:]_])pathlib|(^|[^[:alnum:]_])path[[:space:]]*\(

The boundary anchor (^|[^[:alnum:]_]) is the right fix. Walking through the key cases:

• os.path.normpath( → path preceded by m (normpath) which is [[:alnum:]_] → no match ✓
• os.path.abspath( → path preceded by s → no match ✓
• os.path.commonpath( → path preceded by n → no match ✓
• pathlib.Path( in COMMAND_LC → pathlib preceded by a space/start-of-string → (^|[^[:alnum:]_])pathlib matches ✓; and .path( (lowercased Path) is preceded by . → [^[:alnum:]_])path( matches ✓
• from pathlib import Path; Path('x').write_text('a') → pathlib is still in the raw command string, so (^|[^[:alnum:]_])pathlib catches it ✓

One subtlety worth noting: \.write[[:space:]]*\( does not match .write_text( (there's _text between write and (), so the pathlib match is load-bearing for write_text coverage. This is pre-existing design, unchanged here, and the blocked test confirms it still works.

Test coverage

The 5 new fixtures in block-hook-bypass.test.sh are well-structured:

Test	Expected	Correct
os.path.normpath(	allowed	✓
os.path.abspath(	allowed	✓
os.path.realpath + relpath (combined)	allowed	✓
os.path.join(	allowed	✓
pathlib.Path().write_text	blocked	✓

Minor note — os.path.join test: os.path.join('a','b') was never actually blocked by the old regex — path is followed by .join(, not (, so the old path[[:space:]]*\( never matched it. The test is still worthwhile as defensive coverage, but the PR description and test label ("os.path.join producer (allowed)") slightly overstate its role as a regression case.

Missing fixture — commonpath: The PR description explicitly names commonpath as a false-positive trigger, but the test suite doesn't include an os.path.commonpath( case. The fix does correctly handle it (the h in commonpath is [[:alnum:]_], blocking the match), but a test would make that explicit. Fix this →

Other files

• plugin.json: 0.13.0 → 0.13.1 — correct semver for a bug fix.
• CHANGELOG.md: Entry is accurate, references the correct issue, and calls out both the false-positive class and what still blocks. Well-written.
• Inline comment at block-hook-bypass.sh:309: Explains the intent correctly.

Verdict

The root cause diagnosis is accurate, the fix is minimal and precise, and the true-positive (real write blocked) is confirmed by test. Approve with the one minor suggestion: add a commonpath regression fixture to complete the set mentioned in the description. Everything else is solid.

[claude]

Claude finished @kyle-sexton's task in 2m 2s —— View job

---

Security Review

• Gather PR diff and context
• Read changed files
• Audit regex anchoring and bypass surface
• Audit shell script for injection / logic flaws
• Post findings

---

No security vulnerabilities found in this PR.

The change is confined to one regex variable (_py_write at block-hook-bypass.sh#L313) plus regression tests, a changelog entry, and a version bump. Analysis below.

Regex anchoring is correct and introduces no bypass

Before: pathlib|path[[:space:]]*\(
After: (^|[^[:alnum:]_])pathlib|(^|[^[:alnum:]_])path[[:space:]]*\(

The boundary [^[:alnum:]_] correctly discriminates:

• os.path.normpath( — the char before path is m (alnum) → no match (false positive removed ✓)
• os.path.abspath( — char before path is s → no match ✓
• pathlib.Path('x') lowercased to pathlib.path('x') — char before path is . (non-alnum-non-underscore) → match (write-capable constructor still blocked ✓)
• import pathlib — char before pathlib is space → match ✓

Defense-in-depth means the narrowing cannot create a write bypass

All real Python file-write paths hit at least one unchanged indicator:

Write form	Catching indicator
open('x','w').write(…)	open[[:space:]]*\( (unchanged)
pathlib.Path('x').write_text(…)	\.write[[:space:]]*\( (unchanged) + `(^
pathlib.Path('x').write_bytes(…)	both of the above
aliased: from pathlib import Path as P; P('x').write_text(…)	import pathlib still hits `(^

No Python file write can happen without either open() or a .write*() call, so narrowing the pathlib/path( indicators onto word boundaries does not open a write-bypass surface.

No new injection surfaces in tests

The five new test cases in block-hook-bypass.test.sh (lines 48–57) pipe JSON via stdin to the hook subprocess and assert on exit code — they do not execute the Python commands directly. No injection risk introduced.

Pre-existing residuals (not introduced by this PR, not a finding)

The script's own header documents two known scope gaps (command substitution nesting, group-level redirects). These are unchanged and already covered by accepted-floor tests; this PR does not widen either gap.

---

Verdict: this PR is safe to merge. The anchoring is precise, defense-in-depth is preserved, and no bypass surface is widened.

[claude]

Claude finished @kyle-sexton's task in 1m 23s —— View job

---

Code Review

• Gather context and understand the request
• Read changed files and diff
• Review regex fix correctness
• Review test coverage completeness
• Review CHANGELOG and version bump
• Post final review findings

---

This is a follow-up review of the PR after commit f692868 added the commonpath regression fixture that was the one open item from the previous round of review.

Status: the prior review gap is closed

The second commit responds directly to the prior review's note:

Missing fixture — commonpath: The PR description explicitly names commonpath as a false-positive trigger, but the test suite doesn't include an os.path.commonpath( case.

block-hook-bypass.test.sh:54-55 now includes it:

run "python3 -c os.path.commonpath (allowed)" \
  "python3 -c \"import os; print(os.path.commonpath(['a/b','a/c']))\"" 0

Full fixture set is correct

The *path( helper set from the PR description is now fully covered by tests:

Helper	Tested	Expected	Regex behavior
normpath(	✓	allowed	m before path → [^[:alnum:]_] fails → no match
abspath(	✓	allowed	s before path → no match
realpath(	✓	allowed	l before path → no match
relpath(	✓	allowed	l before path → no match
commonpath(	✓	allowed	n before path → no match
join(	✓	allowed	path followed by .join( not ( → pattern never triggers
pathlib.Path().write_text	✓	blocked	. before path → [^[:alnum:]_] matches; pathlib also still matches

Note on os.path itself: the first path in os.path.normpath( is preceded by . (non-alnum) but followed by . not (, so path[[:space:]]*\( never triggers on it — the anchoring is load-bearing only for the *path( suffix case, which is exactly right.

Regex, version bump, and CHANGELOG unchanged since last review

All three were already verified as correct in the prior review pass. No new issues introduced.

Verdict

The PR is complete. Every os.path.*path( helper from the bug report and PR description now has a regression fixture, the fix is correct, and real writes still block. Ready to merge.

[claude]

Claude finished @kyle-sexton's task in 2m 43s —— View job

---

Security Review

• Gather context and understand the request
• Read PR diff and changed files
• Audit regex anchoring for bypass surfaces
• Audit shell script for injection / logic flaws
• Check for GitHub Actions / supply-chain risks
• Post findings

---

No security vulnerabilities found in this PR.

The change is confined to one regex variable (_py_write at block-hook-bypass.sh:313) plus regression tests, a changelog entry, and a version bump. Analysis below.

Regex narrowing introduces no bypass

Before: pathlib|path[[:space:]]*\(
After: (^|[^[:alnum:]_])pathlib|(^|[^[:alnum:]_])path[[:space:]]*\(

The boundary [^[:alnum:]_] discriminates correctly:

Input (lowercased COMMAND_LC)	Char before path	New match	Write?
os.path.normpath(	m (alnum)	no match (FP removed ✓)	no
os.path.abspath(	s (alnum)	no match ✓	no
os.path.commonpath(	n (alnum)	no match ✓	no
pathlib.Path('x') → .path(	. (non-alnum-non-_)	match ✓	yes
from pathlib import Path	(space)	pathlib matches ✓	yes

Defense-in-depth: narrowing cannot open a write bypass

Every standard Python file-write path hits at least one unchanged indicator:

Write form	Catching indicator
open('x','w').write('a')	open[[:space:]]*\( (unchanged)
pathlib.Path('x').write_text('a')	`(^
from pathlib import Path; Path('x').write_text('a')	`(^
from pathlib import Path; Path('x').write_bytes(b'a')	Same as above — pathlib from the import catches it
pathlib.Path('x').write_bytes(b'a')	`(^
aliased: import pathlib as pl; pl.Path('x').write_text('a')	pathlib appears → `(^

There is no Python file-write form where:

• pathlib is absent from the command string, and
• .write( is absent (since .write_text/.write_bytes have _text/_bytes between write and (), and
• open( is absent, and
• some path( form with an alphanumeric/underscore prefix exists

The intersection of those constraints is empty — no real bypass surface was opened.

No new injection surfaces in tests

The five new test fixtures in block-hook-bypass.test.sh:48–59 pipe JSON via a here-string to the hook subprocess and assert only on exit code. The Python strings inside them are never executed by the test harness — the hook does regex matching on the command string. No injection risk introduced.

Pre-existing residuals (not introduced by this PR)

Two scope gaps are already documented in the hook's header (command substitution inside double quotes; group-level redirects). This PR neither widens nor narrows them.

Supply chain / GitHub Actions

No .github/workflows/ files modified. No new external dependencies or action pins introduced.

---

Verdict: safe to merge. The anchoring is precise, defense-in-depth is preserved, and no bypass surface is widened.

[kyle-sexton]

CI classification: all 25 checks green; claude-review verdict "Ready to merge" and claude-security-review "No security vulnerabilities found — safe to merge." Change is confined to the _py_write boundary-anchor regex + five os.path.*path( regression fixtures + CHANGELOG + semver bump. No unresolved review threads. Merging (squash).

[claude]

Claude finished @kyle-sexton's task in 1m 20s —— View job

---

Code Review (post-sync with main)

• Gather context and understand the request
• Review PR diff against origin/main
• Check what changed in the merge with main
• Post findings

---

This review covers the branch at its current state after merging origin/main (commit 42480ef). The only effect of the merge was a version re-base: main had advanced to 0.14.0 independently, so the merge conflict was resolved to 0.14.1 rather than the earlier 0.13.1. The fix and tests are otherwise unchanged from the prior two review rounds.

Version bump — correctly re-anchored after merge

File	Before merge	After merge
plugin.json	0.13.1 (on branch)	0.14.1 ✓
CHANGELOG.md	## [0.13.1] (on branch)	## [0.14.1] ✓

origin/main is at 0.14.0 (the source-control vendored-resolver bump from #1192). This fix is correctly a patch release on top of that, so 0.14.1 is the right semver. No prior review finding is invalidated by the re-base.

Regex fix — unchanged and still correct

block-hook-bypass.sh:313:

_py_write='open[[:space:]]*\(|\.write[[:space:]]*\(|(^|[^[:alnum:]_])pathlib|(^|[^[:alnum:]_])path[[:space:]]*\('

Boundary anchor is correct and unchanged. os.path.normpath( / abspath( / realpath( / relpath( / commonpath( all fail to match (alnum char before path); pathlib.Path( still matches (. before path). Defense-in-depth through the independent open\( and \.write\( indicators is preserved.

Test fixtures — unchanged and complete

block-hook-bypass.test.sh:44–59: all six fixtures (five MUST-stay-quiet *path( helpers + one MUST-block pathlib.Path().write_text) are intact and correctly cover the full set named in the PR description, including commonpath (the one gap called out in the first review round, then closed).

Verdict

The merge with main introduced only the necessary version re-base. The fix is correct, tests are complete, and the CHANGELOG accurately describes the change under the right version. Ready to merge.

[claude]

Claude finished @kyle-sexton's task in 1m 30s —— View job

---

Security Review

• Gather context and understand the request
• Read PR diff and changed files
• Audit regex anchoring for bypass surfaces
• Audit shell script changes for injection / logic flaws
• Audit test file for injection surfaces
• Check plugin manifest for supply-chain risk
• Post findings

---

No security vulnerabilities found.

The change is confined to one variable assignment in block-hook-bypass.sh:313, five regression test fixtures, a CHANGELOG entry, and a semver bump. Analysis below.

Regex narrowing introduces no bypass

Before: pathlib|path[[:space:]]*\(
After: (^|[^[:alnum:]_])pathlib|(^|[^[:alnum:]_])path[[:space:]]*\(

The check at line 470–472 applies _py_write against COMMAND_LC — the full lowercased raw command including string literals. Walking the cases that matter:

Python write form	Why still blocked
open('x','w').write('a')	open[[:space:]]*\( — unchanged, fires
pathlib.Path('x').write_text('a')	.path('x') after lowercasing — . is [^[:alnum:]_] so path( matches; pathlib also matches
from pathlib import Path; Path('x').write_text('a')	pathlib in the import string — preceded by space ([^[:alnum:]_]), matches
import pathlib as pl; pl.Path('x').write_text('a')	pathlib appears in import — matches
__import__('pathlib').Path('x').write_text('a')	'pathlib' — the ' before pathlib is [^[:alnum:]_] — matches
Path('x').write_bytes(b'a') (no explicit pathlib keyword)	Impossible in real Python — Path must be imported, and any import of Path from pathlib includes the string pathlib in the command

The boundary anchor removes the false-positive surface (normpath(, abspath(, etc. no longer match) but cannot create a write bypass because every real use of pathlib.Path requires the word pathlib to appear somewhere in the command string, and the pathlib arm of the new pattern still catches that.

\.write[[:space:]]*\( scope (pre-existing, unchanged)

This arm matches .write( but not .write_text( or .write_bytes( — that gap is why the pathlib/path( indicators are load-bearing for write_text/write_bytes. This PR does not change that design; it's correctly documented as intentional (the pathlib import is the backstop). No new gap opened.

No injection surfaces in tests

The five new fixtures in block-hook-bypass.test.sh:48–59 pass command strings as arguments to the run() harness. Those strings are passed to the hook for regex evaluation — the Python inside them is never executed by the test runner. No shell injection surface.

Plugin manifest

Version bump 0.14.0 → 0.14.1. No new external dependencies, no new action pins, no permission changes.

Pre-existing residuals (not introduced by this PR)

The hook's own header documents two scope gaps (group-level redirects, non-bare command runners like sudo/nohup). Both are unchanged and covered by accepted-floor tests.

---

Verdict: safe to merge. The anchoring is precise, defense-in-depth is preserved, and no bypass surface is widened.