fix: read a space-separated tool list, so a working fence stops auditing as ineffective (#221)

* 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>

CLAUDE.md

@@ -1,4 +1,4 @@
-<!-- vigiles:sha256:3c6c2542fa6a49c8 compiled from CLAUDE.md.spec.ts -->
+<!-- vigiles:sha256:684ae9f0b90e8b59 compiled from CLAUDE.md.spec.ts -->
 
 # CLAUDE.md
 
@@ -224,7 +224,7 @@ BUILD + TOOLING + GENERATED:
 - `src/core/frontmatter.ts` — Frontmatter-mode parser: `vigiles: enforce:` YAML frontmatter rules in markdown (Level 1 adoption)
 - `src/core/frontmatter.test.ts` — Frontmatter parser test suite (node:test)
 - `src/core/frontmatter-read.ts` — Lenient frontmatter reader — ONE reader for the SKILL.md/subagent --- block, shared by scan + the PreToolUse rail (agent-runtime), replacing three divergent hand-parsers (scan's readField +…
-- `src/core/frontmatter-read.test.ts` — Lenient-reader suite (vitest): valid YAML scalars + flow array, comma-list split, absent→null vs present-empty→[], block-scalar + next-line-quoted, malformed YAML → malformed:true AND salvages a…
+- `src/core/frontmatter-read.test.ts` — Lenient-reader suite (vitest): valid YAML scalars + flow array, the comma / space / YAML-list split (whitespace separates only outside parens, #217), absent→null vs present-empty→[], block-scalar + next-line-quoted, malformed YAML → malformed:true AND salvages a…
 - `src/core/hook-normalize.ts` — Hook settings normalization — the typed boundary (parse-don't-validate) the audit hook detectors read. normalizeHooks(raw) parses the raw settings.hooks `unknown` ONCE into a typed…
 - `src/core/hook-normalize.test.ts` — Hook-normalize suite (vitest): flattens the CC nested shape, reads the Codex flat shape, carries matcher null when absent, drops empty/non-string commands + non-object entries, returns [] for…
 - `action.yml` — GitHub Action — a composite action over the published `npx vigiles` CLI (NOT a node20 entry pointing at an uncommitted dist/): maps every input to a real CLI flag, sets the `valid` output via $GITHUB_OUTPUT, and supports `version: local` so the repo dogfoods it via `uses: ./`. See docs/cli.md and the `prod-grade-gha-cli` rule.

CLAUDE.md.spec.ts

@@ -345,7 +345,7 @@ BUILD + TOOLING + GENERATED:
     "src/core/frontmatter-read.ts":
       "Lenient frontmatter reader — ONE reader for the SKILL.md/subagent --- block, shared by scan + the PreToolUse rail (agent-runtime), replacing three divergent hand-parsers (scan's readField +…",
     "src/core/frontmatter-read.test.ts":
-      "Lenient-reader suite (vitest): valid YAML scalars + flow array, comma-list split, absent→null vs present-empty→[], block-scalar + next-line-quoted, malformed YAML → malformed:true AND salvages a…",
+      "Lenient-reader suite (vitest): valid YAML scalars + flow array, the comma / space / YAML-list split (whitespace separates only outside parens, #217), absent→null vs present-empty→[], block-scalar + next-line-quoted, malformed YAML → malformed:true AND salvages a…",
     "src/core/hook-normalize.ts":
       "Hook settings normalization — the typed boundary (parse-don't-validate) the audit hook detectors read. normalizeHooks(raw) parses the raw settings.hooks `unknown` ONCE into a typed…",
     "src/core/hook-normalize.test.ts":

docs/rules/disallowed-tools-contract.md

@@ -49,8 +49,10 @@ The block-list **inverts** the allow check, so the FP-safe set is different from
 
 ## Scope
 
-Subagent frontmatter (`agents/*.md` `disallowedTools:`), both the comma-list and
-inline-array forms.
+Subagent frontmatter (`agents/*.md` `disallowedTools:`) in every spelling Claude
+Code accepts — a comma-separated string, a space-separated string, or a YAML
+list. Whitespace separates tokens only outside parentheses, so a bounded grant
+like `Bash(git push *)` stays one entry.
 
 > **Not the same field as a skill's `disallowed-tools:`.** A skill's fence is
 > kebab-case and lives in `SKILL.md`; it is read by

src/adapters/claude-code/agent-runtime.test.ts

@@ -964,3 +964,21 @@ test("agent-hook CLI: PreToolUse(Task) with an unknown subagent activates nothin
     cleanupTmpDir(dir);
   }
 });
+
+test("a SPACE-separated tools: contract grants those tools at the rail (#217)", () => {
+  // The second, worse half of #217, on the surface where the contract is
+  // ENFORCED rather than reported. A skill's `allowed-tools:` only pre-approves,
+  // so a mis-split there misleads an audit; a SUBAGENT's `tools:` is the
+  // allowlist this rail denies against — so when the space-separated spelling
+  // arrived as the single token "Read Grep Glob", `includes(tool)` matched
+  // nothing and the agent was denied EVERY tool, including the three it was
+  // explicitly granted. Measured before the fix: Read DENY, Grep DENY, Bash DENY.
+  const md =
+    "---\nname: reader\ndescription: reads\ntools: Read Grep Glob\n---\nbody\n";
+  const allowed = parseAgentTools(md);
+  assert.deepEqual(allowed, ["Read", "Grep", "Glob"]);
+  assert.equal(decidePreToolUse(allowed, "Read").allow, true);
+  assert.equal(decidePreToolUse(allowed, "Grep").allow, true);
+  // …and the fence still holds for what was NOT granted.
+  assert.equal(decidePreToolUse(allowed, "Bash").allow, false);
+});

src/core/frontmatter-read.test.ts

@@ -29,6 +29,97 @@ test("a comma-list tool value splits", () => {
   assert.deepEqual(frontmatterList(fm, "tools"), ["Read", "Grep", "Bash"]);
 });
 
+test("every separator Claude Code documents splits the same way (#217)", () => {
+  // Claude Code accepts a comma-separated string, a SPACE-separated string, or a
+  // YAML list, and treats all three as equivalent. Until #217 only two of the
+  // three worked here: a space-separated fence arrived as one token matching no
+  // built-in, so a fence the harness really enforces audited as closing nothing.
+  // Reported with this table by @vlad-ryzhkov, measured against a real harness.
+  const read = (v: string): string[] | null =>
+    frontmatterList(
+      readFrontmatter(`---\nname: a\ndisallowed-tools: ${v}\n---\n`),
+      "disallowed-tools",
+    );
+  assert.deepEqual(read("WebFetch, WebSearch"), ["WebFetch", "WebSearch"]);
+  assert.deepEqual(read("WebFetch WebSearch"), ["WebFetch", "WebSearch"]);
+  assert.deepEqual(read("WebFetch,  WebSearch ,Bash"), [
+    "WebFetch",
+    "WebSearch",
+    "Bash",
+  ]);
+  // A YAML-quoted scalar whose VALUE is a space-separated list — common in the
+  // wild, and the shape that made this bug invisible: the quotes are YAML syntax,
+  // not token boundaries, so they are stripped and the value still splits.
+  assert.deepEqual(read('"Read Write Glob"'), ["Read", "Write", "Glob"]);
+});
+
+test("whitespace splits ONLY outside parens, so a bounded grant survives (#217)", () => {
+  // The half that makes the naive `split(/[,\s]+/)` wrong: `Bash(git push *)` must
+  // stay ONE token. Shredded, it matches no grant, and `bashGrantIsUnbounded()`
+  // answers "unbounded" when it cannot recognise one — so the fix for a
+  // false-CLEAN verdict would have bought a false-EXPOSED one.
+  const read = (v: string): string[] | null =>
+    frontmatterList(
+      readFrontmatter(`---\nname: a\nallowed-tools: ${v}\n---\n`),
+      "allowed-tools",
+    );
+  assert.deepEqual(read("Bash(git add *) Bash(git commit *)"), [
+    "Bash(git add *)",
+    "Bash(git commit *)",
+  ]);
+  assert.deepEqual(read("Bash(git push *), WebFetch"), [
+    "Bash(git push *)",
+    "WebFetch",
+  ]);
+  assert.deepEqual(read("[Read, Grep]"), ["Read", "Grep"]);
+  // An UNCLOSED paren keeps the rest as one token rather than fragmenting it into
+  // garbage tool names — the conservative side of a shape we cannot parse.
+  assert.deepEqual(read("Bash(git push Read"), ["Bash(git push Read"]);
+});
+
+test("a parenthesis INSIDE a quoted string is a character, not a bracket (#217)", () => {
+  // Found by the Codex review bot on the first cut of this tokenizer. A literal
+  // unmatched paren inside a grant's own command left depth at 1 after the grant
+  // closed, so every following token was swallowed into it — and on a SUBAGENT
+  // that denies a tool the author granted, the exact failure this function
+  // exists to fix. Measured before the fix: one token, `Read` lost.
+  const read = (v: string): string[] | null =>
+    frontmatterList(
+      readFrontmatter(`---\nname: a\ntools: ${v}\n---\n`),
+      "tools",
+    );
+  assert.deepEqual(read("Bash(printf '( %s' foo) Read"), [
+    "Bash(printf '( %s' foo)",
+    "Read",
+  ]);
+  // Balanced parens inside quotes self-corrected even before the fix; pinned so
+  // the common `git commit -m "feat(api): …"` shape cannot regress either.
+  assert.deepEqual(read('Bash(git commit -m "feat(api): x") Read'), [
+    'Bash(git commit -m "feat(api): x")',
+    "Read",
+  ]);
+  // 🔴 THE COUNTERWEIGHT, and why quotes gate ONLY the depth counter: if being
+  // inside quotes also suppressed SPLITTING, a quoted list would collapse back
+  // to one token — reintroducing the very bug #217 reports.
+  //
+  // It must be asserted on a MALFORMED block, and that is the whole point. On
+  // valid YAML js-yaml strips the quotes before `splitList` ever runs, so the
+  // value arrives as bare `Read Write Glob` and NO quote character is present —
+  // measured, after the first version of this assertion used a valid block and
+  // stayed green under the widening mutation, carrying zero information. Quotes
+  // reach the splitter only down the regex SALVAGE path, so that is where the
+  // property lives. (`desc:` below carries an unescaped `: ` — invalid YAML.)
+  const salvaged = frontmatterList(
+    readFrontmatter(
+      '---\nname: a\ndesc: Use the foo: bar tool\ntools: "Read Write Glob"\n---\n',
+    ),
+    "tools",
+  );
+  assert.deepEqual(salvaged, ["Read", "Write", "Glob"]);
+  // …and the same value on the valid-YAML path, where the quotes are gone by then.
+  assert.deepEqual(read('"Read Write Glob"'), ["Read", "Write", "Glob"]);
+});
+
 test("absent list key → null (inherits all); present-but-empty → [] (no tools)", () => {
   const none = readFrontmatter("---\nname: a\n---\n");
   assert.equal(frontmatterList(none, "tools"), null);

src/core/frontmatter-read.ts

@@ -146,11 +146,79 @@ function salvageList(block: string, key: string): string[] | null {
   return splitList(match[1]);
 }
 
-/** Split a comma list or inline-array string into trimmed, de-quoted tokens. */
+/**
+ * Split a tool-list string into trimmed, de-quoted tokens.
+ *
+ * WHITESPACE IS A SEPARATOR, BUT ONLY AT PAREN DEPTH 0 — and that qualifier is
+ * the whole rule. Claude Code documents three equivalent spellings of
+ * `allowed-tools:`/`disallowed-tools:` (a comma-separated string, a
+ * SPACE-separated string, a YAML list), and its own documented example is
+ * space-separated with spaces INSIDE the tokens:
+ *
+ *   allowed-tools: Bash(git add *) Bash(git commit *) Bash(git status *)
+ *
+ * Until #217 this split on `,` alone, so a space-separated fence arrived as ONE
+ * token that matches no built-in name. That failed in the direction that misleads:
+ * a fence the harness really enforces was reported as closing no lethal-trifecta
+ * leg, so an author who wrote a correct fence was told it did nothing. Reported
+ * with a reproduction by @vlad-ryzhkov, measured on a 22-skill harness where the
+ * separator alone moved Safety 81 → 80 → 81.
+ *
+ * THE NAIVE FIX IS WORSE THAN THE BUG, which is why this is a tokenizer and not
+ * `split(/[,\s]+/)`: that shreds `Bash(git push *)` into `Bash(git`, `push`, `*)`,
+ * and since `bashGrantIsUnbounded()` answers "unbounded" when it cannot recognise
+ * a grant, a BOUNDED grant would start reading as an unbounded one. That is the
+ * false-exposed side of the trade `core/lethal-trifecta.ts` already reasons about
+ * — a scarier verdict for a reason the author cannot see anywhere in their file.
+ *
+ * Quotes are stripped per token AFTER splitting, never treated as a delimiter:
+ * `allowed-tools: "Read Write Glob"` is a YAML-quoted scalar whose VALUE is a
+ * space-separated list, and Claude Code splits it. Treating the quotes as token
+ * boundaries would keep exactly the bug this fixes for every skill that quotes
+ * its list.
+ */
 function splitList(raw: string): string[] {
-  return raw
-    .replace(/^\[|\]$/g, "")
-    .split(",")
-    .map((t) => t.trim().replace(/^["']|["']$/g, ""))
-    .filter((t) => t.length > 0);
+  const out: string[] = [];
+  let token = "";
+  let depth = 0;
+  let quote: '"' | "'" | null = null;
+  let escaped = false;
+  const flush = (): void => {
+    const t = token.trim().replace(/^["']|["']$/g, "");
+    if (t.length > 0) out.push(t);
+    token = "";
+  };
+  for (const ch of raw.trim().replace(/^\[|\]$/g, "")) {
+    if (escaped) {
+      escaped = false;
+      token += ch;
+      continue;
+    }
+    if (ch === "\\" && quote !== "'") {
+      escaped = true;
+      token += ch;
+      continue;
+    }
+    if (quote === null && (ch === '"' || ch === "'")) quote = ch;
+    else if (quote === ch) quote = null;
+    // Depth tracks STRUCTURE, so a parenthesis inside a quoted string is a
+    // character, not a bracket. Without this, `Bash(printf '( %s' foo) Read`
+    // leaves depth at 1 after the grant closes and swallows `Read` into the
+    // same token — on a subagent that denies a tool the author granted, which
+    // is the exact failure this function exists to fix.
+    else if (quote === null) {
+      if (ch === "(") depth++;
+      else if (ch === ")") depth = Math.max(0, depth - 1);
+    }
+    // Quotes deliberately do NOT suppress splitting: `allowed-tools: "Read
+    // Write Glob"` is a YAML-quoted scalar whose VALUE is a space-separated
+    // list, and Claude Code splits it. The quotes come off per token below.
+    if (depth === 0 && (ch === "," || /\s/.test(ch))) {
+      flush();
+      continue;
+    }
+    token += ch;
+  }
+  flush();
+  return out;
 }

src/scan.test.ts

@@ -1745,6 +1745,23 @@ test("a disallowed-tools that CLOSES a leg clears the finding", () => {
   cleanupTmpDir(dir);
 });
 
+test("the SPACE-separated spelling of that same fence also clears it (#217)", () => {
+  const dir = makeTmpDir("scan-trifecta-fenced-spaces");
+  // Byte-for-byte the fence above with the commas removed — the spelling Claude
+  // Code documents and honours. It used to read as ONE token named
+  // "WebFetch WebSearch Bash", matching no built-in, so every leg still counted as
+  // supplied and the author was told their working fence closed nothing (#217).
+  write(
+    dir,
+    "skills/fenced/SKILL.md",
+    "---\nname: fenced\ndescription: A model-invocable skill that fences off the network entirely\ndisallowed-tools: WebFetch WebSearch Bash\n---\n# fenced\n",
+  );
+  const r = scanPlugin(dir);
+  assert.equal(r.skills.find((s) => s.name === "fenced")?.trifecta, null);
+  assert.equal(r.trifectaFindings.length, 0);
+  cleanupTmpDir(dir);
+});
+
 test("a PARTIAL disallowed-tools closes no leg and names the suppliers still standing", () => {
   const dir = makeTmpDir("scan-trifecta-partial-fence");
   // Denying `Read` alone leaves Grep/Glob/Bash on the private-data leg — an author