Commit graph

3 commits

Author SHA1 Message Date
fa2404b63c fix(hooks): close the bypasses anchoring opened in the destructive-command rule
Review finding 4638fea9 (MAJOR emitted, catalogue tier BLOCKER). Live in every
session regardless of the STORM flag, so it is not deferrable to the measurement
decision.

Anchoring the rule to command position (S75-S78) was right in intent - the
unanchored word match blocked quoted grep patterns, heredoc data and commit
messages - but it ran against the whitespace-collapsed string. Measured against
normalizeCommand() output, five forms that the previous rule blocked were
allowed:

- newline separator: \s+ -> ' ' collapsed the newline BEFORE the pattern ran,
  making the \n branch of the separator class dead code
- `&` background separator: absent from the class entirely
- `bash -c` / `sh -c`: the wrapped command sits inside quotes, never at
  command position
- `xargs <cmd>`: no separator in front of the command at all

And it missed its own motivating case: `grep "a|b" f` stayed blocked, because
the `|` inside the quotes still read as a separator.

Fix: the rule now runs against a command-position view (`commandView: true`,
per-rule input selection) instead of the collapsed string. The view keeps
newlines, adds `&` to the separator class, and classifies each span:

- quoted spans -> data (one space), so a grep alternation, echoed prose and a
  commit message pass
- EXCEPT the argument of a shell wrapper (`sh -c`, `bash -c`, with optional
  sudo and absolute path) -> spliced back in at command position
- heredoc bodies -> data, keeping the operator line. Restoring the newline
  separator without this would newly block every heredoc line starting with a
  matched word - the exact friction anchoring existed to remove
- `xargs [flags]` -> separator inserted after the flags

Two defects found while verifying, both the same regression class and both
fixed here rather than left:

- `\name` runs name (the backslash only suppresses alias expansion). The old
  unanchored rule blocked it; the anchored one allowed it.
- heredoc bodies, as above - a false positive this change would otherwise have
  introduced.

Known limit, stated rather than implied: `xargs -I {} <cmd>` is not parsed, so
the inserted separator lands before the argument, not the command.

Other BLOCK rules are untouched and still run against the collapsed string.

Verified by a 37-case adversarial probe through the real hook (all five bypass
forms, both wrapper forms, backslash, heredoc, quoted alternation, ordinary
commands, and the unrelated rules): 37/37 as expected. 9 new tests.

Suite 952 (950/0/2, baseline 937 + 15 across both Track A fixes).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013zNqxP8qTWgJhn3wMYUFEh
2026-08-12 22:03:22 +02:00
e1cf545a0c fix(hooks): anchor shutdown rule to command position, not any substring
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-09 13:02:01 +02:00
14c3ff858f test(ultraplan-local): add path-guard + bash-guard baseline hook tests (SC8 baseline)
Pins existing BLOCK rules in the two pre-* executor hooks so a future
silent weakening of BLOCK_RULES surfaces as test failures instead of
slipping through code review.

50 new tests covering both hooks plus allow-list pins (lib/, tests/,
docs/, ls, git, npm) and fail-open on malformed input. Reuses
tests/helpers/hook-helper.mjs child-process spawner.

[skip-docs]
2026-05-04 08:55:49 +02:00