diff --git a/packages/hooks/hook-protocol/README.md b/packages/hooks/hook-protocol/README.md index 1983cd0411..abf822ea29 100644 --- a/packages/hooks/hook-protocol/README.md +++ b/packages/hooks/hook-protocol/README.md @@ -17,8 +17,8 @@ Why a shared lib at all: Codex deliberately reimplements a *subset* of the Claud ## Primitives - **`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. 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)`** — 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. Pure and total. +- **`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. - **`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 ac49526600..1b246a3a14 100644 --- a/packages/hooks/hook-protocol/src/codec.ts +++ b/packages/hooks/hook-protocol/src/codec.ts @@ -64,8 +64,19 @@ function permissionDecisionOf(value: string | undefined): HookOutput['decision'] * JSON on a 0 exit is treated as "no structured output" (the plain stdout is * still on the bridge to use), matching both reference engines' lenient parse of * non-JSON stdout. + * + * `expectedEventName` is the event the hook is FIRING for (e.g. `'PreToolUse'`). + * The reference schemas key the `hookSpecificOutput` block by `hookEventName`, + * so a block whose `hookEventName` names a DIFFERENT event is malformed and its + * event-scoped fields (`permissionDecision`/`permissionDecisionReason`/ + * `additionalContext`/`updatedInput`) are DISCARDED — a `PreToolUse` block on a + * `Stop` hook must not deny the `Stop`. The block's `hookEventName` is still + * surfaced (for the log/diagnostics), and the event-agnostic top-level fields + * (`decision`/`reason`/`continue`/`stopReason`/`suppressOutput`/`systemMessage`) + * are unaffected. Omit `expectedEventName` (or pass a matching one) to apply the + * block as-is — a caller that doesn't key by event opts out of the check. */ -export function parseHookOutput(exitCode: number | undefined, stdout: string, stderr: string): HookOutput { +export function parseHookOutput(exitCode: number | undefined, stdout: string, stderr: string, expectedEventName?: string): HookOutput { const trimmedErr = stderr.trim() const trimmedOut = stdout.trim() // Keep the raw stdout verbatim: a clean-exit hook may emit PLAIN text the @@ -96,15 +107,20 @@ export function parseHookOutput(exitCode: number | undefined, stdout: string, st // reference engines are). The plain stdout remains the bridge's to use. parsed = undefined } - if (parsed) applyStructured(output, parsed) + if (parsed) applyStructured(output, parsed, expectedEventName) } } return output } -/** Fold a parsed structured-stdout object into `output` (mutates in place). */ -function applyStructured(output: HookOutput, parsed: Record): void { +/** + * 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). + */ +function applyStructured(output: HookOutput, parsed: Record, expectedEventName?: string): void { const cont = bool(parsed, 'continue') if (cont !== undefined) output.continue = cont const stopReason = str(parsed, 'stopReason') @@ -121,15 +137,22 @@ function applyStructured(output: HookOutput, parsed: Record): v const topReason = str(parsed, 'reason') if (topReason !== undefined) output.reason = topReason - // hookSpecificOutput: the per-event channel, keyed by `hookEventName`. We - // surface that discriminator so the bridge can DISCARD a block whose event - // doesn't match the firing one (the schemas make it the discriminator). The + // hookSpecificOutput: the per-event channel, keyed by `hookEventName`. The // permissionDecision (allow/deny/ask) OVERRIDES the legacy top-level decision; // additionalContext and updatedInput live here too. const hso = obj(parsed.hookSpecificOutput) if (hso) { const eventName = str(hso, 'hookEventName') + // 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) { + return + } const permission = permissionDecisionOf(str(hso, 'permissionDecision')) if (permission !== undefined) output.decision = permission const permissionReason = str(hso, 'permissionDecisionReason') diff --git a/packages/hooks/hook-protocol/src/runner.ts b/packages/hooks/hook-protocol/src/runner.ts index 9e26607e3d..cea09c1fe7 100644 --- a/packages/hooks/hook-protocol/src/runner.ts +++ b/packages/hooks/hook-protocol/src/runner.ts @@ -31,6 +31,13 @@ export interface RunHookOptions { defaultTimeoutMs: number /** Whether to append a trailing newline to the stdin payload (CC yes, Codex no). */ trailingNewline: boolean + /** + * The event this hook is firing for (e.g. `'PreToolUse'`). When set, a + * structured `hookSpecificOutput` block whose `hookEventName` names a DIFFERENT + * event is treated as malformed and its event-scoped fields are discarded (see + * {@link parseHookOutput}). Omit it to apply any block as-is. + */ + expectedEventName?: string } /** The {@link HookOutput} plus the wall-clock duration of the run (for `hook/result`). */ @@ -75,7 +82,7 @@ export async function runHook( // `undefined` (a non-blocking error — no clean exit code to act on). const exitCode = result.exitCode ?? undefined return { - output: parseHookOutput(exitCode, result.stdout.text, result.stderr.text), + output: parseHookOutput(exitCode, result.stdout.text, result.stderr.text, options.expectedEventName), durationMs: now() - started, } } catch (error: unknown) { diff --git a/packages/hooks/hook-protocol/src/types.ts b/packages/hooks/hook-protocol/src/types.ts index dbc4b57aab..c3b75e7c08 100644 --- a/packages/hooks/hook-protocol/src/types.ts +++ b/packages/hooks/hook-protocol/src/types.ts @@ -132,9 +132,12 @@ export interface HookOutput { reason?: string /** * The `hookSpecificOutput.hookEventName` discriminator, when the hook emitted - * a `hookSpecificOutput` block. The reference schemas key that block by event; - * a bridge compares this to the firing event and DISCARDS a mismatched block - * (a hook claiming `PreToolUse` output on a `Stop` event is malformed). Absent + * a `hookSpecificOutput` block. The reference schemas key that block by event, + * so a block whose `hookEventName` names a DIFFERENT event than the one firing + * is malformed: {@link parseHookOutput} DISCARDS its event-scoped fields when + * given the firing event's `expectedEventName` (a hook claiming `PreToolUse` + * output on a `Stop` event does not affect the `Stop`). This field is still + * surfaced even on a mismatch — the record shows what the block claimed. Absent * when the hook emitted no `hookSpecificOutput`. */ hookEventName?: string diff --git a/packages/hooks/hook-protocol/tests/codec.spec.ts b/packages/hooks/hook-protocol/tests/codec.spec.ts index 7964745056..4f37f804bb 100644 --- a/packages/hooks/hook-protocol/tests/codec.spec.ts +++ b/packages/hooks/hook-protocol/tests/codec.spec.ts @@ -93,6 +93,54 @@ describe('parseHookOutput — structured stdout (exit 0 only)', () => { expect(parseHookOutput(0, JSON.stringify({ decision: 'maybe' }), '').decision).toBeUndefined() }) + it('DISCARDS a hookSpecificOutput block whose hookEventName mismatches the firing event', () => { + // A PreToolUse block emitted on a Stop hook is malformed — its event-scoped + // fields must not take effect (a stray PreToolUse deny must not deny the Stop). + const out = parseHookOutput(0, JSON.stringify({ + hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny', permissionDecisionReason: 'no', additionalContext: 'x', updatedInput: { command: 'y' } }, + }), '', 'Stop') + expect(out.hookEventName).toBe('PreToolUse') // still recorded for the log + expect(out.decision).toBeUndefined() // event-scoped fields discarded + expect(out.reason).toBeUndefined() + expect(out.additionalContext).toBeUndefined() + expect(out.updatedInput).toBeUndefined() + }) + + it('APPLIES a hookSpecificOutput block whose hookEventName matches the firing event', () => { + const out = parseHookOutput(0, JSON.stringify({ + hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny', additionalContext: 'x' }, + }), '', 'PreToolUse') + expect(out.decision).toBe('deny') + expect(out.additionalContext).toBe('x') + }) + + it('applies the block when expectedEventName is omitted (opt-out) even if it names an event', () => { + const out = parseHookOutput(0, JSON.stringify({ + hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny' }, + }), '') + 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). + const out = parseHookOutput(0, JSON.stringify({ + hookSpecificOutput: { permissionDecision: 'deny' }, + }), '', 'Stop') + expect(out.decision).toBe('deny') + }) + + it('a mismatched block does NOT discard the event-agnostic top-level decision/continue', () => { + // Only the per-event block is scoped; top-level fields are event-agnostic. + const out = parseHookOutput(0, JSON.stringify({ + decision: 'block', reason: 'top', continue: false, stopReason: 'halt', + hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'allow' }, + }), '', 'Stop') + expect(out.decision).toBe('block') // top-level survives; the allow block was discarded + expect(out.reason).toBe('top') + expect(out.continue).toBe(false) + expect(out.stopReason).toBe('halt') + }) + it('malformed JSON on a clean exit is lenient (no structured output, no throw)', () => { const out = parseHookOutput(0, '{ not valid json', '') expect(out.decision).toBeUndefined() diff --git a/packages/hooks/hook-protocol/tests/runner.spec.ts b/packages/hooks/hook-protocol/tests/runner.spec.ts index 698d6e0fa3..1cbe1b46de 100644 --- a/packages/hooks/hook-protocol/tests/runner.spec.ts +++ b/packages/hooks/hook-protocol/tests/runner.spec.ts @@ -131,4 +131,17 @@ describe('runHook — outcome decoding + duration', () => { const { output } = await runHook(bash, { command: 'h' }, { payload: {}, defaultTimeoutMs: 1000, trailingNewline: true }, clock()) expect(output.stderr).toBe('plain string fault') }) + + it('threads expectedEventName so a mismatched hookSpecificOutput block is discarded', async () => { + const { bash } = recordingBash(async () => result({ + exitCode: 0, + stdout: { text: JSON.stringify({ hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny' } }), truncated: false }, + })) + const { output } = await runHook(bash, { command: 'h' }, { + payload: {}, defaultTimeoutMs: 1000, trailingNewline: true, expectedEventName: 'Stop', + }, clock()) + // A PreToolUse block on a Stop hook is malformed → its decision is discarded. + expect(output.hookEventName).toBe('PreToolUse') + expect(output.decision).toBeUndefined() + }) })