diff --git a/packages/hooks/hook-protocol/README.md b/packages/hooks/hook-protocol/README.md index abf822ea29..8478f8aa74 100644 --- a/packages/hooks/hook-protocol/README.md +++ b/packages/hooks/hook-protocol/README.md @@ -18,7 +18,7 @@ Why a shared lib at all: Codex deliberately reimplements a *subset* of the Claud - **`matchesMatcher(matcher, query, mode)`** — match-all on absent/`''`/`'*'`; `claude` mode treats a pure `[A-Za-z0-9_|]+` pattern as a literal (pipe = exact-match alternation) and anything else as a regex; `codex` mode is always an unanchored regex. An invalid regex matches nothing (never throws). - **`runHook(bash, hook, options, now)`** — serialize `options.payload` to the hook's stdin (with a trailing newline iff `options.trailingNewline`), merge `options.env` after the executor's credential scrub (the `dsh-bash` trusted-plugin surface), honor the hook's `timeoutSec` (else `defaultTimeoutMs`), and decode the result (threading `options.expectedEventName` to the codec). Never throws: an executor rejection (infra fault) becomes a `HookOutput` with `exitCode: undefined` (a non-blocking error). `now` is injected for testable durations. -- **`parseHookOutput(exitCode, stdout, stderr, expectedEventName?)`** — the exit-code + structured-stdout codec. Exit `0` → parse JSON stdout (lenient: non-JSON is left for the bridge); exit `2` → blocking error, `stderr` is the block reason (surfaced as `decision: 'block'`); other → non-blocking error. `hookSpecificOutput.permissionDecision` (allow/deny/ask) overrides a legacy top-level `decision`; `additionalContext`/`updatedInput`/`systemMessage`/`continue`/`stopReason`/`suppressOutput` are parsed too. The schemas key the `hookSpecificOutput` block by `hookEventName`, so passing `expectedEventName` (the firing event) DISCARDS a block whose `hookEventName` names a different event — its event-scoped fields don't take effect (a `PreToolUse` block on a `Stop` hook is malformed), while the event-agnostic top-level fields still apply. Pure and total. +- **`parseHookOutput(exitCode, stdout, stderr, expectedEventName?)`** — the exit-code + structured-stdout codec. Exit `0` → parse JSON stdout (lenient: non-JSON is left for the bridge); exit `2` → blocking error, `stderr` is the block reason (surfaced as `decision: 'block'`); other → non-blocking error. `hookSpecificOutput.permissionDecision` (allow/deny/ask) overrides a legacy top-level `decision`; `additionalContext`/`updatedInput`/`systemMessage`/`continue`/`stopReason`/`suppressOutput` are parsed too. The schemas key the `hookSpecificOutput` block by `hookEventName`, so passing `expectedEventName` (the firing event) DISCARDS a block whose `hookEventName` names a different event — or omits it entirely — its event-scoped fields don't take effect (a `PreToolUse` block on a `Stop` hook is malformed, and so is a discriminator-less block that would otherwise apply to any event), while the event-agnostic top-level fields still apply. Pure and total. - **`mergeHookOutputs(outputs)`** — fold the results of every hook that matched one point: permission precedence **deny > ask > allow**, halt sticky on the first `continue:false`, block reasons joined with `\n\n`, `additionalContext`/`systemMessages` accumulated in order. ## `hook/*` session events diff --git a/packages/hooks/hook-protocol/src/codec.ts b/packages/hooks/hook-protocol/src/codec.ts index 1b246a3a14..b5170028c2 100644 --- a/packages/hooks/hook-protocol/src/codec.ts +++ b/packages/hooks/hook-protocol/src/codec.ts @@ -117,8 +117,8 @@ export function parseHookOutput(exitCode: number | undefined, stdout: string, st /** * Fold a parsed structured-stdout object into `output` (mutates in place). * `expectedEventName` (the firing event) gates the per-event `hookSpecificOutput` - * block: a block whose `hookEventName` names a different event has its - * event-scoped fields discarded (only its `hookEventName` is recorded). + * block: a block whose `hookEventName` names a different event — OR omits it — has + * its event-scoped fields discarded (any present `hookEventName` is still recorded). */ function applyStructured(output: HookOutput, parsed: Record, expectedEventName?: string): void { const cont = bool(parsed, 'continue') @@ -146,11 +146,14 @@ function applyStructured(output: HookOutput, parsed: Record, ex // Always surface the discriminator (for the log/diagnostics), even on a // mismatch — the record should show what the malformed block claimed. if (eventName !== undefined) output.hookEventName = eventName - // The schemas key this block by event: if it names a DIFFERENT event than the - // one firing, it is malformed — discard its event-scoped fields (a PreToolUse - // block must not deny a Stop hook). A caller that passes no expectedEventName - // opts out of the check (applies the block as-is). - if (expectedEventName !== undefined && eventName !== undefined && eventName !== expectedEventName) { + // The schemas key this block by event: when a caller passes the firing event + // (`expectedEventName`), the block's `hookEventName` MUST name it. A different + // name — or a MISSING one — is malformed under the keyed schema, so discard the + // event-scoped fields (a PreToolUse block must not deny a Stop hook; nor may a + // discriminator-less block silently apply PreToolUse-scoped permission fields to + // whatever event is firing). A caller that passes no expectedEventName opts out + // of the check (applies the block as-is). + if (expectedEventName !== undefined && eventName !== expectedEventName) { return } const permission = permissionDecisionOf(str(hso, 'permissionDecision')) diff --git a/packages/hooks/hook-protocol/tests/codec.spec.ts b/packages/hooks/hook-protocol/tests/codec.spec.ts index 4f37f804bb..5f72753c57 100644 --- a/packages/hooks/hook-protocol/tests/codec.spec.ts +++ b/packages/hooks/hook-protocol/tests/codec.spec.ts @@ -121,11 +121,23 @@ describe('parseHookOutput — structured stdout (exit 0 only)', () => { expect(out.decision).toBe('deny') }) - it('applies a block that has NO hookEventName regardless of expectedEventName', () => { - // No discriminator to mismatch — the block applies (a hook that omits the key). + it('DISCARDS a block with NO hookEventName when a firing event is expected', () => { + // Under the keyed schema a missing discriminator is as malformed as a + // mismatched one: a discriminator-less block must not apply its event-scoped + // permission fields to whatever event happens to be firing. + const out = parseHookOutput(0, JSON.stringify({ + hookSpecificOutput: { permissionDecision: 'deny', additionalContext: 'x' }, + }), '', 'Stop') + expect(out.hookEventName).toBeUndefined() // none to record + expect(out.decision).toBeUndefined() // event-scoped fields discarded + expect(out.additionalContext).toBeUndefined() + }) + + it('applies a discriminator-less block when expectedEventName is omitted (opt-out)', () => { + // With no firing event to validate against, the block applies as-is. const out = parseHookOutput(0, JSON.stringify({ hookSpecificOutput: { permissionDecision: 'deny' }, - }), '', 'Stop') + }), '') expect(out.decision).toBe('deny') })