From dad3db40d3a7e7e686aa0ec0a1497ec016ca0424 Mon Sep 17 00:00:00 2001 From: VACInc <3279061+VACInc@users.noreply.github.com> Date: Fri, 15 May 2026 20:53:30 -0400 Subject: [PATCH] fix(codex): honor denied app-server tool policy --- .../codex/src/app-server/run-attempt.test.ts | 85 +++++++++++++++++++ .../codex/src/app-server/run-attempt.ts | 39 +++++++-- .../src/app-server/thread-lifecycle.test.ts | 33 +++++++ .../codex/src/app-server/thread-lifecycle.ts | 34 ++++++-- 4 files changed, 179 insertions(+), 12 deletions(-) diff --git a/extensions/codex/src/app-server/run-attempt.test.ts b/extensions/codex/src/app-server/run-attempt.test.ts index 9261799d79e..00089347826 100644 --- a/extensions/codex/src/app-server/run-attempt.test.ts +++ b/extensions/codex/src/app-server/run-attempt.test.ts @@ -953,6 +953,35 @@ describe("runCodexAppServerAttempt", () => { ).toEqual(["exec", "apply_patch", "read"]); }); + it("treats an explicit empty Codex dynamic toolsAllow as no tools", () => { + const tools = ["message", "web_search"].map((name) => ({ name })); + + expect(__testing.filterCodexDynamicToolsForAllowlist(tools, [])).toEqual([]); + }); + + it("treats wildcard Codex dynamic toolsAllow as unrestricted", () => { + const tools = ["message", "web_search"].map((name) => ({ name })); + + expect(__testing.filterCodexDynamicToolsForAllowlist(tools, [" * "])).toEqual(tools); + }); + + it("disables Codex native tool surfaces for restricted runtime allowlists", () => { + const workspaceDir = path.join(tempDir, "workspace"); + const params = createParams(path.join(tempDir, "session.jsonl"), workspaceDir); + params.disableTools = false; + + expect(__testing.shouldEnableCodexAppServerNativeToolSurface(params)).toBe(true); + + params.toolsAllow = ["*"]; + expect(__testing.shouldEnableCodexAppServerNativeToolSurface(params)).toBe(true); + + params.toolsAllow = []; + expect(__testing.shouldEnableCodexAppServerNativeToolSurface(params)).toBe(false); + + params.toolsAllow = ["message"]; + expect(__testing.shouldEnableCodexAppServerNativeToolSurface(params)).toBe(false); + }); + it("forces the message dynamic tool for message-tool-only source replies", () => { const workspaceDir = path.join(tempDir, "workspace"); const params = createParams(path.join(tempDir, "session.jsonl"), workspaceDir); @@ -1072,6 +1101,62 @@ describe("runCodexAppServerAttempt", () => { expect(heartbeat?.deferLoading).toBe(true); }); + it("disables Codex native tool surfaces when runtime toolsAllow is empty", async () => { + __testing.setOpenClawCodingToolsFactoryForTests(() => [ + createRuntimeDynamicTool("message"), + createRuntimeDynamicTool("web_search"), + ]); + const harness = createStartedThreadHarness(async (method) => { + if (method === "app/list") { + throw new Error("app/list should not run when runtime toolsAllow is empty."); + } + return undefined; + }); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.disableTools = false; + params.runtimePlan = createCodexRuntimePlanFixture(); + params.toolsAllow = []; + + const run = runCodexAppServerAttempt(params, { + pluginConfig: { + appServer: { mode: "yolo" }, + codexPlugins: { + enabled: true, + plugins: { + "google-calendar": { + marketplaceName: "openai-curated", + pluginName: "google-calendar", + }, + }, + }, + }, + }); + await harness.waitForMethod("turn/start", 120_000); + await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" }); + await run; + + const startRequest = harness.requests.find((entry) => entry.method === "thread/start"); + const startParams = startRequest?.params as + | { + dynamicTools?: Array<{ name?: string }>; + config?: { + "features.code_mode"?: boolean; + "features.code_mode_only"?: boolean; + apps?: Record; + }; + } + | undefined; + + expect(startParams?.dynamicTools).toEqual([]); + expect(startParams?.config?.["features.code_mode"]).toBe(false); + expect(startParams?.config?.["features.code_mode_only"]).toBe(false); + expect(startParams?.config?.apps?.["google-calendar-app"]?.enabled).toBeUndefined(); + expect(harness.requests.map((entry) => entry.method)).not.toContain("app/list"); + }); + it("returns a run context report without deferred Codex dynamic tool schemas", async () => { __testing.setOpenClawCodingToolsFactoryForTests(() => [ createRuntimeDynamicTool("message"), diff --git a/extensions/codex/src/app-server/run-attempt.ts b/extensions/codex/src/app-server/run-attempt.ts index 954164b5ca2..795d717a288 100644 --- a/extensions/codex/src/app-server/run-attempt.ts +++ b/extensions/codex/src/app-server/run-attempt.ts @@ -859,6 +859,7 @@ export async function runCodexAppServerAttempt( disableTools: params.disableTools, toolsAllow: params.toolsAllow, }); + const nativeToolSurfaceEnabled = shouldEnableCodexAppServerNativeToolSurface(params); for (const diagnostic of bundleMcpThreadConfig.diagnostics) { embeddedAgentLog.warn(`bundle-mcp: ${diagnostic.pluginId}: ${diagnostic.message}`); } @@ -1136,7 +1137,8 @@ export async function runCodexAppServerAttempt( const threadConfig = mergeCodexThreadConfigs( bundleMcpThreadConfig?.configPatch as JsonObject | undefined, ); - const pluginThreadConfigEnabled = shouldBuildCodexPluginThreadConfig(pluginConfig); + const pluginThreadConfigEnabled = + nativeToolSurfaceEnabled && shouldBuildCodexPluginThreadConfig(pluginConfig); const pluginAppCacheKey = buildCodexPluginAppCacheKey({ appServer, agentDir, @@ -1197,6 +1199,7 @@ export async function runCodexAppServerAttempt( developerInstructions: promptBuild.developerInstructions, config: threadConfig, finalConfigPatch: nativeHookRelayConfig, + nativeCodeModeEnabled: nativeToolSurfaceEnabled, mcpServersFingerprint: bundleMcpThreadConfig.fingerprint, mcpServersFingerprintEvaluated: bundleMcpThreadConfig.evaluated, contextEngineProjection, @@ -3109,10 +3112,11 @@ function includeForcedMessageToolAllow( toolsAllow: string[] | undefined, params: EmbeddedRunAttemptParams, ): string[] | undefined { - if (!shouldForceMessageTool(params)) { - return toolsAllow; - } - if (toolsAllow === undefined) { + if ( + !shouldForceMessageTool(params) || + toolsAllow === undefined || + hasWildcardCodexToolsAllow(toolsAllow) + ) { return toolsAllow; } if (toolsAllow.length === 0) { @@ -3122,11 +3126,28 @@ function includeForcedMessageToolAllow( return normalized.has("message") ? toolsAllow : [...toolsAllow, "message"]; } +function shouldEnableCodexAppServerNativeToolSurface(params: EmbeddedRunAttemptParams): boolean { + const toolsAllow = includeForcedMessageToolAllow(params.toolsAllow, params); + if (toolsAllow === undefined) { + return true; + } + // Codex native code mode exposes its shell/file surface as one app-server + // capability, so narrow OpenClaw allowlists must fail closed rather than + // widening `message` or `web_search` into shell access. + return hasWildcardCodexToolsAllow(toolsAllow); +} + function filterCodexDynamicToolsForAllowlist( tools: T[], toolsAllow?: string[], ): T[] { - if (!toolsAllow || toolsAllow.length === 0) { + if (!toolsAllow) { + return tools; + } + if (toolsAllow.length === 0) { + return []; + } + if (hasWildcardCodexToolsAllow(toolsAllow)) { return tools; } const allowSet = new Set( @@ -3135,6 +3156,10 @@ function filterCodexDynamicToolsForAllowlist( return tools.filter((tool) => allowSet.has(normalizeCodexDynamicToolName(tool.name))); } +function hasWildcardCodexToolsAllow(toolsAllow: string[]): boolean { + return toolsAllow.some((name) => normalizeCodexDynamicToolName(name) === "*"); +} + function shouldForceMessageTool(params: EmbeddedRunAttemptParams): boolean { return params.sourceReplyDeliveryMode === "message_tool_only"; } @@ -4221,6 +4246,7 @@ export const __testing = { buildDynamicTools, filterCodexDynamicToolsForAllowlist, filterToolsForVisionInputs, + hasWildcardCodexToolsAllow, handleDynamicToolCallWithTimeout, isInvalidCodexImagePayloadError, remapCodexContextFilePath, @@ -4230,6 +4256,7 @@ export const __testing = { restrictCodexAppServerSandboxForOpenClawSandbox, resolveCodexAppServerForOpenClawToolPolicy, resolveOpenClawCodingToolsSessionKeys, + shouldEnableCodexAppServerNativeToolSurface, shouldForceMessageTool, setOpenClawCodingToolsFactoryForTests(factory: OpenClawCodingToolsFactory): void { openClawCodingToolsFactoryForTests = factory; diff --git a/extensions/codex/src/app-server/thread-lifecycle.test.ts b/extensions/codex/src/app-server/thread-lifecycle.test.ts index 6eb1d1b2a97..072d2c889e0 100644 --- a/extensions/codex/src/app-server/thread-lifecycle.test.ts +++ b/extensions/codex/src/app-server/thread-lifecycle.test.ts @@ -91,6 +91,39 @@ describe("Codex app-server native code mode config", () => { }); }); + it("disables Codex native code mode on thread/start when runtime policy denies it", () => { + const request = buildThreadStartParams(createAttemptParams({ provider: "openai" }), { + cwd: "/repo", + dynamicTools: [], + appServer: createAppServerOptions() as never, + developerInstructions: "test instructions", + nativeCodeModeEnabled: false, + config: { + "features.code_mode": true, + "features.code_mode_only": true, + }, + }); + + expect(request.config).toEqual({ + "features.code_mode": false, + "features.code_mode_only": false, + }); + }); + + it("disables Codex native code mode on thread/resume when runtime policy denies it", () => { + const request = buildThreadResumeParams(createAttemptParams({ provider: "openai" }), { + threadId: "thread-1", + appServer: createAppServerOptions() as never, + developerInstructions: "test instructions", + nativeCodeModeEnabled: false, + }); + + expect(request.config).toEqual({ + "features.code_mode": false, + "features.code_mode_only": false, + }); + }); + it("disables native Codex project docs for lightweight context threads", () => { const request = buildThreadStartParams( createAttemptParams({ diff --git a/extensions/codex/src/app-server/thread-lifecycle.ts b/extensions/codex/src/app-server/thread-lifecycle.ts index 45f3db47c6a..34f40d3e93e 100644 --- a/extensions/codex/src/app-server/thread-lifecycle.ts +++ b/extensions/codex/src/app-server/thread-lifecycle.ts @@ -73,6 +73,11 @@ export const CODEX_CODE_MODE_THREAD_CONFIG: JsonObject = { "features.code_mode_only": true, }; +export const CODEX_CODE_MODE_DISABLED_THREAD_CONFIG: JsonObject = { + "features.code_mode": false, + "features.code_mode_only": false, +}; + const CODEX_LIGHTWEIGHT_CONTEXT_THREAD_CONFIG: JsonObject = { project_doc_max_bytes: 0, }; @@ -87,6 +92,7 @@ export async function startOrResumeThread(params: { developerInstructions?: string; config?: JsonObject; finalConfigPatch?: JsonObject; + nativeCodeModeEnabled?: boolean; mcpServersFingerprint?: string; mcpServersFingerprintEvaluated?: boolean; pluginThreadConfig?: CodexPluginThreadConfigProvider; @@ -246,6 +252,7 @@ export async function startOrResumeThread(params: { appServer: params.appServer, developerInstructions: params.developerInstructions, config: resumeConfig, + nativeCodeModeEnabled: params.nativeCodeModeEnabled, }), ), ); @@ -341,6 +348,7 @@ export async function startOrResumeThread(params: { appServer: params.appServer, developerInstructions: params.developerInstructions, config, + nativeCodeModeEnabled: params.nativeCodeModeEnabled, }), ), ); @@ -539,6 +547,7 @@ export function buildThreadStartParams( appServer: CodexAppServerRuntimeOptions; developerInstructions?: string; config?: JsonObject; + nativeCodeModeEnabled?: boolean; }, ): CodexThreadStartParams { const modelProvider = resolveCodexAppServerModelProvider({ @@ -557,7 +566,9 @@ export function buildThreadStartParams( sandbox: options.appServer.sandbox, ...(options.appServer.serviceTier ? { serviceTier: options.appServer.serviceTier } : {}), serviceName: "OpenClaw", - config: buildCodexRuntimeThreadConfigForRun(params, options.config), + config: buildCodexRuntimeThreadConfigForRun(params, options.config, { + nativeCodeModeEnabled: options.nativeCodeModeEnabled, + }), developerInstructions: options.developerInstructions ?? buildDeveloperInstructions(params), dynamicTools: options.dynamicTools, experimentalRawEvents: true, @@ -573,6 +584,7 @@ export function buildThreadResumeParams( appServer: CodexAppServerRuntimeOptions; developerInstructions?: string; config?: JsonObject; + nativeCodeModeEnabled?: boolean; }, ): CodexThreadResumeParams { const modelProvider = resolveCodexAppServerModelProvider({ @@ -590,15 +602,24 @@ export function buildThreadResumeParams( approvalsReviewer: options.appServer.approvalsReviewer, sandbox: options.appServer.sandbox, ...(options.appServer.serviceTier ? { serviceTier: options.appServer.serviceTier } : {}), - config: buildCodexRuntimeThreadConfigForRun(params, options.config), + config: buildCodexRuntimeThreadConfigForRun(params, options.config, { + nativeCodeModeEnabled: options.nativeCodeModeEnabled, + }), developerInstructions: options.developerInstructions ?? buildDeveloperInstructions(params), persistExtendedHistory: true, }; } -export function buildCodexRuntimeThreadConfig(config: JsonObject | undefined): JsonObject { - const runtimeConfig = mergeCodexThreadConfigs(config, CODEX_CODE_MODE_THREAD_CONFIG) ?? { - ...CODEX_CODE_MODE_THREAD_CONFIG, +export function buildCodexRuntimeThreadConfig( + config: JsonObject | undefined, + options: { nativeCodeModeEnabled?: boolean } = {}, +): JsonObject { + const codeModeConfig = + options.nativeCodeModeEnabled === false + ? CODEX_CODE_MODE_DISABLED_THREAD_CONFIG + : CODEX_CODE_MODE_THREAD_CONFIG; + const runtimeConfig = mergeCodexThreadConfigs(config, codeModeConfig) ?? { + ...codeModeConfig, }; return runtimeConfig; } @@ -606,8 +627,9 @@ export function buildCodexRuntimeThreadConfig(config: JsonObject | undefined): J function buildCodexRuntimeThreadConfigForRun( params: EmbeddedRunAttemptParams, config: JsonObject | undefined, + options: { nativeCodeModeEnabled?: boolean } = {}, ): JsonObject { - const runtimeConfig = buildCodexRuntimeThreadConfig(config); + const runtimeConfig = buildCodexRuntimeThreadConfig(config, options); if (params.bootstrapContextMode !== "lightweight") { return runtimeConfig; }