From ec46d30fbfea48d00e44c0d05668e8b1d5c27db7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 29 Jul 2026 18:16:51 -0700 Subject: [PATCH] fix(codex): preserve requester across approval bridge (#116152) --- .../src/app-server/approval-bridge.test.ts | 46 ++++++ .../codex/src/app-server/approval-bridge.ts | 5 + ...pproval-requester.real-binary.live.test.ts | 154 ++++++++++++++++++ .../codex/src/app-server/hook-requester.ts | 27 +++ .../src/app-server/run-attempt-resources.ts | 12 +- 5 files changed, 235 insertions(+), 9 deletions(-) create mode 100644 extensions/codex/src/app-server/approval-requester.real-binary.live.test.ts create mode 100644 extensions/codex/src/app-server/hook-requester.ts diff --git a/extensions/codex/src/app-server/approval-bridge.test.ts b/extensions/codex/src/app-server/approval-bridge.test.ts index 84ca3cb27a08..559aa75db921 100644 --- a/extensions/codex/src/app-server/approval-bridge.test.ts +++ b/extensions/codex/src/app-server/approval-bridge.test.ts @@ -232,6 +232,48 @@ describe("Codex app-server approval bridge", () => { }, ); + it("preserves the initiating requester when re-running policy for a promoted file approval", async () => { + const params = createParams(); + params.senderId = "owner-1"; + params.senderIsOwner = true; + params.memberRoleIds = ["role-a", "role-b"]; + mockRunBeforeToolCallHook.mockImplementation(async ({ params: hookParams, ctx }) => + ctx?.requester?.senderIsOwner === true + ? { blocked: false, params: hookParams } + : { blocked: true, reason: "owner required", kind: "veto" }, + ); + + const result = await handleCodexAppServerApprovalRequest({ + method: "item/fileChange/requestApproval", + requestParams: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "patch-owner-policy", + }, + paramsForRun: params, + threadId: "thread-1", + turnId: "turn-1", + autoApproveOpenClawToolPolicy: true, + }); + + expect(result).toEqual({ decision: "accept" }); + expect(mockRunBeforeToolCallHook).toHaveBeenCalledWith( + expect.objectContaining({ + toolName: "apply_patch", + ctx: expect.objectContaining({ + requester: { + channel: "telegram", + accountId: "default", + senderId: "owner-1", + senderIsOwner: true, + roleIds: ["role-a", "role-b"], + }, + }), + }), + ); + expect(mockCallGatewayTool).not.toHaveBeenCalled(); + }); + it.each([ ["promoted tool policy", { autoApproveOpenClawToolPolicy: true }], ["full-auto runtime policy", { autoApprove: true }], @@ -328,6 +370,10 @@ describe("Codex app-server approval bridge", () => { agentId: "main", sessionKey: "agent:main:session-1", channelId: "chat-1", + requester: { + channel: "telegram", + accountId: "default", + }, workspaceDir: undefined, turnSourceChannel: "telegram", turnSourceTo: "chat-1", diff --git a/extensions/codex/src/app-server/approval-bridge.ts b/extensions/codex/src/app-server/approval-bridge.ts index d15dce649872..3e8c99df87b1 100644 --- a/extensions/codex/src/app-server/approval-bridge.ts +++ b/extensions/codex/src/app-server/approval-bridge.ts @@ -19,6 +19,7 @@ import { normalizeTrimmedStringList } from "openclaw/plugin-sdk/string-coerce-ru import { sliceUtf16Safe, truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime"; import { formatCodexDisplayText } from "../command-formatters.js"; import { resolveCodexToolAbortTerminalReason } from "./dynamic-tool-execution.js"; +import { buildCodexHookRequester } from "./hook-requester.js"; import { approvalRequestExplicitlyUnavailable, mapExecDecisionToOutcome, @@ -470,6 +471,7 @@ async function runOpenClawToolPolicyForApprovalRequest(params: { currentChannelId: params.paramsForRun.currentChannelId, messageTo: params.paramsForRun.messageTo, }).channelId; + const requester = buildCodexHookRequester(params.paramsForRun); const outcome = await runBeforeToolCallHook({ toolName: policyRequest.toolName, params: policyRequest.params, @@ -485,6 +487,9 @@ async function runOpenClawToolPolicyForApprovalRequest(params: { ...(params.paramsForRun.sessionId ? { sessionId: params.paramsForRun.sessionId } : {}), ...(params.paramsForRun.runId ? { runId: params.paramsForRun.runId } : {}), ...(hookChannelId ? { channelId: hookChannelId } : {}), + // This is the same concrete call already seen by native PreToolUse. Preserve + // its host-proven actor so sender-aware policy cannot authorize two identities. + ...(requester ? { requester } : {}), trigger: params.paramsForRun.trigger, approvalReviewerDeviceId: params.paramsForRun.approvalReviewerDeviceId, turnSourceChannel: params.paramsForRun.messageChannel ?? params.paramsForRun.messageProvider, diff --git a/extensions/codex/src/app-server/approval-requester.real-binary.live.test.ts b/extensions/codex/src/app-server/approval-requester.real-binary.live.test.ts new file mode 100644 index 000000000000..d25efe8896b0 --- /dev/null +++ b/extensions/codex/src/app-server/approval-requester.real-binary.live.test.ts @@ -0,0 +1,154 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import type { EmbeddedRunAttemptParams } from "openclaw/plugin-sdk/agent-harness-runtime"; +import { + initializeGlobalHookRunner, + resetGlobalHookRunner, +} from "openclaw/plugin-sdk/hook-runtime"; +import { createMockPluginRegistry } from "openclaw/plugin-sdk/plugin-test-runtime"; +import { withTempDir } from "openclaw/plugin-sdk/test-env"; +import type { PluginHookToolContext } from "openclaw/plugin-sdk/types"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { resolveCodexAppServerRuntimeOptions } from "./config.js"; +import type { CodexModelListResponse } from "./protocol.js"; +import { runCodexAppServerAttempt } from "./run-attempt.js"; +import { createCodexTestBindingStore } from "./session-binding.test-helpers.js"; +import { createIsolatedCodexAppServerClient } from "./shared-client.js"; + +const LIVE = + process.env.OPENCLAW_LIVE_TEST === "1" && + process.env.OPENCLAW_LIVE_CODEX_APPROVAL_REQUESTER === "1"; +const describeLive = LIVE ? describe : describe.skip; + +afterEach(() => { + resetGlobalHookRunner(); + vi.unstubAllEnvs(); +}); + +describeLive("Codex app-server approval requester real-binary bridge", () => { + it("preserves an owner requester through promoted apply_patch approval", async () => { + await withTempDir("openclaw-codex-approval-requester-", async (root) => { + const workspace = path.join(root, "workspace"); + const agentDir = path.join(root, "agent"); + const target = path.join(workspace, "memory", "real-binary-owner.md"); + await fs.mkdir(path.dirname(target), { recursive: true }); + vi.stubEnv("OPENCLAW_STATE_DIR", path.join(root, "state")); + + const runtime = resolveCodexAppServerRuntimeOptions({ + pluginConfig: { appServer: { homeScope: "user" } }, + env: {}, + }); + const client = await createIsolatedCodexAppServerClient({ + startOptions: runtime.start, + agentDir, + authProfileId: null, + timeoutMs: 120_000, + }); + try { + const listed = await client.request( + "model/list", + { limit: 100, cursor: null, includeHidden: false }, + { timeoutMs: 60_000 }, + ); + const modelId = + listed.data.find((model) => model.isDefault)?.model ?? listed.data[0]?.model; + if (!modelId) { + throw new Error("Codex model/list returned no models"); + } + + const hookRequesters: unknown[] = []; + initializeGlobalHookRunner( + createMockPluginRegistry([ + { + hookName: "before_tool_call", + handler: async (_event, ctx) => { + const hookContext = ctx as PluginHookToolContext; + hookRequesters.push(hookContext.requester); + return hookContext.requester?.senderIsOwner === true + ? undefined + : { block: true, blockReason: "owner required" }; + }, + }, + ]), + ); + const serverRequestMethods: string[] = []; + client.addRequestHandler((request) => { + serverRequestMethods.push(request.method); + return undefined; + }); + const agentEvents: Array<{ stream: string; data: Record }> = []; + const params = { + sessionId: "approval-requester-session", + sessionKey: "agent:approval-requester:main", + sessionFile: path.join(root, "session.jsonl"), + workspaceDir: workspace, + cwd: workspace, + agentDir, + provider: "codex", + modelId, + model: { + id: modelId, + name: modelId, + provider: "codex", + api: "openai-chatgpt-responses", + reasoning: true, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 200_000, + maxTokens: 8_000, + compat: { supportsTools: false }, + }, + prompt: + "Use the exec tool exactly once. In its JavaScript call tools.apply_patch to create memory/real-binary-owner.md containing exactly REAL_BINARY_OWNER_OK, then reply done. Do not call apply_patch directly.", + runId: "approval-requester-run", + contextTokenBudget: 150_000, + contextWindowInfo: { + tokens: 150_000, + referenceTokens: 200_000, + source: "agentContextTokens", + }, + thinkLevel: "medium", + disableTools: false, + config: { tools: { web: { search: { enabled: false } } } }, + timeoutMs: 180_000, + trigger: "user", + oneShotCliRun: true, + senderIsOwner: true, + authStorage: {}, + authProfileStore: { version: 1, profiles: {} }, + modelRegistry: {}, + onAgentEvent: (event: { stream: string; data: Record }) => { + agentEvents.push(event); + }, + } as unknown as EmbeddedRunAttemptParams; + + const result = await runCodexAppServerAttempt(params, { + bindingStore: createCodexTestBindingStore(), + pluginConfig: { appServer: { homeScope: "user" } }, + nativeHookRelay: { enabled: true, events: ["pre_tool_use"] }, + clientFactory: async () => client, + }); + + expect(result.terminal.kind, JSON.stringify(result.terminal)).toBe("ok"); + expect(await fs.readFile(target, "utf8")).toBe("REAL_BINARY_OWNER_OK\n"); + expect(serverRequestMethods).toContain("item/fileChange/requestApproval"); + expect(hookRequesters).toEqual( + expect.arrayContaining([expect.objectContaining({ senderIsOwner: true })]), + ); + expect(agentEvents).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + stream: "approval", + data: expect.objectContaining({ + status: "approved", + message: "Codex app-server approval accepted by OpenClaw tool policy.", + }), + }), + ]), + ); + } finally { + await client.closeAndWait(); + } + }); + }, 240_000); +}); diff --git a/extensions/codex/src/app-server/hook-requester.ts b/extensions/codex/src/app-server/hook-requester.ts new file mode 100644 index 000000000000..848013157597 --- /dev/null +++ b/extensions/codex/src/app-server/hook-requester.ts @@ -0,0 +1,27 @@ +import type { EmbeddedRunAttemptParams } from "openclaw/plugin-sdk/agent-harness-runtime"; +import type { PluginHookToolContext } from "openclaw/plugin-sdk/types"; + +type CodexHookRequester = NonNullable; + +/** Rebuilds the host-proven requester identity shared by native and bridged tool hooks. */ +export function buildCodexHookRequester( + params: Pick< + EmbeddedRunAttemptParams, + | "messageChannel" + | "messageProvider" + | "agentAccountId" + | "senderId" + | "senderIsOwner" + | "memberRoleIds" + >, +): CodexHookRequester | undefined { + const channel = params.messageChannel ?? params.messageProvider; + const requester: CodexHookRequester = { + ...(channel ? { channel } : {}), + ...(params.agentAccountId ? { accountId: params.agentAccountId } : {}), + ...(params.senderId ? { senderId: params.senderId } : {}), + ...(params.senderIsOwner !== undefined ? { senderIsOwner: params.senderIsOwner } : {}), + ...(params.memberRoleIds?.length ? { roleIds: [...params.memberRoleIds] } : {}), + }; + return Object.keys(requester).length > 0 ? requester : undefined; +} diff --git a/extensions/codex/src/app-server/run-attempt-resources.ts b/extensions/codex/src/app-server/run-attempt-resources.ts index 72188eb5e79e..638380f459e0 100644 --- a/extensions/codex/src/app-server/run-attempt-resources.ts +++ b/extensions/codex/src/app-server/run-attempt-resources.ts @@ -7,6 +7,7 @@ import { resolveCodexStartupTimeoutMs } from "./attempt-timeouts.js"; import type { CodexAppServerClient } from "./client.js"; import { resolveCodexToolAbortTerminalReason } from "./dynamic-tool-execution.js"; import { CodexAppServerEventProjector } from "./event-projector.js"; +import { buildCodexHookRequester } from "./hook-requester.js"; import { buildCodexNativeHookRelayDisabledConfig, buildCodexNativeHookRelayConfig, @@ -183,14 +184,7 @@ export function prepareCodexAttemptResources(prompt: CodexAttemptPrompt) { timeoutFloorMs: options.startupTimeoutFloorMs, }); const requesterChannel = params.messageChannel ?? params.messageProvider; - const requester = { - ...(requesterChannel ? { channel: requesterChannel } : {}), - ...(params.agentAccountId ? { accountId: params.agentAccountId } : {}), - ...(params.senderId ? { senderId: params.senderId } : {}), - ...(params.senderIsOwner !== undefined ? { senderIsOwner: params.senderIsOwner } : {}), - ...(params.memberRoleIds?.length ? { roleIds: [...params.memberRoleIds] } : {}), - }; - const hasRequester = Object.keys(requester).length > 0; + const requester = buildCodexHookRequester(params); const buildNativeHookRelayFinalConfigPatch = ( decision: { action: "resume"; binding: CodexAppServerThreadBinding } | { action: "start" }, ) => { @@ -210,7 +204,7 @@ export function prepareCodexAttemptResources(prompt: CodexAttemptPrompt) { config: params.config, runId: params.runId, channelId: hookChannelId, - ...(hasRequester ? { requester } : {}), + ...(requester ? { requester } : {}), approvalContext: { trigger: params.trigger, approvalReviewerDeviceId: params.approvalReviewerDeviceId,