diff --git a/.changeset/jolly-orcas-snooze.md b/.changeset/jolly-orcas-snooze.md new file mode 100644 index 000000000..319a7664b --- /dev/null +++ b/.changeset/jolly-orcas-snooze.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1638 +--- +**Capability `settings.json` hooks no longer fire on every tool and no longer fail when non-executable** — installing a capability that declared a tool-scoped `PreToolUse`/`PostToolUse` hook wrote the entry with no `matcher`, so a guard intended for only `Write|Edit` fired on every tool call (including `Bash`) and a fail-closed guard could block the whole session; the emitted command was also a bare script path, so a `.js`-family hook delivered via `git`/tarball that lost the executable bit failed with `Permission denied` on every matching call. Install now honors a declared `matcher` (absent = match-all, unchanged for existing capabilities) and emits a `node`-prefixed command for `.js`/`.cjs`/`.mjs` hooks so they run regardless of file-mode bits. (#1634) diff --git a/CONTEXT.md b/CONTEXT.md index ebb9f8344..a61a10534 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -722,6 +722,12 @@ A legal deferred state of an Execute step (`external_job_waiting`): the executor `DEFECT.WINDOWS-TEST-PORTABILITY.fix-forward=gate platform-specific execution with if (process.platform !== 'win32'); normalize path expectations to forward slashes with .replace(/\\/g, '/'); invoke scripts via explicit interpreter (sh ) rather than relying on exec-bit; annotate // windows-portability-ok: when a bypass is intentional` `DEFECT.WINDOWS-TEST-PORTABILITY.prevention=run lint:ci before opening a PR; treat the CI windows lane as the only true Windows signal — gsd-test (Mac/Linux only) cannot substitute for it` +`DEFECT.WINDOWS-POSIX-MODE-BIT-ASSERT.symptom=a test writes a file with a POSIX mode (fs.writeFileSync(p, data, {mode: 0o644}) or fs.chmodSync) then asserts fs.statSync(p).mode & 0o777 === ; passes on macOS/Linux/ubuntu CI, FAILS on the windows-latest CI lane — Windows fs does NOT honor POSIX write modes, Node reports the mode derived from the DOS readonly attribute (0o666 for writable / 0o444 for readonly), never the requested 0o644/0o755` +`DEFECT.WINDOWS-POSIX-MODE-BIT-ASSERT.examples=#1634/PR #1638 tests/capability-lifecycle.test.cjs "a .cjs hook command is node-prefixed so it runs without the executable bit" failed windows-latest,24 on "precondition: file staged without +x" (expected 420/0o644, got 438/0o666); the node-prefix behavioral assertion was correct — only the mode-bit precondition was the POSIX-only fact` +`DEFECT.WINDOWS-POSIX-MODE-BIT-ASSERT.detect=grep tests for \`.mode & 0o777\` / \`.mode) === 0o\` / \`writeFileSync(...{ mode: 0o\` / \`chmodSync\` paired with a strict-equality assertion on the resulting mode; any such assertion is a POSIX-only fact that will diverge on Windows (write reads back as 0o666)` +`DEFECT.WINDOWS-POSIX-MODE-BIT-ASSERT.fix-forward=gate the mode-bit precondition on if (process.platform !== 'win32') — the executable-bit/mode is a POSIX concept meaningless on Windows; KEEP the platform-independent behavioral assertion (the actual behavior under test) running on every OS; do NOT delete the precondition, scope it to POSIX` +`DEFECT.WINDOWS-POSIX-MODE-BIT-ASSERT.prevention=ref DEFECT.WINDOWS-TEST-PORTABILITY — gsd-test is Mac/Linux only (no Windows host), only the CI windows-latest lane catches this; run npm run lint:ci (lint-windows-test-portability) before push; prefer asserting the BEHAVIOR (command shape, runnability) over the filesystem mode bit` + `DEFECT.WINDOWS-PATH-LEAK-IN-MARKDOWN-CONTENT.symptom=path.join() result on Windows (backslashes) substituted verbatim into markdown body (@-references, workflow files, generated docs); content gains mixed separators; cross-platform substring assertions fail on windows-latest CI lane only; macOS/Linux CI green so defect ships undetected` `DEFECT.WINDOWS-PATH-LEAK-IN-MARKDOWN-CONTENT.examples=PR #1622 computePathPrefix returned ${resolvedTarget}/ verbatim — rewrites of @~/.claude/gsd-core/commands/gsd/X.md wrote @C:\...\gsd-ial-windsurf-XXX\gsd-core/commands/gsd/help.md (trailing forward slashes from the original literal survived, prefix backslashes did not); tests/install-runtime-artifacts.test.cjs:318 + tests/install.test.cjs:1323 failed on windows-latest only` `DEFECT.WINDOWS-PATH-LEAK-IN-MARKDOWN-CONTENT.detect=any function returning a filesystem path that flows into markdown/text body substitution; grep for path.join/raw resolvedTarget/${configDir}/ in code paths writing workflow .md, agent .md, or generated docs; smoke pattern is ${resolvedTarget}/ or ${configDir}/... templates that bypass normalization` diff --git a/docs/adr/894-capability-declaration-format.md b/docs/adr/894-capability-declaration-format.md index 45ae203ff..6e480ab24 100644 --- a/docs/adr/894-capability-declaration-format.md +++ b/docs/adr/894-capability-declaration-format.md @@ -48,7 +48,7 @@ Schema-validated JSON. Common envelope + role-typed body (`role: feature | runti | Field | Type | Notes | |---|---|---| | `skills` / `agents` | string[] | owned stems — exactly one owner each across all capabilities | -| `hooks` | `{event, script}[]` | lifecycle hooks | +| `hooks` | `{event, script, matcher?}[]` | lifecycle hooks; optional `matcher` is a settings.json tool-scoping pattern (exact tool name, pipe-separated list, wildcard, or regex, e.g. `Write|Edit`); absent = match-all (see *#1634 amendment* below) | | `config` | object | federated config-key schema slice | | `steps` / `contributions` / `gates` | arrays | loop hooks (below) | @@ -204,6 +204,8 @@ This ADR was stress-tested in two rounds before merge; the format changed materi 5. **Hook activation `when`** — declarative config-level gating; deeper context applicability self-gates in the skill (no phase-context vocabulary). 6. **`byLoopPoint` ordering materialized** in the registry; resolver filters active + renders. Same-capability hooks must degrade gracefully when an entry step self-gates. +**Amendment — #1634 (lifecycle hook `matcher`):** the `role: "feature"` `hooks[]` entry gained an optional `matcher` field. `matcher` is a settings.json tool-scoping pattern — exact tool name, pipe-separated list, wildcard, or regex (e.g. `Write|Edit`). The capability install path projects a declared `matcher` onto the emitted settings.json hook entry (an entry-level sibling of `hooks`, exactly matching the runtime's native shape); **absent means match-all** — the field is omitted, so the shipped capabilities' wiring is byte-for-byte unchanged. This closes #1634, where a tool-scoped `PreToolUse`/`PostToolUse` hook otherwise fired on every tool (a fail-closed guard could block the whole session). `matcher` is a settings.json-family concept; per-runtime matcher projection (ADR-857 D8, runtimes-as-descriptors) is deliberately left as a separate concern rather than baking a raw Claude regex into every runtime's projection. The validator gates `matcher` to a non-empty string with no control characters. + ## Consequences **Positive** diff --git a/gsd-core/bin/lib/capability-validator.cjs b/gsd-core/bin/lib/capability-validator.cjs index 1f1a08b2f..6f3cad16b 100644 --- a/gsd-core/bin/lib/capability-validator.cjs +++ b/gsd-core/bin/lib/capability-validator.cjs @@ -582,6 +582,27 @@ function validateFeatureBody(cap) { 'absolute path, or ".." segment) — it contains unsafe characters: ' + JSON.stringify(h.script), ); } + // #1634: optional tool-scoping `matcher` (a settings.json concept — exact tool name, + // pipe-separated list, wildcard, or regex; e.g. "Write|Edit"). When present it must be a + // non-empty string without control characters; absent => match-all (omitted at projection + // so existing shipped capabilities are unchanged). + if (h.matcher !== undefined) { + if (typeof h.matcher !== 'string' || h.matcher.length === 0) { + errors.push('hooks[' + i + '].matcher must be a non-empty string when present'); + } else { + // Reject ASCII control characters (0x00-0x1f and 0x7f DEL) via char codes — a literal + // control-char range regex trips ESLint's no-control-regex rule, and char codes are + // equally precise. + let hasControl = false; + for (let c = 0; c < h.matcher.length; c++) { + const code = h.matcher.charCodeAt(c); + if (code < 0x20 || code === 0x7f) { hasControl = true; break; } + } + if (hasControl) { + errors.push('hooks[' + i + '].matcher must not contain control characters'); + } + } + } } } } diff --git a/src/capability-lifecycle.cts b/src/capability-lifecycle.cts index a590f1f12..c6a258334 100644 --- a/src/capability-lifecycle.cts +++ b/src/capability-lifecycle.cts @@ -414,6 +414,21 @@ function shellSingleQuote(value: string): string { return "'" + value.replace(/'/g, "'\\''") + "'"; } +/** + * #1634: build the emitted hook `command` for an ABSOLUTE confined script path. For `.js`-family + * hooks (`.js`/`.cjs`/`.mjs`) prefix with `node` so the hook runs regardless of the source's + * executable bit — a `git`/tarball source that lost `+x` would otherwise yield + * `/bin/sh: Permission denied` on every matching call (defect #2). This mirrors first-party hooks + * (`node "${CLAUDE_PLUGIN_ROOT}/hooks/x.js"`). The path stays POSIX single-quoted (#1460 (R) HIGH) + * so a space-containing install prefix cannot word-split or inject. Non-JS scripts (e.g. `.sh`) + * keep the bare single-quoted absolute path (unchanged) — they remain responsible for their own + * executability, exactly as before; per-runtime command projection is a separate concern (ADR-857 D8). + */ +const JS_HOOK_EXT_RE = /\.(?:js|cjs|mjs)$/; +function runnableHookCommand(absScript: string): string { + return JS_HOOK_EXT_RE.test(absScript) ? 'node ' + shellSingleQuote(absScript) : shellSingleQuote(absScript); +} + /** * #1460 CONF-1: resolve a hook `script` (declared RELATIVE to the bundle) against the capability's * own install dir and CONFINE it via realpath, returning the ABSOLUTE confined path or null when it @@ -609,6 +624,11 @@ function applyCapabilitySharedEdits(args: { const event = typeof rec['event'] === 'string' ? rec['event'] : ''; const script = typeof rec['script'] === 'string' ? rec['script'] : ''; if (!event || !script || isUnsafeKey(event)) continue; + // #1634: optional tool-scoping `matcher` (a settings.json concept — entry-level sibling of + // `hooks`). Absent => match-all (field OMITTED so the existing shipped capabilities' wiring + // is byte-for-byte unchanged, Hyrum's Law). The validator gates this to a non-empty string. + const matcherRaw = rec['matcher']; + const matcher = typeof matcherRaw === 'string' && matcherRaw.length > 0 ? matcherRaw : null; // #1460 CONF-1: resolve the declared (relative) script against the capability's OWN install // dir and CONFINE via realpath, then write the ABSOLUTE confined path as the hook command — // never the raw relative path (which would resolve against the CWD at hook-exec time and could @@ -616,14 +636,17 @@ function applyCapabilitySharedEdits(args: { // through a symlinked subdir) return null and are SKIPPED, exactly as before. const absScript = confinedBundleScript(capDir(runtimeDir, capId), script); if (absScript === null) continue; - // #1460 (R) HIGH: the hook `command` is consumed by a shell (first-party hooks emit - // `node "${CLAUDE_PLUGIN_ROOT}/hooks/x.js"`). The absolute path begins with the - // (non-manifest) install-prefix, which commonly contains spaces — emit it POSIX - // single-quoted so the prefix cannot word-split or inject. The script BASENAME is - // already restricted to a shell-safe allowlist by isSafeHookScriptPath above. - const command = shellSingleQuote(absScript); + // #1460 (R) HIGH + #1634: the hook `command` is consumed by a shell. `runnableHookCommand` + // emits a `node`-prefixed POSIX-single-quoted absolute path for `.js`-family hooks (runs + // without `+x`; mirrors first-party) and a bare single-quoted path otherwise. Single-quoting + // keeps a space-containing install prefix as one shell token (cannot word-split or inject). + const command = runnableHookCommand(absScript); const arr = Array.isArray(hooksObj[event]) ? (hooksObj[event] as unknown[]) : []; - arr.push({ [CAP_MARKER]: capId, hooks: [{ type: 'command', command }] }); + // #1634: stamp the marker so the entry is surgically strippable, and carry the declared + // `matcher` (entry-level sibling of `hooks`) only when the author declared one. + const entry: Record = { [CAP_MARKER]: capId, hooks: [{ type: 'command', command }] }; + if (matcher !== null) entry['matcher'] = matcher; + arr.push(entry); hooksObj[event] = arr; touched = true; } diff --git a/tests/capability-lifecycle.test.cjs b/tests/capability-lifecycle.test.cjs index 960b64caf..d1b649b55 100644 --- a/tests/capability-lifecycle.test.cjs +++ b/tests/capability-lifecycle.test.cjs @@ -307,7 +307,7 @@ test('upgrade: a changed executable set without consent aborts and leaves the OL assert.strictEqual(capManifestVersion(dir, 'e'), '1.0.0', 'old bundle untouched'); assert.strictEqual(readLedgerEntry(dir, 'e').version, '1.0.0', 'old ledger untouched'); // #1460 CONF-1/(R): command is the ABSOLUTE confined path inside the bundle (POSIX single-quoted), not the raw relative form. - assert.strictEqual(readSettings(dir).hooks.PostToolUse[0].hooks[0].command, shQuote(expectedBundleCommand(dir, 'e', 'hooks/a.js'))); + assert.strictEqual(readSettings(dir).hooks.PostToolUse[0].hooks[0].command, nodeQuoted(expectedBundleCommand(dir, 'e', 'hooks/a.js'))); }); test('upgrade: a changed executable set WITH consent upgrades and re-derives shared edits', async () => { @@ -324,7 +324,7 @@ test('upgrade: a changed executable set WITH consent upgrades and re-derives sha const hooks = readSettings(dir).hooks.PostToolUse; assert.strictEqual(hooks.length, 1); // #1460 CONF-1/(R): re-derived command is the ABSOLUTE confined path inside the bundle (POSIX single-quoted). - assert.strictEqual(hooks[0].hooks[0].command, shQuote(expectedBundleCommand(dir, 'e', 'hooks/b.js')), 'old shared edit stripped, new applied (quoted absolute confined path)'); + assert.strictEqual(hooks[0].hooks[0].command, nodeQuoted(expectedBundleCommand(dir, 'e', 'hooks/b.js')), 'old shared edit stripped, new applied (node + quoted absolute confined path)'); }); // --------------------------------------------------------------------------- @@ -349,6 +349,10 @@ function runtimeWithSpace() { function shQuote(s) { return "'" + String(s).replace(/'/g, "'\\''") + "'"; } +/** #1634: a `.js`-family hook command is emitted as `node ` + the POSIX-quoted absolute path. */ +function nodeQuoted(p) { + return 'node ' + shQuote(p); +} test('#1460 (R): emitted hook command is single-quoted when the install prefix contains a space', async () => { const dir = runtimeWithSpace(); @@ -382,7 +386,7 @@ test('#1460 (R): a normal script under a normal prefix emits the quoted absolute assert.strictEqual(before.length, 1); assert.strictEqual(before[0][CAP_MARKER], 'e'); // revert-fails: without quoting the control command is the bare absolute path. - assert.strictEqual(before[0].hooks[0].command, shQuote(expectedBundleCommand(dir, 'e', 'hooks/run.js'))); + assert.strictEqual(before[0].hooks[0].command, nodeQuoted(expectedBundleCommand(dir, 'e', 'hooks/run.js'))); // Strip is keyed on CAP_MARKER===capId, NOT the command string — quoting does not break it. const rem = await lifecycle.removeCapability('e', { runtimeDir: dir, hostVersion: '1.6.0', sharedFiles: ['settings.json'], @@ -411,7 +415,88 @@ test('#1460 (R): idempotent strip-then-reapply yields the identical quoted comma const second = readSettings(dir).hooks.PostToolUse; assert.strictEqual(second.length, 1, 'idempotent: still exactly one stamped hook after strip+reapply'); assert.strictEqual(second[0].hooks[0].command, first[0].hooks[0].command, 'identical quoted command on re-apply'); - assert.strictEqual(second[0].hooks[0].command, shQuote(expectedBundleCommand(dir, capId, 'hooks/run.js'))); + assert.strictEqual(second[0].hooks[0].command, nodeQuoted(expectedBundleCommand(dir, capId, 'hooks/run.js'))); +}); + +// --------------------------------------------------------------------------- +// #1634: a declared tool-scoping `matcher` must be honored, and a .js-family +// hook command must run without relying on the source's executable bit. +// --------------------------------------------------------------------------- +test('#1634: a declared matcher is preserved on the emitted shared-config hook entry', () => { + const dir = runtime(); + const capId = 'toolkit'; + const capDirPath = path.join(dir, '.gsd', 'capabilities', capId); + fs.mkdirSync(path.join(capDirPath, 'hooks'), { recursive: true }); + fs.writeFileSync(path.join(capDirPath, 'hooks', 'genfile-guard.cjs'), '// guard\n', 'utf8'); + const manifest = declarativeCap(capId, '1.0.0'); + manifest.hooks = [{ event: 'PreToolUse', script: 'hooks/genfile-guard.cjs', matcher: 'Write|Edit' }]; + + lifecycle.applyCapabilitySharedEdits({ runtimeDir: dir, capId, manifest, sharedFiles: ['settings.json'] }); + + const entry = readSettings(dir).hooks.PreToolUse[0]; + assert.strictEqual(entry[CAP_MARKER], capId, 'entry is stamped with the capability marker'); + // revert-fails: matcher was dropped, so the hook fired on every tool (Bash/Read/Write/Edit). + assert.strictEqual(entry.matcher, 'Write|Edit', 'declared matcher is preserved so the hook is tool-scoped'); +}); + +test('#1634: an absent matcher is omitted (match-all) so shipped capabilities stay unchanged', () => { + const dir = runtime(); + const capId = 'plain'; + const capDirPath = path.join(dir, '.gsd', 'capabilities', capId); + fs.mkdirSync(path.join(capDirPath, 'hooks'), { recursive: true }); + fs.writeFileSync(path.join(capDirPath, 'hooks', 'run.js'), '// x\n', 'utf8'); + const manifest = declarativeCap(capId, '1.0.0'); + manifest.hooks = [{ event: 'PostToolUse', script: 'hooks/run.js' }]; + + lifecycle.applyCapabilitySharedEdits({ runtimeDir: dir, capId, manifest, sharedFiles: ['settings.json'] }); + + const entry = readSettings(dir).hooks.PostToolUse[0]; + assert.ok(!('matcher' in entry), 'no matcher declared => field omitted (match-all), preserving prior behavior'); +}); + +test('#1634: a .cjs hook command is node-prefixed so it runs without the executable bit', () => { + const dir = runtime(); + const capId = 'toolkit'; + const capDirPath = path.join(dir, '.gsd', 'capabilities', capId); + fs.mkdirSync(path.join(capDirPath, 'hooks'), { recursive: true }); + // Stage at 0644 (no +x) — a git/tarball source that lost the executable bit. + fs.writeFileSync(path.join(capDirPath, 'hooks', 'genfile-guard.cjs'), '// guard\n', { encoding: 'utf8', mode: 0o644 }); + const manifest = declarativeCap(capId, '1.0.0'); + manifest.hooks = [{ event: 'PreToolUse', script: 'hooks/genfile-guard.cjs' }]; + + lifecycle.applyCapabilitySharedEdits({ runtimeDir: dir, capId, manifest, sharedFiles: ['settings.json'] }); + + const command = readSettings(dir).hooks.PreToolUse[0].hooks[0].command; + // The executable bit is a POSIX concept; on Windows fs modes are not POSIX (a 0o644 write reads + // back as 0o666), so the precondition is checked on POSIX only. The node-prefix assertion below + // is the actual fix and is platform-independent. + if (process.platform !== 'win32') { + assert.strictEqual((fs.statSync(path.join(capDirPath, 'hooks', 'genfile-guard.cjs')).mode & 0o777), 0o644, + 'precondition: file staged without +x'); + } + // revert-fails: command was a bare single-quoted path -> /bin/sh: Permission denied on non-+x. + assert.ok(/^node '/.test(command), 'command is node-prefixed for a .cjs hook: ' + command); +}); + +test('#1634: matcher + node-prefix together; marker strip round-trip unaffected', () => { + const dir = runtime(); + const capId = 'toolkit'; + const capDirPath = path.join(dir, '.gsd', 'capabilities', capId); + fs.mkdirSync(path.join(capDirPath, 'hooks'), { recursive: true }); + fs.writeFileSync(path.join(capDirPath, 'hooks', 'genfile-guard.cjs'), '// guard\n', 'utf8'); + const manifest = declarativeCap(capId, '1.0.0'); + manifest.hooks = [{ event: 'PreToolUse', script: 'hooks/genfile-guard.cjs', matcher: 'Write|Edit' }]; + + const edits = lifecycle.applyCapabilitySharedEdits({ runtimeDir: dir, capId, manifest, sharedFiles: ['settings.json'] }); + const before = readSettings(dir).hooks.PreToolUse[0]; + assert.strictEqual(before.matcher, 'Write|Edit', 'matcher preserved on apply'); + assert.ok(/^node '/.test(before.hooks[0].command), 'command node-prefixed on apply'); + + // Strip is keyed on CAP_MARKER, not command/matcher shape — must still remove the entry. + lifecycle.stripCapabilitySharedEdits({ runtimeDir: dir, capId, sharedEdits: edits }); + const after = readSettings(dir); + assert.ok(!after || !after.hooks || !after.hooks.PreToolUse || after.hooks.PreToolUse.length === 0, + 'marker-stamped entry is stripped regardless of matcher/command shape'); }); test('#1460 (R): confinedBundleScript returns null for an unsafe-char script (defense-in-depth)', () => { @@ -955,11 +1040,11 @@ test('#1460 CONF-1: a relative hook script is emitted as the absolute path insid const s = readSettings(dir); const command = s.hooks.PostToolUse[0].hooks[0].command; const expectedAbs = path.join(fs.realpathSync(path.join(capDir, 'subdir')), 'run.js'); - // #1460 (R): the emitted command is the absolute confined path, POSIX single-quoted. - assert.strictEqual(command, shQuote(expectedAbs), 'command must be the quoted absolute confined path inside capDir'); - const unquoted = command.slice(1, -1); // strip the wrapping single quotes for the path-shape checks - assert.ok(path.isAbsolute(unquoted), 'command must be absolute (CWD-independent)'); - assert.ok(unquoted.startsWith(fs.realpathSync(capDir) + path.sep), 'command must live inside the bundle'); + // #1460 (R) + #1634: the emitted command is `node ` + the absolute confined path, POSIX single-quoted. + assert.strictEqual(command, nodeQuoted(expectedAbs), 'command must be node + the quoted absolute confined path inside capDir'); + const unquoted = command.slice('node '.length + 1, -1); // drop `node ` prefix, strip the wrapping single quotes + assert.ok(path.isAbsolute(unquoted), 'command path must be absolute (CWD-independent)'); + assert.ok(unquoted.startsWith(fs.realpathSync(capDir) + path.sep), 'command path must live inside the bundle'); }); test('#1460 CONF-1: a script resolving OUTSIDE capDir via a symlinked subdir is NOT written (skipped)', (t) => { diff --git a/tests/capability-registry.test.cjs b/tests/capability-registry.test.cjs index b44fd07b2..871f75d34 100644 --- a/tests/capability-registry.test.cjs +++ b/tests/capability-registry.test.cjs @@ -290,6 +290,57 @@ describe('validateCapability adversarial cases', () => { 'Expected error about agentVerdict forcing blocking:false, got: ' + JSON.stringify(errors), ); }); + + test('#1634: a valid tool-scoping matcher is accepted on a lifecycle hook', () => { + const cap = { + ...UI_CAP, + hooks: [{ event: 'PreToolUse', script: 'hooks/genfile-guard.cjs', matcher: 'Write|Edit' }], + }; + const errors = validateCapability(cap, 'ui'); + assert.ok( + !errors.some((e) => e.includes('matcher')), + 'A valid matcher must not produce a matcher error, got: ' + JSON.stringify(errors), + ); + }); + + test('#1634: an absent matcher is accepted (match-all)', () => { + const cap = { ...UI_CAP, hooks: [{ event: 'PreToolUse', script: 'hooks/g.js' }] }; + const errors = validateCapability(cap, 'ui'); + assert.ok( + !errors.some((e) => e.includes('matcher')), + 'An absent matcher must not error, got: ' + JSON.stringify(errors), + ); + }); + + test('#1634: an empty-string matcher is rejected', () => { + const cap = { ...UI_CAP, hooks: [{ event: 'PreToolUse', script: 'hooks/g.js', matcher: '' }] }; + const errors = validateCapability(cap, 'ui'); + assert.ok( + errors.some((e) => e.includes('matcher') && e.includes('non-empty')), + 'Expected a non-empty matcher error, got: ' + JSON.stringify(errors), + ); + }); + + test('#1634: a non-string matcher is rejected', () => { + const cap = { ...UI_CAP, hooks: [{ event: 'PreToolUse', script: 'hooks/g.js', matcher: 42 }] }; + const errors = validateCapability(cap, 'ui'); + assert.ok( + errors.some((e) => e.includes('matcher')), + 'Expected a matcher type error, got: ' + JSON.stringify(errors), + ); + }); + + test('#1634: a matcher containing control characters is rejected', () => { + const cap = { + ...UI_CAP, + hooks: [{ event: 'PreToolUse', script: 'hooks/g.js', matcher: 'Write\n|Edit' }], + }; + const errors = validateCapability(cap, 'ui'); + assert.ok( + errors.some((e) => e.includes('matcher') && e.includes('control')), + 'Expected a control-character matcher error, got: ' + JSON.stringify(errors), + ); + }); }); describe('validateAgainstContract adversarial cases', () => {