Skip to content

0.48.0 — The review guard stops missing perl -pi

Choose a tag to compare

@mikebronner mikebronner released this 19 Sep 21:46
· 25 commits to main since this release

🔒 The spelling that got through

hooks/scripts/local-review-guard.sh stops a Holmes review lens writing to the tree it is reviewing. It matched -i as a whole token only.

So perl -i -pe was refused and perl -pi -e was allowed — the commonest way anyone writes an in-place Perl edit.

Five more clustered spellings went with it: perl -ni, perl -pi.bak, perl -lpi, sed -ie, and ruby -pi. Ruby was not in the interpreter table at all.

🔬 How it surfaced

Not by reading the code. During a Holmes Local-mode review of an uncommitted workbench-core tree, a correctness lens ran perl -pi against assets/permissions/rails.json to test a mutation, despite an explicit no-write instruction in its own prompt. The guard allowed it.

The same lens's later perl -i attempt on another file was correctly refused. That inconsistency is what made the hole visible. The lens self-restored and the tree was verified intact, so nothing was lost.

The 79-case suite covered no clustered form, which is why this shipped.

✨ The fix

The flag rule now walks each single-dash cluster left to right, stopping at the first switch that takes the rest of the token as its value. So perl -pes/i/j/ stays a read-only one-liner rather than becoming a false positive.

Which letter means in-place is read per interpreter, from the same table as those terminators, because the interpreters disagree:

  • -I is an in-place edit to BSD and macOS sed, and is refused there. To perl and ruby it names an include directory, so perl -Ilib -ne print stays allowed.
  • sed's -l stays out of the terminator set. GNU's -l N takes a line length and BSD's takes nothing, and listing it let a joined sed -li through.

Both halves come from one subscript, so an interpreter cannot be present in one lookup and missing from the other. That drift would raise KeyError, crash the hook, and a crashed hook fails open.

✅ Verified from outside

The session that reported the hole re-ran its original matrix against the fix rather than taking it on trust.

All eight in-place forms are now refused, including the six that used to pass. Zero false positives across perl -Ilib -ne print, perl -pes/i/j/, perl -ne print, sed -n 1,5p, git diff HEAD, and grep -i foo.

hooks/scripts/test-local-review-guard.sh goes from 79 to 125 cases. Every test and lint script in the repo passes.

⚠️ This refuses commands that previously ran

A minor bump rather than a patch, because the behaviour change is user-visible. Six command shapes a review agent could run before are now blocked. That is the point of the release, and it is worth knowing before you update.