Skip to content

Commit 4cb94cc

Browse files
jesse-merhijesse-merhi
authored
fix: keep trusted policies with hook registry (#94545)
* fix: keep trusted policies with hook registry * fix: compose trusted policies with live hooks * fix: preserve trusted policy order * test: update hook runner mock * fix: keep hook policy registry internal
1 parent 298a0cd commit 4cb94cc

9 files changed

Lines changed: 471 additions & 154 deletions

‎src/agents/agent-tools.before-tool-call.embedded-mode.test.ts‎

Lines changed: 100 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,18 @@
55
*/
66
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
77
import { setEmbeddedMode } from "../infra/embedded-mode.js";
8-
import { getGlobalHookRunner } from "../plugins/hook-runner-global.js";
8+
import {
9+
getGlobalHookRunner,
10+
initializeGlobalHookRunner,
11+
resetGlobalHookRunner,
12+
} from "../plugins/hook-runner-global.js";
913
import type { HookRunner } from "../plugins/hooks.js";
1014
import { createEmptyPluginRegistry } from "../plugins/registry-empty.js";
11-
import { setActivePluginRegistry } from "../plugins/runtime.js";
15+
import {
16+
pinActivePluginChannelRegistry,
17+
releasePinnedPluginChannelRegistry,
18+
setActivePluginRegistry,
19+
} from "../plugins/runtime.js";
1220
import { PluginApprovalResolutions } from "../plugins/types.js";
1321
import { runBeforeToolCallHook } from "./agent-tools.before-tool-call.js";
1422
import { callGatewayTool } from "./tools/gateway.js";
@@ -69,6 +77,7 @@ describe("runBeforeToolCallHook — embedded mode approvals", () => {
6977
let runBeforeToolCallMock: ReturnType<typeof vi.fn<HookRunner["runBeforeToolCall"]>>;
7078

7179
beforeEach(() => {
80+
resetGlobalHookRunner();
7281
runBeforeToolCallMock = vi.fn<HookRunner["runBeforeToolCall"]>();
7382
hookRunner = {
7483
hasHooks: vi.fn<HookRunner["hasHooks"]>().mockReturnValue(true),
@@ -82,6 +91,7 @@ describe("runBeforeToolCallHook — embedded mode approvals", () => {
8291
afterEach(() => {
8392
setEmbeddedMode(false);
8493
setActivePluginRegistry(createEmptyPluginRegistry());
94+
resetGlobalHookRunner();
8595
});
8696

8797
it("blocks approval-required tools in embedded mode when no gateway approval route exists", async () => {
@@ -445,6 +455,94 @@ describe("runBeforeToolCallHook — embedded mode approvals", () => {
445455
expect(runBeforeToolCallMock).not.toHaveBeenCalled();
446456
});
447457

458+
it("runs trusted policies from the global hook registry after the active registry changes", async () => {
459+
const evaluatePolicy = vi.fn(() => ({
460+
block: true,
461+
blockReason: "gateway registry policy blocked",
462+
}));
463+
const gatewayRegistry = createEmptyPluginRegistry();
464+
gatewayRegistry.trustedToolPolicies = [
465+
{
466+
pluginId: "gateway-policy",
467+
pluginName: "Gateway Policy",
468+
source: "test",
469+
policy: {
470+
id: "gateway-block",
471+
description: "Gateway policy",
472+
evaluate: evaluatePolicy,
473+
},
474+
},
475+
];
476+
initializeGlobalHookRunner(gatewayRegistry);
477+
setActivePluginRegistry(createEmptyPluginRegistry());
478+
runBeforeToolCallMock.mockResolvedValue(undefined);
479+
480+
const result = await runBeforeToolCallHook({
481+
toolName: "bash",
482+
params: { command: "deploy" },
483+
toolCallId: "call-gateway-policy",
484+
ctx: { agentId: "main", sessionKey: "main" },
485+
});
486+
487+
expect(result).toEqual({
488+
blocked: true,
489+
kind: "veto",
490+
deniedReason: "plugin-before-tool-call",
491+
reason: "gateway registry policy blocked",
492+
params: { command: "deploy" },
493+
});
494+
expect(evaluatePolicy).toHaveBeenCalledTimes(1);
495+
expect(runBeforeToolCallMock).not.toHaveBeenCalled();
496+
});
497+
498+
it("runs pinned gateway trusted policies after a later global runner initialization", async () => {
499+
const evaluatePolicy = vi.fn(() => ({
500+
block: true,
501+
blockReason: "pinned gateway policy blocked",
502+
}));
503+
const gatewayRegistry = createEmptyPluginRegistry();
504+
gatewayRegistry.trustedToolPolicies = [
505+
{
506+
pluginId: "gateway-policy",
507+
pluginName: "Gateway Policy",
508+
source: "test",
509+
policy: {
510+
id: "gateway-block",
511+
description: "Gateway policy",
512+
evaluate: evaluatePolicy,
513+
},
514+
},
515+
];
516+
setActivePluginRegistry(gatewayRegistry);
517+
initializeGlobalHookRunner(gatewayRegistry);
518+
pinActivePluginChannelRegistry(gatewayRegistry);
519+
try {
520+
const laterRegistry = createEmptyPluginRegistry();
521+
setActivePluginRegistry(laterRegistry);
522+
initializeGlobalHookRunner(laterRegistry);
523+
runBeforeToolCallMock.mockResolvedValue(undefined);
524+
525+
const result = await runBeforeToolCallHook({
526+
toolName: "bash",
527+
params: { command: "deploy" },
528+
toolCallId: "call-pinned-gateway-policy",
529+
ctx: { agentId: "main", sessionKey: "main" },
530+
});
531+
532+
expect(result).toEqual({
533+
blocked: true,
534+
kind: "veto",
535+
deniedReason: "plugin-before-tool-call",
536+
reason: "pinned gateway policy blocked",
537+
params: { command: "deploy" },
538+
});
539+
expect(evaluatePolicy).toHaveBeenCalledTimes(1);
540+
expect(runBeforeToolCallMock).not.toHaveBeenCalled();
541+
} finally {
542+
releasePinnedPluginChannelRegistry(gatewayRegistry);
543+
}
544+
});
545+
448546
it("does not require skill_workshop lifecycle approval in auto mode", async () => {
449547
(hookRunner.hasHooks as ReturnType<typeof vi.fn>).mockReturnValue(false);
450548

‎src/agents/agent-tools.before-tool-call.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
import type { SessionState } from "../logging/diagnostic-session-state.js";
3838
import { redactToolDetail } from "../logging/redact.js";
3939
import { createSubsystemLogger } from "../logging/subsystem.js";
40+
import { getGlobalHookRunnerRegistry } from "../plugins/hook-runner-global-state.js";
4041
import { getGlobalHookRunner } from "../plugins/hook-runner-global.js";
4142
import { deriveToolParams } from "../plugins/host-tool-param-parsers.js";
4243
import { copyPluginToolMeta, getPluginToolMeta } from "../plugins/tools.js";
@@ -197,9 +198,10 @@ export type BeforeToolCallPolicyDiagnosticState = {
197198

198199
/** Return whether before_tool_call hooks or trusted policies are active. */
199200
export function getBeforeToolCallPolicyDiagnosticState(): BeforeToolCallPolicyDiagnosticState {
201+
const policyRegistry = getGlobalHookRunnerRegistry() ?? undefined;
200202
return {
201203
hasBeforeToolCallHook: getGlobalHookRunner()?.hasHooks("before_tool_call") === true,
202-
trustedToolPolicies: getTrustedToolPolicyDiagnosticEntries(),
204+
trustedToolPolicies: getTrustedToolPolicyDiagnosticEntries(policyRegistry),
203205
};
204206
}
205207

@@ -1114,7 +1116,8 @@ export async function runBeforeToolCallHook(args: {
11141116
const hookRunner = getGlobalHookRunner();
11151117
try {
11161118
const hasBeforeToolCallHooks = hookRunner?.hasHooks("before_tool_call") === true;
1117-
const shouldRunTrustedPolicies = hasTrustedToolPolicies();
1119+
const policyRegistry = getGlobalHookRunnerRegistry() ?? undefined;
1120+
const shouldRunTrustedPolicies = hasTrustedToolPolicies(policyRegistry);
11181121
const normalizedParams = isPlainObject(params) ? params : {};
11191122
const initialCorePolicyResult = resolveSkillWorkshopToolApproval({
11201123
toolName,
@@ -1166,6 +1169,7 @@ export async function runBeforeToolCallHook(args: {
11661169
},
11671170
toolContext,
11681171
{
1172+
...(policyRegistry ? { registry: policyRegistry } : {}),
11691173
...(args.ctx?.config ? { config: args.ctx.config } : {}),
11701174
deriveEvent: deriveToolEventParams,
11711175
normalizeEvent(eventValue) {

‎src/agents/bash-tools.exec.resolve-env-hook.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ const mocks = vi.hoisted(() => ({
3131

3232
vi.mock("../plugins/hook-runner-global.js", () => ({
3333
getGlobalHookRunner: () => mocks.hookRunner,
34+
getGlobalHookRunnerRegistry: () => null,
3435
}));
3536

3637
vi.mock("../infra/shell-env.js", () => ({

‎src/plugins/contracts/plugin-sdk-runtime-api-guardrails.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,22 @@ describe("runtime api guardrails", () => {
355355
}
356356
});
357357

358+
it("keeps the composed hook-runner registry internal", () => {
359+
const pluginRuntime = readFileSync(resolve(ROOT_DIR, "plugin-sdk/plugin-runtime.ts"), "utf8");
360+
const hookRunnerGlobal = readFileSync(
361+
resolve(ROOT_DIR, "plugins/hook-runner-global.ts"),
362+
"utf8",
363+
);
364+
const hookRegistryTypes = readFileSync(
365+
resolve(ROOT_DIR, "plugins/hook-registry.types.ts"),
366+
"utf8",
367+
);
368+
369+
expect(pluginRuntime).toContain('export * from "../plugins/hook-runner-global.js";');
370+
expect(hookRunnerGlobal).not.toContain("getGlobalHookRunnerRegistry");
371+
expect(hookRegistryTypes).not.toContain("trustedToolPolicies");
372+
});
373+
358374
it("keeps Slack's narrow runtime-setter entrypoint pinned to a single export", () => {
359375
// Regression for #69317. The bundled channel entry's runtime.specifier
360376
// now points at runtime-setter-api.ts. The whole point of that file is

0 commit comments

Comments
 (0)