mirror of
https://github.com/zernie/vigiles.git
synced 2026-09-14 20:53:57 +08:00
zernie
c4e75ae538
* fix: read a space-separated tool list, so a working fence stops auditing as ineffective Claude Code documents three equivalent spellings of `allowed-tools:` / `disallowed-tools:` / a subagent's `tools:` — a comma-separated string, a SPACE-separated string, and a YAML list. `splitList` in core/frontmatter-read.ts split on `,` alone, so the space-separated form arrived as ONE token matching no built-in name. Reported with a reproduction by @vlad-ryzhkov in #217: on a 22-skill harness, changing only the separator moved Safety 81 -> 80 -> 81, reversibly. Reproduced here before changing anything — three of the seven documented shapes came back as a single token: `WebFetch WebSearch`, `Bash(git add *) Bash(git commit *)`, and `"Read Write Glob"` (a YAML-quoted scalar whose VALUE is a space-separated list, which is what most skills in the wild write). TWO SURFACES, AND THE SECOND IS WORSE THAN THE REPORTED ONE. #217 asks about a skill's fence, where `allowed-tools:` only pre-approves, so a mis-split MISLEADS: a fence Claude Code really enforces was reported as closing no lethal-trifecta leg, naming as still-supplied the very tools the author had just denied. But a SUBAGENT's `tools:` is the allowlist the PreToolUse rail denies against, and it reads the same `splitList`. Measured on `tools: Read Grep Glob`, pre-fix: Read DENY · Grep DENY · Bash DENY — the joined token matched nothing, so the agent was denied EVERY tool, including the three it was explicitly granted. #217 raised this path as an open question ("I have not worked out whether anything downstream depends on that"); it did. THE NAIVE FIX IS WORSE THAN THE BUG. `split(/[,\s]+/)` shreds `Bash(git push *)` into `Bash(git`, `push`, `*)`, and `bashGrantIsUnbounded()` answers "unbounded" when it cannot recognise a grant — so a BOUNDED grant would begin reading as an unbounded one. That trades a false-clean verdict for a false-exposed one, which is the worse side of the trade core/lethal-trifecta.ts already reasons about, for a reason the author cannot see anywhere in their file. So this is a tokenizer: whitespace separates only at paren depth 0. Quotes are stripped per token AFTER splitting, never treated as a delimiter — the outer quotes of `"Read Write Glob"` are YAML syntax, and treating them as token boundaries would keep exactly this bug for every skill that quotes its list. An unclosed `(` keeps the remainder as one token rather than fragmenting it into garbage tool names. The tokenizer is the one @vlad-ryzhkov proposed, with his six-row table plus irregular spacing and an unbalanced paren as tests. Every new test is mutation-proven with the patch verified landed: separator back to `,` only (pre-fix) -> the 4 new tests fail whitespace splits at ANY depth (the naive fix) -> the parens test fails One home, so the surfaces cannot drift: `frontmatterList` is the single reader behind scan, adopt, and the rail. Closes #217 Co-Authored-By: vlad-ryzhkov <211849729+vlad-ryzhkov@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: a parenthesis inside a quoted string is a character, not a bracket Follow-up within #217, found by the Codex review bot on the first cut of the tokenizer. The depth counter read every `(` as structure, so a grant carrying a literal unmatched paren in its own command left depth at 1 after the grant closed and swallowed everything after it: tools: Bash(printf '( %s' foo) Read -> ["Bash(printf '( %s' foo) Read"] one token On a subagent that is the same failure this PR exists to fix: the rail denies `Read`, a tool the author granted. Reproduced before changing anything. Quote state now gates the DEPTH COUNTER ONLY, never the splitting. That asymmetry is the fix, not an accident: if being inside quotes also suppressed whitespace splitting, a quoted list would collapse back to one token and reintroduce #217 itself for every skill that writes `allowed-tools: "Read Write Glob"`. Backslash escapes are honoured outside single quotes, per shell rules. MUTATION-PROVEN BOTH WAYS, each with the patch verified landed: depth counted inside quotes (the pre-fix state) -> the new test fails quotes suppress splitting too (the over-fix) -> the new test fails 🔴 AND THE SECOND MUTATION EXPOSED A DEFECT IN THE TEST FIRST. Its original counterweight asserted the quoted list on a VALID YAML block and stayed GREEN under the widening mutation — because js-yaml strips the quotes before `splitList` ever runs, so no quote character was present and the mutation could not bite. Measured, then moved onto a MALFORMED block, where the regex salvage path hands the quotes through: that is the only path on which the property is observable at all. A green mutation is a finding about the test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: vlad-ryzhkov <211849729+vlad-ryzhkov@users.noreply.github.com>