mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 05:51:15 +08:00
fix(codex): defer report-mode plugin approvals
Route Codex app-server report-mode PreToolUse plugin approval requirements through the matching app-server approval request instead of failing closed. Shares duplicate in-flight approvals, preserves block/rewrite fail-closed behavior, and keeps generic plugin allow-always scoped to one Codex request. Supersedes #86978; thanks @clawSean for the original docs clarification.
This commit is contained in:
committed by
GitHub
parent
44dc29f397
commit
56a5d7e865
@@ -1,2 +1,2 @@
|
||||
7039b60f2cea732a90db633328952faaddd919f0d098b303b29d554e64184073 plugin-sdk-api-baseline.json
|
||||
1a78f4df81562af070c5379c6369a8bea9c704f985b5382a463364757b26db0d plugin-sdk-api-baseline.jsonl
|
||||
91cb45dc1e8aaa3dac9a2c1d3c98c8ff22112e41c305de17f30d0d4420635ee4 plugin-sdk-api-baseline.json
|
||||
3aa4802ffcb68c4f15e367030994eae10e73b55b5f14c8e23d4e9467fae325fe plugin-sdk-api-baseline.jsonl
|
||||
|
||||
@@ -99,6 +99,16 @@ OpenClaw can mirror selected events, but it cannot rewrite the native Codex
|
||||
thread unless Codex exposes that operation through app-server or native hook
|
||||
callbacks.
|
||||
|
||||
Codex app-server report-mode `PreToolUse` events defer plugin approval requests
|
||||
to the matching app-server approval. If an OpenClaw `before_tool_call` hook
|
||||
returns `requireApproval` while the native payload sets report approval mode
|
||||
(`openclaw_approval_mode` is `"report"`), the native hook relay records the
|
||||
plugin approval requirement and returns no native decision. When Codex sends the
|
||||
app-server approval request for the same tool use, OpenClaw opens the plugin
|
||||
approval prompt and maps the decision back to Codex. Codex `PermissionRequest`
|
||||
events are a separate approval path and can still route through OpenClaw
|
||||
approvals when the runtime is configured for that bridge.
|
||||
|
||||
Codex app-server item notifications also provide async `after_tool_call`
|
||||
observations for native tool completions that are not already covered by the
|
||||
native `PostToolUse` relay. These observations are for telemetry and plugin
|
||||
|
||||
@@ -211,6 +211,8 @@ Hook guard behavior for typed lifecycle hooks:
|
||||
- `params` rewrites the tool parameters for execution.
|
||||
- `requireApproval` pauses the agent run and asks the user through plugin
|
||||
approvals. The `/approve` command can approve both exec and plugin approvals.
|
||||
In Codex app-server report-mode native `PreToolUse` relays, this is deferred
|
||||
to the matching app-server approval request; see [Codex harness runtime](/plugins/codex-harness-runtime#hook-boundaries).
|
||||
- A lower-priority `block: true` can still block after a higher-priority hook
|
||||
requested approval.
|
||||
- `onResolution` receives the resolved approval decision - `allow-once`,
|
||||
|
||||
@@ -2,6 +2,7 @@ import {
|
||||
callGatewayTool,
|
||||
hasNativeHookRelayInvocation,
|
||||
invokeNativeHookRelay,
|
||||
resolveNativeHookRelayDeferredToolApproval,
|
||||
runBeforeToolCallHook,
|
||||
type EmbeddedRunAttemptParams,
|
||||
} from "openclaw/plugin-sdk/agent-harness-runtime";
|
||||
@@ -13,6 +14,7 @@ vi.mock("openclaw/plugin-sdk/agent-harness-runtime", async (importOriginal) => (
|
||||
callGatewayTool: vi.fn(),
|
||||
hasNativeHookRelayInvocation: vi.fn(() => false),
|
||||
invokeNativeHookRelay: vi.fn(),
|
||||
resolveNativeHookRelayDeferredToolApproval: vi.fn(),
|
||||
runBeforeToolCallHook: vi.fn(async ({ params }: { params: unknown }) => ({
|
||||
blocked: false,
|
||||
params,
|
||||
@@ -22,6 +24,9 @@ vi.mock("openclaw/plugin-sdk/agent-harness-runtime", async (importOriginal) => (
|
||||
const mockCallGatewayTool = vi.mocked(callGatewayTool);
|
||||
const mockHasNativeHookRelayInvocation = vi.mocked(hasNativeHookRelayInvocation);
|
||||
const mockInvokeNativeHookRelay = vi.mocked(invokeNativeHookRelay);
|
||||
const mockResolveNativeHookRelayDeferredToolApproval = vi.mocked(
|
||||
resolveNativeHookRelayDeferredToolApproval,
|
||||
);
|
||||
const mockRunBeforeToolCallHook = vi.mocked(runBeforeToolCallHook);
|
||||
|
||||
function requireRecord(value: unknown, label: string): Record<string, unknown> {
|
||||
@@ -103,6 +108,8 @@ describe("Codex app-server approval bridge", () => {
|
||||
mockHasNativeHookRelayInvocation.mockReset();
|
||||
mockHasNativeHookRelayInvocation.mockReturnValue(false);
|
||||
mockInvokeNativeHookRelay.mockReset();
|
||||
mockResolveNativeHookRelayDeferredToolApproval.mockReset();
|
||||
mockResolveNativeHookRelayDeferredToolApproval.mockResolvedValue(undefined);
|
||||
mockRunBeforeToolCallHook.mockReset();
|
||||
mockRunBeforeToolCallHook.mockImplementation(async ({ params }) => ({
|
||||
blocked: false,
|
||||
@@ -132,7 +139,7 @@ describe("Codex app-server approval bridge", () => {
|
||||
expect(mockRunBeforeToolCallHook).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
toolName: "exec",
|
||||
approvalMode: "report",
|
||||
approvalMode: "request",
|
||||
}),
|
||||
);
|
||||
findApprovalEvent(params, {
|
||||
@@ -212,7 +219,7 @@ describe("Codex app-server approval bridge", () => {
|
||||
},
|
||||
},
|
||||
toolCallId: "cmd-1",
|
||||
approvalMode: "report",
|
||||
approvalMode: "request",
|
||||
signal: undefined,
|
||||
ctx: {
|
||||
agentId: "main",
|
||||
@@ -385,6 +392,11 @@ describe("Codex app-server approval bridge", () => {
|
||||
expect(result).toEqual({ decision: "accept" });
|
||||
expect(mockRunBeforeToolCallHook).not.toHaveBeenCalled();
|
||||
expect(mockInvokeNativeHookRelay).toHaveBeenCalledTimes(1);
|
||||
expect(mockResolveNativeHookRelayDeferredToolApproval).toHaveBeenCalledWith({
|
||||
relayId: "relay-1",
|
||||
toolUseId: "cmd-native-relay-noop",
|
||||
signal: undefined,
|
||||
});
|
||||
expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([
|
||||
"plugin.approval.request",
|
||||
"plugin.approval.waitDecision",
|
||||
@@ -432,12 +444,53 @@ describe("Codex app-server approval bridge", () => {
|
||||
event: "pre_tool_use",
|
||||
toolUseId: "cmd-native-relay-observed",
|
||||
});
|
||||
expect(mockResolveNativeHookRelayDeferredToolApproval).toHaveBeenCalledWith({
|
||||
relayId: "relay-1",
|
||||
toolUseId: "cmd-native-relay-observed",
|
||||
signal: undefined,
|
||||
});
|
||||
expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([
|
||||
"plugin.approval.request",
|
||||
"plugin.approval.waitDecision",
|
||||
]);
|
||||
});
|
||||
|
||||
it("accepts command approvals from deferred native PreToolUse plugin approvals", async () => {
|
||||
const params = createParams();
|
||||
mockHasNativeHookRelayInvocation.mockReturnValueOnce(true);
|
||||
mockResolveNativeHookRelayDeferredToolApproval.mockResolvedValueOnce({
|
||||
handled: true,
|
||||
outcome: "approved-once",
|
||||
});
|
||||
|
||||
const result = await handleCodexAppServerApprovalRequest({
|
||||
method: "item/commandExecution/requestApproval",
|
||||
requestParams: {
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
itemId: "cmd-native-relay-deferred",
|
||||
command: "pnpm test extensions/codex/src/app-server",
|
||||
cwd: "/workspace",
|
||||
},
|
||||
paramsForRun: params,
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
nativeHookRelay: {
|
||||
relayId: "relay-1",
|
||||
allowedEvents: ["pre_tool_use"],
|
||||
},
|
||||
});
|
||||
|
||||
expect(result).toEqual({ decision: "accept" });
|
||||
expect(mockRunBeforeToolCallHook).not.toHaveBeenCalled();
|
||||
expect(mockInvokeNativeHookRelay).not.toHaveBeenCalled();
|
||||
expect(mockCallGatewayTool).not.toHaveBeenCalled();
|
||||
findApprovalEvent(params, {
|
||||
status: "approved",
|
||||
message: "Codex app-server approval granted for this turn.",
|
||||
});
|
||||
});
|
||||
|
||||
it("fails closed when the native hook relay returns unreadable approval output", async () => {
|
||||
const params = createParams();
|
||||
mockInvokeNativeHookRelay.mockResolvedValueOnce({
|
||||
@@ -676,6 +729,43 @@ describe("Codex app-server approval bridge", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps OpenClaw plugin allow-always approvals scoped to one Codex request", async () => {
|
||||
const params = createParams();
|
||||
mockRunBeforeToolCallHook.mockResolvedValueOnce({
|
||||
blocked: false,
|
||||
params: {
|
||||
command: "pnpm test",
|
||||
approval: {
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
itemId: "cmd-needs-approval",
|
||||
command: "pnpm test",
|
||||
},
|
||||
},
|
||||
approvalResolution: "allow-always",
|
||||
});
|
||||
|
||||
const result = await handleCodexAppServerApprovalRequest({
|
||||
method: "item/commandExecution/requestApproval",
|
||||
requestParams: {
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
itemId: "cmd-needs-approval",
|
||||
command: "pnpm test",
|
||||
},
|
||||
paramsForRun: params,
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
});
|
||||
|
||||
expect(result).toEqual({ decision: "accept" });
|
||||
expect(mockCallGatewayTool).not.toHaveBeenCalled();
|
||||
findApprovalEvent(params, {
|
||||
status: "approved",
|
||||
message: "Codex app-server approval granted for this turn.",
|
||||
});
|
||||
});
|
||||
|
||||
it("denies command approvals when OpenClaw tool policy requires approval", async () => {
|
||||
const params = createParams();
|
||||
mockRunBeforeToolCallHook.mockResolvedValueOnce({
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
formatApprovalDisplayPath,
|
||||
hasNativeHookRelayInvocation,
|
||||
invokeNativeHookRelay,
|
||||
resolveNativeHookRelayDeferredToolApproval,
|
||||
type EmbeddedRunAttemptParams,
|
||||
type NativeHookRelayProcessResponse,
|
||||
type NativeHookRelayRegistrationHandle,
|
||||
@@ -101,6 +102,21 @@ export async function handleCodexAppServerApprovalRequest(params: {
|
||||
});
|
||||
return buildApprovalResponse(params.method, context.requestParams, "denied");
|
||||
}
|
||||
if (
|
||||
policyOutcome?.outcome === "approved-once" ||
|
||||
policyOutcome?.outcome === "approved-session"
|
||||
) {
|
||||
emitApprovalEvent(params.paramsForRun, {
|
||||
phase: "resolved",
|
||||
kind: context.kind,
|
||||
status: "approved",
|
||||
title: context.title,
|
||||
...context.eventDetails,
|
||||
...approvalEventScope(params.method, policyOutcome.outcome),
|
||||
message: approvalResolutionMessage(policyOutcome.outcome),
|
||||
});
|
||||
return buildApprovalResponse(params.method, context.requestParams, policyOutcome.outcome);
|
||||
}
|
||||
if (params.autoApprove === true) {
|
||||
emitApprovalEvent(params.paramsForRun, {
|
||||
phase: "resolved",
|
||||
@@ -312,7 +328,10 @@ function buildApprovalContext(params: {
|
||||
}
|
||||
|
||||
type ApprovalContext = ReturnType<typeof buildApprovalContext>;
|
||||
type ApprovalPolicyOutcome = { outcome: "denied"; reason: string } | { outcome: "no-decision" };
|
||||
type ApprovalPolicyOutcome =
|
||||
| { outcome: "denied"; reason: string }
|
||||
| { outcome: "approved-once" | "approved-session" }
|
||||
| { outcome: "no-decision" };
|
||||
|
||||
async function runOpenClawToolPolicyForApprovalRequest(params: {
|
||||
method: string;
|
||||
@@ -337,10 +356,17 @@ async function runOpenClawToolPolicyForApprovalRequest(params: {
|
||||
policyRequest,
|
||||
nativeHookRelay: params.nativeHookRelay,
|
||||
cwd,
|
||||
signal: params.signal,
|
||||
});
|
||||
if (nativeRelayOutcome?.blocked) {
|
||||
return { outcome: "denied", reason: nativeRelayOutcome.reason };
|
||||
}
|
||||
if (
|
||||
nativeRelayOutcome?.outcome === "approved-once" ||
|
||||
nativeRelayOutcome?.outcome === "approved-session"
|
||||
) {
|
||||
return { outcome: nativeRelayOutcome.outcome };
|
||||
}
|
||||
if (nativeRelayOutcome?.handled) {
|
||||
return { outcome: "no-decision" };
|
||||
}
|
||||
@@ -355,7 +381,7 @@ async function runOpenClawToolPolicyForApprovalRequest(params: {
|
||||
toolName: policyRequest.toolName,
|
||||
params: policyRequest.params,
|
||||
...(params.context.itemId ? { toolCallId: params.context.itemId } : {}),
|
||||
approvalMode: "report",
|
||||
approvalMode: "request",
|
||||
signal: params.signal,
|
||||
ctx: {
|
||||
...(params.paramsForRun.agentId ? { agentId: params.paramsForRun.agentId } : {}),
|
||||
@@ -377,6 +403,13 @@ async function runOpenClawToolPolicyForApprovalRequest(params: {
|
||||
"OpenClaw tool policy rewrote Codex app-server approval params; refusing original request.",
|
||||
};
|
||||
}
|
||||
if (outcome.approvalResolution) {
|
||||
return {
|
||||
// Generic plugin approval `allow-always` is plugin-owned durability, not
|
||||
// Codex session trust. Keep the app-server request scoped to this item.
|
||||
outcome: "approved-once",
|
||||
};
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@@ -390,6 +423,7 @@ async function runNativeRelayToolPolicyForApprovalRequest(params: {
|
||||
"allowedEvents" | "generation" | "relayId"
|
||||
>;
|
||||
cwd?: string;
|
||||
signal?: AbortSignal;
|
||||
}): Promise<
|
||||
| {
|
||||
handled: true;
|
||||
@@ -399,6 +433,7 @@ async function runNativeRelayToolPolicyForApprovalRequest(params: {
|
||||
| {
|
||||
handled: true;
|
||||
blocked?: false;
|
||||
outcome?: "approved-once" | "approved-session";
|
||||
}
|
||||
| undefined
|
||||
> {
|
||||
@@ -426,6 +461,17 @@ async function runNativeRelayToolPolicyForApprovalRequest(params: {
|
||||
toolUseId: params.context.itemId,
|
||||
})
|
||||
) {
|
||||
const approvalOutcome = await resolveNativeHookRelayDeferredToolApproval({
|
||||
relayId: params.nativeHookRelay.relayId,
|
||||
toolUseId: params.context.itemId,
|
||||
signal: params.signal,
|
||||
});
|
||||
if (approvalOutcome?.outcome === "denied") {
|
||||
return { handled: true, blocked: true, reason: approvalOutcome.reason };
|
||||
}
|
||||
if (approvalOutcome?.outcome === "approved-once") {
|
||||
return { handled: true, outcome: approvalOutcome.outcome };
|
||||
}
|
||||
return { handled: true };
|
||||
}
|
||||
try {
|
||||
@@ -441,6 +487,17 @@ async function runNativeRelayToolPolicyForApprovalRequest(params: {
|
||||
if (decision.blocked) {
|
||||
return { handled: true, blocked: true, reason: decision.reason };
|
||||
}
|
||||
const approvalOutcome = await resolveNativeHookRelayDeferredToolApproval({
|
||||
relayId: params.nativeHookRelay.relayId,
|
||||
toolUseId: params.context.itemId,
|
||||
signal: params.signal,
|
||||
});
|
||||
if (approvalOutcome?.outcome === "denied") {
|
||||
return { handled: true, blocked: true, reason: approvalOutcome.reason };
|
||||
}
|
||||
if (approvalOutcome?.outcome === "approved-once") {
|
||||
return { handled: true, outcome: approvalOutcome.outcome };
|
||||
}
|
||||
return { handled: true };
|
||||
} catch (error) {
|
||||
return {
|
||||
|
||||
@@ -160,6 +160,37 @@ describe("runBeforeToolCallHook — embedded mode approvals", () => {
|
||||
expect(mockCallGatewayTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("defers approval-required tools without opening an approval request", async () => {
|
||||
runBeforeToolCallMock.mockResolvedValue({
|
||||
requireApproval: {
|
||||
pluginId: "test-plugin",
|
||||
title: "Needs approval",
|
||||
description: "Review before running",
|
||||
severity: "info",
|
||||
},
|
||||
params: { adjusted: true },
|
||||
});
|
||||
|
||||
const result = await runBeforeToolCallHook({
|
||||
toolName: "exec",
|
||||
params: { command: "ls" },
|
||||
toolCallId: "call-defer",
|
||||
approvalMode: "defer",
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({
|
||||
blocked: false,
|
||||
params: { command: "ls" },
|
||||
deferredApproval: {
|
||||
toolName: "exec",
|
||||
toolCallId: "call-defer",
|
||||
baseParams: { command: "ls" },
|
||||
overrideParams: { adjusted: true },
|
||||
},
|
||||
});
|
||||
expect(mockCallGatewayTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("sends approval to gateway when NOT in embedded mode", async () => {
|
||||
setEmbeddedMode(false);
|
||||
|
||||
@@ -263,7 +294,11 @@ describe("runBeforeToolCallHook — embedded mode approvals", () => {
|
||||
ctx: { agentId: "main", sessionKey: "main" },
|
||||
});
|
||||
|
||||
expect(result).toEqual({ blocked: false, params: { command: "deploy" } });
|
||||
expect(result).toEqual({
|
||||
blocked: false,
|
||||
params: { command: "deploy" },
|
||||
approvalResolution: PluginApprovalResolutions.ALLOW_ONCE,
|
||||
});
|
||||
const approvalCall = requireApprovalRequestCall("trusted policy approval request");
|
||||
expect(approvalCall.timeoutParams.timeoutMs).toBe(130_000);
|
||||
expect(approvalCall.request.pluginId).toBe("trusted-policy");
|
||||
|
||||
@@ -104,10 +104,24 @@ type HookOutcome =
|
||||
reason: string;
|
||||
params?: unknown;
|
||||
}
|
||||
| { blocked: false; params: unknown };
|
||||
| {
|
||||
blocked: false;
|
||||
params: unknown;
|
||||
approvalResolution?: PluginApprovalResolution;
|
||||
deferredApproval?: DeferredPluginToolApproval;
|
||||
};
|
||||
type PluginApprovalRequest = NonNullable<PluginHookBeforeToolCallResult["requireApproval"]>;
|
||||
export type DeferredPluginToolApproval = {
|
||||
approval: PluginApprovalRequest;
|
||||
toolName: string;
|
||||
toolCallId?: string;
|
||||
ctx?: HookContext;
|
||||
baseParams: unknown;
|
||||
overrideParams?: unknown;
|
||||
};
|
||||
|
||||
type BeforeToolCallWrapperOptions = {
|
||||
approvalMode?: "request" | "report";
|
||||
approvalMode?: "request" | "report" | "defer";
|
||||
emitDiagnostics: boolean;
|
||||
};
|
||||
|
||||
@@ -491,6 +505,7 @@ async function requestPluginToolApproval(params: {
|
||||
return {
|
||||
blocked: false,
|
||||
params: mergeParamsWithApprovalOverrides(params.baseParams, params.overrideParams),
|
||||
approvalResolution: resolution,
|
||||
};
|
||||
}
|
||||
if (decision === PluginApprovalResolutions.DENY) {
|
||||
@@ -507,6 +522,7 @@ async function requestPluginToolApproval(params: {
|
||||
return {
|
||||
blocked: false,
|
||||
params: mergeParamsWithApprovalOverrides(params.baseParams, params.overrideParams),
|
||||
approvalResolution: resolution,
|
||||
};
|
||||
}
|
||||
return {
|
||||
@@ -539,6 +555,28 @@ async function requestPluginToolApproval(params: {
|
||||
}
|
||||
}
|
||||
|
||||
export async function requestDeferredPluginToolApproval(params: {
|
||||
deferredApproval: DeferredPluginToolApproval;
|
||||
signal?: AbortSignal;
|
||||
}): Promise<HookOutcome> {
|
||||
const deferred = params.deferredApproval;
|
||||
return requestPluginToolApproval({
|
||||
approval: deferred.approval,
|
||||
toolName: deferred.toolName,
|
||||
...(deferred.toolCallId ? { toolCallId: deferred.toolCallId } : {}),
|
||||
...(deferred.ctx ? { ctx: deferred.ctx } : {}),
|
||||
signal: params.signal,
|
||||
baseParams: deferred.baseParams,
|
||||
overrideParams: deferred.overrideParams,
|
||||
});
|
||||
}
|
||||
|
||||
export function cancelDeferredPluginToolApproval(
|
||||
deferredApproval: DeferredPluginToolApproval,
|
||||
): void {
|
||||
notifyPluginApprovalResolution(deferredApproval.approval, PluginApprovalResolutions.CANCELLED);
|
||||
}
|
||||
|
||||
export function buildBlockedToolResult(params: {
|
||||
reason: string;
|
||||
deniedReason?: HookBlockedReason;
|
||||
@@ -647,7 +685,7 @@ export async function runBeforeToolCallHook(args: {
|
||||
toolCallId?: string;
|
||||
ctx?: HookContext;
|
||||
signal?: AbortSignal;
|
||||
approvalMode?: "request" | "report";
|
||||
approvalMode?: "request" | "report" | "defer";
|
||||
}): Promise<HookOutcome> {
|
||||
const toolName = normalizeToolName(args.toolName || "tool");
|
||||
const params = args.params;
|
||||
@@ -805,6 +843,20 @@ export async function runBeforeToolCallHook(args: {
|
||||
};
|
||||
}
|
||||
if (trustedPolicyResult?.requireApproval) {
|
||||
if (args.approvalMode === "defer") {
|
||||
return {
|
||||
blocked: false,
|
||||
params,
|
||||
deferredApproval: {
|
||||
approval: trustedPolicyResult.requireApproval,
|
||||
toolName,
|
||||
...(args.toolCallId ? { toolCallId: args.toolCallId } : {}),
|
||||
...(args.ctx ? { ctx: args.ctx } : {}),
|
||||
baseParams: params,
|
||||
overrideParams: trustedPolicyResult.params,
|
||||
},
|
||||
};
|
||||
}
|
||||
if (args.approvalMode === "report") {
|
||||
notifyPluginApprovalResolution(
|
||||
trustedPolicyResult.requireApproval,
|
||||
@@ -875,6 +927,20 @@ export async function runBeforeToolCallHook(args: {
|
||||
}
|
||||
|
||||
if (hookResult?.requireApproval) {
|
||||
if (args.approvalMode === "defer") {
|
||||
return {
|
||||
blocked: false,
|
||||
params: policyAdjustedParams,
|
||||
deferredApproval: {
|
||||
approval: hookResult.requireApproval,
|
||||
toolName,
|
||||
...(args.toolCallId ? { toolCallId: args.toolCallId } : {}),
|
||||
...(args.ctx ? { ctx: args.ctx } : {}),
|
||||
baseParams: policyAdjustedParams,
|
||||
overrideParams: hookResult.params,
|
||||
},
|
||||
};
|
||||
}
|
||||
if (args.approvalMode === "report") {
|
||||
notifyPluginApprovalResolution(
|
||||
hookResult.requireApproval,
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
invokeNativeHookRelay,
|
||||
invokeNativeHookRelayBridge,
|
||||
registerNativeHookRelay,
|
||||
resolveNativeHookRelayDeferredToolApproval,
|
||||
} from "./native-hook-relay.js";
|
||||
|
||||
afterEach(() => {
|
||||
@@ -1844,7 +1845,7 @@ describe("native hook relay registry", () => {
|
||||
expect(beforeToolCall).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("reports synthetic app-server PreToolUse approval requirements without opening plugin approvals", async () => {
|
||||
it("defers synthetic app-server PreToolUse approval requirements to the app-server approval", async () => {
|
||||
const beforeToolCall = vi.fn(async () => ({
|
||||
requireApproval: {
|
||||
title: "Needs approval",
|
||||
@@ -1876,14 +1877,81 @@ describe("native hook relay registry", () => {
|
||||
},
|
||||
});
|
||||
|
||||
expect(JSON.parse(response.stdout)).toEqual({
|
||||
hookSpecificOutput: {
|
||||
hookEventName: "PreToolUse",
|
||||
permissionDecision: "deny",
|
||||
permissionDecisionReason: "native command needs approval",
|
||||
expect(response).toEqual({ stdout: "", stderr: "", exitCode: 0 });
|
||||
expect(beforeToolCall).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("shares in-flight deferred PreToolUse approvals for duplicate app-server requests", async () => {
|
||||
const beforeToolCall = vi.fn(async () => ({
|
||||
requireApproval: {
|
||||
title: "Needs approval",
|
||||
description: "native command needs approval",
|
||||
},
|
||||
}));
|
||||
initializeGlobalHookRunner(
|
||||
createMockPluginRegistry([{ hookName: "before_tool_call", handler: beforeToolCall }]),
|
||||
);
|
||||
const relay = registerNativeHookRelay({
|
||||
provider: "codex",
|
||||
agentId: "agent-1",
|
||||
sessionId: "session-1",
|
||||
sessionKey: "agent:main:session-1",
|
||||
runId: "run-1",
|
||||
});
|
||||
|
||||
await invokeNativeHookRelay({
|
||||
provider: "codex",
|
||||
relayId: relay.relayId,
|
||||
event: "pre_tool_use",
|
||||
rawPayload: {
|
||||
hook_event_name: "PreToolUse",
|
||||
openclaw_approval_mode: "report",
|
||||
cwd: "/repo",
|
||||
tool_name: "exec_command",
|
||||
tool_use_id: "native-approval-report-duplicate",
|
||||
tool_input: { cmd: "cat /tmp/private_key" },
|
||||
},
|
||||
});
|
||||
expect(beforeToolCall).toHaveBeenCalledTimes(1);
|
||||
|
||||
let resolveApproval:
|
||||
| ((value: { blocked: false; params: unknown; approvalResolution: "allow-once" }) => void)
|
||||
| undefined;
|
||||
const approvalRequester = vi.fn(
|
||||
() =>
|
||||
new Promise<{ blocked: false; params: unknown; approvalResolution: "allow-once" }>(
|
||||
(resolve) => {
|
||||
resolveApproval = resolve;
|
||||
},
|
||||
),
|
||||
);
|
||||
testing.setNativeHookRelayDeferredToolApprovalRequesterForTests(approvalRequester);
|
||||
|
||||
const firstApproval = resolveNativeHookRelayDeferredToolApproval({
|
||||
relayId: relay.relayId,
|
||||
toolUseId: "native-approval-report-duplicate",
|
||||
});
|
||||
const duplicateApproval = resolveNativeHookRelayDeferredToolApproval({
|
||||
relayId: relay.relayId,
|
||||
toolUseId: "native-approval-report-duplicate",
|
||||
});
|
||||
|
||||
await vi.waitFor(() => expect(approvalRequester).toHaveBeenCalledTimes(1));
|
||||
resolveApproval?.({
|
||||
blocked: false,
|
||||
params: { cmd: "cat /tmp/private_key", command: "cat /tmp/private_key" },
|
||||
approvalResolution: "allow-once",
|
||||
});
|
||||
|
||||
await expect(Promise.all([firstApproval, duplicateApproval])).resolves.toEqual([
|
||||
{ handled: true, outcome: "approved-once" },
|
||||
{ handled: true, outcome: "approved-once" },
|
||||
]);
|
||||
await expect(
|
||||
resolveNativeHookRelayDeferredToolApproval({
|
||||
relayId: relay.relayId,
|
||||
toolUseId: "native-approval-report-duplicate",
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("passes config to trusted policies for native pre-tool session extension reads", async () => {
|
||||
|
||||
@@ -23,7 +23,13 @@ import { privateFileStoreSync } from "../../infra/private-file-store.js";
|
||||
import { createSubsystemLogger } from "../../logging/subsystem.js";
|
||||
import { hasGlobalHooks } from "../../plugins/hook-runner-global.js";
|
||||
import { PluginApprovalResolutions } from "../../plugins/types.js";
|
||||
import { hasBeforeToolCallPolicy, runBeforeToolCallHook } from "../agent-tools.before-tool-call.js";
|
||||
import {
|
||||
cancelDeferredPluginToolApproval,
|
||||
hasBeforeToolCallPolicy,
|
||||
requestDeferredPluginToolApproval,
|
||||
runBeforeToolCallHook,
|
||||
type DeferredPluginToolApproval,
|
||||
} from "../agent-tools.before-tool-call.js";
|
||||
import { stableStringify } from "../stable-stringify.js";
|
||||
import { resolveToolLoopDetectionConfig } from "../tool-loop-detection-config.js";
|
||||
import { normalizeToolName } from "../tool-policy.js";
|
||||
@@ -212,6 +218,7 @@ type NativeHookRelaySharedState = {
|
||||
relayBridges: Map<string, NativeHookRelayBridgeRegistration>;
|
||||
invocations: NativeHookRelayInvocation[];
|
||||
pendingPermissionApprovals: Map<string, Promise<NativeHookRelayPermissionApprovalResult>>;
|
||||
pendingPreToolUseApprovals: Map<string, NativeHookRelayPreToolUseApproval>;
|
||||
permissionApprovalWindows: Map<string, number[]>;
|
||||
permissionAllowAlwaysApprovals: Map<string, { expiresAtMs: number }>;
|
||||
};
|
||||
@@ -235,6 +242,7 @@ function getNativeHookRelaySharedState(): NativeHookRelaySharedState {
|
||||
relayBridges: new Map<string, NativeHookRelayBridgeRegistration>(),
|
||||
invocations: [],
|
||||
pendingPermissionApprovals: new Map<string, Promise<NativeHookRelayPermissionApprovalResult>>(),
|
||||
pendingPreToolUseApprovals: new Map<string, NativeHookRelayPreToolUseApproval>(),
|
||||
permissionApprovalWindows: new Map<string, number[]>(),
|
||||
permissionAllowAlwaysApprovals: new Map<string, { expiresAtMs: number }>(),
|
||||
};
|
||||
@@ -246,6 +254,7 @@ const relays = nativeHookRelayState.relays;
|
||||
const relayBridges = nativeHookRelayState.relayBridges;
|
||||
const invocations = nativeHookRelayState.invocations;
|
||||
const pendingPermissionApprovals = nativeHookRelayState.pendingPermissionApprovals;
|
||||
const pendingPreToolUseApprovals = nativeHookRelayState.pendingPreToolUseApprovals;
|
||||
const permissionApprovalWindows = nativeHookRelayState.permissionApprovalWindows;
|
||||
const permissionAllowAlwaysApprovals = nativeHookRelayState.permissionAllowAlwaysApprovals;
|
||||
|
||||
@@ -267,6 +276,25 @@ type NativeHookRelayPermissionApprovalRequester = (
|
||||
request: NativeHookRelayPermissionApprovalRequest,
|
||||
) => Promise<NativeHookRelayPermissionApprovalResult>;
|
||||
|
||||
type NativeHookRelayDeferredToolApprovalRequester = typeof requestDeferredPluginToolApproval;
|
||||
|
||||
type NativeHookRelayPreToolUseApproval = {
|
||||
deferredApproval: DeferredPluginToolApproval;
|
||||
originalParamsFingerprint: string;
|
||||
resolutionPromise?: Promise<NativeHookRelayDeferredApprovalOutcome>;
|
||||
};
|
||||
|
||||
export type NativeHookRelayDeferredApprovalOutcome =
|
||||
| {
|
||||
handled: true;
|
||||
outcome: "approved-once";
|
||||
}
|
||||
| {
|
||||
handled: true;
|
||||
outcome: "denied";
|
||||
reason: string;
|
||||
};
|
||||
|
||||
type NativeHookRelayBridgeRegistration = {
|
||||
relayId: string;
|
||||
registryPath: string;
|
||||
@@ -294,6 +322,8 @@ type NativeHookRelayBridgeRequestAuth = {
|
||||
|
||||
let nativeHookRelayPermissionApprovalRequester: NativeHookRelayPermissionApprovalRequester =
|
||||
requestNativeHookRelayPermissionApproval;
|
||||
let nativeHookRelayDeferredToolApprovalRequester: NativeHookRelayDeferredToolApprovalRequester =
|
||||
requestDeferredPluginToolApproval;
|
||||
|
||||
const NATIVE_HOOK_TOOL_NAME_ALIASES: Record<string, string> = {
|
||||
exec_command: "exec",
|
||||
@@ -429,6 +459,7 @@ function unregisterNativeHookRelay(
|
||||
unregisterNativeHookRelayBridge(relayId);
|
||||
relays.delete(relayId);
|
||||
removeNativeHookRelayInvocations(relayId);
|
||||
removeNativeHookRelayPreToolUseApprovals(relayId);
|
||||
removeNativeHookRelayPermissionState(relayId);
|
||||
}
|
||||
|
||||
@@ -596,6 +627,60 @@ export function hasNativeHookRelayInvocation(params: {
|
||||
);
|
||||
}
|
||||
|
||||
export async function resolveNativeHookRelayDeferredToolApproval(params: {
|
||||
relayId: string;
|
||||
toolUseId?: string;
|
||||
signal?: AbortSignal;
|
||||
}): Promise<NativeHookRelayDeferredApprovalOutcome | undefined> {
|
||||
const pendingApprovalKey = nativeHookRelayPreToolUseApprovalKey({
|
||||
relayId: params.relayId,
|
||||
toolUseId: params.toolUseId,
|
||||
});
|
||||
if (!pendingApprovalKey) {
|
||||
return undefined;
|
||||
}
|
||||
const pendingApproval = pendingPreToolUseApprovals.get(pendingApprovalKey);
|
||||
if (!pendingApproval) {
|
||||
return undefined;
|
||||
}
|
||||
pendingApproval.resolutionPromise ??= resolveNativeHookRelayPreToolUseApproval(
|
||||
pendingApproval,
|
||||
params.signal,
|
||||
).finally(() => {
|
||||
if (pendingPreToolUseApprovals.get(pendingApprovalKey) === pendingApproval) {
|
||||
pendingPreToolUseApprovals.delete(pendingApprovalKey);
|
||||
}
|
||||
});
|
||||
return pendingApproval.resolutionPromise;
|
||||
}
|
||||
|
||||
async function resolveNativeHookRelayPreToolUseApproval(
|
||||
pendingApproval: NativeHookRelayPreToolUseApproval,
|
||||
signal?: AbortSignal,
|
||||
): Promise<NativeHookRelayDeferredApprovalOutcome> {
|
||||
const outcome = await nativeHookRelayDeferredToolApprovalRequester({
|
||||
deferredApproval: pendingApproval.deferredApproval,
|
||||
signal,
|
||||
});
|
||||
if (outcome.blocked) {
|
||||
return { handled: true, outcome: "denied", reason: outcome.reason };
|
||||
}
|
||||
if (
|
||||
nativeHookRelayParamsWereRewritten(pendingApproval.originalParamsFingerprint, outcome.params)
|
||||
) {
|
||||
return {
|
||||
handled: true,
|
||||
outcome: "denied",
|
||||
reason:
|
||||
"OpenClaw tool policy rewrote Codex app-server approval params; refusing original request.",
|
||||
};
|
||||
}
|
||||
return {
|
||||
handled: true,
|
||||
outcome: "approved-once",
|
||||
};
|
||||
}
|
||||
|
||||
export async function invokeNativeHookRelayBridge(
|
||||
params: InvokeNativeHookRelayBridgeParams,
|
||||
): Promise<NativeHookRelayProcessResponse> {
|
||||
@@ -700,6 +785,55 @@ function canAcceptNativeHookRelayGenerationMismatch(
|
||||
return true;
|
||||
}
|
||||
|
||||
function nativeHookRelayPreToolUseApprovalKey(params: {
|
||||
relayId: string;
|
||||
toolUseId?: string;
|
||||
}): string | undefined {
|
||||
const toolUseId = params.toolUseId?.trim();
|
||||
return toolUseId ? `${params.relayId}:${toolUseId}` : undefined;
|
||||
}
|
||||
|
||||
function setNativeHookRelayPreToolUseApproval(params: {
|
||||
relayId: string;
|
||||
toolUseId?: string;
|
||||
deferredApproval: DeferredPluginToolApproval;
|
||||
originalParamsFingerprint: string;
|
||||
}): boolean {
|
||||
const key = nativeHookRelayPreToolUseApprovalKey(params);
|
||||
if (!key) {
|
||||
return false;
|
||||
}
|
||||
const previousApproval = pendingPreToolUseApprovals.get(key);
|
||||
if (previousApproval) {
|
||||
cancelDeferredPluginToolApproval(previousApproval.deferredApproval);
|
||||
}
|
||||
pendingPreToolUseApprovals.set(key, {
|
||||
deferredApproval: params.deferredApproval,
|
||||
originalParamsFingerprint: params.originalParamsFingerprint,
|
||||
});
|
||||
if (pendingPreToolUseApprovals.size > MAX_NATIVE_HOOK_RELAY_INVOCATIONS) {
|
||||
const oldestKey = pendingPreToolUseApprovals.keys().next().value;
|
||||
if (oldestKey) {
|
||||
const oldestApproval = pendingPreToolUseApprovals.get(oldestKey);
|
||||
if (oldestApproval) {
|
||||
cancelDeferredPluginToolApproval(oldestApproval.deferredApproval);
|
||||
}
|
||||
pendingPreToolUseApprovals.delete(oldestKey);
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function removeNativeHookRelayPreToolUseApprovals(relayId: string): void {
|
||||
const prefix = `${relayId}:`;
|
||||
for (const [key, pendingApproval] of pendingPreToolUseApprovals) {
|
||||
if (key.startsWith(prefix)) {
|
||||
cancelDeferredPluginToolApproval(pendingApproval.deferredApproval);
|
||||
pendingPreToolUseApprovals.delete(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function pruneExpiredNativeHookRelays(now = Date.now()): void {
|
||||
for (const [relayId, registration] of relays) {
|
||||
if (now > registration.expiresAtMs) {
|
||||
@@ -1160,13 +1294,12 @@ async function runNativeHookRelayPreToolUse(params: {
|
||||
const toolName = normalizeNativeHookToolName(params.invocation.toolName);
|
||||
const toolInput = params.adapter.readToolInput(params.invocation.rawPayload);
|
||||
const originalToolInputFingerprint = stableStringify(toolInput);
|
||||
const approvalMode = readNativeHookRelayApprovalMode(params.invocation.rawPayload);
|
||||
const outcome = await runBeforeToolCallHook({
|
||||
toolName,
|
||||
params: toolInput,
|
||||
...(params.invocation.toolUseId ? { toolCallId: params.invocation.toolUseId } : {}),
|
||||
...(readNativeHookRelayApprovalMode(params.invocation.rawPayload) === "report"
|
||||
? { approvalMode: "report" }
|
||||
: {}),
|
||||
...(approvalMode === "report" ? { approvalMode: "defer" } : {}),
|
||||
signal: params.registration.signal,
|
||||
ctx: {
|
||||
...(params.registration.agentId ? { agentId: params.registration.agentId } : {}),
|
||||
@@ -1181,6 +1314,22 @@ async function runNativeHookRelayPreToolUse(params: {
|
||||
if (outcome.blocked) {
|
||||
return params.adapter.renderPreToolUseBlockResponse(outcome.reason);
|
||||
}
|
||||
if (outcome.deferredApproval) {
|
||||
if (
|
||||
!setNativeHookRelayPreToolUseApproval({
|
||||
relayId: params.registration.relayId,
|
||||
toolUseId: params.invocation.toolUseId,
|
||||
deferredApproval: outcome.deferredApproval,
|
||||
originalParamsFingerprint: originalToolInputFingerprint,
|
||||
})
|
||||
) {
|
||||
cancelDeferredPluginToolApproval(outcome.deferredApproval);
|
||||
return params.adapter.renderPreToolUseBlockResponse(
|
||||
"Plugin approval required but Codex tool id unavailable.",
|
||||
);
|
||||
}
|
||||
return params.adapter.renderNoopResponse(params.invocation.event);
|
||||
}
|
||||
if (nativeHookRelayParamsWereRewritten(originalToolInputFingerprint, outcome.params)) {
|
||||
// Codex app-server may continue with the original params when updatedInput
|
||||
// is unsupported, so rewrites must fail closed here.
|
||||
@@ -2061,9 +2210,14 @@ export const testing = {
|
||||
relays.clear();
|
||||
invocations.length = 0;
|
||||
pendingPermissionApprovals.clear();
|
||||
for (const pendingApproval of pendingPreToolUseApprovals.values()) {
|
||||
cancelDeferredPluginToolApproval(pendingApproval.deferredApproval);
|
||||
}
|
||||
pendingPreToolUseApprovals.clear();
|
||||
permissionApprovalWindows.clear();
|
||||
permissionAllowAlwaysApprovals.clear();
|
||||
nativeHookRelayPermissionApprovalRequester = requestNativeHookRelayPermissionApproval;
|
||||
nativeHookRelayDeferredToolApprovalRequester = requestDeferredPluginToolApproval;
|
||||
},
|
||||
getNativeHookRelayInvocationsForTests(): NativeHookRelayInvocation[] {
|
||||
return [...invocations];
|
||||
@@ -2099,5 +2253,10 @@ export const testing = {
|
||||
): void {
|
||||
nativeHookRelayPermissionApprovalRequester = requester;
|
||||
},
|
||||
setNativeHookRelayDeferredToolApprovalRequesterForTests(
|
||||
requester: NativeHookRelayDeferredToolApprovalRequester,
|
||||
): void {
|
||||
nativeHookRelayDeferredToolApprovalRequester = requester;
|
||||
},
|
||||
} as const;
|
||||
export { testing as __testing };
|
||||
|
||||
@@ -201,10 +201,12 @@ export {
|
||||
getBeforeToolCallPolicyDiagnosticState,
|
||||
hasBeforeToolCallPolicy,
|
||||
isToolWrappedWithBeforeToolCallHook,
|
||||
requestDeferredPluginToolApproval,
|
||||
runBeforeToolCallHook,
|
||||
setBeforeToolCallDiagnosticsEnabled,
|
||||
wrapToolWithBeforeToolCallHook,
|
||||
type BeforeToolCallPolicyDiagnosticState,
|
||||
type DeferredPluginToolApproval,
|
||||
} from "../agents/agent-tools.before-tool-call.js";
|
||||
export {
|
||||
resolveAgentHarnessBeforePromptBuildResult,
|
||||
@@ -260,6 +262,7 @@ export {
|
||||
buildNativeHookRelayCommand,
|
||||
hasNativeHookRelayInvocation,
|
||||
invokeNativeHookRelay,
|
||||
resolveNativeHookRelayDeferredToolApproval,
|
||||
testing as nativeHookRelayTesting,
|
||||
registerNativeHookRelay,
|
||||
} from "../agents/harness/native-hook-relay.js";
|
||||
|
||||
Reference in New Issue
Block a user