Skip to content

Commit c138db7

Browse files
committed
fix(runtime): enforce hook lifecycle bounds
1 parent 7766fb0 commit c138db7

6 files changed

Lines changed: 167 additions & 4 deletions

File tree

packages/runtime-host/src/server/host-hook-composition.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import type {
1010
import type { RuntimeEvent } from '@maka/core/runtime-event';
1111
import type { SessionHeader } from '@maka/core/session';
1212
import {
13+
createHookExecutionLimiter,
1314
createPreToolUseHookDispatcher,
15+
type HookExecutionLimiter,
1416
type PreToolUseHookDispatcher,
1517
} from '@maka/runtime/hooks/engine';
1618
import {
@@ -35,12 +37,14 @@ export function createHostHookComposition(input: {
3537
}): HostHookComposition {
3638
const userConfig = createHookConfigStore(input.stateRoot);
3739
const trust = createHookTrustStore(input.stateRoot);
40+
const executionLimiter = createHookExecutionLimiter();
3841
return {
3942
dispatcherFor(header) {
4043
return createSessionHookDispatcher({
4144
header,
4245
userConfig,
4346
trust,
47+
executionLimiter,
4448
runtimeEvents: input.runtimeEvents,
4549
});
4650
},
@@ -51,10 +55,12 @@ function createSessionHookDispatcher(input: {
5155
header: SessionHeader;
5256
userConfig: HookConfigStore;
5357
trust: HookTrustStore;
58+
executionLimiter: HookExecutionLimiter;
5459
runtimeEvents: HookRuntimeEventWriter;
5560
}): PreToolUseHookDispatcher {
5661
return createPreToolUseHookDispatcher({
5762
loadSnapshot: () => loadSnapshot(input),
63+
executionLimiter: input.executionLimiter,
5864
recordAudit: (hookInput, audit, context) =>
5965
recordAudit(input.runtimeEvents, hookInput, audit, context.invocationId),
6066
});

packages/runtime/src/__tests__/hooks-engine.test.ts

Lines changed: 111 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
import assert from 'node:assert/strict';
22
import { describe, it } from 'node:test';
33
import type { PreToolUseHookInput, ResolvedHookDefinition } from '@maka/core/hooks';
4-
import { createHookCommandRunner, type HookCommandRunner } from '../hooks/command-runner.js';
5-
import { createPreToolUseHookDispatcher } from '../hooks/engine.js';
4+
import {
5+
createHookCommandRunner,
6+
HOOK_EXECUTION_MARKER,
7+
type HookCommandRunner,
8+
} from '../hooks/command-runner.js';
9+
import { createHookExecutionLimiter, createPreToolUseHookDispatcher } from '../hooks/engine.js';
610

711
describe('PreToolUse Hook engine', () => {
812
it('freezes one snapshot per turn and skips untrusted matching definitions', async () => {
@@ -37,6 +41,37 @@ describe('PreToolUse Hook engine', () => {
3741
assert.equal(first.audits[0]?.status, 'skipped_untrusted');
3842
});
3943

44+
it('retains every active Turn snapshot until that Turn is released', async () => {
45+
let revision = 1;
46+
let loads = 0;
47+
const dispatcher = createPreToolUseHookDispatcher({
48+
loadSnapshot: async () => {
49+
loads += 1;
50+
return [definition({ id: `revision-${revision}`, trusted: false })];
51+
},
52+
});
53+
for (let index = 1; index <= 9; index += 1) dispatcher.prepareTurn(`turn-${index}`);
54+
await Promise.resolve();
55+
revision = 2;
56+
57+
const retained = await dispatcher.runPreToolUse(
58+
hookInput('turn-1'),
59+
new AbortController().signal,
60+
{ invocationId: 'invocation-1' },
61+
);
62+
assert.equal(retained.audits[0]?.handlerId, 'revision-1');
63+
assert.equal(loads, 9);
64+
65+
dispatcher.releaseTurn('turn-1');
66+
const reloaded = await dispatcher.runPreToolUse(
67+
hookInput('turn-1'),
68+
new AbortController().signal,
69+
{ invocationId: 'invocation-1-reloaded' },
70+
);
71+
assert.equal(reloaded.audits[0]?.handlerId, 'revision-2');
72+
assert.equal(loads, 10);
73+
});
74+
4075
it('runs matching handlers concurrently and reports denials in configuration order', async () => {
4176
let active = 0;
4277
let maxActive = 0;
@@ -73,6 +108,49 @@ describe('PreToolUse Hook engine', () => {
73108
);
74109
});
75110

111+
it('shares one concurrency ceiling across simultaneous dispatchers and sessions', async () => {
112+
let active = 0;
113+
let maxActive = 0;
114+
let release!: () => void;
115+
const gate = new Promise<void>((resolve) => {
116+
release = resolve;
117+
});
118+
const runner: HookCommandRunner = {
119+
run: async () => {
120+
active += 1;
121+
maxActive = Math.max(maxActive, active);
122+
await gate;
123+
active -= 1;
124+
return result(0);
125+
},
126+
};
127+
const limiter = createHookExecutionLimiter(2);
128+
const createDispatcher = () =>
129+
createPreToolUseHookDispatcher({
130+
loadSnapshot: async () => [
131+
definition({ id: 'first', definitionOrder: 0 }),
132+
definition({ id: 'second', definitionOrder: 1 }),
133+
],
134+
commandRunner: runner,
135+
executionLimiter: limiter,
136+
});
137+
const first = createDispatcher().runPreToolUse(
138+
hookInput('turn-1'),
139+
new AbortController().signal,
140+
{ invocationId: 'invocation-1' },
141+
);
142+
const second = createDispatcher().runPreToolUse(
143+
{ ...hookInput('turn-2'), session_id: 'session-2' },
144+
new AbortController().signal,
145+
{ invocationId: 'invocation-2' },
146+
);
147+
await new Promise<void>((resolve) => setImmediate(resolve));
148+
assert.equal(maxActive, 2);
149+
release();
150+
await Promise.all([first, second]);
151+
assert.equal(maxActive, 2);
152+
});
153+
76154
it('fails open on handler and audit failures but preserves an explicit denial', async () => {
77155
const dispatcher = createPreToolUseHookDispatcher({
78156
loadSnapshot: async () => [
@@ -133,6 +211,37 @@ describe('PreToolUse Hook engine', () => {
133211
assert.equal(output.reason, 'structured policy denial');
134212
});
135213

214+
it('marks Hook children and refuses execution in a Host recursively started by a Hook', async () => {
215+
const runner = createHookCommandRunner();
216+
const markerProbe = await runner.run(
217+
definition({
218+
command: process.execPath,
219+
args: [
220+
'-e',
221+
`process.exit(process.env[${JSON.stringify(HOOK_EXECUTION_MARKER)}] === '1' ? 0 : 9)`,
222+
],
223+
}),
224+
hookInput('turn-1'),
225+
new AbortController().signal,
226+
);
227+
assert.equal(markerProbe.exitCode, 0);
228+
229+
const previous = process.env[HOOK_EXECUTION_MARKER];
230+
process.env[HOOK_EXECUTION_MARKER] = '1';
231+
try {
232+
const recursive = await runner.run(
233+
definition({ command: process.execPath, args: ['-e', 'process.exit(99)'] }),
234+
hookInput('turn-recursive'),
235+
new AbortController().signal,
236+
);
237+
assert.equal(recursive.exitCode, null);
238+
assert.equal(recursive.spawnError, 'Recursive Hook execution is not allowed');
239+
} finally {
240+
if (previous === undefined) delete process.env[HOOK_EXECUTION_MARKER];
241+
else process.env[HOOK_EXECUTION_MARKER] = previous;
242+
}
243+
});
244+
136245
it('terminates timed-out Hook process trees and fails open', async () => {
137246
const dispatcher = createPreToolUseHookDispatcher({
138247
loadSnapshot: async () => [

packages/runtime/src/__tests__/tool-runtime-sqlite-boundary.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ describe('ToolRuntime with real SQLite boundary', () => {
3737
runtimeCommitSink: store,
3838
preToolUseHooks: {
3939
prepareTurn: () => {},
40+
releaseTurn: () => {},
4041
runPreToolUse: async () => ({
4142
denied: true,
4243
reason: 'Policy denied this command',

packages/runtime/src/hooks/command-runner.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
import { DEFAULT_PROCESS_TERMINATION_GRACE_MS } from '../process-tree-terminator.js';
99

1010
const HOOK_OUTPUT_LIMIT = 64 * 1024;
11+
export const HOOK_EXECUTION_MARKER = 'MAKA_HOOK_EXECUTION' as const;
1112

1213
export interface HookCommandResult {
1314
exitCode: number | null;
@@ -36,6 +37,9 @@ async function runHookCommand(
3637
abortSignal: AbortSignal,
3738
): Promise<HookCommandResult> {
3839
if (abortSignal.aborted) return emptyResult({ aborted: true });
40+
if (process.env[HOOK_EXECUTION_MARKER] !== undefined) {
41+
return emptyResult({ spawnError: 'Recursive Hook execution is not allowed' });
42+
}
3943
let child: ReturnType<typeof spawn>;
4044
try {
4145
child = spawn(definition.command, definition.args, {
@@ -142,6 +146,7 @@ function minimalHookEnvironment(): NodeJS.ProcessEnv {
142146
for (const key of keep) {
143147
if (process.env[key] !== undefined) env[key] = process.env[key];
144148
}
149+
env[HOOK_EXECUTION_MARKER] = '1';
145150
return env;
146151
}
147152

packages/runtime/src/hooks/engine.ts

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export interface HookDispatchResult {
2323

2424
export interface PreToolUseHookDispatcher {
2525
prepareTurn(turnId: string): void;
26+
releaseTurn(turnId: string): void;
2627
runPreToolUse(
2728
input: PreToolUseHookInput,
2829
abortSignal: AbortSignal,
@@ -44,6 +45,41 @@ export interface PreToolUseHookDispatcherInput {
4445
commandRunner?: HookCommandRunner;
4546
now?: () => number;
4647
concurrency?: number;
48+
executionLimiter?: HookExecutionLimiter;
49+
}
50+
51+
export interface HookExecutionLimiter {
52+
run<T>(operation: () => Promise<T>): Promise<T>;
53+
}
54+
55+
export function createHookExecutionLimiter(
56+
concurrency = DEFAULT_CONCURRENCY,
57+
): HookExecutionLimiter {
58+
if (!Number.isInteger(concurrency) || concurrency < 1) {
59+
throw new Error('Hook concurrency must be a positive integer');
60+
}
61+
let active = 0;
62+
const queued: Array<() => void> = [];
63+
const drain = () => {
64+
while (active < concurrency && queued.length > 0) queued.shift()?.();
65+
};
66+
return {
67+
run<T>(operation: () => Promise<T>): Promise<T> {
68+
return new Promise<T>((resolve, reject) => {
69+
queued.push(() => {
70+
active += 1;
71+
void Promise.resolve()
72+
.then(operation)
73+
.then(resolve, reject)
74+
.finally(() => {
75+
active -= 1;
76+
drain();
77+
});
78+
});
79+
drain();
80+
});
81+
},
82+
};
4783
}
4884

4985
export function createPreToolUseHookDispatcher(
@@ -52,14 +88,14 @@ export function createPreToolUseHookDispatcher(
5288
const commandRunner = input.commandRunner ?? createHookCommandRunner();
5389
const now = input.now ?? Date.now;
5490
const concurrency = input.concurrency ?? DEFAULT_CONCURRENCY;
91+
const executionLimiter = input.executionLimiter ?? createHookExecutionLimiter(concurrency);
5592
const snapshots = new Map<string, Promise<readonly ResolvedHookDefinition[]>>();
5693

5794
const snapshotForTurn = (turnId: string): Promise<readonly ResolvedHookDefinition[]> => {
5895
let snapshot = snapshots.get(turnId);
5996
if (!snapshot) {
6097
snapshot = input.loadSnapshot(turnId);
6198
snapshots.set(turnId, snapshot);
62-
while (snapshots.size > 8) snapshots.delete(snapshots.keys().next().value!);
6399
}
64100
return snapshot;
65101
};
@@ -68,6 +104,9 @@ export function createPreToolUseHookDispatcher(
68104
prepareTurn(turnId) {
69105
void snapshotForTurn(turnId).catch(() => {});
70106
},
107+
releaseTurn(turnId) {
108+
snapshots.delete(turnId);
109+
},
71110
async runPreToolUse(hookInput, abortSignal, context) {
72111
const definitions = await snapshotForTurn(hookInput.turn_id);
73112
const matching = definitions.filter((definition) =>
@@ -78,7 +117,9 @@ export function createPreToolUseHookDispatcher(
78117
return auditFor(definition, hookInput, 'skipped_untrusted', 0, 'Review required');
79118
}
80119
const startedAt = now();
81-
const result = await commandRunner.run(definition, hookInput, abortSignal);
120+
const result = await executionLimiter.run(() =>
121+
commandRunner.run(definition, hookInput, abortSignal),
122+
);
82123
return auditFromCommandResult(
83124
definition,
84125
hookInput,

packages/runtime/src/tool-runtime.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,7 @@ export class ToolRuntime {
593593
// Bounded, not open-ended: every rejection above has already been
594594
// dispatched, and running impls observe the turn abort signal.
595595
await Promise.allSettled([...this.activeToolSettlements]);
596+
this.input.preToolUseHooks?.releaseTurn(turnId);
596597
if (boundarySettlementErrors.length > 0) {
597598
throw new AggregateError(
598599
boundarySettlementErrors,

0 commit comments

Comments
 (0)