From 80227005a01667ef2ef243afad0bda80f8202b90 Mon Sep 17 00:00:00 2001 From: joshavant <830519+joshavant@users.noreply.github.com> Date: Thu, 28 May 2026 12:55:30 -0700 Subject: [PATCH] feat(exec): add normalized auto mode Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com> Co-authored-by: jesse-merhi <79823012+jesse-merhi@users.noreply.github.com> --- docs/.generated/config-baseline.sha256 | 8 +- .../.generated/plugin-sdk-api-baseline.sha256 | 4 +- docs/plugins/codex-harness-reference.md | 6 +- docs/plugins/codex-harness.md | 23 +- docs/plugins/sdk-migration.md | 1 + docs/plugins/sdk-subpaths.md | 1 + docs/tools/exec-approvals.md | 19 +- docs/tools/exec.md | 5 +- extensions/codex/index.ts | 8 +- extensions/codex/openclaw.plugin.json | 5 +- .../codex/src/app-server/app-server-policy.ts | 8 +- .../codex/src/app-server/config.test.ts | 759 ++++++++++++++++++ extensions/codex/src/app-server/config.ts | 431 +++++++++- .../src/app-server/dynamic-tool-build.test.ts | 35 + .../src/app-server/dynamic-tool-build.ts | 1 + .../codex/src/app-server/run-attempt.test.ts | 21 + .../codex/src/app-server/run-attempt.ts | 25 +- extensions/codex/src/command-handlers.ts | 1 + extensions/codex/src/commands.test.ts | 2 + .../codex/src/conversation-binding.test.ts | 324 +++++++- extensions/codex/src/conversation-binding.ts | 200 ++++- extensions/discord/src/components.builders.ts | 55 +- extensions/discord/src/components.test.ts | 22 + package.json | 4 + .../src/schema/exec-approvals.ts | 2 + packages/plugin-sdk/package.json | 4 + .../plugin-sdk/src/exec-approvals-runtime.ts | 1 + scripts/lib/plugin-sdk-entrypoints.json | 1 + .../agent-tools-agent-config.exec.test.ts | 80 ++ src/agents/agent-tools.ts | 42 +- .../bash-tools.exec-approval-request.ts | 8 + .../bash-tools.exec-host-gateway.test.ts | 337 +++++++- src/agents/bash-tools.exec-host-gateway.ts | 204 +++-- .../bash-tools.exec-host-node-phases.ts | 19 +- src/agents/bash-tools.exec-host-node.test.ts | 275 ++++++- src/agents/bash-tools.exec-host-node.ts | 483 ++++++----- src/agents/bash-tools.exec-host-node.types.ts | 3 + .../bash-tools.exec-host-shared.test.ts | 17 + src/agents/bash-tools.exec-host-shared.ts | 9 +- src/agents/bash-tools.exec-types.ts | 15 +- .../bash-tools.exec.security-floor.test.ts | 266 +++++- src/agents/bash-tools.exec.ts | 86 +- ...nt-runner.buildembeddedsandboxinfo.test.ts | 179 ++++- src/agents/embedded-agent-runner/compact.ts | 17 +- .../embedded-agent-runner/compact.types.ts | 3 +- .../embedded-agent-runner/run/attempt.ts | 16 +- .../embedded-agent-runner/sandbox-info.ts | 62 +- src/agents/exec-auto-reviewer.prompt.ts | 11 + src/agents/exec-auto-reviewer.test.ts | 311 +++++++ src/agents/exec-auto-reviewer.ts | 291 +++++++ src/agents/exec-defaults.test.ts | 171 ++++ src/agents/exec-defaults.ts | 137 +++- src/config/schema.help.quality.test.ts | 1 + src/config/schema.help.ts | 8 + src/config/schema.labels.ts | 4 + src/config/schema.tags.ts | 1 + src/config/schema.test.ts | 63 ++ src/config/types.tools.ts | 10 + src/config/zod-schema.agent-runtime.ts | 29 +- src/gateway/operator-approvals-client.test.ts | 6 +- src/gateway/operator-approvals-client.ts | 18 +- .../server-methods/approval-shared.test.ts | 51 ++ src/gateway/server-methods/approval-shared.ts | 55 +- src/gateway/server-methods/exec-approval.ts | 4 + src/infra/exec-approvals-effective.ts | 193 ++++- src/infra/exec-approvals-policy.test.ts | 195 +++++ src/infra/exec-approvals.ts | 184 ++++- src/infra/exec-auto-review.test.ts | 50 ++ src/infra/exec-auto-review.ts | 59 ++ src/node-host/invoke-system-run.test.ts | 203 ++++- src/node-host/invoke-system-run.ts | 187 ++++- src/plugin-sdk/exec-approvals-runtime.ts | 7 + ...tension-package-project-boundaries.test.ts | 3 + 73 files changed, 5822 insertions(+), 527 deletions(-) create mode 100644 packages/plugin-sdk/src/exec-approvals-runtime.ts create mode 100644 src/agents/exec-auto-reviewer.prompt.ts create mode 100644 src/agents/exec-auto-reviewer.test.ts create mode 100644 src/agents/exec-auto-reviewer.ts create mode 100644 src/infra/exec-auto-review.test.ts create mode 100644 src/infra/exec-auto-review.ts create mode 100644 src/plugin-sdk/exec-approvals-runtime.ts diff --git a/docs/.generated/config-baseline.sha256 b/docs/.generated/config-baseline.sha256 index a8f7387eccd..c9a1f697f00 100644 --- a/docs/.generated/config-baseline.sha256 +++ b/docs/.generated/config-baseline.sha256 @@ -1,4 +1,4 @@ -feec5d92420f2008f02beed1f09c373a9392bf54fec547087bfa0581d8c5fe85 config-baseline.json -e51fb382c686637ba6c413648fead48950982b72595c9c6aa1a50da109f4024d config-baseline.core.json -6e64ca2148297da39348568c1faf8c6c431efe3c7b53fb29914905f5b88322a4 config-baseline.channel.json -1b763a5524aca2d7ecf1eea38f845ad1ffed5c1b37e85e62f6a7902a3ee0f920 config-baseline.plugin.json +c80dea63b0a3786c8999d06aae62c110786f440b4d6748f9838577aaa2816971 config-baseline.json +948323a1507817b6580ed976f9f9449239008f40283cc7e6005148ecf0ca4582 config-baseline.core.json +f833ffca6bd88162f062bbea4f0eede783373f46674ebbfc3a390c80353930a2 config-baseline.channel.json +bc38b58b67132401a030b3b3a77efdb6c88f207ea1fab9abcb4599e1f9552dda config-baseline.plugin.json diff --git a/docs/.generated/plugin-sdk-api-baseline.sha256 b/docs/.generated/plugin-sdk-api-baseline.sha256 index ce718f481b1..eace4a215ef 100644 --- a/docs/.generated/plugin-sdk-api-baseline.sha256 +++ b/docs/.generated/plugin-sdk-api-baseline.sha256 @@ -1,2 +1,2 @@ -260286745285203c06b49fb0397217439bd8ab48be87b9de05fd62603bd720c1 plugin-sdk-api-baseline.json -9934014d22861b813f02c3fcb01e3b79d13a47d06a75e9096f2465f3c2f1dd5a plugin-sdk-api-baseline.jsonl +59de21361cab0622926ad313caf3f8dc43c28d420a82ba060680ecc30c472453 plugin-sdk-api-baseline.json +05adee9037669db4e834d1a0ca9705d5d94df770083862ab149d2f3e559010d2 plugin-sdk-api-baseline.jsonl diff --git a/docs/plugins/codex-harness-reference.md b/docs/plugins/codex-harness-reference.md index 044be4e5bf2..c614eaf96a6 100644 --- a/docs/plugins/codex-harness-reference.md +++ b/docs/plugins/codex-harness-reference.md @@ -118,7 +118,11 @@ prompts that nobody is around to answer. If Codex's local system requirements file disallows implicit YOLO approval, reviewer, or sandbox values, OpenClaw treats the implicit default as guardian -instead and selects allowed guardian permissions. Hostname-matching +instead and selects allowed guardian permissions. `tools.exec.mode: "auto"` +also forces guardian-reviewed Codex approvals and does not preserve unsafe +legacy `approvalPolicy: "never"` or `sandbox: "danger-full-access"` overrides; +set `tools.exec.mode: "full"` for an intentional no-approval posture. +Hostname-matching `[[remote_sandbox_config]]` entries in the same requirements file are honored for the sandbox default decision. diff --git a/docs/plugins/codex-harness.md b/docs/plugins/codex-harness.md index 0f65a1b9f4a..aced51c4bf5 100644 --- a/docs/plugins/codex-harness.md +++ b/docs/plugins/codex-harness.md @@ -362,30 +362,35 @@ turn instead of relying on Codex host-side sandboxing. Shell access is exposed through OpenClaw sandbox-backed dynamic tools such as `sandbox_exec` and `sandbox_process` when the normal exec/process tools are available. -Use guardian mode when you want Codex native auto-review before sandbox escapes -or extra permissions: +Use normalized OpenClaw exec mode when you want Codex native auto-review before +sandbox escapes or extra permissions: ```json5 { + tools: { + exec: { + mode: "auto", + }, + }, plugins: { entries: { codex: { enabled: true, - config: { - appServer: { - mode: "guardian", - serviceTier: "priority", - }, - }, }, }, }, } ``` -Guardian mode expands to Codex app-server approvals, usually +For Codex app-server sessions, OpenClaw maps `tools.exec.mode: "auto"` to Codex +Guardian-reviewed approvals, usually `approvalPolicy: "on-request"`, `approvalsReviewer: "auto_review"`, and `sandbox: "workspace-write"` when the local requirements allow those values. +In `tools.exec.mode: "auto"`, OpenClaw does not preserve legacy unsafe Codex +`approvalPolicy: "never"` or `sandbox: "danger-full-access"` overrides; use +`tools.exec.mode: "full"` for an intentional no-approval Codex posture. The +legacy `plugins.entries.codex.config.appServer.mode: "guardian"` preset still +works, but `tools.exec.mode: "auto"` is the normalized OpenClaw surface. For every app-server field, auth order, environment isolation, discovery, and timeout behavior, see [Codex harness reference](/plugins/codex-harness-reference). diff --git a/docs/plugins/sdk-migration.md b/docs/plugins/sdk-migration.md index 12b289c3711..364c551241d 100644 --- a/docs/plugins/sdk-migration.md +++ b/docs/plugins/sdk-migration.md @@ -568,6 +568,7 @@ releases. | `plugin-sdk/dedupe-runtime` | Dedupe helpers | In-memory dedupe caches | | `plugin-sdk/file-access-runtime` | File access helpers | Safe local-file/media path helpers | | `plugin-sdk/transport-ready-runtime` | Transport readiness helpers | `waitForTransportReady` | + | `plugin-sdk/exec-approvals-runtime` | Exec approval policy helpers | `loadExecApprovals`, `resolveExecApprovalsFromFile`, `ExecApprovalsFile` | | `plugin-sdk/collection-runtime` | Bounded cache helpers | `pruneMapToMaxSize` | | `plugin-sdk/diagnostic-runtime` | Diagnostic gating helpers | `isDiagnosticFlagEnabled`, `isDiagnosticsEnabled` | | `plugin-sdk/error-runtime` | Error formatting helpers | `formatUncaughtError`, `isApprovalNotFoundError`, error graph helpers | diff --git a/docs/plugins/sdk-subpaths.md b/docs/plugins/sdk-subpaths.md index f09f8cd7e15..fbc47c84460 100644 --- a/docs/plugins/sdk-subpaths.md +++ b/docs/plugins/sdk-subpaths.md @@ -282,6 +282,7 @@ and pairing-path families. | `plugin-sdk/secure-random-runtime` | Secure token/UUID helpers | | `plugin-sdk/system-event-runtime` | System event queue helpers | | `plugin-sdk/transport-ready-runtime` | Transport readiness wait helper | + | `plugin-sdk/exec-approvals-runtime` | Exec approval policy file helpers without the broad infra-runtime barrel | | `plugin-sdk/infra-runtime` | Deprecated compatibility shim; use the focused runtime subpaths above | | `plugin-sdk/collection-runtime` | Small bounded cache helpers | | `plugin-sdk/diagnostic-runtime` | Diagnostic flag, event, and trace-context helpers | diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index 7887b3e1481..f9c8f39eea5 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -114,6 +114,20 @@ Example schema: ## Policy knobs +### `tools.exec.mode` + +`tools.exec.mode` is the preferred normalized policy surface for host exec. +Values are: + +- `deny` - block host exec. +- `allowlist` - run only allowlisted commands without asking. +- `ask` - use allowlist policy and ask on misses. +- `auto` - use allowlist policy, run deterministic matches directly, and send approval misses through OpenClaw's native auto reviewer before falling back to a human approval route. +- `full` - run host exec without approval prompts. + +Legacy `tools.exec.security` / `tools.exec.ask` remain supported and still win +when set at the narrower session or agent scope. + ### `exec.security` @@ -289,7 +303,10 @@ EOF ### Session-only shortcut - `/exec security=full ask=off` changes only the current session. -- `/elevated full` is a break-glass shortcut that also skips exec approvals for that session. +- `/elevated full` is a break-glass shortcut that skips exec approvals only when + both the requested policy and the host approvals file resolve to + `security: "full"` and `ask: "off"`. A stricter host file, such as + `ask: "always"`, still prompts. If the host approvals file stays stricter than config, the stricter host policy still wins. diff --git a/docs/tools/exec.md b/docs/tools/exec.md index 016d6872b61..4b44ed6cff9 100644 --- a/docs/tools/exec.md +++ b/docs/tools/exec.md @@ -68,6 +68,7 @@ Notes: - `host` defaults to `auto`: sandbox when sandbox runtime is active for the session, otherwise gateway. - `host` only accepts `auto`, `sandbox`, `gateway`, or `node`. It is not a hostname selector; hostname-like values are rejected before the command runs. - `auto` is the default routing strategy, not a wildcard. Per-call `host=node` is allowed from `auto`; per-call `host=gateway` is only allowed when no sandbox runtime is active. +- `tools.exec.mode` is the normalized policy knob. Values are `deny`, `allowlist`, `ask`, `auto`, and `full`. `auto` runs deterministic allowlist/safe-bin matches directly and routes every remaining exec approval case through OpenClaw's native auto reviewer before asking a human. `ask` / `ask=always` still asks a human every time. - With no extra config, `host=auto` still "just works": no sandbox means it resolves to `gateway`; a live sandbox means it stays in the sandbox. - `elevated` escapes the sandbox onto the configured host path: `gateway` by default, or `node` when `tools.exec.host=node` (or the session default is `host=node`). It is only available when elevated access is enabled for the current session/provider. - `gateway`/`node` approvals are controlled by `~/.openclaw/exec-approvals.json`. @@ -108,7 +109,7 @@ Notes: - YOLO comes from the host-policy defaults (`security=full`, `ask=off`), not from `host=auto`. If you want to force gateway or node routing, set `tools.exec.host` or use `/exec host=...`. - In `security=full` plus `ask=off` mode, host exec follows the configured policy directly; there is no extra heuristic command-obfuscation prefilter or script-preflight rejection layer. - `tools.exec.node` (default: unset) -- `tools.exec.strictInlineEval` (default: false): when true, inline interpreter eval forms such as `python -c`, `node -e`, `ruby -e`, `perl -e`, `php -r`, `lua -e`, and `osascript -e` always require explicit approval. `allow-always` can still persist benign interpreter/script invocations, but inline-eval forms still prompt each time. +- `tools.exec.strictInlineEval` (default: false): when true, inline interpreter eval forms such as `python -c`, `node -e`, `ruby -e`, `perl -e`, `php -r`, `lua -e`, and `osascript -e` require reviewer or explicit approval. In `mode=auto`, the native auto reviewer may allow a clearly low-risk one-off command; if the reviewer asks, the request goes to a human. `allow-always` can still persist benign interpreter/script invocations, but inline-eval forms do not become durable allow rules. - `tools.exec.commandHighlighting` (default: false): when true, approval prompts can highlight parser-derived command spans in the command text. Set to `true` globally or per agent to enable command text highlighting without changing exec approval policy. - `tools.exec.pathPrepend`: list of directories to prepend to `PATH` for exec runs (gateway + sandbox only). - `tools.exec.safeBins`: stdin-only safe binaries that can run without explicit allowlist entries. For behavior details, see [Safe bins](/tools/exec-approvals-advanced#safe-bins-stdin-only). @@ -209,7 +210,7 @@ Use the two controls for different jobs: Do not treat `safeBins` as a generic allowlist, and do not add interpreter/runtime binaries (for example `python3`, `node`, `ruby`, `bash`). If you need those, use explicit allowlist entries and keep approval prompts enabled. `openclaw security audit` warns when interpreter/runtime `safeBins` entries are missing explicit profiles, and `openclaw doctor --fix` can scaffold missing custom `safeBinProfiles` entries. `openclaw security audit` and `openclaw doctor` also warn when you explicitly add broad-behavior bins such as `jq` back into `safeBins`. -If you explicitly allowlist interpreters, enable `tools.exec.strictInlineEval` so inline code-eval forms still require a fresh approval. +If you explicitly allowlist interpreters, enable `tools.exec.strictInlineEval` so inline code-eval forms still require reviewer or explicit approval. For full policy details and examples, see [Exec approvals](/tools/exec-approvals-advanced#safe-bins-stdin-only) and [Safe bins versus allowlist](/tools/exec-approvals-advanced#safe-bins-versus-allowlist). diff --git a/extensions/codex/index.ts b/extensions/codex/index.ts index ce0fad99899..f85666a1b04 100644 --- a/extensions/codex/index.ts +++ b/extensions/codex/index.ts @@ -25,11 +25,11 @@ export default definePluginEntry({ name: "Codex", description: "Codex app-server harness and Codex-managed GPT model catalog.", register(api) { + const resolveCurrentConfig = () => + api.runtime.config?.current ? (api.runtime.config.current() as OpenClawConfig) : undefined; const resolveCurrentPluginConfig = () => resolveLivePluginConfigObject( - api.runtime.config?.current - ? () => api.runtime.config.current() as OpenClawConfig - : undefined, + resolveCurrentConfig, "codex", api.pluginConfig as Record, ) ?? api.pluginConfig; @@ -114,8 +114,8 @@ export default definePluginEntry({ ); api.on("inbound_claim", (event, ctx) => handleCodexConversationInboundClaim(event, ctx, { - config: api.runtime.config?.current?.() as OpenClawConfig | undefined, pluginConfig: resolveCurrentPluginConfig(), + config: resolveCurrentConfig(), resumeCodexCliSessionOnNode: (params) => resumeCodexCliSessionOnNode({ runtime: api.runtime, ...params }), }), diff --git a/extensions/codex/openclaw.plugin.json b/extensions/codex/openclaw.plugin.json index 1b651445ec4..49ea774364f 100644 --- a/extensions/codex/openclaw.plugin.json +++ b/extensions/codex/openclaw.plugin.json @@ -133,8 +133,7 @@ "properties": { "mode": { "type": "string", - "enum": ["yolo", "guardian"], - "default": "yolo" + "enum": ["yolo", "guardian"] }, "transport": { "type": "string", @@ -305,7 +304,7 @@ }, "appServer.mode": { "label": "Execution Mode", - "help": "Use yolo for unchained local execution or guardian for Codex guardian-reviewed approvals.", + "help": "Legacy Codex app-server preset. Prefer tools.exec.mode=auto for normalized Guardian-reviewed approvals.", "advanced": true }, "appServer.transport": { diff --git a/extensions/codex/src/app-server/app-server-policy.ts b/extensions/codex/src/app-server/app-server-policy.ts index 74b4e6b1593..47fac78ba1f 100644 --- a/extensions/codex/src/app-server/app-server-policy.ts +++ b/extensions/codex/src/app-server/app-server-policy.ts @@ -1,4 +1,8 @@ -import type { CodexAppServerRuntimeOptions, CodexPluginConfig } from "./config.js"; +import type { + CodexAppServerRuntimeOptions, + CodexPluginConfig, + OpenClawExecPolicyForCodexAppServer, +} from "./config.js"; export function resolveCodexAppServerForOpenClawToolPolicy(params: { appServer: CodexAppServerRuntimeOptions; @@ -6,6 +10,7 @@ export function resolveCodexAppServerForOpenClawToolPolicy(params: { env: NodeJS.ProcessEnv; shouldPromote: boolean; canUseUntrustedApprovalPolicy: boolean; + execPolicy?: OpenClawExecPolicyForCodexAppServer; }): CodexAppServerRuntimeOptions { if ( !params.shouldPromote || @@ -15,6 +20,7 @@ export function resolveCodexAppServerForOpenClawToolPolicy(params: { return params.appServer; } const explicitMode = + params.execPolicy?.mode === "full" || params.pluginConfig.appServer?.mode !== undefined || isCodexAppServerPolicyMode(params.env.OPENCLAW_CODEX_APP_SERVER_MODE); const explicitApprovalPolicy = diff --git a/extensions/codex/src/app-server/config.test.ts b/extensions/codex/src/app-server/config.test.ts index df8f149cad2..f31eda16f9d 100644 --- a/extensions/codex/src/app-server/config.test.ts +++ b/extensions/codex/src/app-server/config.test.ts @@ -10,6 +10,9 @@ import { readCodexPluginConfig, resolveCodexAppServerRuntimeOptions, resolveCodexComputerUseConfig, + resolveOpenClawExecModeForCodexAppServer, + resolveOpenClawExecModeFromConfig, + resolveOpenClawExecPolicyForCodexAppServer, resolveCodexPluginsPolicy, shouldAutoApproveCodexAppServerApprovals, } from "./config.js"; @@ -763,6 +766,761 @@ allowed_sandbox_modes = ["read-only", "workspace-write"] }); }); + it("maps normalized OpenClaw auto exec mode to guardian-reviewed local execution", () => { + const runtime = resolveRuntimeForTest({ + pluginConfig: {}, + execMode: "auto", + }); + + expectRuntimePolicy(runtime, { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "auto_review", + }); + }); + + it("forces guarded app-server policy fields for auto mode", () => { + const runtime = resolveRuntimeForTest({ + pluginConfig: { + appServer: { + approvalPolicy: "never", + sandbox: "danger-full-access", + approvalsReviewer: "user", + }, + }, + env: { + OPENCLAW_CODEX_APP_SERVER_APPROVAL_POLICY: "never", + OPENCLAW_CODEX_APP_SERVER_SANDBOX: "danger-full-access", + }, + execMode: "auto", + }); + + expectRuntimePolicy(runtime, { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "auto_review", + }); + }); + + it("preserves explicit read-only app-server sandbox for auto mode", () => { + const configRuntime = resolveRuntimeForTest({ + pluginConfig: { + appServer: { + mode: "yolo", + approvalPolicy: "never", + sandbox: "read-only", + approvalsReviewer: "user", + }, + }, + execMode: "auto", + env: {}, + }); + const envRuntime = resolveRuntimeForTest({ + pluginConfig: {}, + execMode: "auto", + env: { + OPENCLAW_CODEX_APP_SERVER_MODE: "yolo", + OPENCLAW_CODEX_APP_SERVER_APPROVAL_POLICY: "never", + OPENCLAW_CODEX_APP_SERVER_SANDBOX: "read-only", + }, + }); + + expectRuntimePolicy(configRuntime, { + approvalPolicy: "on-request", + sandbox: "read-only", + approvalsReviewer: "auto_review", + }); + expectRuntimePolicy(envRuntime, { + approvalPolicy: "on-request", + sandbox: "read-only", + approvalsReviewer: "auto_review", + }); + }); + + it.each(["deny", "allowlist"] as const)( + "blocks Codex app-server local execution for normalized OpenClaw %s exec mode", + (execMode) => { + expect(() => + resolveRuntimeForTest({ + pluginConfig: {}, + execMode, + }), + ).toThrow( + `Codex app-server local execution is not available when tools.exec.mode=${execMode}`, + ); + }, + ); + + it("maps normalized OpenClaw ask exec mode away from Codex yolo", () => { + const runtime = resolveRuntimeForTest({ + pluginConfig: {}, + execMode: "ask", + }); + + expectRuntimePolicy(runtime, { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "user", + }); + }); + + it("keeps user approvals for ask mode with explicit legacy guardian mode", () => { + const configRuntime = resolveRuntimeForTest({ + pluginConfig: { + appServer: { + mode: "guardian", + }, + }, + execMode: "ask", + env: {}, + }); + const envRuntime = resolveRuntimeForTest({ + pluginConfig: {}, + execMode: "ask", + env: { OPENCLAW_CODEX_APP_SERVER_MODE: "guardian" }, + }); + + expectRuntimePolicy(configRuntime, { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "user", + }); + expectRuntimePolicy(envRuntime, { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "user", + }); + }); + + it("overrides explicit app-server policy fields for ask mode", () => { + const configRuntime = resolveRuntimeForTest({ + pluginConfig: { + appServer: { + mode: "yolo", + approvalPolicy: "never", + sandbox: "danger-full-access", + approvalsReviewer: "auto_review", + }, + }, + execMode: "ask", + env: {}, + }); + const envRuntime = resolveRuntimeForTest({ + pluginConfig: {}, + execMode: "ask", + env: { + OPENCLAW_CODEX_APP_SERVER_MODE: "yolo", + OPENCLAW_CODEX_APP_SERVER_APPROVAL_POLICY: "never", + OPENCLAW_CODEX_APP_SERVER_SANDBOX: "danger-full-access", + }, + }); + + expectRuntimePolicy(configRuntime, { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "user", + }); + expectRuntimePolicy(envRuntime, { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "user", + }); + }); + + it("preserves explicit read-only app-server sandbox for ask mode", () => { + const configRuntime = resolveRuntimeForTest({ + pluginConfig: { + appServer: { + mode: "yolo", + approvalPolicy: "never", + sandbox: "read-only", + approvalsReviewer: "auto_review", + }, + }, + execMode: "ask", + env: {}, + }); + const envRuntime = resolveRuntimeForTest({ + pluginConfig: {}, + execMode: "ask", + env: { + OPENCLAW_CODEX_APP_SERVER_MODE: "yolo", + OPENCLAW_CODEX_APP_SERVER_APPROVAL_POLICY: "never", + OPENCLAW_CODEX_APP_SERVER_SANDBOX: "read-only", + }, + }); + + expectRuntimePolicy(configRuntime, { + approvalPolicy: "on-request", + sandbox: "read-only", + approvalsReviewer: "user", + }); + expectRuntimePolicy(envRuntime, { + approvalPolicy: "on-request", + sandbox: "read-only", + approvalsReviewer: "user", + }); + }); + + it("fails closed when normalized OpenClaw ask mode cannot use user approvals", () => { + expect(() => + resolveRuntimeForTest({ + pluginConfig: {}, + execMode: "ask", + requirementsToml: 'allowed_approvals_reviewers = ["auto_review"]\n', + }), + ).toThrow("tools.exec.mode=ask requires Codex app-server user approvals"); + expect(() => + resolveRuntimeForTest({ + pluginConfig: { appServer: { mode: "guardian" } }, + execMode: "ask", + requirementsToml: 'allowed_approvals_reviewers = ["auto_review"]\n', + }), + ).toThrow("tools.exec.mode=ask requires Codex app-server user approvals"); + }); + + it.each([ + { execMode: "auto", policies: ["never"] }, + { execMode: "auto", policies: ["on-failure"] }, + { execMode: "auto", policies: ["untrusted"] }, + { execMode: "ask", policies: ["never"] }, + { execMode: "ask", policies: ["on-failure"] }, + { execMode: "ask", policies: ["untrusted"] }, + ] as const)( + "fails closed when normalized OpenClaw $execMode mode can only use $policies approvals", + ({ execMode, policies }) => { + expect(() => + resolveRuntimeForTest({ + pluginConfig: {}, + execMode, + requirementsToml: `allowed_approval_policies = [${policies + .map((policy) => `"${policy}"`) + .join(", ")}]\n`, + }), + ).toThrow(`tools.exec.mode=${execMode} requires Codex app-server prompting approvals`); + }, + ); + + it("keeps normalized OpenClaw full exec mode on default Codex yolo", () => { + const runtime = resolveRuntimeForTest({ + pluginConfig: {}, + execMode: "full", + }); + + expectRuntimePolicy(runtime, { + approvalPolicy: "never", + sandbox: "danger-full-access", + approvalsReviewer: "user", + }); + }); + + it("fails closed when normalized OpenClaw auto mode can only use on-failure approvals", () => { + expect(() => + resolveRuntimeForTest({ + pluginConfig: {}, + execMode: "auto", + requirementsToml: + 'allowed_sandbox_modes = ["read-only"]\nallowed_approval_policies = ["on-failure"]\nallowed_approvals_reviewers = ["user"]\n', + }), + ).toThrow("tools.exec.mode=auto requires Codex app-server prompting approvals"); + }); + + it("fails closed when normalized OpenClaw auto mode cannot force prompting over yolo", () => { + expect(() => + resolveRuntimeForTest({ + pluginConfig: {}, + execMode: "auto", + requirementsToml: + 'allowed_sandbox_modes = ["danger-full-access", "read-only"]\nallowed_approval_policies = ["never", "on-failure"]\nallowed_approvals_reviewers = ["user"]\n', + }), + ).toThrow("tools.exec.mode=auto requires Codex app-server prompting approvals"); + }); + + it("fails closed when normalized OpenClaw auto mode cannot use an auto reviewer", () => { + expect(() => + resolveRuntimeForTest({ + pluginConfig: {}, + execMode: "auto", + requirementsToml: + 'allowed_approval_policies = ["on-request"]\nallowed_approvals_reviewers = ["user"]\n', + }), + ).toThrow("tools.exec.mode=auto requires Codex app-server auto approvals"); + }); + + it("keeps normalized OpenClaw auto mode when legacy app-server yolo was schema-defaulted", () => { + const runtime = resolveRuntimeForTest({ + pluginConfig: { + appServer: { + command: "codex", + mode: "yolo", + transport: "stdio", + requestTimeoutMs: 60_000, + turnCompletionIdleTimeoutMs: 60_000, + }, + codexDynamicToolsLoading: "searchable", + codexDynamicToolsExclude: [], + }, + execMode: "auto", + }); + + expectRuntimePolicy(runtime, { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "auto_review", + }); + expectFields(runtime, "runtime start", { + start: { + transport: "stdio", + command: "codex", + commandSource: "config", + args: ["app-server", "--listen", "stdio://"], + headers: {}, + }, + }); + }); + + it("forces guarded policy fields for normalized OpenClaw auto mode", () => { + const runtime = resolveRuntimeForTest({ + pluginConfig: { + appServer: { + approvalPolicy: "never", + sandbox: "danger-full-access", + approvalsReviewer: "user", + }, + }, + execMode: "auto", + }); + + expectRuntimePolicy(runtime, { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "auto_review", + }); + }); + + it("resolves agent-scoped normalized OpenClaw exec mode for Codex app-server mapping", () => { + const config = { + tools: { + exec: { + mode: "ask", + }, + }, + agents: { + list: [ + { + id: "Codex-Agent", + tools: { + exec: { + mode: "auto", + }, + }, + }, + ], + }, + }; + + expect(resolveOpenClawExecModeFromConfig({ config, agentId: "codex-agent" })).toBe("auto"); + expect(resolveOpenClawExecModeFromConfig({ config, agentId: "other-agent" })).toBe("ask"); + }); + + it("keeps legacy exec security overrides ahead of normalized OpenClaw exec mode", () => { + expect( + resolveOpenClawExecModeFromConfig({ + config: { + tools: { + exec: { + mode: "auto", + }, + }, + agents: { + list: [ + { + id: "codex-agent", + tools: { + exec: { + security: "full", + }, + }, + }, + ], + }, + }, + agentId: "codex-agent", + }), + ).toBe("full"); + }); + + it.each(["always"] as const)( + "keeps legacy full exec security with ask=%s on prompting Codex policy", + (ask) => { + const config = { + tools: { + exec: { + security: "full", + ask, + }, + }, + }; + const execPolicy = resolveOpenClawExecPolicyForCodexAppServer({ config }); + + expect(resolveOpenClawExecModeForCodexAppServer({ config })).toBe("ask"); + expectRuntimePolicy( + resolveRuntimeForTest({ + pluginConfig: { + appServer: { + mode: "yolo", + approvalPolicy: "never", + sandbox: "workspace-write", + approvalsReviewer: "auto_review", + }, + }, + execPolicy, + }), + { + approvalPolicy: "on-request", + sandbox: "danger-full-access", + approvalsReviewer: "user", + }, + ); + }, + ); + + it("keeps legacy full exec security with ask=on-miss on default Codex yolo", () => { + const config = { + tools: { + exec: { + security: "full", + ask: "on-miss", + }, + }, + }; + const execPolicy = resolveOpenClawExecPolicyForCodexAppServer({ config }); + + expect(resolveOpenClawExecModeForCodexAppServer({ config })).toBe("full"); + expectRuntimePolicy(resolveRuntimeForTest({ execPolicy }), { + approvalPolicy: "never", + sandbox: "danger-full-access", + approvalsReviewer: "user", + }); + }); + + it("fails closed when legacy full exec with ask cannot use full Codex sandbox", () => { + const config = { + tools: { + exec: { + security: "full", + ask: "always", + }, + }, + }; + + expect(() => + resolveRuntimeForTest({ + execPolicy: resolveOpenClawExecPolicyForCodexAppServer({ config }), + requirementsToml: 'allowed_sandbox_modes = ["read-only", "workspace-write"]\n', + }), + ).toThrow("legacy full exec security with ask requires Codex app-server danger-full-access"); + }); + + it("clamps legacy full exec with ask when an OpenClaw sandbox is active", () => { + const config = { + tools: { + exec: { + security: "full", + ask: "always", + }, + }, + }; + + expectRuntimePolicy( + resolveRuntimeForTest({ + execPolicy: resolveOpenClawExecPolicyForCodexAppServer({ config }), + openClawSandboxActive: true, + requirementsToml: 'allowed_sandbox_modes = ["read-only", "workspace-write"]\n', + }), + { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "user", + }, + ); + }); + + it("applies host exec approval security floors before starting Codex app-server", () => { + const execPolicy = resolveOpenClawExecPolicyForCodexAppServer({ + config: { + tools: { + exec: { + mode: "full", + }, + }, + }, + approvals: { + version: 1, + defaults: { + security: "deny", + }, + agents: {}, + }, + }); + + expect(execPolicy.mode).toBe("deny"); + expect(() => + resolveRuntimeForTest({ + pluginConfig: { + appServer: { + mode: "yolo", + approvalPolicy: "never", + sandbox: "danger-full-access", + }, + }, + execPolicy, + }), + ).toThrow("Codex app-server local execution is not available when tools.exec.mode=deny"); + }); + + it("applies host exec approval ask floors before starting Codex app-server", () => { + const execPolicy = resolveOpenClawExecPolicyForCodexAppServer({ + config: { + tools: { + exec: { + mode: "full", + }, + }, + }, + approvals: { + version: 1, + defaults: { + ask: "always", + }, + agents: {}, + }, + }); + + expect(execPolicy.mode).toBe("ask"); + expectRuntimePolicy( + resolveRuntimeForTest({ + pluginConfig: { + appServer: { + mode: "yolo", + approvalPolicy: "never", + sandbox: "workspace-write", + approvalsReviewer: "auto_review", + }, + }, + execPolicy, + }), + { + approvalPolicy: "on-request", + sandbox: "danger-full-access", + approvalsReviewer: "user", + }, + ); + }); + + it("applies agent-scoped exec approval security floors before starting Codex app-server", () => { + const execPolicy = resolveOpenClawExecPolicyForCodexAppServer({ + config: { + tools: { + exec: { + mode: "full", + }, + }, + }, + agentId: "codex-agent", + approvals: { + version: 1, + defaults: { + security: "full", + }, + agents: { + "codex-agent": { + security: "deny", + }, + }, + }, + }); + + expect(execPolicy.mode).toBe("deny"); + expect(() => + resolveRuntimeForTest({ + pluginConfig: { + appServer: { + mode: "yolo", + approvalPolicy: "never", + sandbox: "danger-full-access", + }, + }, + execPolicy, + }), + ).toThrow("Codex app-server local execution is not available when tools.exec.mode=deny"); + }); + + it("applies agent-scoped exec approval ask floors before starting Codex app-server", () => { + const execPolicy = resolveOpenClawExecPolicyForCodexAppServer({ + config: { + tools: { + exec: { + mode: "full", + }, + }, + }, + agentId: "codex-agent", + approvals: { + version: 1, + defaults: { + ask: "off", + }, + agents: { + "codex-agent": { + ask: "always", + }, + }, + }, + }); + + expect(execPolicy.mode).toBe("ask"); + expectRuntimePolicy( + resolveRuntimeForTest({ + pluginConfig: { + appServer: { + mode: "yolo", + approvalPolicy: "never", + sandbox: "workspace-write", + approvalsReviewer: "auto_review", + }, + }, + execPolicy, + }), + { + approvalPolicy: "on-request", + sandbox: "danger-full-access", + approvalsReviewer: "user", + }, + ); + }); + + it("treats ask-only legacy overrides as normalized mode overrides", () => { + const config = { + tools: { + exec: { + mode: "auto", + }, + }, + agents: { + list: [ + { + id: "codex-agent", + tools: { + exec: { + ask: "off", + }, + }, + }, + ], + }, + }; + + expect(resolveOpenClawExecModeFromConfig({ config, agentId: "codex-agent" })).toBe("allowlist"); + const execMode = resolveOpenClawExecModeForCodexAppServer({ + config, + agentId: "main", + execOverrides: { + ask: "always", + }, + }); + expect(execMode).toBe("ask"); + expectRuntimePolicy(resolveRuntimeForTest({ execMode }), { + approvalPolicy: "on-request", + sandbox: "workspace-write", + approvalsReviewer: "user", + }); + }); + + it("keeps current legacy exec security overrides ahead of configured normalized mode", () => { + const config = { + tools: { + exec: { + mode: "auto", + }, + }, + }; + + expect( + resolveOpenClawExecModeForCodexAppServer({ + config, + agentId: "main", + execOverrides: { + security: "full", + }, + }), + ).toBe("full"); + expect( + resolveOpenClawExecModeForCodexAppServer({ + config, + agentId: "main", + execOverrides: { + ask: "always", + }, + }), + ).toBe("ask"); + expect( + resolveOpenClawExecModeForCodexAppServer({ + config, + agentId: "main", + execOverrides: { + security: "full", + ask: "off", + }, + }), + ).toBe("full"); + }); + + it("preserves legacy full exec security before applying current ask overrides", () => { + expect( + resolveOpenClawExecModeForCodexAppServer({ + config: { + tools: { + exec: { + security: "full", + ask: "on-miss", + }, + }, + }, + }), + ).toBe("full"); + expect( + resolveOpenClawExecModeForCodexAppServer({ + config: { + tools: { + exec: { + security: "full", + ask: "always", + }, + }, + }, + execOverrides: { + ask: "off", + }, + }), + ).toBe("full"); + expect( + resolveOpenClawExecModeForCodexAppServer({ + config: { + tools: { + exec: { + security: "full", + ask: "on-miss", + }, + }, + }, + execOverrides: { + ask: "off", + }, + }), + ).toBe("full"); + }); + it("accepts the latest auto_review reviewer and legacy guardian_subagent alias", () => { expect( resolveRuntimeForTest({ @@ -980,6 +1738,7 @@ allowed_sandbox_modes = ["read-only", "workspace-write"] }; const appServerProperties = manifest.configSchema.properties.appServer.properties; + expect(appServerProperties.mode?.default).toBeUndefined(); expect(appServerProperties.command?.default).toBeUndefined(); expect(appServerProperties.approvalPolicy?.default).toBeUndefined(); expect(appServerProperties.sandbox?.default).toBeUndefined(); diff --git a/extensions/codex/src/app-server/config.ts b/extensions/codex/src/app-server/config.ts index a8e09809b1a..f21a7f28431 100644 --- a/extensions/codex/src/app-server/config.ts +++ b/extensions/codex/src/app-server/config.ts @@ -1,6 +1,11 @@ import { createHmac, randomBytes } from "node:crypto"; import { readFileSync } from "node:fs"; import { hostname as readHostName } from "node:os"; +import { + resolveExecApprovalsFromFile, + type ExecApprovalsFile, +} from "openclaw/plugin-sdk/exec-approvals-runtime"; +import { normalizeAgentId } from "openclaw/plugin-sdk/routing"; import { normalizeTrimmedStringList } from "openclaw/plugin-sdk/string-coerce-runtime"; import { detectWindowsSpawnCommandInlineArgs } from "openclaw/plugin-sdk/windows-spawn"; import { z } from "zod"; @@ -14,11 +19,26 @@ const PLAIN_DECIMAL_NUMBER_RE = /^[+-]?(?:(?:\d+\.?\d*)|(?:\.\d+))$/; type CodexAppServerTransportMode = "stdio" | "websocket"; type CodexAppServerPolicyMode = "yolo" | "guardian"; +type OpenClawExecMode = "deny" | "allowlist" | "ask" | "auto" | "full"; +type OpenClawExecSecurity = "deny" | "allowlist" | "full"; +type OpenClawExecAsk = "off" | "on-miss" | "always"; +type OpenClawExecApprovalFloorsForCodexAppServer = { + security?: OpenClawExecSecurity; + ask?: OpenClawExecAsk; +}; +export type OpenClawExecPolicyForCodexAppServer = { + mode?: OpenClawExecMode; + security: OpenClawExecSecurity; + ask: OpenClawExecAsk; + touched: boolean; +}; +type OpenClawExecPolicy = OpenClawExecPolicyForCodexAppServer; type CodexAppServerDefaultPolicy = { mode: CodexAppServerPolicyMode; approvalPolicy?: CodexAppServerApprovalPolicy; approvalsReviewer?: CodexAppServerApprovalsReviewer; sandbox?: CodexAppServerSandboxMode; + dangerFullAccessAllowed?: boolean; }; export type CodexAppServerApprovalPolicy = "never" | "on-request" | "on-failure" | "untrusted"; export type CodexAppServerApprovalPolicySource = "config" | "env" | "requirements" | "implicit"; @@ -369,12 +389,15 @@ export function resolveCodexPluginsPolicy(pluginConfig?: unknown): ResolvedCodex export function resolveCodexAppServerRuntimeOptions( params: { pluginConfig?: unknown; + execMode?: OpenClawExecMode; + execPolicy?: OpenClawExecPolicyForCodexAppServer; env?: NodeJS.ProcessEnv; requirementsToml?: string | null; requirementsPath?: string; readRequirementsFile?: (path: string) => string | undefined; platform?: NodeJS.Platform; hostName?: string; + openClawSandboxActive?: boolean; } = {}, ): CodexAppServerRuntimeOptions { const env = params.env ?? process.env; @@ -396,20 +419,68 @@ export function resolveCodexAppServerRuntimeOptions( const clearEnv = normalizeStringList(config.clearEnv); const authToken = readNonEmptyString(config.authToken); const url = readNonEmptyString(config.url); + const execMode = resolveEffectiveOpenClawExecModeForCodexAppServer({ + execMode: params.execMode, + execPolicy: params.execPolicy, + }); + assertCodexAppServerAllowedForOpenClawExecMode(execMode); const explicitPolicyMode = resolvePolicyMode(config.mode) ?? resolvePolicyMode(env.OPENCLAW_CODEX_APP_SERVER_MODE); - const defaultPolicy = explicitPolicyMode - ? undefined - : resolveDefaultCodexAppServerPolicy({ - transport, - env, - requirementsToml: params.requirementsToml, - requirementsPath: params.requirementsPath, - readRequirementsFile: params.readRequirementsFile, - platform: params.platform, - hostName: params.hostName, - }); - const policyMode = explicitPolicyMode ?? defaultPolicy?.mode ?? "yolo"; + const explicitApprovalPolicy = + resolveApprovalPolicy(config.approvalPolicy) ?? + resolveApprovalPolicy(env.OPENCLAW_CODEX_APP_SERVER_APPROVAL_POLICY); + const configuredSandbox = + resolveSandbox(config.sandbox) ?? resolveSandbox(env.OPENCLAW_CODEX_APP_SERVER_SANDBOX); + const explicitApprovalsReviewer = resolveApprovalsReviewer(config.approvalsReviewer); + const normalizedPolicyMode = resolveCodexPolicyModeForOpenClawExecMode(execMode); + const ignoreLegacyYoloPolicyMode = + normalizedPolicyMode === "guardian" && explicitPolicyMode === "yolo"; + const forceUserReviewer = execMode !== undefined && execMode !== "auto" && execMode !== "full"; + const forceGuardianReviewer = execMode === "auto"; + const forceDangerFullAccessSandbox = + params.execPolicy?.touched === true && + params.execPolicy.security === "full" && + params.execPolicy.ask === "always"; + const forceRuntimePolicy = + forceUserReviewer || forceGuardianReviewer || forceDangerFullAccessSandbox; + const defaultPolicy = + explicitPolicyMode && !forceRuntimePolicy && !ignoreLegacyYoloPolicyMode + ? undefined + : resolveDefaultCodexAppServerPolicy({ + transport, + env, + forceGuardian: normalizedPolicyMode === "guardian", + forceUserReviewer, + execModeRequiringPromptingApprovals: + execMode === "auto" || execMode === "ask" ? execMode : undefined, + requirementsToml: params.requirementsToml, + requirementsPath: params.requirementsPath, + readRequirementsFile: params.readRequirementsFile, + platform: params.platform, + hostName: params.hostName, + }); + const preserveExplicitAutoSandbox = forceGuardianReviewer && configuredSandbox === "read-only"; + const forcedPolicy = forceRuntimePolicy + ? { + approvalPolicy: defaultPolicy?.approvalPolicy ?? "on-request", + sandbox: preserveExplicitAutoSandbox + ? undefined + : forceDangerFullAccessSandbox + ? selectForcedDangerFullAccessSandbox({ + defaultPolicy, + openClawSandboxActive: params.openClawSandboxActive === true, + }) + : selectForcedPromptingSandbox({ + configuredSandbox, + defaultSandbox: defaultPolicy?.sandbox, + }), + approvalsReviewer: + defaultPolicy?.approvalsReviewer ?? (forceUserReviewer ? "user" : "auto_review"), + } + : undefined; + const policyMode = ignoreLegacyYoloPolicyMode + ? normalizedPolicyMode + : (explicitPolicyMode ?? normalizedPolicyMode ?? defaultPolicy?.mode ?? "yolo"); const serviceTier = normalizeCodexServiceTier(config.serviceTier); if (transport === "websocket" && !url) { throw new Error( @@ -457,15 +528,16 @@ export function resolveCodexAppServerRuntimeOptions( ), } : {}), - approvalPolicy, + approvalPolicy: forcedPolicy?.approvalPolicy ?? approvalPolicy, approvalPolicySource, sandbox: - resolveSandbox(config.sandbox) ?? - resolveSandbox(env.OPENCLAW_CODEX_APP_SERVER_SANDBOX) ?? + forcedPolicy?.sandbox ?? + configuredSandbox ?? defaultPolicy?.sandbox ?? (policyMode === "guardian" ? "workspace-write" : "danger-full-access"), approvalsReviewer: - resolveApprovalsReviewer(config.approvalsReviewer) ?? + forcedPolicy?.approvalsReviewer ?? + explicitApprovalsReviewer ?? defaultPolicy?.approvalsReviewer ?? (policyMode === "guardian" ? "auto_review" : "user"), ...(serviceTier ? { serviceTier } : {}), @@ -634,6 +706,9 @@ function resolvePolicyMode(value: unknown): CodexAppServerPolicyMode | undefined function resolveDefaultCodexAppServerPolicy(params: { transport: CodexAppServerTransportMode; + forceGuardian?: boolean; + forceUserReviewer?: boolean; + execModeRequiringPromptingApprovals?: Extract; env?: NodeJS.ProcessEnv; requirementsToml?: string | null; requirementsPath?: string; @@ -642,11 +717,28 @@ function resolveDefaultCodexAppServerPolicy(params: { hostName?: string; }): CodexAppServerDefaultPolicy { if (params.transport !== "stdio") { - return { mode: "yolo" }; + return { mode: "yolo", dangerFullAccessAllowed: true }; } const content = readCodexRequirementsToml(params); if (content === undefined) { - return { mode: "yolo" }; + if (!params.forceGuardian) { + return { mode: "yolo", dangerFullAccessAllowed: true }; + } + return { + mode: "guardian", + dangerFullAccessAllowed: true, + approvalPolicy: selectGuardianApprovalPolicy( + undefined, + params.execModeRequiringPromptingApprovals, + ), + approvalsReviewer: params.forceUserReviewer + ? selectUserApprovalsReviewer(undefined) + : selectGuardianApprovalsReviewer( + undefined, + params.execModeRequiringPromptingApprovals === "auto" ? "auto" : undefined, + ), + sandbox: selectGuardianSandbox(undefined), + }; } const allowedSandboxModes = parseAllowedSandboxModesFromCodexRequirements( content, @@ -660,13 +752,22 @@ function resolveDefaultCodexAppServerPolicy(params: { allowedApprovalPolicies === undefined || allowedApprovalPolicies.has("never"); const yoloReviewerAllowed = allowedApprovalsReviewers === undefined || allowedApprovalsReviewers.has("user"); - if (yoloSandboxAllowed && yoloApprovalAllowed && yoloReviewerAllowed) { - return { mode: "yolo" }; + if (!params.forceGuardian && yoloSandboxAllowed && yoloApprovalAllowed && yoloReviewerAllowed) { + return { mode: "yolo", dangerFullAccessAllowed: true }; } return { mode: "guardian", - approvalPolicy: selectGuardianApprovalPolicy(allowedApprovalPolicies), - approvalsReviewer: selectGuardianApprovalsReviewer(allowedApprovalsReviewers), + dangerFullAccessAllowed: yoloSandboxAllowed, + approvalPolicy: selectGuardianApprovalPolicy( + allowedApprovalPolicies, + params.execModeRequiringPromptingApprovals, + ), + approvalsReviewer: params.forceUserReviewer + ? selectUserApprovalsReviewer(allowedApprovalsReviewers) + : selectGuardianApprovalsReviewer( + allowedApprovalsReviewers, + params.execModeRequiringPromptingApprovals === "auto" ? "auto" : undefined, + ), sandbox: selectGuardianSandbox(allowedSandboxModes), }; } @@ -913,10 +1014,16 @@ function normalizeRequirementsApprovalsReviewer( function selectGuardianApprovalPolicy( allowedApprovalPolicies: Set | undefined, + execModeRequiringPromptingApprovals?: Extract, ): CodexAppServerApprovalPolicy { if (allowedApprovalPolicies === undefined || allowedApprovalPolicies.has("on-request")) { return "on-request"; } + if (execModeRequiringPromptingApprovals) { + throw new Error( + `tools.exec.mode=${execModeRequiringPromptingApprovals} requires Codex app-server prompting approvals`, + ); + } if (allowedApprovalPolicies.has("on-failure")) { return "on-failure"; } @@ -931,6 +1038,7 @@ function selectGuardianApprovalPolicy( function selectGuardianApprovalsReviewer( allowedApprovalsReviewers: Set | undefined, + execModeRequiringAutoReviewer?: Extract, ): CodexAppServerApprovalsReviewer { if (allowedApprovalsReviewers === undefined || allowedApprovalsReviewers.has("auto_review")) { return "auto_review"; @@ -938,12 +1046,51 @@ function selectGuardianApprovalsReviewer( if (allowedApprovalsReviewers.has("guardian_subagent")) { return "guardian_subagent"; } + if (execModeRequiringAutoReviewer) { + throw new Error( + `tools.exec.mode=${execModeRequiringAutoReviewer} requires Codex app-server auto approvals`, + ); + } if (allowedApprovalsReviewers.has("user")) { return "user"; } return "auto_review"; } +function selectUserApprovalsReviewer( + allowedApprovalsReviewers: Set | undefined, +): CodexAppServerApprovalsReviewer { + if (allowedApprovalsReviewers === undefined || allowedApprovalsReviewers.has("user")) { + return "user"; + } + throw new Error("tools.exec.mode=ask requires Codex app-server user approvals"); +} + +function selectForcedPromptingSandbox(params: { + configuredSandbox?: CodexAppServerSandboxMode; + defaultSandbox?: CodexAppServerSandboxMode; +}): CodexAppServerSandboxMode { + if (params.configuredSandbox === "read-only" || params.defaultSandbox === "read-only") { + return "read-only"; + } + return params.defaultSandbox ?? "workspace-write"; +} + +function selectForcedDangerFullAccessSandbox(params: { + defaultPolicy: CodexAppServerDefaultPolicy | undefined; + openClawSandboxActive: boolean; +}): CodexAppServerSandboxMode { + if (params.defaultPolicy?.dangerFullAccessAllowed === false) { + if (params.openClawSandboxActive) { + return params.defaultPolicy.sandbox ?? "workspace-write"; + } + throw new Error( + "legacy full exec security with ask requires Codex app-server danger-full-access", + ); + } + return "danger-full-access"; +} + function selectGuardianSandbox( allowedSandboxModes: Set | undefined, ): CodexAppServerSandboxMode { @@ -980,6 +1127,238 @@ function resolveApprovalsReviewer(value: unknown): CodexAppServerApprovalsReview : undefined; } +export function resolveOpenClawExecModeFromConfig(params: { + config?: unknown; + agentId?: string; +}): OpenClawExecMode | undefined { + const policy = resolveOpenClawExecPolicyFromConfig(params); + return policy.touched ? policy.mode : undefined; +} + +function resolveOpenClawExecPolicyFromConfig(params: { + config?: unknown; + agentId?: string; +}): OpenClawExecPolicy { + const root = readRecord(params.config); + const globalExec = readRecord(readRecord(root?.tools)?.exec); + const globalPolicy = applyOpenClawExecPolicyLayer(createDefaultOpenClawExecPolicy(), globalExec); + const agentId = params.agentId?.trim(); + if (!agentId) { + return globalPolicy; + } + const agents = readRecord(root?.agents); + const agentList = Array.isArray(agents?.list) ? agents.list : []; + const normalizedAgentId = normalizeAgentId(agentId); + const agentEntry = agentList.find((entry) => { + const id = readRecord(entry)?.id; + return typeof id === "string" && normalizeAgentId(id) === normalizedAgentId; + }); + const agentExec = readRecord(readRecord(readRecord(agentEntry)?.tools)?.exec); + return applyOpenClawExecPolicyLayer(globalPolicy, agentExec); +} + +export function resolveOpenClawExecModeForCodexAppServer(params: { + execOverrides?: { + security?: unknown; + ask?: unknown; + }; + approvals?: ExecApprovalsFile; + config?: unknown; + agentId?: string; +}): OpenClawExecMode | undefined { + const policy = resolveOpenClawExecPolicyForCodexAppServer(params); + return policy.touched ? policy.mode : undefined; +} + +export function resolveOpenClawExecPolicyForCodexAppServer(params: { + execOverrides?: { + security?: unknown; + ask?: unknown; + }; + approvals?: ExecApprovalsFile; + config?: unknown; + agentId?: string; +}): OpenClawExecPolicyForCodexAppServer { + const basePolicy = resolveOpenClawExecPolicyFromConfig({ + config: params.config, + agentId: params.agentId, + }); + const overridePolicy = applyOpenClawExecPolicyLayer(basePolicy, params.execOverrides); + const approvalFloors = resolveOpenClawExecApprovalFloorsForCodexAppServer({ + approvals: params.approvals, + agentId: params.agentId, + policy: overridePolicy, + }); + return applyOpenClawExecApprovalFloors(overridePolicy, approvalFloors); +} + +function resolveEffectiveOpenClawExecModeForCodexAppServer(params: { + execMode?: OpenClawExecMode; + execPolicy?: OpenClawExecPolicyForCodexAppServer; +}): OpenClawExecMode | undefined { + if (params.execPolicy?.touched === true) { + return params.execPolicy.mode; + } + return params.execMode; +} + +function resolveCodexPolicyModeForOpenClawExecMode( + mode: OpenClawExecMode | undefined, +): CodexAppServerPolicyMode | undefined { + if (!mode || mode === "full") { + return undefined; + } + return "guardian"; +} + +function assertCodexAppServerAllowedForOpenClawExecMode(mode: OpenClawExecMode | undefined): void { + if (mode === "deny" || mode === "allowlist") { + throw new Error( + `Codex app-server local execution is not available when tools.exec.mode=${mode}`, + ); + } +} + +function createDefaultOpenClawExecPolicy(): OpenClawExecPolicy { + return { + security: "full", + ask: "off", + touched: false, + }; +} + +function applyOpenClawExecPolicyLayer( + base: OpenClawExecPolicy, + exec?: { mode?: unknown; security?: unknown; ask?: unknown }, +): OpenClawExecPolicy { + if (!exec) { + return base; + } + const mode = readExecMode(exec.mode); + if (mode !== undefined) { + return { + ...resolveOpenClawExecPolicyForMode(mode), + touched: true, + }; + } + const security = readExecSecurity(exec.security); + const ask = readExecAsk(exec.ask); + if (security === undefined && ask === undefined) { + return base; + } + const nextSecurity = security ?? base.security; + const nextAsk = ask ?? base.ask; + return { + mode: resolveOpenClawExecModeFromPolicy({ security: nextSecurity, ask: nextAsk }), + security: nextSecurity, + ask: nextAsk, + touched: true, + }; +} + +function resolveOpenClawExecApprovalFloorsForCodexAppServer(params: { + approvals?: ExecApprovalsFile; + agentId?: string; + policy: OpenClawExecPolicy; +}): OpenClawExecApprovalFloorsForCodexAppServer | undefined { + if (!params.approvals) { + return undefined; + } + return resolveExecApprovalsFromFile({ + file: params.approvals, + agentId: params.agentId, + overrides: { + security: params.policy.security, + ask: params.policy.ask, + }, + }).agent; +} + +function applyOpenClawExecApprovalFloors( + base: OpenClawExecPolicy, + approvalFloors?: OpenClawExecApprovalFloorsForCodexAppServer, +): OpenClawExecPolicy { + if (!approvalFloors) { + return base; + } + const nextSecurity = approvalFloors.security + ? minOpenClawExecSecurity(base.security, approvalFloors.security) + : base.security; + const nextAsk = approvalFloors.ask ? maxOpenClawExecAsk(base.ask, approvalFloors.ask) : base.ask; + if (nextSecurity === base.security && nextAsk === base.ask) { + return base; + } + return { + mode: resolveOpenClawExecModeFromPolicy({ security: nextSecurity, ask: nextAsk }), + security: nextSecurity, + ask: nextAsk, + touched: true, + }; +} + +function resolveOpenClawExecPolicyForMode( + mode: OpenClawExecMode, +): Omit { + switch (mode) { + case "deny": + return { mode, security: "deny", ask: "off" }; + case "allowlist": + return { mode, security: "allowlist", ask: "off" }; + case "ask": + case "auto": + return { mode, security: "allowlist", ask: "on-miss" }; + case "full": + return { mode, security: "full", ask: "off" }; + } + const exhaustiveMode: never = mode; + return exhaustiveMode; +} + +function resolveOpenClawExecModeFromPolicy(params: { + security: OpenClawExecSecurity; + ask: OpenClawExecAsk; +}): OpenClawExecMode { + if (params.security === "deny") { + return "deny"; + } + if (params.security === "allowlist" && params.ask === "off") { + return "allowlist"; + } + if (params.security === "full" && params.ask !== "always") { + return "full"; + } + return "ask"; +} + +function minOpenClawExecSecurity( + left: OpenClawExecSecurity, + right: OpenClawExecSecurity, +): OpenClawExecSecurity { + const order: Record = { deny: 0, allowlist: 1, full: 2 }; + return order[left] <= order[right] ? left : right; +} + +function maxOpenClawExecAsk(left: OpenClawExecAsk, right: OpenClawExecAsk): OpenClawExecAsk { + const order: Record = { off: 0, "on-miss": 1, always: 2 }; + return order[left] >= order[right] ? left : right; +} + +function readExecMode(value: unknown): OpenClawExecMode | undefined { + return value === "deny" || + value === "allowlist" || + value === "ask" || + value === "auto" || + value === "full" + ? value + : undefined; +} + +function readRecord(value: unknown): Record | undefined { + return value && typeof value === "object" && !Array.isArray(value) + ? (value as Record) + : undefined; +} + export function normalizeCodexServiceTier(value: unknown): CodexServiceTier | undefined { if (typeof value !== "string") { return undefined; @@ -1035,6 +1414,14 @@ function readBooleanEnv(value: string | undefined): boolean | undefined { return undefined; } +function readExecSecurity(value: unknown): OpenClawExecSecurity | undefined { + return value === "deny" || value === "allowlist" || value === "full" ? value : undefined; +} + +function readExecAsk(value: unknown): OpenClawExecAsk | undefined { + return value === "off" || value === "on-miss" || value === "always" ? value : undefined; +} + function readNumberEnv(value: string | undefined): number | undefined { const trimmed = value?.trim(); if (!trimmed || !PLAIN_DECIMAL_NUMBER_RE.test(trimmed)) { diff --git a/extensions/codex/src/app-server/dynamic-tool-build.test.ts b/extensions/codex/src/app-server/dynamic-tool-build.test.ts index 03056155258..935851351e8 100644 --- a/extensions/codex/src/app-server/dynamic-tool-build.test.ts +++ b/extensions/codex/src/app-server/dynamic-tool-build.test.ts @@ -469,6 +469,41 @@ describe("Codex app-server dynamic tool build", () => { ); }); + it("passes runtime config into Codex exec dynamic tool construction", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const workspaceDir = path.join(tempDir, "workspace"); + const params = createParams(sessionFile, workspaceDir); + const runtimeConfig = { + tools: { + exec: { + mode: "auto", + reviewer: { + timeoutMs: 1234, + }, + }, + }, + } as EmbeddedRunAttemptParams["config"]; + params.disableTools = false; + params.config = runtimeConfig; + params.runtimePlan = createCodexRuntimePlanFixture(); + const factoryOptions: unknown[] = []; + setOpenClawCodingToolsFactoryForTests((options) => { + factoryOptions.push(options); + return []; + }); + + await buildDynamicToolsForTest(params, workspaceDir, { sandbox: null as never }); + + const toolOptions = factoryOptions[0] as { + config?: unknown; + exec?: { config?: unknown; mode?: unknown }; + }; + expect(factoryOptions).toHaveLength(1); + expect(toolOptions.config).toBe(runtimeConfig); + expect(toolOptions.exec?.config).toBe(runtimeConfig); + expect(toolOptions.exec?.mode).toBeUndefined(); + }); + it("uses the tool auth profile store for Codex dynamic tool construction", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); const workspaceDir = path.join(tempDir, "workspace"); diff --git a/extensions/codex/src/app-server/dynamic-tool-build.ts b/extensions/codex/src/app-server/dynamic-tool-build.ts index 9993637a1f3..d6237468f51 100644 --- a/extensions/codex/src/app-server/dynamic-tool-build.ts +++ b/extensions/codex/src/app-server/dynamic-tool-build.ts @@ -187,6 +187,7 @@ export async function buildDynamicTools(input: DynamicToolBuildParams) { ...buildEmbeddedAttemptToolRunContext(params), exec: { ...params.execOverrides, + config: params.config, elevated: params.bashElevated, }, sandbox: input.sandbox, diff --git a/extensions/codex/src/app-server/run-attempt.test.ts b/extensions/codex/src/app-server/run-attempt.test.ts index 5ab925bd6fe..c9a9ce55f2e 100644 --- a/extensions/codex/src/app-server/run-attempt.test.ts +++ b/extensions/codex/src/app-server/run-attempt.test.ts @@ -2518,6 +2518,27 @@ describe("runCodexAppServerAttempt", () => { expect(startParams?.sandbox).toBe("danger-full-access"); }); + it("keeps normalized full exec mode unpromoted when OpenClaw tool policy exists", async () => { + initializeGlobalHookRunner( + createMockPluginRegistry([{ hookName: "before_tool_call", handler: vi.fn() }]), + ); + const sessionFile = path.join(tempDir, "session.jsonl"); + const workspaceDir = path.join(tempDir, "workspace"); + const harness = createStartedThreadHarness(); + const params = createParams(sessionFile, workspaceDir); + params.config = { tools: { exec: { mode: "full" } } } as never; + + const run = runCodexAppServerAttempt(params); + await harness.waitForMethod("turn/start"); + await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" }); + await run; + + const startRequest = harness.requests.find((request) => request.method === "thread/start"); + const startParams = startRequest?.params as Record | undefined; + expect(startParams?.approvalPolicy).toBe("never"); + expect(startParams?.sandbox).toBe("danger-full-access"); + }); + it("ignores invalid Codex app-server env overrides when promoting tool policy approval", async () => { initializeGlobalHookRunner( createMockPluginRegistry([{ hookName: "before_tool_call", handler: vi.fn() }]), diff --git a/extensions/codex/src/app-server/run-attempt.ts b/extensions/codex/src/app-server/run-attempt.ts index 285ea337cbb..024220b5e33 100644 --- a/extensions/codex/src/app-server/run-attempt.ts +++ b/extensions/codex/src/app-server/run-attempt.ts @@ -42,6 +42,7 @@ import { onInternalDiagnosticEvent, resolveDiagnosticModelContentCapturePolicy, } from "openclaw/plugin-sdk/diagnostic-runtime"; +import { loadExecApprovals } from "openclaw/plugin-sdk/exec-approvals-runtime"; import { pathExists } from "openclaw/plugin-sdk/security-runtime"; import { resolveCodexAppServerForOpenClawToolPolicy } from "./app-server-policy.js"; import { handleCodexAppServerApprovalRequest } from "./approval-bridge.js"; @@ -118,6 +119,7 @@ import { readCodexPluginConfig, resolveCodexComputerUseConfig, resolveCodexAppServerRuntimeOptions, + resolveOpenClawExecPolicyForCodexAppServer, shouldAutoApproveCodexAppServerApprovals, type CodexAppServerRuntimeOptions, } from "./config.js"; @@ -342,7 +344,11 @@ export async function runCodexAppServerAttempt( const attemptClientFactory = options.clientFactory ?? defaultLeasedCodexAppServerClientFactory; const pluginConfig = readCodexPluginConfig(options.pluginConfig); const computerUseConfig = resolveCodexComputerUseConfig({ pluginConfig }); - const configuredAppServer = resolveCodexAppServerRuntimeOptions({ pluginConfig }); + const { sessionAgentId } = resolveSessionAgentIds({ + sessionKey: params.sessionKey, + config: params.config, + agentId: params.agentId, + }); const beforeToolCallPolicy = getBeforeToolCallPolicyDiagnosticState(); preDynamicStartupStages.mark("config"); const resolvedWorkspace = resolveUserPath(params.workspaceDir); @@ -357,6 +363,17 @@ export async function runCodexAppServerAttempt( workspaceDir: resolvedWorkspace, }); preDynamicStartupStages.mark("sandbox"); + const execPolicy = resolveOpenClawExecPolicyForCodexAppServer({ + execOverrides: params.execOverrides, + approvals: loadExecApprovals(), + config: params.config, + agentId: sessionAgentId, + }); + const configuredAppServer = resolveCodexAppServerRuntimeOptions({ + pluginConfig, + execPolicy, + openClawSandboxActive: sandbox?.enabled === true, + }); const effectiveWorkspace = sandbox?.enabled ? sandbox.workspaceAccess === "rw" ? resolvedWorkspace @@ -378,6 +395,7 @@ export async function runCodexAppServerAttempt( shouldPromote: beforeToolCallPolicy.hasBeforeToolCallHook || beforeToolCallPolicy.trustedToolPolicies.length > 0, + execPolicy, canUseUntrustedApprovalPolicy: configuredAppServer.start.transport !== "stdio" || isCodexAppServerApprovalPolicyAllowedByRequirements("untrusted"), @@ -408,11 +426,6 @@ export async function runCodexAppServerAttempt( params.abortSignal?.addEventListener("abort", abortFromUpstream, { once: true }); } - const { sessionAgentId } = resolveSessionAgentIds({ - sessionKey: params.sessionKey, - config: params.config, - agentId: params.agentId, - }); const agentDir = params.agentDir ?? resolveAgentDir(params.config ?? {}, sessionAgentId); preDynamicStartupStages.mark("session-agent"); let startupBinding = await readCodexAppServerBinding(params.sessionFile); diff --git a/extensions/codex/src/command-handlers.ts b/extensions/codex/src/command-handlers.ts index a57f2c99190..2c94ead3f0b 100644 --- a/extensions/codex/src/command-handlers.ts +++ b/extensions/codex/src/command-handlers.ts @@ -546,6 +546,7 @@ async function bindConversation( sessionFile: ctx.sessionFile, workspaceDir, agentDir: scope.agentDir, + sessionKey: ctx.sessionKey, threadId: parsed.threadId, model: parsed.model, modelProvider: parsed.provider, diff --git a/extensions/codex/src/commands.test.ts b/extensions/codex/src/commands.test.ts index 1efedbc5bb8..0e9bfd61b9a 100644 --- a/extensions/codex/src/commands.test.ts +++ b/extensions/codex/src/commands.test.ts @@ -3367,6 +3367,7 @@ describe("codex command", () => { sessionFile, workspaceDir: "/repo", agentDir: path.join(tempDir, "agents", "main", "agent"), + sessionKey: undefined, threadId: "thread-123", model: "gpt-5.4", modelProvider: "openai", @@ -3426,6 +3427,7 @@ describe("codex command", () => { sessionFile, workspaceDir: "/repo with space", agentDir: path.join(tempDir, "agents", "main", "agent"), + sessionKey: undefined, threadId: "thread-123", model: undefined, modelProvider: undefined, diff --git a/extensions/codex/src/conversation-binding.test.ts b/extensions/codex/src/conversation-binding.test.ts index 6246c36f2cb..481d0e09fea 100644 --- a/extensions/codex/src/conversation-binding.test.ts +++ b/extensions/codex/src/conversation-binding.test.ts @@ -14,10 +14,40 @@ const agentRuntimeMocks = vi.hoisted(() => ({ resolveAuthProfileOrder: vi.fn(), resolveDefaultAgentDir: vi.fn(() => "/agent"), resolvePersistedAuthProfileOwnerAgentDir: vi.fn(), - resolveProviderIdForAuth: vi.fn((provider: string) => provider), + resolveProviderIdForAuth: vi.fn((provider: string, _lookup?: { config?: unknown }) => provider), + resolveSessionAgentIds: vi.fn(() => ({ defaultAgentId: "main", sessionAgentId: "main" })), saveAuthProfileStore: vi.fn(), })); +const codexRequirementsTomlMock = vi.hoisted(() => vi.fn<() => string | undefined>()); +const resolveSandboxContextMock = vi.hoisted(() => + vi.fn<(...args: unknown[]) => Promise<{ enabled: boolean } | null>>(async () => null), +); + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readFileSync(filePath: string | URL | number, options?: BufferEncoding | object | null) { + if (filePath === "/etc/codex/requirements.toml") { + const content = codexRequirementsTomlMock(); + if (content !== undefined) { + return content; + } + } + return actual.readFileSync(filePath, options); + }, + }; +}); + +vi.mock("openclaw/plugin-sdk/agent-harness-runtime", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + resolveSandboxContext: resolveSandboxContextMock, + }; +}); + vi.mock("./app-server/shared-client.js", () => ({ ...sharedClientMocks, getLeasedSharedCodexAppServerClient: sharedClientMocks.getSharedCodexAppServerClient, @@ -55,7 +85,11 @@ describe("codex conversation binding", () => { agentRuntimeMocks.resolveDefaultAgentDir.mockClear(); agentRuntimeMocks.resolvePersistedAuthProfileOwnerAgentDir.mockReset(); agentRuntimeMocks.resolveProviderIdForAuth.mockClear(); + agentRuntimeMocks.resolveSessionAgentIds.mockClear(); agentRuntimeMocks.saveAuthProfileStore.mockReset(); + codexRequirementsTomlMock.mockReset(); + resolveSandboxContextMock.mockReset(); + resolveSandboxContextMock.mockResolvedValue(null); await fs.rm(tempDir, { recursive: true, force: true }); }); @@ -66,7 +100,13 @@ describe("codex conversation binding", () => { }); agentRuntimeMocks.resolveAuthProfileOrder.mockReturnValue([]); agentRuntimeMocks.resolveDefaultAgentDir.mockReturnValue("/agent"); - agentRuntimeMocks.resolveProviderIdForAuth.mockImplementation((provider: string) => provider); + agentRuntimeMocks.resolveProviderIdForAuth.mockImplementation( + (provider: string, _lookup?: { config?: unknown }) => provider, + ); + agentRuntimeMocks.resolveSessionAgentIds.mockReturnValue({ + defaultAgentId: "main", + sessionAgentId: "main", + }); }); it("uses the default Codex auth profile and omits the public OpenAI provider for new binds", async () => { @@ -208,6 +248,70 @@ describe("codex conversation binding", () => { expect(data.agentDir).toBe(agentDir); }); + it("rejects binding when configured exec auto mode may need unrouted human approvals", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const requests: Array<{ method: string; params: Record }> = []; + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue({ + request: vi.fn(async (method: string, requestParams: Record) => { + requests.push({ method, params: requestParams }); + return { + thread: { id: "thread-new", sessionId: "session-1", cwd: tempDir }, + model: "gpt-5.4-mini", + }; + }), + }); + + await expect( + startCodexConversationThread({ + config: { + tools: { + exec: { + mode: "auto", + }, + }, + } as never, + sessionFile, + workspaceDir: tempDir, + model: "gpt-5.4-mini", + }), + ).rejects.toThrow( + "OpenClaw native Codex conversation binding cannot route interactive approvals yet", + ); + expect(requests).toEqual([]); + }); + + it("rejects binding when configured exec ask mode needs unrouted user approvals", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + const requests: Array<{ method: string; params: Record }> = []; + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue({ + request: vi.fn(async (method: string, requestParams: Record) => { + requests.push({ method, params: requestParams }); + return { + thread: { id: "thread-new", sessionId: "session-1", cwd: tempDir }, + model: "gpt-5.4-mini", + }; + }), + }); + + await expect( + startCodexConversationThread({ + config: { + tools: { + exec: { + mode: "ask", + }, + }, + } as never, + sessionFile, + workspaceDir: tempDir, + model: "gpt-5.4-mini", + }), + ).rejects.toThrow( + "OpenClaw native Codex conversation binding cannot route interactive approvals yet", + ); + expect(requests).toEqual([]); + }); + it("clears the Codex app-server sidecar when a pending bind is denied", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); const sidecar = `${sessionFile}.codex-app-server.json`; @@ -595,6 +699,104 @@ describe("codex conversation binding", () => { expect(savedBinding).not.toHaveProperty("modelProvider"); }); + it("does not silently decline auto-mode approvals during missing thread recovery", async () => { + const sessionFile = path.join(tempDir, "session.jsonl"); + await fs.writeFile( + `${sessionFile}.codex-app-server.json`, + JSON.stringify({ + schemaVersion: 1, + threadId: "thread-old", + cwd: tempDir, + approvalPolicy: "never", + sandbox: "danger-full-access", + }), + ); + const requests: Array<{ method: string; params: Record }> = []; + const notificationHandlers: Array<(notification: Record) => void> = []; + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue({ + request: vi.fn(async (method: string, requestParams: Record) => { + requests.push({ method, params: requestParams }); + if (method === "turn/start" && requestParams.threadId === "thread-old") { + throw new Error("thread not found: thread-old"); + } + if (method === "thread/start") { + return { + thread: { id: "thread-new", sessionId: "session-1", cwd: tempDir }, + model: "gpt-5.4-mini", + }; + } + if (method === "turn/start" && requestParams.threadId === "thread-new") { + setImmediate(() => { + for (const handler of notificationHandlers) { + handler({ + method: "turn/completed", + params: { + threadId: "thread-new", + turn: { + id: "turn-new", + status: "completed", + items: [{ id: "assistant-1", type: "agentMessage", text: "Recovered" }], + }, + }, + }); + } + }); + return { turn: { id: "turn-new" } }; + } + throw new Error(`unexpected method: ${method}`); + }), + addNotificationHandler: vi.fn((handler) => { + notificationHandlers.push(handler); + return () => undefined; + }), + addRequestHandler: vi.fn(() => () => undefined), + }); + + const result = await handleCodexConversationInboundClaim( + { + content: "hi again", + bodyForAgent: "hi again", + channel: "telegram", + isGroup: false, + commandAuthorized: true, + }, + { + channelId: "telegram", + pluginBinding: { + bindingId: "binding-1", + pluginId: "codex", + pluginRoot: tempDir, + channel: "telegram", + accountId: "default", + conversationId: "5185575566", + boundAt: Date.now(), + data: { + kind: "codex-app-server-session", + version: 1, + sessionFile, + workspaceDir: tempDir, + }, + }, + }, + { + timeoutMs: 500, + config: { + tools: { + exec: { + mode: "auto", + }, + }, + } as never, + }, + ); + + expect(result?.handled).toBe(true); + expect(result?.reply?.text).toContain( + "OpenClaw native Codex conversation binding cannot route interactive approvals yet", + ); + expect(requests).toEqual([]); + }); + it("creates a fresh thread when recovery finds the binding already cleared", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); const requests: Array<{ method: string; params: Record }> = []; @@ -674,6 +876,119 @@ describe("codex conversation binding", () => { expect(savedBinding.threadId).toBe("thread-new"); }); + it("passes sandbox state when resolving bound turn policy", async () => { + codexRequirementsTomlMock.mockReturnValue( + [ + 'allowed_sandbox_modes = ["read-only", "workspace-write"]', + 'allowed_approval_policies = ["never", "on-request"]', + 'allowed_approvals_reviewers = ["user"]', + ].join("\n"), + ); + resolveSandboxContextMock.mockResolvedValue({ enabled: true }); + const sessionFile = path.join(tempDir, "session.jsonl"); + await fs.writeFile( + `${sessionFile}.codex-app-server.json`, + JSON.stringify({ + schemaVersion: 1, + threadId: "thread-1", + cwd: tempDir, + approvalPolicy: "never", + sandbox: "danger-full-access", + }), + ); + let notificationHandler: ((notification: unknown) => void) | undefined; + const turnStartParams: Record[] = []; + sharedClientMocks.getSharedCodexAppServerClient.mockResolvedValue({ + request: vi.fn(async (method: string, requestParams: Record) => { + if (method === "turn/start") { + turnStartParams.push(requestParams); + setImmediate(() => + notificationHandler?.({ + method: "turn/completed", + params: { + threadId: "thread-1", + turn: { + id: "turn-1", + status: "completed", + items: [{ type: "agentMessage", id: "item-1", text: "done" }], + }, + }, + }), + ); + return { turn: { id: "turn-1" } }; + } + throw new Error(`unexpected method: ${method}`); + }), + addNotificationHandler: vi.fn((handler: (notification: unknown) => void) => { + notificationHandler = handler; + return () => undefined; + }), + addRequestHandler: vi.fn(() => () => undefined), + }); + + const result = await handleCodexConversationInboundClaim( + { + content: "continue", + bodyForAgent: "continue", + channel: "telegram", + isGroup: false, + commandAuthorized: true, + sessionKey: "agent:main:session-1", + }, + { + channelId: "telegram", + sessionKey: "agent:main:session-1", + pluginBinding: { + bindingId: "binding-1", + pluginId: "codex", + pluginRoot: tempDir, + channel: "telegram", + accountId: "default", + conversationId: "5185575566", + boundAt: Date.now(), + data: { + kind: "codex-app-server-session", + version: 1, + sessionFile, + workspaceDir: tempDir, + }, + }, + }, + { + timeoutMs: 50, + config: { + tools: { + exec: { + security: "full", + ask: "on-miss", + }, + }, + } as never, + }, + ); + + expect(result?.handled).toBe(true); + expect(result?.reply?.text).toContain( + "OpenClaw native Codex conversation binding cannot route interactive approvals yet", + ); + expect(result?.reply?.text).not.toContain( + "legacy full exec security with ask requires Codex app-server danger-full-access", + ); + expect(resolveSandboxContextMock).toHaveBeenCalledWith({ + config: { + tools: { + exec: { + security: "full", + ask: "on-miss", + }, + }, + }, + sessionKey: "agent:main:session-1", + workspaceDir: tempDir, + }); + expect(turnStartParams).toEqual([]); + }); + it("returns a clean failure reply when app-server turn start rejects", async () => { const sessionFile = path.join(tempDir, "session.jsonl"); const agentDir = path.join(tempDir, "agents", "bot-b", "agent"); @@ -831,5 +1146,10 @@ describe("codex conversation binding", () => { expect(turnStartParams[0]?.input).toEqual([ { type: "text", text: "use the fallback prompt", text_elements: [] }, ]); + expect(turnStartParams[0]?.approvalPolicy).toBe("never"); + expect(turnStartParams[0]?.approvalsReviewer).toBe("user"); + expect(turnStartParams[0]?.sandboxPolicy).toEqual({ + type: "dangerFullAccess", + }); }); }); diff --git a/extensions/codex/src/conversation-binding.ts b/extensions/codex/src/conversation-binding.ts index b7208b64a73..dd40cfc7b6e 100644 --- a/extensions/codex/src/conversation-binding.ts +++ b/extensions/codex/src/conversation-binding.ts @@ -1,18 +1,29 @@ -import { formatErrorMessage } from "openclaw/plugin-sdk/agent-harness-runtime"; -import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { + formatErrorMessage, + resolveSandboxContext, +} from "openclaw/plugin-sdk/agent-harness-runtime"; +import { resolveSessionAgentIds } from "openclaw/plugin-sdk/agent-runtime"; +import { loadExecApprovals } from "openclaw/plugin-sdk/exec-approvals-runtime"; import type { PluginConversationBindingResolvedEvent, PluginHookInboundClaimContext, PluginHookInboundClaimEvent, } from "openclaw/plugin-sdk/plugin-entry"; import type { ReplyPayload } from "openclaw/plugin-sdk/reply-payload"; +import { + loadSessionStore, + resolveSessionStoreEntry, + resolveStorePath, +} from "openclaw/plugin-sdk/session-store-runtime"; import { resolveCodexAppServerAuthProfileIdForAgent } from "./app-server/auth-bridge.js"; import { CODEX_CONTROL_METHODS } from "./app-server/capabilities.js"; import { codexSandboxPolicyForTurn, + resolveOpenClawExecPolicyForCodexAppServer, resolveCodexAppServerRuntimeOptions, type CodexAppServerApprovalPolicy, type CodexAppServerSandboxMode, + type OpenClawExecPolicyForCodexAppServer, } from "./app-server/config.js"; import { type CodexServiceTier, @@ -52,6 +63,8 @@ import { buildCodexConversationTurnInput } from "./conversation-turn-input.js"; import { resumeCodexCliSessionOnNode } from "./node-cli-sessions.js"; const DEFAULT_BOUND_TURN_TIMEOUT_MS = 20 * 60_000; +const NATIVE_CONVERSATION_INTERACTIVE_APPROVALS_UNAVAILABLE = + "OpenClaw native Codex conversation binding cannot route interactive approvals yet; use the Codex harness or explicit /acp spawn codex for that workflow."; export { createCodexCliNodeConversationBindingData, @@ -61,7 +74,7 @@ export { type CodexConversationRunOptions = { pluginConfig?: unknown; - config?: OpenClawConfig; + config?: CodexConversationConfig; timeoutMs?: number; resumeCodexCliSessionOnNode?: ResumeCodexCliSessionOnNodeFn; }; @@ -72,10 +85,11 @@ type ResumeCodexCliSessionOnNodeFn = ( type CodexConversationStartParams = { pluginConfig?: unknown; - config?: Parameters[0]["config"]; + config?: CodexConversationConfig; sessionFile: string; workspaceDir?: string; agentDir?: string; + sessionKey?: string; threadId?: string; model?: string; modelProvider?: string; @@ -89,10 +103,46 @@ type BoundTurnResult = { reply: ReplyPayload; }; +type CodexConversationConfig = Parameters< + typeof resolveCodexAppServerAuthProfileIdForAgent +>[0]["config"]; + type CodexConversationGlobalState = { queues: Map>; }; +async function resolveConversationAppServerRuntime(params: { + pluginConfig?: unknown; + config?: CodexConversationConfig; + agentId?: string; + sessionKey?: string; + workspaceDir: string; +}): Promise<{ + execPolicy?: OpenClawExecPolicyForCodexAppServer; + runtime: ReturnType; +}> { + const execPolicy = resolveConversationExecPolicy({ + config: params.config, + agentId: params.agentId, + sessionKey: params.sessionKey, + }); + const sandboxForPolicy = + execPolicy?.touched === true && execPolicy.security === "full" && execPolicy.ask !== "off" + ? await resolveSandboxContext({ + config: params.config, + sessionKey: params.sessionKey, + workspaceDir: params.workspaceDir, + }) + : undefined; + const runtime = resolveCodexAppServerRuntimeOptions({ + pluginConfig: params.pluginConfig, + execPolicy, + openClawSandboxActive: sandboxForPolicy?.enabled === true, + }); + assertNativeConversationApprovalPolicySupported({ execPolicy, runtime }); + return { execPolicy, runtime }; +} + const CODEX_CONVERSATION_GLOBAL_STATE = Symbol.for("openclaw.codex.conversationBinding"); function getGlobalState(): CodexConversationGlobalState { @@ -131,6 +181,7 @@ export async function startCodexConversationThread( sandbox: params.sandbox, serviceTier: params.serviceTier, config: params.config, + sessionKey: params.sessionKey, }); } else { await createThread({ @@ -145,6 +196,7 @@ export async function startCodexConversationThread( sandbox: params.sandbox, serviceTier: params.serviceTier, config: params.config, + sessionKey: params.sessionKey, }); } return createCodexConversationBindingData({ @@ -222,6 +274,8 @@ export async function handleCodexConversationInboundClaim( data, prompt, event, + config: options.config, + sessionKey: event.sessionKey ?? ctx.sessionKey, pluginConfig: options.pluginConfig, timeoutMs: options.timeoutMs, }), @@ -263,9 +317,15 @@ async function attachExistingThread(params: { sandbox?: CodexAppServerSandboxMode; serviceTier?: CodexServiceTier; config?: CodexAppServerAuthProfileLookup["config"]; + agentId?: string; + sessionKey?: string; }): Promise { - const runtime = resolveCodexAppServerRuntimeOptions({ + const { execPolicy, runtime } = await resolveConversationAppServerRuntime({ pluginConfig: params.pluginConfig, + config: params.config, + agentId: params.agentId, + sessionKey: params.sessionKey, + workspaceDir: params.workspaceDir, }); const agentLookup = buildAgentLookup({ agentDir: params.agentDir, config: params.config }); const modelProvider = resolveThreadRequestModelProvider({ @@ -287,9 +347,11 @@ async function attachExistingThread(params: { ...(params.model ? { model: params.model } : {}), ...(modelProvider ? { modelProvider } : {}), personality: CODEX_NATIVE_PERSONALITY_NONE, - approvalPolicy: params.approvalPolicy ?? runtime.approvalPolicy, + approvalPolicy: execPolicy?.touched + ? runtime.approvalPolicy + : (params.approvalPolicy ?? runtime.approvalPolicy), approvalsReviewer: runtime.approvalsReviewer, - sandbox: params.sandbox ?? runtime.sandbox, + sandbox: execPolicy?.touched ? runtime.sandbox : (params.sandbox ?? runtime.sandbox), ...((params.serviceTier ?? runtime.serviceTier) ? { serviceTier: params.serviceTier ?? runtime.serviceTier } : {}), @@ -312,8 +374,10 @@ async function attachExistingThread(params: { modelProvider: response.modelProvider ?? params.modelProvider, ...agentLookup, }), - approvalPolicy: params.approvalPolicy ?? runtimeApprovalPolicy, - sandbox: params.sandbox ?? runtime.sandbox, + approvalPolicy: execPolicy?.touched + ? runtimeApprovalPolicy + : (params.approvalPolicy ?? runtimeApprovalPolicy), + sandbox: execPolicy?.touched ? runtime.sandbox : (params.sandbox ?? runtime.sandbox), serviceTier: params.serviceTier ?? runtime.serviceTier, }, { @@ -337,9 +401,15 @@ async function createThread(params: { sandbox?: CodexAppServerSandboxMode; serviceTier?: CodexServiceTier; config?: CodexAppServerAuthProfileLookup["config"]; + agentId?: string; + sessionKey?: string; }): Promise { - const runtime = resolveCodexAppServerRuntimeOptions({ + const { execPolicy, runtime } = await resolveConversationAppServerRuntime({ pluginConfig: params.pluginConfig, + config: params.config, + agentId: params.agentId, + sessionKey: params.sessionKey, + workspaceDir: params.workspaceDir, }); const agentLookup = buildAgentLookup({ agentDir: params.agentDir, config: params.config }); const modelProvider = resolveThreadRequestModelProvider({ @@ -361,9 +431,11 @@ async function createThread(params: { ...(params.model ? { model: params.model } : {}), ...(modelProvider ? { modelProvider } : {}), personality: CODEX_NATIVE_PERSONALITY_NONE, - approvalPolicy: params.approvalPolicy ?? runtime.approvalPolicy, + approvalPolicy: execPolicy?.touched + ? runtime.approvalPolicy + : (params.approvalPolicy ?? runtime.approvalPolicy), approvalsReviewer: runtime.approvalsReviewer, - sandbox: params.sandbox ?? runtime.sandbox, + sandbox: execPolicy?.touched ? runtime.sandbox : (params.sandbox ?? runtime.sandbox), ...((params.serviceTier ?? runtime.serviceTier) ? { serviceTier: params.serviceTier ?? runtime.serviceTier } : {}), @@ -388,8 +460,10 @@ async function createThread(params: { modelProvider: response.modelProvider ?? params.modelProvider, ...agentLookup, }), - approvalPolicy: params.approvalPolicy ?? runtimeApprovalPolicy, - sandbox: params.sandbox ?? runtime.sandbox, + approvalPolicy: execPolicy?.touched + ? runtimeApprovalPolicy + : (params.approvalPolicy ?? runtimeApprovalPolicy), + sandbox: execPolicy?.touched ? runtime.sandbox : (params.sandbox ?? runtime.sandbox), serviceTier: params.serviceTier ?? runtime.serviceTier, }, { @@ -406,17 +480,24 @@ async function runBoundTurn(params: { prompt: string; event: PluginHookInboundClaimEvent; pluginConfig?: unknown; + config?: CodexConversationConfig; + sessionKey?: string; timeoutMs?: number; }): Promise { - const runtime = resolveCodexAppServerRuntimeOptions({ - pluginConfig: params.pluginConfig, - }); - const agentLookup = buildAgentLookup({ agentDir: params.data.agentDir }); + const agentLookup = buildAgentLookup({ agentDir: params.data.agentDir, config: params.config }); const binding = await readCodexAppServerBinding(params.data.sessionFile, agentLookup); const threadId = binding?.threadId; if (!threadId) { throw new Error("bound Codex conversation has no thread binding"); } + const workspaceDir = binding.cwd || params.data.workspaceDir; + const { execPolicy, runtime } = await resolveConversationAppServerRuntime({ + pluginConfig: params.pluginConfig, + config: params.config, + sessionKey: params.sessionKey, + workspaceDir, + }); + assertNativeConversationApprovalPolicySupported({ execPolicy, runtime }); const client = await getLeasedSharedCodexAppServerClient({ startOptions: runtime.start, @@ -473,12 +554,14 @@ async function runBoundTurn(params: { prompt: params.prompt, event: params.event, }), - cwd: binding.cwd || params.data.workspaceDir, - approvalPolicy: binding.approvalPolicy ?? runtime.approvalPolicy, + cwd: workspaceDir, + approvalPolicy: execPolicy?.touched + ? runtime.approvalPolicy + : (binding.approvalPolicy ?? runtime.approvalPolicy), approvalsReviewer: runtime.approvalsReviewer, sandboxPolicy: codexSandboxPolicyForTurn( - binding.sandbox ?? runtime.sandbox, - binding.cwd || params.data.workspaceDir, + execPolicy?.touched ? runtime.sandbox : (binding.sandbox ?? runtime.sandbox), + workspaceDir, ), ...(binding.model ? { model: binding.model } : {}), personality: CODEX_NATIVE_PERSONALITY_NONE, @@ -513,11 +596,22 @@ async function runBoundTurn(params: { } } +function assertNativeConversationApprovalPolicySupported(params: { + execPolicy?: OpenClawExecPolicyForCodexAppServer; + runtime: ReturnType; +}): void { + if (params.execPolicy?.touched === true && params.runtime.approvalPolicy !== "never") { + throw new Error(NATIVE_CONVERSATION_INTERACTIVE_APPROVALS_UNAVAILABLE); + } +} + async function runBoundTurnWithMissingThreadRecovery(params: { data: CodexAppServerConversationBindingData; prompt: string; event: PluginHookInboundClaimEvent; pluginConfig?: unknown; + config?: CodexConversationConfig; + sessionKey?: string; timeoutMs?: number; }): Promise { try { @@ -526,8 +620,13 @@ async function runBoundTurnWithMissingThreadRecovery(params: { if (!isCodexThreadNotFoundError(error)) { throw error; } - const agentLookup = buildAgentLookup({ agentDir: params.data.agentDir }); + const agentLookup = buildAgentLookup({ agentDir: params.data.agentDir, config: params.config }); const binding = await readCodexAppServerBinding(params.data.sessionFile, agentLookup); + const execPolicy = resolveConversationExecPolicy({ + config: params.config, + sessionKey: params.sessionKey, + }); + const useCurrentRuntimePolicy = execPolicy?.touched === true; await startCodexConversationThread({ pluginConfig: params.pluginConfig, sessionFile: params.data.sessionFile, @@ -536,14 +635,65 @@ async function runBoundTurnWithMissingThreadRecovery(params: { model: binding?.model, modelProvider: binding?.modelProvider, authProfileId: binding?.authProfileId, - approvalPolicy: binding?.approvalPolicy, - sandbox: binding?.sandbox, + approvalPolicy: useCurrentRuntimePolicy ? undefined : binding?.approvalPolicy, + sandbox: useCurrentRuntimePolicy ? undefined : binding?.sandbox, serviceTier: binding?.serviceTier, + config: params.config, + sessionKey: params.sessionKey, }); return await runBoundTurn(params); } } +function resolveConversationExecPolicy(params: { + config?: CodexConversationConfig; + agentId?: string; + sessionKey?: string; +}) { + if (!params.config) { + return undefined; + } + const agentId = + params.agentId ?? + resolveSessionAgentIds({ + sessionKey: params.sessionKey, + config: params.config, + }).sessionAgentId; + return resolveOpenClawExecPolicyForCodexAppServer({ + config: params.config, + agentId, + execOverrides: readSessionExecOverrides({ + config: params.config, + agentId, + sessionKey: params.sessionKey, + }), + approvals: loadExecApprovals(), + }); +} + +function readSessionExecOverrides(params: { + config?: CodexConversationConfig; + agentId?: string; + sessionKey?: string; +}): { security?: string; ask?: string } | undefined { + const sessionKey = params.sessionKey?.trim(); + if (!params.config || !sessionKey) { + return undefined; + } + const storePath = resolveStorePath(params.config.session?.store, { agentId: params.agentId }); + const entry = resolveSessionStoreEntry({ + store: loadSessionStore(storePath, { skipCache: true }), + sessionKey, + }).existing; + if (!entry?.execSecurity && !entry?.execAsk) { + return undefined; + } + return { + security: entry.execSecurity, + ask: entry.execAsk, + }; +} + function isCodexThreadNotFoundError(error: unknown): boolean { const message = formatErrorMessage(error); return ( diff --git a/extensions/discord/src/components.builders.ts b/extensions/discord/src/components.builders.ts index 6d69fb5b438..0dd723b6348 100644 --- a/extensions/discord/src/components.builders.ts +++ b/extensions/discord/src/components.builders.ts @@ -93,10 +93,10 @@ function createButtonComponent(params: { id: componentId, kind: params.modalId ? "modal-trigger" : "button", label: params.spec.label, - callbackData: params.spec.callbackData, - modalId: params.modalId, - reusable: params.spec.reusable, - allowedUsers: params.spec.allowedUsers, + ...(params.spec.callbackData !== undefined ? { callbackData: params.spec.callbackData } : {}), + ...(params.modalId !== undefined ? { modalId: params.modalId } : {}), + ...(params.spec.reusable !== undefined ? { reusable: params.spec.reusable } : {}), + ...(params.spec.allowedUsers !== undefined ? { allowedUsers: params.spec.allowedUsers } : {}), }, }; } @@ -126,10 +126,10 @@ function createSelectComponent(params: { id: componentId, kind: "select", label, - callbackData: params.spec.callbackData, + ...(params.spec.callbackData !== undefined ? { callbackData: params.spec.callbackData } : {}), selectType, ...(options ? { options } : {}), - allowedUsers: params.spec.allowedUsers, + ...(params.spec.allowedUsers !== undefined ? { allowedUsers: params.spec.allowedUsers } : {}), }); if (type === "string") { @@ -252,12 +252,13 @@ export function buildDiscordComponentMessage(params: { > = []; const addEntry = (entry: DiscordComponentEntry) => { + const reusable = entry.reusable ?? params.spec.reusable; entries.push({ ...entry, - sessionKey: params.sessionKey, - agentId: params.agentId, - accountId: params.accountId, - reusable: entry.reusable ?? params.spec.reusable, + ...(params.sessionKey !== undefined ? { sessionKey: params.sessionKey } : {}), + ...(params.agentId !== undefined ? { agentId: params.agentId } : {}), + ...(params.accountId !== undefined ? { accountId: params.accountId } : {}), + ...(reusable !== undefined ? { reusable } : {}), consumptionGroupId, }); }; @@ -339,26 +340,30 @@ export function buildDiscordComponentMessage(params: { name: normalizeModalFieldName(field.name, index), label: field.label, type: field.type, - description: field.description, - placeholder: field.placeholder, - required: field.required, - options: field.options, - minValues: field.minValues, - maxValues: field.maxValues, - minLength: field.minLength, - maxLength: field.maxLength, - style: field.style, + ...(field.description !== undefined ? { description: field.description } : {}), + ...(field.placeholder !== undefined ? { placeholder: field.placeholder } : {}), + ...(field.required !== undefined ? { required: field.required } : {}), + ...(field.options !== undefined ? { options: field.options } : {}), + ...(field.minValues !== undefined ? { minValues: field.minValues } : {}), + ...(field.maxValues !== undefined ? { maxValues: field.maxValues } : {}), + ...(field.minLength !== undefined ? { minLength: field.minLength } : {}), + ...(field.maxLength !== undefined ? { maxLength: field.maxLength } : {}), + ...(field.style !== undefined ? { style: field.style } : {}), })); modals.push({ id: modalId, title: params.spec.modal.title, - callbackData: params.spec.modal.callbackData, fields, - sessionKey: params.sessionKey, - agentId: params.agentId, - accountId: params.accountId, - reusable: params.spec.reusable, - allowedUsers: params.spec.modal.allowedUsers, + ...(params.spec.modal.callbackData !== undefined + ? { callbackData: params.spec.modal.callbackData } + : {}), + ...(params.sessionKey !== undefined ? { sessionKey: params.sessionKey } : {}), + ...(params.agentId !== undefined ? { agentId: params.agentId } : {}), + ...(params.accountId !== undefined ? { accountId: params.accountId } : {}), + ...(params.spec.reusable !== undefined ? { reusable: params.spec.reusable } : {}), + ...(params.spec.modal.allowedUsers !== undefined + ? { allowedUsers: params.spec.modal.allowedUsers } + : {}), }); const triggerSpec: DiscordComponentButtonSpec = { diff --git a/extensions/discord/src/components.test.ts b/extensions/discord/src/components.test.ts index 3f005199f8e..b78c11d1653 100644 --- a/extensions/discord/src/components.test.ts +++ b/extensions/discord/src/components.test.ts @@ -147,6 +147,28 @@ describe("discord components", () => { expect(result.entries).toHaveLength(0); }); + it("omits unset optional fields from persisted button entries", () => { + const spec = readDiscordComponentSpec({ + blocks: [ + { + type: "actions", + buttons: [{ label: "Allow Once", style: "success" }], + }, + ], + }); + if (!spec) { + throw new Error("Expected component spec to be parsed"); + } + + const result = buildDiscordComponentMessage({ spec }); + const entry = result.entries[0]; + if (!entry) { + throw new Error("Expected button entry"); + } + + expect(Object.entries(entry).filter(([, value]) => value === undefined)).toEqual([]); + }); + it("requires options for modal select fields", () => { expect(() => readDiscordComponentSpec({ diff --git a/package.json b/package.json index 3520bc1a1df..9e45076a27a 100644 --- a/package.json +++ b/package.json @@ -427,6 +427,10 @@ "types": "./dist/plugin-sdk/transport-ready-runtime.d.ts", "default": "./dist/plugin-sdk/transport-ready-runtime.js" }, + "./plugin-sdk/exec-approvals-runtime": { + "types": "./dist/plugin-sdk/exec-approvals-runtime.d.ts", + "default": "./dist/plugin-sdk/exec-approvals-runtime.js" + }, "./plugin-sdk/infra-runtime": { "types": "./dist/plugin-sdk/infra-runtime.d.ts", "default": "./dist/plugin-sdk/infra-runtime.js" diff --git a/packages/gateway-protocol/src/schema/exec-approvals.ts b/packages/gateway-protocol/src/schema/exec-approvals.ts index be7310904bb..ce328984aaa 100644 --- a/packages/gateway-protocol/src/schema/exec-approvals.ts +++ b/packages/gateway-protocol/src/schema/exec-approvals.ts @@ -158,6 +158,8 @@ export const ExecApprovalRequestParamsSchema = Type.Object( turnSourceTo: Type.Optional(Type.Union([Type.String(), Type.Null()])), turnSourceAccountId: Type.Optional(Type.Union([Type.String(), Type.Null()])), turnSourceThreadId: Type.Optional(Type.Union([Type.String(), Type.Number(), Type.Null()])), + requireDeliveryRoute: Type.Optional(Type.Boolean()), + suppressDelivery: Type.Optional(Type.Boolean()), timeoutMs: Type.Optional(Type.Integer({ minimum: 1 })), twoPhase: Type.Optional(Type.Boolean()), }, diff --git a/packages/plugin-sdk/package.json b/packages/plugin-sdk/package.json index 0753c79f73c..0f2f0015bd7 100644 --- a/packages/plugin-sdk/package.json +++ b/packages/plugin-sdk/package.json @@ -68,6 +68,10 @@ "types": "./dist/src/plugin-sdk/transport-ready-runtime.d.ts", "default": "./src/transport-ready-runtime.ts" }, + "./exec-approvals-runtime": { + "types": "./dist/src/plugin-sdk/exec-approvals-runtime.d.ts", + "default": "./src/exec-approvals-runtime.ts" + }, "./core": { "types": "./dist/src/plugin-sdk/core.d.ts", "default": "./src/core.ts" diff --git a/packages/plugin-sdk/src/exec-approvals-runtime.ts b/packages/plugin-sdk/src/exec-approvals-runtime.ts new file mode 100644 index 00000000000..fa374876360 --- /dev/null +++ b/packages/plugin-sdk/src/exec-approvals-runtime.ts @@ -0,0 +1 @@ +export * from "../../../src/plugin-sdk/exec-approvals-runtime.js"; diff --git a/scripts/lib/plugin-sdk-entrypoints.json b/scripts/lib/plugin-sdk-entrypoints.json index ec9e9b67ee6..f566c582a4d 100644 --- a/scripts/lib/plugin-sdk-entrypoints.json +++ b/scripts/lib/plugin-sdk-entrypoints.json @@ -70,6 +70,7 @@ "secure-random-runtime", "system-event-runtime", "transport-ready-runtime", + "exec-approvals-runtime", "infra-runtime", "runtime-config-snapshot", "runtime-group-policy", diff --git a/src/agents/agent-tools-agent-config.exec.test.ts b/src/agents/agent-tools-agent-config.exec.test.ts index f5c50725401..5003d939ee3 100644 --- a/src/agents/agent-tools-agent-config.exec.test.ts +++ b/src/agents/agent-tools-agent-config.exec.test.ts @@ -99,6 +99,86 @@ describe("Agent-specific exec tool defaults", () => { expect(resultDetails?.status).toBe("completed"); }); + it("passes normalized exec mode defaults into the exec tool", async () => { + const tools = createOpenClawCodingTools({ + config: { + tools: { + exec: { + mode: "deny", + }, + }, + }, + sessionKey: "agent:main:main", + workspaceDir: "/tmp/test-main-mode-deny", + agentDir: "/tmp/agent-main-mode-deny", + }); + const execTool = requireExecTool(tools); + + await expect( + execTool.execute("call-mode-deny", { + command: "echo blocked", + }), + ).rejects.toThrow("security=deny"); + }); + + it("ignores per-call legacy security when configured mode is full", async () => { + const tools = createOpenClawCodingTools({ + config: { + tools: { + exec: { + mode: "full", + }, + }, + }, + sessionKey: "agent:main:main", + workspaceDir: "/tmp/test-main-mode-call-security", + agentDir: "/tmp/agent-main-mode-call-security", + }); + const execTool = requireExecTool(tools); + + const result = await execTool.execute("call-mode-security-deny", { + command: "echo allowed", + security: "deny", + }); + const text = (result.content[0] as { text?: string } | undefined)?.text ?? ""; + expect(text).toContain("allowed"); + }); + + it("preserves mode-derived security for partial agent exec overrides", async () => { + const tools = createOpenClawCodingTools({ + config: { + tools: { + exec: { + mode: "auto", + safeBins: [], + }, + }, + agents: { + list: [ + { + id: "main", + tools: { + exec: { + ask: "off", + }, + }, + }, + ], + }, + }, + sessionKey: "agent:main:main", + workspaceDir: "/tmp/test-main-mode-partial-agent", + agentDir: "/tmp/agent-main-mode-partial-agent", + }); + const execTool = requireExecTool(tools); + + await expect( + execTool.execute("call-mode-partial-agent", { + command: "echo blocked", + }), + ).rejects.toThrow(/allowlist miss/); + }); + it("fails closed when exec host=sandbox is requested without sandbox runtime", async () => { const tools = createOpenClawCodingTools({ config: {}, diff --git a/src/agents/agent-tools.ts b/src/agents/agent-tools.ts index de6f1f2f755..640c687e8d4 100644 --- a/src/agents/agent-tools.ts +++ b/src/agents/agent-tools.ts @@ -7,6 +7,12 @@ import type { ModelCompatConfig } from "../config/types.models.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { DiagnosticTraceContext } from "../infra/diagnostic-trace-context.js"; import { resolveEventSessionRoutingPolicy } from "../infra/event-session-routing.js"; +import { + type ExecAsk, + type ExecMode, + type ExecSecurity, + resolveExecPolicyForMode, +} from "../infra/exec-approvals.js"; import { resolveMergedSafeBinProfileFixtures } from "../infra/exec-safe-bin-runtime-policy.js"; import { logWarn } from "../logger.js"; import { getPluginToolMeta } from "../plugins/tools.js"; @@ -291,15 +297,46 @@ function isApplyPatchAllowedForModel(params: { }); } +type ExecPolicyLayer = { + mode?: ExecMode; + security?: ExecSecurity; + ask?: ExecAsk; +}; + +function hasLegacyExecPolicy(exec?: ExecPolicyLayer): boolean { + return exec?.security !== undefined || exec?.ask !== undefined; +} + +function applyExecPolicyLayer(base: ExecPolicyLayer, layer?: ExecPolicyLayer): ExecPolicyLayer { + if (!layer) { + return base; + } + if (layer.mode) { + return { + mode: layer.mode, + ...resolveExecPolicyForMode(layer.mode), + }; + } + if (hasLegacyExecPolicy(layer)) { + return { + security: layer.security ?? base.security, + ask: layer.ask ?? base.ask, + }; + } + return base; +} + function resolveExecConfig(params: { cfg?: OpenClawConfig; agentId?: string }) { const cfg = params.cfg; const globalExec = cfg?.tools?.exec; const agentExec = cfg && params.agentId ? resolveAgentConfig(cfg, params.agentId)?.tools?.exec : undefined; + const layeredPolicy = applyExecPolicyLayer(applyExecPolicyLayer({}, globalExec), agentExec); return { host: agentExec?.host ?? globalExec?.host, - security: agentExec?.security ?? globalExec?.security, - ask: agentExec?.ask ?? globalExec?.ask, + mode: layeredPolicy.mode, + security: layeredPolicy.security, + ask: layeredPolicy.ask, node: agentExec?.node ?? globalExec?.node, pathPrepend: agentExec?.pathPrepend ?? globalExec?.pathPrepend, safeBins: agentExec?.safeBins ?? globalExec?.safeBins, @@ -313,6 +350,7 @@ function resolveExecConfig(params: { cfg?: OpenClawConfig; agentId?: string }) { global: globalExec, local: agentExec, }), + reviewer: agentExec?.reviewer ?? globalExec?.reviewer, backgroundMs: agentExec?.backgroundMs ?? globalExec?.backgroundMs, timeoutSec: agentExec?.timeoutSec ?? globalExec?.timeoutSec, approvalRunningNoticeMs: diff --git a/src/agents/bash-tools.exec-approval-request.ts b/src/agents/bash-tools.exec-approval-request.ts index e25d1b6df2a..20b09c204c2 100644 --- a/src/agents/bash-tools.exec-approval-request.ts +++ b/src/agents/bash-tools.exec-approval-request.ts @@ -53,6 +53,8 @@ export type RequestExecApprovalDecisionParams = { turnSourceTo?: string; turnSourceAccountId?: string; turnSourceThreadId?: string | number; + requireDeliveryRoute?: boolean; + suppressDelivery?: boolean; }; type ExecApprovalRequestToolParams = RequestExecApprovalDecisionParams & { @@ -83,6 +85,8 @@ function buildExecApprovalRequestToolParams( turnSourceTo: params.turnSourceTo, turnSourceAccountId: params.turnSourceAccountId, turnSourceThreadId: params.turnSourceThreadId, + requireDeliveryRoute: params.requireDeliveryRoute, + suppressDelivery: params.suppressDelivery, timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS, twoPhase: true, }; @@ -193,6 +197,8 @@ type HostExecApprovalParams = { turnSourceTo?: string; turnSourceAccountId?: string; turnSourceThreadId?: string | number; + requireDeliveryRoute?: boolean; + suppressDelivery?: boolean; }; type ExecApprovalRequesterContext = { @@ -294,6 +300,8 @@ async function buildHostApprovalDecisionParams( sessionKey: params.sessionKey, }), resolvedPath: params.resolvedPath, + requireDeliveryRoute: params.requireDeliveryRoute, + suppressDelivery: params.suppressDelivery, ...buildExecApprovalTurnSourceContext(params), }; } diff --git a/src/agents/bash-tools.exec-host-gateway.test.ts b/src/agents/bash-tools.exec-host-gateway.test.ts index b17fa2c9240..ccf0d33eab8 100644 --- a/src/agents/bash-tools.exec-host-gateway.test.ts +++ b/src/agents/bash-tools.exec-host-gateway.test.ts @@ -6,6 +6,7 @@ type StrictInlineEvalBoundary = typeof import("./bash-tools.exec-host-shared.js").enforceStrictInlineEvalApprovalBoundary; type SendExecApprovalFollowupResult = typeof import("./bash-tools.exec-host-shared.js").sendExecApprovalFollowupResult; +type ExecAutoReviewer = typeof import("../infra/exec-auto-review.js").defaultExecAutoReviewer; type BuildExecApprovalFollowupTargetMock = ( value: ExecApprovalFollowupTarget, ) => ExecApprovalFollowupTarget | null; @@ -59,12 +60,20 @@ const analyzeShellCommandMock = vi.hoisted(() => })), ); const hasDurableExecApprovalMock = vi.hoisted(() => vi.fn(() => true)); +const requiresExecApprovalMock = vi.hoisted(() => vi.fn(() => false)); const buildEnforcedShellCommandMock = vi.hoisted(() => vi.fn((): { ok: boolean; reason?: string; command?: string } => ({ ok: false, reason: "segment execution plan unavailable", })), ); +const defaultExecAutoReviewerMock = vi.hoisted(() => + vi.fn(async () => ({ + decision: "allow-once", + risk: "low", + rationale: "allowed", + })), +); const recordAllowlistMatchesUseMock = vi.hoisted(() => vi.fn()); const resolveApprovalDecisionOrUndefinedMock = vi.hoisted(() => vi.fn(async (): Promise => undefined), @@ -98,12 +107,13 @@ const detectInterpreterInlineEvalArgvMock = vi.hoisted(() => ), ); -vi.mock("../infra/exec-approvals.js", () => ({ +vi.mock("../infra/exec-approvals.js", async (importOriginal) => ({ + ...(await importOriginal()), evaluateShellAllowlist: evaluateShellAllowlistMock, analyzeShellCommand: analyzeShellCommandMock, hasDurableExecApproval: hasDurableExecApprovalMock, buildEnforcedShellCommand: buildEnforcedShellCommandMock, - requiresExecApproval: vi.fn(() => false), + requiresExecApproval: requiresExecApprovalMock, recordAllowlistUse: vi.fn(), recordAllowlistMatchesUse: recordAllowlistMatchesUseMock, resolveApprovalAuditTrustPath: vi.fn(() => null), @@ -113,6 +123,10 @@ vi.mock("../infra/exec-approvals.js", () => ({ addDurableCommandApproval: vi.fn(), })); +vi.mock("../infra/exec-auto-review.js", () => ({ + defaultExecAutoReviewer: defaultExecAutoReviewerMock, +})); + vi.mock("./bash-tools.exec-approval-request.js", () => ({ buildExecApprovalRequesterContext: vi.fn(() => ({})), buildExecApprovalTurnSourceContext: vi.fn(() => ({})), @@ -228,11 +242,19 @@ describe("processGatewayAllowlist", () => { })); hasDurableExecApprovalMock.mockReset(); hasDurableExecApprovalMock.mockReturnValue(true); + requiresExecApprovalMock.mockReset(); + requiresExecApprovalMock.mockReturnValue(false); buildEnforcedShellCommandMock.mockReset(); buildEnforcedShellCommandMock.mockReturnValue({ ok: false, reason: "segment execution plan unavailable", }); + defaultExecAutoReviewerMock.mockReset(); + defaultExecAutoReviewerMock.mockResolvedValue({ + decision: "allow-once", + risk: "low", + rationale: "allowed", + }); recordAllowlistMatchesUseMock.mockReset(); resolveApprovalDecisionOrUndefinedMock.mockReset(); resolveApprovalDecisionOrUndefinedMock.mockResolvedValue(undefined); @@ -331,26 +353,274 @@ describe("processGatewayAllowlist", () => { expect(result.pendingResult?.details.status).toBe("approval-pending"); }); - it("allows durable exact-command trust to bypass the synchronous allowlist miss", async () => { + it("auto-reviews simple read-only approval misses without prompting", async () => { + requiresExecApprovalMock.mockReturnValue(true); evaluateShellAllowlistMock.mockReturnValue({ allowlistMatches: [], - analysisOk: false, + analysisOk: true, allowlistSatisfied: false, - segments: [{ resolution: null, argv: ["node", "--version"] }], + segments: [{ resolution: null, argv: ["echo", "ok"] }], segmentAllowlistEntries: [], }); - hasDurableExecApprovalMock.mockReturnValue(true); - buildEnforcedShellCommandMock.mockReturnValue({ - ok: true, - command: "node --version", + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "allowlist", + hostAsk: "on-miss", + askFallback: "deny", }); const result = await runGatewayAllowlist({ - command: "node --version", + command: "echo ok", + ask: "on-miss", + autoReview: true, }); + expect(defaultExecAutoReviewerMock).toHaveBeenCalledWith( + expect.objectContaining({ + command: "echo ok", + argv: ["echo", "ok"], + host: "gateway", + reason: "approval-required", + }), + ); expect(createAndRegisterDefaultExecApprovalRequestMock).not.toHaveBeenCalled(); - expect(result).toEqual({ execCommandOverride: undefined }); + expect(result).toEqual({ + execCommandOverride: undefined, + allowWithoutEnforcedCommand: true, + }); + }); + + it("auto-reviews heredoc commands instead of forcing human approval", async () => { + const command = "python3 - <<'PY'\nprint('ok')\nPY"; + requiresExecApprovalMock.mockReturnValue(false); + buildEnforcedShellCommandMock.mockReturnValue({ + ok: true, + command, + }); + evaluateShellAllowlistMock.mockReturnValue({ + allowlistMatches: [], + analysisOk: true, + allowlistSatisfied: true, + segments: [ + { + raw: command, + resolution: null, + argv: ["python3", "-", "<<'PY'"], + }, + ], + segmentAllowlistEntries: [], + }); + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "allowlist", + hostAsk: "on-miss", + askFallback: "deny", + }); + + const result = await runGatewayAllowlist({ + command, + ask: "on-miss", + autoReview: true, + }); + + expect(defaultExecAutoReviewerMock).toHaveBeenCalledWith( + expect.objectContaining({ + command, + argv: ["python3", "-", "<<'PY'"], + host: "gateway", + reason: "heredoc", + }), + ); + expect(createAndRegisterDefaultExecApprovalRequestMock).not.toHaveBeenCalled(); + expect(result).toEqual({ + execCommandOverride: command, + allowWithoutEnforcedCommand: false, + }); + }); + + it("auto-reviews strict inline-eval commands instead of forcing human approval", async () => { + const command = "python3 -c 'print(1)'"; + requiresExecApprovalMock.mockReturnValue(false); + detectInterpreterInlineEvalArgvMock.mockReturnValue(INLINE_EVAL_HIT); + buildEnforcedShellCommandMock.mockReturnValue({ + ok: true, + command, + }); + evaluateShellAllowlistMock.mockReturnValue({ + allowlistMatches: [], + analysisOk: true, + allowlistSatisfied: true, + segments: [ + { + raw: command, + resolution: null, + argv: ["python3", "-c", "print(1)"], + }, + ], + segmentAllowlistEntries: [], + }); + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "allowlist", + hostAsk: "on-miss", + askFallback: "deny", + }); + const warnings: string[] = []; + + const result = await runGatewayAllowlist({ + command, + ask: "on-miss", + autoReview: true, + strictInlineEval: true, + warnings, + }); + + expect(defaultExecAutoReviewerMock).toHaveBeenCalledWith( + expect.objectContaining({ + command, + argv: ["python3", "-c", "print(1)"], + host: "gateway", + reason: "strict-inline-eval", + analysis: expect.objectContaining({ + inlineEval: true, + }), + }), + ); + expect(createAndRegisterDefaultExecApprovalRequestMock).not.toHaveBeenCalled(); + expect(warnings[0]).toContain("reviewer or explicit approval"); + expect(result).toEqual({ + execCommandOverride: command, + allowWithoutEnforcedCommand: false, + }); + }); + + it("auto-reviews allowlist plan misses instead of forcing human approval", async () => { + const command = "echo ok"; + requiresExecApprovalMock.mockReturnValue(false); + buildEnforcedShellCommandMock.mockReturnValue({ + ok: false, + reason: "segment execution plan unavailable", + }); + evaluateShellAllowlistMock.mockReturnValue({ + allowlistMatches: [], + analysisOk: true, + allowlistSatisfied: true, + segments: [{ raw: command, resolution: null, argv: ["echo", "ok"] }], + segmentAllowlistEntries: [], + }); + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "allowlist", + hostAsk: "on-miss", + askFallback: "deny", + }); + + const result = await runGatewayAllowlist({ + command, + ask: "on-miss", + autoReview: true, + }); + + expect(defaultExecAutoReviewerMock).toHaveBeenCalledWith( + expect.objectContaining({ + command, + argv: ["echo", "ok"], + host: "gateway", + reason: "execution-plan-miss", + }), + ); + expect(createAndRegisterDefaultExecApprovalRequestMock).not.toHaveBeenCalled(); + expect(result).toEqual({ + execCommandOverride: undefined, + allowWithoutEnforcedCommand: true, + }); + }); + + it("requests human approval when auto-review asks on an approval miss", async () => { + requiresExecApprovalMock.mockReturnValue(true); + evaluateShellAllowlistMock.mockReturnValue({ + allowlistMatches: [], + analysisOk: true, + allowlistSatisfied: false, + segments: [{ resolution: null, argv: ["echo", "ok"] }], + segmentAllowlistEntries: [], + }); + defaultExecAutoReviewerMock.mockResolvedValue({ + decision: "ask", + risk: "medium", + rationale: "needs a person", + }); + const warnings: string[] = []; + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "allowlist", + hostAsk: "on-miss", + askFallback: "deny", + }); + + const result = await runGatewayAllowlist({ + command: "echo ok", + ask: "on-miss", + autoReview: true, + warnings, + }); + + expect(defaultExecAutoReviewerMock).toHaveBeenCalledTimes(1); + expect(createAndRegisterDefaultExecApprovalRequestMock).toHaveBeenCalledTimes(1); + expect(warnings.join("\n")).toContain("needs a person"); + expect(result.pendingResult?.details.status).toBe("approval-pending"); + }); + + it("does not use fallback-full when auto-review asks for human approval", async () => { + requiresExecApprovalMock.mockReturnValue(true); + evaluateShellAllowlistMock.mockReturnValue({ + allowlistMatches: [], + analysisOk: true, + allowlistSatisfied: false, + segments: [{ resolution: null, argv: ["echo", "ok"] }], + segmentAllowlistEntries: [], + }); + defaultExecAutoReviewerMock.mockResolvedValue({ + decision: "ask", + risk: "medium", + rationale: "needs a person", + }); + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "allowlist", + hostAsk: "on-miss", + askFallback: "full", + }); + createExecApprovalDecisionStateMock.mockReturnValue({ + baseDecision: { timedOut: true }, + approvedByAsk: true, + deniedReason: null, + }); + resolveApprovalDecisionOrUndefinedMock.mockResolvedValue(null); + enforceStrictInlineEvalApprovalBoundaryMock.mockImplementation((value) => + value.requiresAutoReviewHumanApproval === true && value.baseDecision.timedOut + ? { approvedByAsk: false, deniedReason: "approval-timeout" } + : { approvedByAsk: value.approvedByAsk, deniedReason: value.deniedReason }, + ); + + const result = await runGatewayAllowlist({ + command: "echo ok", + ask: "on-miss", + autoReview: true, + turnSourceChannel: "webchat", + }); + + expect(enforceStrictInlineEvalApprovalBoundaryMock).toHaveBeenCalledWith( + expect.objectContaining({ + requiresAutoReviewHumanApproval: true, + }), + ); + expect(result.deniedResult?.details.status).toBe("failed"); + expect(result.deniedResult?.content[0]).toEqual( + expect.objectContaining({ + text: "Exec denied (gateway id=req-1, approval-timeout): echo ok", + }), + ); }); it("requires approval for security audit suppression edits unless yolo mode is active", async () => { @@ -371,6 +641,29 @@ describe("processGatewayAllowlist", () => { expect(result.pendingResult?.details.status).toBe("approval-pending"); }); + it("keeps security audit suppression edits off the auto-review path", async () => { + const warnings: string[] = []; + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "full", + hostAsk: "on-miss", + askFallback: "deny", + }); + + const result = await runGatewayAllowlist({ + command: "openclaw config set security.audit.suppressions '[]'", + security: "full", + ask: "on-miss", + autoReview: true, + warnings, + }); + + expect(defaultExecAutoReviewerMock).not.toHaveBeenCalled(); + expect(createAndRegisterDefaultExecApprovalRequestMock).toHaveBeenCalledTimes(1); + expect(warnings[0]).toContain("explicit approval"); + expect(result.pendingResult?.details.status).toBe("approval-pending"); + }); + it("does not require approval for security audit suppression edits in yolo mode", async () => { resolveExecHostApprovalContextMock.mockReturnValue({ approvals: { allowlist: [], file: { version: 1, agents: {} } }, @@ -550,6 +843,28 @@ EOF`, expect(result.pendingResult?.details.status).toBe("approval-pending"); }); + it("allows durable exact-command trust to bypass the synchronous allowlist miss", async () => { + evaluateShellAllowlistMock.mockReturnValue({ + allowlistMatches: [], + analysisOk: false, + allowlistSatisfied: false, + segments: [{ resolution: null, argv: ["node", "--version"] }], + segmentAllowlistEntries: [], + }); + hasDurableExecApprovalMock.mockReturnValue(true); + buildEnforcedShellCommandMock.mockReturnValue({ + ok: true, + command: "node --version", + }); + + const result = await runGatewayAllowlist({ + command: "node --version", + }); + + expect(createAndRegisterDefaultExecApprovalRequestMock).not.toHaveBeenCalled(); + expect(result).toEqual({ execCommandOverride: undefined }); + }); + it("keeps denying allowlist misses when durable trust does not match", async () => { evaluateShellAllowlistMock.mockReturnValue({ allowlistMatches: [], diff --git a/src/agents/bash-tools.exec-host-gateway.ts b/src/agents/bash-tools.exec-host-gateway.ts index ec612103d45..91c1b10253f 100644 --- a/src/agents/bash-tools.exec-host-gateway.ts +++ b/src/agents/bash-tools.exec-host-gateway.ts @@ -2,7 +2,7 @@ import { describeInterpreterInlineEval } from "../infra/command-analysis/inline- import { detectPolicyInlineEval } from "../infra/command-analysis/policy.js"; import { addDurableCommandApproval, - analyzeShellCommand, + commandRequiresSecurityAuditSuppressionApproval, type ExecAsk, resolveExecApprovalAllowedDecisions, type ExecSecurity, @@ -14,6 +14,11 @@ import { resolveApprovalAuditTrustPath, requiresExecApproval, } from "../infra/exec-approvals.js"; +import { + defaultExecAutoReviewer, + type ExecAutoReviewer, + type ExecAutoReviewInput, +} from "../infra/exec-auto-review.js"; import type { SafeBinProfile } from "../infra/exec-safe-bin-policy.js"; import { isRecord } from "../shared/record-coerce.js"; import { normalizeStringEntries } from "../shared/string-normalization.js"; @@ -62,6 +67,8 @@ export type ProcessGatewayAllowlistParams = { defaultTimeoutSec: number; security: ExecSecurity; ask: ExecAsk; + autoReview?: boolean; + autoReviewer?: ExecAutoReviewer; safeBins: Set; safeBinProfiles: Readonly>; strictInlineEval?: boolean; @@ -106,109 +113,35 @@ function hasGatewayAllowlistMiss(params: { ); } -function normalizeCommandName(value: string | undefined): string { - return (value ?? "").split(/[\\/]/).pop()?.toLowerCase() ?? ""; -} - -function textMentionsSecurityAuditSuppressions(value: string): boolean { - const normalized = value.toLowerCase(); - return ( - normalized.includes("security.audit.suppressions") || - /["']?security["']?[\s\S]{0,200}["']?audit["']?[\s\S]{0,200}["']?suppressions["']?/.test( - normalized, - ) - ); -} - -function isReadOnlySecurityAuditSuppressionInspection(argv: string[]): boolean { - const command = normalizeCommandName(argv[0]); - let offset = command === "pnpm" && argv[1] === "openclaw" ? 1 : 0; - if (normalizeCommandName(argv[offset]) !== "openclaw") { - return false; +function resolveGatewayAutoReviewReason(params: { + requiresInlineEvalApproval: boolean; + requiresHeredocApproval: boolean; + requiresAllowlistPlanApproval: boolean; + hostSecurity: ExecSecurity; + analysisOk: boolean; + allowlistSatisfied: boolean; + durableApprovalSatisfied: boolean; +}): ExecAutoReviewInput["reason"] { + if (params.requiresInlineEvalApproval) { + return "strict-inline-eval"; } - offset += 1; - while (offset < argv.length) { - const arg = argv[offset]; - if (["--dev", "--no-color"].includes(arg ?? "")) { - offset += 1; - continue; - } - if (["--profile", "--container", "--log-level"].includes(arg ?? "")) { - offset += 2; - continue; - } - if ( - arg?.startsWith("--profile=") || - arg?.startsWith("--container=") || - arg?.startsWith("--log-level=") - ) { - offset += 1; - continue; - } - break; + if (params.requiresHeredocApproval) { + return "heredoc"; } - return ( - argv[offset] === "config" && ["get", "schema", "validate"].includes(argv[offset + 1] ?? "") - ); -} - -function removeParsedSegmentText(command: string, segments: Array<{ raw?: string }>): string { - let remaining = command; - for (const segment of segments) { - const raw = segment.raw?.trim(); - if (!raw) { - continue; - } - remaining = remaining.replace(raw, " "); + if (params.requiresAllowlistPlanApproval) { + return "execution-plan-miss"; } - return remaining; -} - -function commandRequiresSecurityAuditSuppressionApproval(params: { - command: string; - cwd?: string; - env?: NodeJS.ProcessEnv; - segments: Array<{ argv: string[]; raw?: string }>; -}): boolean { - let sawSegmentMention = false; - for (const segment of params.segments) { - const segmentText = `${segment.raw ?? ""} ${segment.argv.join(" ")}`; - if (!textMentionsSecurityAuditSuppressions(segmentText)) { - continue; - } - sawSegmentMention = true; - if (!isReadOnlySecurityAuditSuppressionInspection(segment.argv)) { - return true; - } + if ( + hasGatewayAllowlistMiss({ + hostSecurity: params.hostSecurity, + analysisOk: params.analysisOk, + allowlistSatisfied: params.allowlistSatisfied, + durableApprovalSatisfied: params.durableApprovalSatisfied, + }) + ) { + return "allowlist-miss"; } - if (sawSegmentMention) { - const rawAnalysis = analyzeShellCommand({ - command: params.command, - cwd: params.cwd, - env: params.env, - platform: process.platform, - }); - if (!rawAnalysis.ok) { - return textMentionsSecurityAuditSuppressions(params.command); - } - for (const segment of rawAnalysis.segments) { - if ( - textMentionsSecurityAuditSuppressions(`${segment.raw} ${segment.argv.join(" ")}`) && - !isReadOnlySecurityAuditSuppressionInspection(segment.argv) - ) { - return true; - } - } - if ( - textMentionsSecurityAuditSuppressions( - removeParsedSegmentText(params.command, rawAnalysis.segments), - ) - ) { - return true; - } - return false; - } - return textMentionsSecurityAuditSuppressions(params.command); + return "approval-required"; } function formatOutcomeExitLabel(outcome: { exitCode: number | null; timedOut: boolean }): string { @@ -433,7 +366,7 @@ export async function processGatewayAllowlist( params.strictInlineEval === true ? detectPolicyInlineEval(allowlistEval.segments) : null; if (inlineEvalHit) { params.warnings.push( - `Warning: strict inline-eval mode requires explicit approval for ${describeInterpreterInlineEval( + `Warning: strict inline-eval mode requires reviewer or explicit approval for ${describeInterpreterInlineEval( inlineEvalHit, )}.`, ); @@ -493,12 +426,12 @@ export async function processGatewayAllowlist( requiresSecurityAuditSuppressionApproval; if (requiresHeredocApproval) { params.warnings.push( - "Warning: heredoc execution requires explicit approval in allowlist mode.", + "Warning: heredoc execution requires reviewer or explicit approval in allowlist mode.", ); } if (requiresAllowlistPlanApproval) { params.warnings.push( - `Warning: allowlist auto-execution is unavailable on ${process.platform}; explicit approval is required.`, + `Warning: allowlist auto-execution is unavailable on ${process.platform}; reviewer or explicit approval is required.`, ); } if (requiresSecurityAuditSuppressionApproval) { @@ -506,8 +439,69 @@ export async function processGatewayAllowlist( "Warning: security audit suppression changes require explicit approval unless exec is running in yolo mode.", ); } - if (requiresAsk) { + const [autoReviewSegment] = allowlistEval.segments; + const autoReviewArgv = + allowlistEval.segments.length === 1 && + (autoReviewSegment?.raw === undefined || + autoReviewSegment.raw.trim() === params.command.trim()) + ? autoReviewSegment.argv + : undefined; + const canAutoReviewApprovalMiss = + params.autoReview === true && + hostAsk !== "always" && + !requiresSecurityAuditSuppressionApproval; + let autoReviewRequiresHumanApproval = false; + if (canAutoReviewApprovalMiss) { + const reviewer = params.autoReviewer ?? defaultExecAutoReviewer; + const decision = await reviewer({ + command: params.command, + argv: autoReviewArgv, + cwd: params.workdir, + envKeys: Object.keys(params.requestedEnv ?? {}).toSorted(), + host: "gateway", + reason: resolveGatewayAutoReviewReason({ + requiresInlineEvalApproval, + requiresHeredocApproval, + requiresAllowlistPlanApproval, + hostSecurity, + analysisOk, + allowlistSatisfied, + durableApprovalSatisfied, + }), + analysis: { + parsed: analysisOk, + allowlistMatched: allowlistSatisfied, + durableApprovalMatched: durableApprovalSatisfied, + inlineEval: requiresInlineEvalApproval, + heredoc: requiresHeredocApproval, + }, + agent: { + id: params.agentId, + sessionKey: params.sessionKey, + }, + }); + if (decision.decision === "allow-once") { + params.warnings.push( + `Exec auto-review allowed once (risk=${decision.risk}): ${decision.rationale}`, + ); + recordMatchedAllowlistUse( + resolveApprovalAuditTrustPath( + allowlistEval.segments[0]?.resolution ?? null, + params.workdir, + ), + ); + return { + execCommandOverride: enforcedCommand, + allowWithoutEnforcedCommand: enforcedCommand === undefined, + }; + } + params.warnings.push( + `Exec auto-review deferred to human approval (risk=${decision.risk}): ${decision.rationale}`, + ); + autoReviewRequiresHumanApproval = true; + } + const requestArgs = buildDefaultExecApprovalRequestArgs({ warnings: params.warnings, approvalRunningNoticeMs: params.approvalRunningNoticeMs, @@ -565,6 +559,7 @@ export async function processGatewayAllowlist( approvedByAsk, deniedReason, requiresInlineEvalApproval, + requiresAutoReviewHumanApproval: autoReviewRequiresHumanApproval, }); if (strictInlineEvalDecision.deniedReason || !strictInlineEvalDecision.approvedByAsk) { @@ -649,6 +644,7 @@ export async function processGatewayAllowlist( approvedByAsk, deniedReason, requiresInlineEvalApproval, + requiresAutoReviewHumanApproval: autoReviewRequiresHumanApproval, })); if ( diff --git a/src/agents/bash-tools.exec-host-node-phases.ts b/src/agents/bash-tools.exec-host-node-phases.ts index fcf59c80826..d704a9e6809 100644 --- a/src/agents/bash-tools.exec-host-node-phases.ts +++ b/src/agents/bash-tools.exec-host-node-phases.ts @@ -9,6 +9,7 @@ import { type ExecAsk, type ExecSecurity, type SystemRunApprovalPlan, + commandRequiresSecurityAuditSuppressionApproval, evaluateShellAllowlist, hasDurableExecApproval, resolveExecApprovalsFromFile, @@ -48,6 +49,8 @@ type NodeApprovalAnalysis = { allowlistSatisfied: boolean; durableApprovalSatisfied: boolean; inlineEvalHit: InterpreterInlineEvalHit | null; + requiresSecurityAuditSuppressionApproval: boolean; + autoReviewArgv?: string[]; }; export function shouldSkipNodeApprovalPrepare(params: { @@ -317,11 +320,18 @@ export async function analyzeNodeApprovalRequirement(params: { : null; if (inlineEvalHit) { params.request.warnings.push( - `Warning: strict inline-eval mode requires explicit approval for ${describeInterpreterInlineEval( + `Warning: strict inline-eval mode requires reviewer or explicit approval for ${describeInterpreterInlineEval( inlineEvalHit, )}.`, ); } + const requiresSecurityAuditSuppressionApproval = + commandRequiresSecurityAuditSuppressionApproval({ + command: params.request.command, + cwd: params.request.workdir, + env: params.request.env, + segments: baseAllowlistEval.segments, + }) && !(params.hostSecurity === "full" && params.hostAsk === "off"); if ((params.hostAsk === "always" || params.hostSecurity === "allowlist") && analysisOk) { try { const approvalsSnapshot = await callGatewayTool<{ file: string }>( @@ -367,5 +377,12 @@ export async function analyzeNodeApprovalRequirement(params: { allowlistSatisfied, durableApprovalSatisfied, inlineEvalHit, + requiresSecurityAuditSuppressionApproval, + autoReviewArgv: + baseAllowlistEval.segments.length === 1 && + (baseAllowlistEval.segments[0]?.raw === undefined || + baseAllowlistEval.segments[0].raw.trim() === params.request.command.trim()) + ? baseAllowlistEval.segments[0].argv + : undefined, }; } diff --git a/src/agents/bash-tools.exec-host-node.test.ts b/src/agents/bash-tools.exec-host-node.test.ts index a3aad68557e..6c17a319679 100644 --- a/src/agents/bash-tools.exec-host-node.test.ts +++ b/src/agents/bash-tools.exec-host-node.test.ts @@ -2,6 +2,7 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; type StrictInlineEvalBoundary = typeof import("./bash-tools.exec-host-shared.js").enforceStrictInlineEvalApprovalBoundary; +type ExecAutoReviewer = typeof import("../infra/exec-auto-review.js").defaultExecAutoReviewer; const INLINE_EVAL_HIT = { executable: "python3", @@ -27,6 +28,16 @@ const preparedPlan = vi.hoisted(() => ({ const callGatewayToolMock = vi.hoisted(() => vi.fn()); const listNodesMock = vi.hoisted(() => vi.fn()); const parsePreparedSystemRunPayloadMock = vi.hoisted(() => vi.fn()); +const commandRequiresSecurityAuditSuppressionApprovalMock = vi.hoisted(() => vi.fn(() => false)); +const evaluateShellAllowlistMock = vi.hoisted(() => + vi.fn(() => ({ + allowlistMatches: [], + analysisOk: true, + allowlistSatisfied: false, + segments: [{ resolution: null, argv: ["bun", "./script.ts"] }], + segmentAllowlistEntries: [], + })), +); const requiresExecApprovalMock = vi.hoisted(() => vi.fn(() => true)); const resolveExecHostApprovalContextMock = vi.hoisted(() => vi.fn(() => ({ @@ -76,13 +87,9 @@ const detectInterpreterInlineEvalArgvMock = vi.hoisted(() => ); vi.mock("../infra/exec-approvals.js", () => ({ - evaluateShellAllowlist: vi.fn(() => ({ - allowlistMatches: [], - analysisOk: true, - allowlistSatisfied: false, - segments: [{ resolution: null, argv: ["bun", "./script.ts"] }], - segmentAllowlistEntries: [], - })), + evaluateShellAllowlist: evaluateShellAllowlistMock, + commandRequiresSecurityAuditSuppressionApproval: + commandRequiresSecurityAuditSuppressionApprovalMock, hasDurableExecApproval: vi.fn(() => false), requiresExecApproval: requiresExecApprovalMock, resolveExecApprovalAllowedDecisions: vi.fn(() => ["allow-once", "allow-always", "deny"]), @@ -225,6 +232,9 @@ describe("executeNodeHostCommand", () => { callGatewayToolMock.mockReset(); callGatewayToolMock.mockImplementation( async (method: string, _options: unknown, params: MockNodeInvokeParams | undefined) => { + if (method === "exec.approval.resolve") { + return { payload: {} }; + } if (method !== "node.invoke") { throw new Error(`unexpected gateway method: ${method}`); } @@ -255,6 +265,16 @@ describe("executeNodeHostCommand", () => { ]); parsePreparedSystemRunPayloadMock.mockReset(); parsePreparedSystemRunPayloadMock.mockReturnValue({ plan: preparedPlan }); + commandRequiresSecurityAuditSuppressionApprovalMock.mockReset(); + commandRequiresSecurityAuditSuppressionApprovalMock.mockReturnValue(false); + evaluateShellAllowlistMock.mockReset(); + evaluateShellAllowlistMock.mockReturnValue({ + allowlistMatches: [], + analysisOk: true, + allowlistSatisfied: false, + segments: [{ resolution: null, argv: ["bun", "./script.ts"] }], + segmentAllowlistEntries: [], + }); requiresExecApprovalMock.mockReset(); requiresExecApprovalMock.mockReturnValue(true); resolveExecHostApprovalContextMock.mockReset(); @@ -352,6 +372,247 @@ describe("executeNodeHostCommand", () => { expect(runParams.turnSourceThreadId).toBe("42"); }); + it("does not build a human approval prompt for node auto-review allows", async () => { + const autoReviewer = vi.fn(async () => ({ + decision: "allow-once", + risk: "low", + rationale: "safe read", + })); + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "allowlist", + hostAsk: "on-miss", + askFallback: "deny", + }); + + const result = await executeNodeHostCommand({ + command: "bun ./script.ts", + workdir: "/tmp/work", + env: {}, + security: "allowlist", + ask: "on-miss", + autoReview: true, + autoReviewer, + defaultTimeoutSec: 30, + approvalRunningNoticeMs: 0, + warnings: [], + agentId: "requested-agent", + sessionKey: "requested-session", + }); + + expect(result.details?.status).toBe("completed"); + expect(autoReviewer).toHaveBeenCalledWith( + expect.objectContaining({ + command: "bun ./script.ts", + argv: ["bun", "./script.ts"], + host: "node", + reason: "allowlist-miss", + }), + ); + expect(createAndRegisterDefaultExecApprovalRequestMock).not.toHaveBeenCalled(); + expect(registerExecApprovalRequestForHostOrThrowMock).toHaveBeenCalledWith( + expect.objectContaining({ + host: "node", + requireDeliveryRoute: false, + suppressDelivery: true, + }), + ); + expect(callGatewayToolMock).toHaveBeenCalledWith( + "exec.approval.resolve", + { timeoutMs: 15_000 }, + { id: expect.any(String), decision: "allow-once" }, + { scopes: ["operator.approvals"] }, + ); + }); + + it("requests human approval when node auto-review asks on an approval miss", async () => { + const autoReviewer = vi.fn(async () => ({ + decision: "ask", + risk: "medium", + rationale: "needs a person", + })); + const warnings: string[] = []; + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "allowlist", + hostAsk: "on-miss", + askFallback: "deny", + }); + + const result = await executeNodeHostCommand({ + command: "bun ./script.ts", + workdir: "/tmp/work", + env: {}, + security: "allowlist", + ask: "on-miss", + autoReview: true, + autoReviewer, + defaultTimeoutSec: 30, + approvalRunningNoticeMs: 0, + warnings, + agentId: "requested-agent", + sessionKey: "requested-session", + }); + + expect(result.details?.status).toBe("approval-pending"); + expect(autoReviewer).toHaveBeenCalledTimes(1); + expect(createAndRegisterDefaultExecApprovalRequestMock).toHaveBeenCalledTimes(1); + expect(warnings.join("\n")).toContain("needs a person"); + }); + + it("auto-reviews strict inline-eval commands before asking a human", async () => { + const autoReviewer = vi.fn(async () => ({ + decision: "allow-once", + risk: "low", + rationale: "safe inline eval", + })); + detectInterpreterInlineEvalArgvMock.mockReturnValue(INLINE_EVAL_HIT); + evaluateShellAllowlistMock.mockReturnValue({ + allowlistMatches: [], + analysisOk: true, + allowlistSatisfied: false, + segments: [ + { + resolution: null, + argv: ["python3", "-c", "print(1)"], + raw: "python3 -c 'print(1)'", + }, + ], + segmentAllowlistEntries: [], + }); + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "allowlist", + hostAsk: "on-miss", + askFallback: "deny", + }); + const warnings: string[] = []; + + const result = await executeNodeHostCommand({ + command: "python3 -c 'print(1)'", + workdir: "/tmp/work", + env: {}, + security: "allowlist", + ask: "on-miss", + autoReview: true, + autoReviewer, + strictInlineEval: true, + defaultTimeoutSec: 30, + approvalRunningNoticeMs: 0, + warnings, + agentId: "requested-agent", + sessionKey: "requested-session", + }); + + expect(result.details?.status).toBe("completed"); + expect(autoReviewer).toHaveBeenCalledWith( + expect.objectContaining({ + command: "python3 -c 'print(1)'", + argv: ["python3", "-c", "print(1)"], + host: "node", + reason: "strict-inline-eval", + analysis: expect.objectContaining({ + inlineEval: true, + }), + }), + ); + expect(createAndRegisterDefaultExecApprovalRequestMock).not.toHaveBeenCalled(); + expect(warnings[0]).toContain("requires reviewer or explicit approval"); + }); + + it("keeps security audit suppression edits off the auto-review path", async () => { + const autoReviewer = vi.fn(async () => ({ + decision: "allow-once", + risk: "low", + rationale: "test reviewer would allow it", + })); + const warnings: string[] = []; + commandRequiresSecurityAuditSuppressionApprovalMock.mockReturnValue(true); + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "allowlist", + hostAsk: "on-miss", + askFallback: "deny", + }); + + const result = await executeNodeHostCommand({ + command: "openclaw config set security.audit.suppressions '[]'", + workdir: "/tmp/work", + env: {}, + security: "allowlist", + ask: "on-miss", + autoReview: true, + autoReviewer, + defaultTimeoutSec: 30, + approvalRunningNoticeMs: 0, + warnings, + agentId: "requested-agent", + sessionKey: "requested-session", + }); + + expect(result.details?.status).toBe("approval-pending"); + expect(autoReviewer).not.toHaveBeenCalled(); + expect(createAndRegisterDefaultExecApprovalRequestMock).toHaveBeenCalledTimes(1); + expect(warnings).toContain( + "Warning: security audit suppression changes require explicit approval unless exec is running in yolo mode.", + ); + }); + + it("does not use fallback-full when node auto-review asks for human approval", async () => { + const autoReviewer = vi.fn(async () => ({ + decision: "ask", + risk: "medium", + rationale: "needs a person", + })); + resolveExecHostApprovalContextMock.mockReturnValue({ + approvals: { allowlist: [], file: { version: 1, agents: {} } }, + hostSecurity: "allowlist", + hostAsk: "on-miss", + askFallback: "full", + }); + resolveApprovalDecisionOrUndefinedMock.mockResolvedValue(null); + createExecApprovalDecisionStateMock.mockReturnValue({ + baseDecision: { timedOut: true }, + approvedByAsk: true, + deniedReason: null, + }); + enforceStrictInlineEvalApprovalBoundaryMock.mockImplementation((value) => + value.requiresAutoReviewHumanApproval === true && value.baseDecision.timedOut + ? { approvedByAsk: false, deniedReason: "approval-timeout" } + : { approvedByAsk: value.approvedByAsk, deniedReason: value.deniedReason }, + ); + + const result = await executeNodeHostCommand({ + command: "bun ./script.ts", + workdir: "/tmp/work", + env: {}, + security: "allowlist", + ask: "on-miss", + autoReview: true, + autoReviewer, + defaultTimeoutSec: 30, + approvalRunningNoticeMs: 0, + warnings: [], + agentId: "requested-agent", + sessionKey: "requested-session", + }); + + expect(result.details?.status).toBe("approval-pending"); + await vi.waitFor(() => { + expect(sendExecApprovalFollowupResultMock).toHaveBeenCalledWith( + { approvalId: "approval-1" }, + "Exec denied (node=node-1 id=approval-1, approval-timeout): bun ./script.ts", + ); + }); + expect( + callGatewayToolMock.mock.calls.some( + ([method, , params]) => + method === "node.invoke" && + (params as MockNodeInvokeParams | undefined)?.command === "system.run", + ), + ).toBe(false); + }); + it("builds a local systemRunPlan when approval is required and the node omits prepare", async () => { listNodesMock.mockResolvedValueOnce([ { diff --git a/src/agents/bash-tools.exec-host-node.ts b/src/agents/bash-tools.exec-host-node.ts index 798c3175002..d1bd90922cd 100644 --- a/src/agents/bash-tools.exec-host-node.ts +++ b/src/agents/bash-tools.exec-host-node.ts @@ -1,8 +1,12 @@ +import { randomUUID } from "node:crypto"; import { APPROVALS_SCOPE, WRITE_SCOPE } from "../gateway/operator-scopes.js"; +import type { InterpreterInlineEvalHit } from "../infra/command-analysis/inline-eval.js"; import { + type ExecSecurity, requiresExecApproval, resolveExecApprovalAllowedDecisions, } from "../infra/exec-approvals.js"; +import { defaultExecAutoReviewer, type ExecAutoReviewInput } from "../infra/exec-auto-review.js"; import { buildExecApprovalRequesterContext, buildExecApprovalTurnSourceContext, @@ -32,6 +36,26 @@ export type { ExecuteNodeHostCommandParams } from "./bash-tools.exec-host-node.t const APPROVED_NODE_INVOKE_SCOPES = [WRITE_SCOPE, APPROVALS_SCOPE]; +function resolveNodeAutoReviewReason(params: { + inlineEvalHit: InterpreterInlineEvalHit | null; + hostSecurity: ExecSecurity; + analysisOk: boolean; + allowlistSatisfied: boolean; + durableApprovalSatisfied: boolean; +}): ExecAutoReviewInput["reason"] { + if (params.inlineEvalHit !== null) { + return "strict-inline-eval"; + } + if ( + params.hostSecurity === "allowlist" && + (!params.analysisOk || !params.allowlistSatisfied) && + !params.durableApprovalSatisfied + ) { + return "allowlist-miss"; + } + return "approval-required"; +} + export async function executeNodeHostCommand( params: ExecuteNodeHostCommandParams, ): Promise> { @@ -60,8 +84,14 @@ export async function executeNodeHostCommand( hostSecurity, hostAsk, }); - const { analysisOk, allowlistSatisfied, durableApprovalSatisfied, inlineEvalHit } = - approvalAnalysis; + const { + analysisOk, + allowlistSatisfied, + durableApprovalSatisfied, + inlineEvalHit, + requiresSecurityAuditSuppressionApproval, + autoReviewArgv, + } = approvalAnalysis; const requiresAsk = requiresExecApproval({ ask: hostAsk, @@ -69,214 +99,291 @@ export async function executeNodeHostCommand( analysisOk, allowlistSatisfied, durableApprovalSatisfied, - }) || inlineEvalHit !== null; + }) || + inlineEvalHit !== null || + requiresSecurityAuditSuppressionApproval; + if (requiresSecurityAuditSuppressionApproval) { + params.warnings.push( + "Warning: security audit suppression changes require explicit approval unless exec is running in yolo mode.", + ); + } + const registerNodeApproval = async ( + approvalId: string, + options: { requireDeliveryRoute?: boolean; suppressDelivery?: boolean } = {}, + ) => + await registerExecApprovalRequestForHostOrThrow({ + approvalId, + systemRunPlan: prepared.plan, + env: target.env, + workdir: prepared.cwd, + host: "node", + nodeId: target.nodeId, + security: hostSecurity, + ask: hostAsk, + commandHighlighting: params.commandHighlighting, + ...buildExecApprovalRequesterContext({ + agentId: prepared.agentId, + sessionKey: prepared.sessionKey, + }), + ...(options.requireDeliveryRoute !== undefined + ? { requireDeliveryRoute: options.requireDeliveryRoute } + : {}), + ...(options.suppressDelivery !== undefined + ? { suppressDelivery: options.suppressDelivery } + : {}), + ...buildExecApprovalTurnSourceContext(params), + }); let inlineApprovedByAsk = false; let inlineApprovalDecision: "allow-once" | "allow-always" | null = null; let inlineApprovalId: string | undefined; if (requiresAsk) { - const requestArgs = execHostShared.buildDefaultExecApprovalRequestArgs({ - warnings: params.warnings, - approvalRunningNoticeMs: params.approvalRunningNoticeMs, - createApprovalSlug, - turnSourceChannel: params.turnSourceChannel, - turnSourceAccountId: params.turnSourceAccountId, - }); - const registerNodeApproval = async (approvalId: string) => - await registerExecApprovalRequestForHostOrThrow({ - approvalId, - systemRunPlan: prepared.plan, - env: target.env, - workdir: prepared.cwd, - host: "node", - nodeId: target.nodeId, - security: hostSecurity, - ask: hostAsk, - commandHighlighting: params.commandHighlighting, - ...buildExecApprovalRequesterContext({ - agentId: prepared.agentId, - sessionKey: prepared.sessionKey, - }), - ...buildExecApprovalTurnSourceContext(params), - }); - const { - approvalId, - approvalSlug, - warningText, - expiresAtMs, - preResolvedDecision, - initiatingSurface, - sentApproverDms, - unavailableReason, - } = await execHostShared.createAndRegisterDefaultExecApprovalRequest({ - ...requestArgs, - register: registerNodeApproval, - }); + let autoReviewRequiresHumanApproval = false; if ( - execHostShared.shouldResolveExecApprovalUnavailableInline({ - trigger: params.trigger, - unavailableReason, - preResolvedDecision, - }) + params.autoReview === true && + hostAsk !== "always" && + !requiresSecurityAuditSuppressionApproval ) { - const { baseDecision, approvedByAsk, deniedReason } = - execHostShared.createExecApprovalDecisionState({ - decision: preResolvedDecision, - askFallback, - }); - const strictInlineEvalDecision = execHostShared.enforceStrictInlineEvalApprovalBoundary({ - baseDecision, - approvedByAsk, - deniedReason, - requiresInlineEvalApproval: inlineEvalHit !== null, + const reviewer = params.autoReviewer ?? defaultExecAutoReviewer; + const decision = await reviewer({ + command: params.command, + argv: autoReviewArgv, + cwd: prepared.cwd, + envKeys: Object.keys(params.requestedEnv ?? {}).toSorted(), + host: "node", + reason: resolveNodeAutoReviewReason({ + inlineEvalHit, + hostSecurity, + analysisOk, + allowlistSatisfied, + durableApprovalSatisfied, + }), + analysis: { + parsed: analysisOk, + allowlistMatched: allowlistSatisfied, + durableApprovalMatched: durableApprovalSatisfied, + inlineEval: inlineEvalHit !== null, + }, + agent: { + id: params.agentId, + sessionKey: params.sessionKey, + }, }); - if (strictInlineEvalDecision.deniedReason || !strictInlineEvalDecision.approvedByAsk) { - throw new Error( - execHostShared.buildHeadlessExecApprovalDeniedMessage({ - trigger: params.trigger, - host: "node", - security: hostSecurity, - ask: hostAsk, - askFallback, - }), + if (decision.decision === "allow-once") { + const approvalId = randomUUID(); + await registerNodeApproval(approvalId, { + requireDeliveryRoute: false, + suppressDelivery: true, + }); + await callGatewayTool( + "exec.approval.resolve", + { timeoutMs: 15_000 }, + { id: approvalId, decision: "allow-once" }, + { scopes: [APPROVALS_SCOPE] }, + ); + inlineApprovedByAsk = true; + inlineApprovalDecision = "allow-once"; + inlineApprovalId = approvalId; + } + if (decision.decision !== "allow-once") { + autoReviewRequiresHumanApproval = true; + params.warnings.push( + `Exec auto-review deferred to human approval (risk=${decision.risk}): ${decision.rationale}`, ); } - inlineApprovedByAsk = strictInlineEvalDecision.approvedByAsk; - inlineApprovalDecision = strictInlineEvalDecision.approvedByAsk ? "allow-once" : null; - inlineApprovalId = approvalId; - } else { - const followupTarget = execHostShared.buildExecApprovalFollowupTarget({ - approvalId, - sessionKey: params.notifySessionKey ?? params.sessionKey, - bashElevated: params.bashElevated, + } + + if (!inlineApprovedByAsk) { + const requestArgs = execHostShared.buildDefaultExecApprovalRequestArgs({ + warnings: params.warnings, + approvalRunningNoticeMs: params.approvalRunningNoticeMs, + createApprovalSlug, turnSourceChannel: params.turnSourceChannel, - turnSourceTo: params.turnSourceTo, turnSourceAccountId: params.turnSourceAccountId, - turnSourceThreadId: params.turnSourceThreadId, }); - - void (async () => { - const decision = await execHostShared.resolveApprovalDecisionOrUndefined({ - approvalId, + const { + approvalId, + approvalSlug, + warningText, + expiresAtMs, + preResolvedDecision, + initiatingSurface, + sentApproverDms, + unavailableReason, + } = await execHostShared.createAndRegisterDefaultExecApprovalRequest({ + ...requestArgs, + register: registerNodeApproval, + }); + if ( + execHostShared.shouldResolveExecApprovalUnavailableInline({ + trigger: params.trigger, + unavailableReason, preResolvedDecision, - onFailure: () => - void execHostShared.sendExecApprovalFollowupResult( - followupTarget, - `Exec denied (node=${target.nodeId} id=${approvalId}, approval-request-failed): ${params.command}`, - ), - }); - if (decision === undefined) { - return; - } - - const { - baseDecision, - approvedByAsk: initialApprovedByAsk, - deniedReason: initialDeniedReason, - } = execHostShared.createExecApprovalDecisionState({ - decision, - askFallback, - }); - let approvedByAsk = initialApprovedByAsk; - let approvalDecision: "allow-once" | "allow-always" | null = null; - let deniedReason = initialDeniedReason; - - if (baseDecision.timedOut && askFallback === "full" && approvedByAsk) { - approvalDecision = "allow-once"; - } else if (decision === "allow-once") { - approvedByAsk = true; - approvalDecision = "allow-once"; - } else if (decision === "allow-always") { - approvedByAsk = true; - approvalDecision = "allow-always"; - } - - ({ approvedByAsk, deniedReason } = execHostShared.enforceStrictInlineEvalApprovalBoundary({ + }) + ) { + const { baseDecision, approvedByAsk, deniedReason } = + execHostShared.createExecApprovalDecisionState({ + decision: preResolvedDecision, + askFallback, + }); + const strictInlineEvalDecision = execHostShared.enforceStrictInlineEvalApprovalBoundary({ baseDecision, approvedByAsk, deniedReason, requiresInlineEvalApproval: inlineEvalHit !== null, - })); - if (deniedReason) { - approvalDecision = null; - } - - if (deniedReason) { - await execHostShared.sendExecApprovalFollowupResult( - followupTarget, - `Exec denied (node=${target.nodeId} id=${approvalId}, ${deniedReason}): ${params.command}`, - ); - return; - } - - try { - const raw = await callGatewayTool( - "node.invoke", - { timeoutMs: target.invokeTimeoutMs }, - buildNodeSystemRunInvoke({ - target, - command: prepared.argv, - rawCommand: prepared.rawCommand, - cwd: prepared.cwd, - agentId: prepared.agentId, - sessionKey: prepared.sessionKey, - turnSourceChannel: params.turnSourceChannel, - turnSourceTo: params.turnSourceTo, - turnSourceAccountId: params.turnSourceAccountId, - turnSourceThreadId: params.turnSourceThreadId, - approved: approvedByAsk, - approvalDecision: - approvalDecision === "allow-always" && inlineEvalHit !== null - ? "allow-once" - : approvalDecision, - runId: approvalId, - suppressNotifyOnExit: true, - notifyOnExit: params.notifyOnExit, - systemRunPlan: prepared.plan, + requiresAutoReviewHumanApproval: autoReviewRequiresHumanApproval, + }); + if (strictInlineEvalDecision.deniedReason || !strictInlineEvalDecision.approvedByAsk) { + throw new Error( + execHostShared.buildHeadlessExecApprovalDeniedMessage({ + trigger: params.trigger, + host: "node", + security: hostSecurity, + ask: hostAsk, + askFallback, }), - { scopes: APPROVED_NODE_INVOKE_SCOPES }, - ); - const payload = - raw?.payload && typeof raw.payload === "object" - ? (raw.payload as { - stdout?: string; - stderr?: string; - error?: string | null; - exitCode?: number | null; - timedOut?: boolean; - }) - : {}; - const combined = [payload.stdout, payload.stderr, payload.error] - .filter(Boolean) - .join("\n"); - const output = normalizeNotifyOutput(combined.slice(-DEFAULT_NOTIFY_TAIL_CHARS)); - const exitLabel = payload.timedOut ? "timeout" : `code ${payload.exitCode ?? "?"}`; - const summary = output - ? `Exec finished (node=${target.nodeId} id=${approvalId}, ${exitLabel})\n${output}` - : `Exec finished (node=${target.nodeId} id=${approvalId}, ${exitLabel})`; - await execHostShared.sendExecApprovalFollowupResult(followupTarget, summary); - } catch { - await execHostShared.sendExecApprovalFollowupResult( - followupTarget, - `Exec denied (node=${target.nodeId} id=${approvalId}, invoke-failed): ${params.command}`, ); } - })(); + inlineApprovedByAsk = strictInlineEvalDecision.approvedByAsk; + inlineApprovalDecision = strictInlineEvalDecision.approvedByAsk ? "allow-once" : null; + inlineApprovalId = approvalId; + } else { + const followupTarget = execHostShared.buildExecApprovalFollowupTarget({ + approvalId, + sessionKey: params.notifySessionKey ?? params.sessionKey, + bashElevated: params.bashElevated, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + }); - return execHostShared.buildExecApprovalPendingToolResult({ - host: "node", - command: params.command, - cwd: params.workdir, - warningText, - approvalId, - approvalSlug, - expiresAtMs, - initiatingSurface, - sentApproverDms, - unavailableReason, - allowedDecisions: resolveExecApprovalAllowedDecisions({ ask: hostAsk }), - nodeId: target.nodeId, - }); + void (async () => { + const decision = await execHostShared.resolveApprovalDecisionOrUndefined({ + approvalId, + preResolvedDecision, + onFailure: () => + void execHostShared.sendExecApprovalFollowupResult( + followupTarget, + `Exec denied (node=${target.nodeId} id=${approvalId}, approval-request-failed): ${params.command}`, + ), + }); + if (decision === undefined) { + return; + } + + const { + baseDecision, + approvedByAsk: initialApprovedByAsk, + deniedReason: initialDeniedReason, + } = execHostShared.createExecApprovalDecisionState({ + decision, + askFallback, + }); + let approvedByAsk = initialApprovedByAsk; + let approvalDecision: "allow-once" | "allow-always" | null = null; + let deniedReason = initialDeniedReason; + + if (baseDecision.timedOut && askFallback === "full" && approvedByAsk) { + approvalDecision = "allow-once"; + } else if (decision === "allow-once") { + approvedByAsk = true; + approvalDecision = "allow-once"; + } else if (decision === "allow-always") { + approvedByAsk = true; + approvalDecision = "allow-always"; + } + + ({ approvedByAsk, deniedReason } = execHostShared.enforceStrictInlineEvalApprovalBoundary( + { + baseDecision, + approvedByAsk, + deniedReason, + requiresInlineEvalApproval: inlineEvalHit !== null, + requiresAutoReviewHumanApproval: autoReviewRequiresHumanApproval, + }, + )); + if (deniedReason) { + approvalDecision = null; + } + + if (deniedReason) { + await execHostShared.sendExecApprovalFollowupResult( + followupTarget, + `Exec denied (node=${target.nodeId} id=${approvalId}, ${deniedReason}): ${params.command}`, + ); + return; + } + + try { + const raw = await callGatewayTool( + "node.invoke", + { timeoutMs: target.invokeTimeoutMs }, + buildNodeSystemRunInvoke({ + target, + command: prepared.argv, + rawCommand: prepared.rawCommand, + cwd: prepared.cwd, + agentId: prepared.agentId, + sessionKey: prepared.sessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + approved: approvedByAsk, + approvalDecision: + approvalDecision === "allow-always" && inlineEvalHit !== null + ? "allow-once" + : approvalDecision, + runId: approvalId, + suppressNotifyOnExit: true, + notifyOnExit: params.notifyOnExit, + systemRunPlan: prepared.plan, + }), + { scopes: APPROVED_NODE_INVOKE_SCOPES }, + ); + const payload = + raw?.payload && typeof raw.payload === "object" + ? (raw.payload as { + stdout?: string; + stderr?: string; + error?: string | null; + exitCode?: number | null; + timedOut?: boolean; + }) + : {}; + const combined = [payload.stdout, payload.stderr, payload.error] + .filter(Boolean) + .join("\n"); + const output = normalizeNotifyOutput(combined.slice(-DEFAULT_NOTIFY_TAIL_CHARS)); + const exitLabel = payload.timedOut ? "timeout" : `code ${payload.exitCode ?? "?"}`; + const summary = output + ? `Exec finished (node=${target.nodeId} id=${approvalId}, ${exitLabel})\n${output}` + : `Exec finished (node=${target.nodeId} id=${approvalId}, ${exitLabel})`; + await execHostShared.sendExecApprovalFollowupResult(followupTarget, summary); + } catch { + await execHostShared.sendExecApprovalFollowupResult( + followupTarget, + `Exec denied (node=${target.nodeId} id=${approvalId}, invoke-failed): ${params.command}`, + ); + } + })(); + + return execHostShared.buildExecApprovalPendingToolResult({ + host: "node", + command: params.command, + cwd: params.workdir, + warningText, + approvalId, + approvalSlug, + expiresAtMs, + initiatingSurface, + sentApproverDms, + unavailableReason, + allowedDecisions: resolveExecApprovalAllowedDecisions({ ask: hostAsk }), + nodeId: target.nodeId, + }); + } } } diff --git a/src/agents/bash-tools.exec-host-node.types.ts b/src/agents/bash-tools.exec-host-node.types.ts index c44fd00167c..94a33e909fb 100644 --- a/src/agents/bash-tools.exec-host-node.types.ts +++ b/src/agents/bash-tools.exec-host-node.types.ts @@ -1,4 +1,5 @@ import type { ExecAsk, ExecSecurity } from "../infra/exec-approvals.js"; +import type { ExecAutoReviewer } from "../infra/exec-auto-review.js"; import type { ExecElevatedDefaults } from "./bash-tools.exec-types.js"; export type ExecuteNodeHostCommandParams = { @@ -18,6 +19,8 @@ export type ExecuteNodeHostCommandParams = { agentId?: string; security: ExecSecurity; ask: ExecAsk; + autoReview?: boolean; + autoReviewer?: ExecAutoReviewer; strictInlineEval?: boolean; commandHighlighting?: boolean; timeoutSec?: number; diff --git a/src/agents/bash-tools.exec-host-shared.test.ts b/src/agents/bash-tools.exec-host-shared.test.ts index 527825677a8..4e9d643063e 100644 --- a/src/agents/bash-tools.exec-host-shared.test.ts +++ b/src/agents/bash-tools.exec-host-shared.test.ts @@ -330,6 +330,23 @@ describe("enforceStrictInlineEvalApprovalBoundary", () => { }); }); + it("denies timeout-based fallback when auto-review defers to human approval", () => { + const params = { + baseDecision: { timedOut: true }, + approvedByAsk: true, + deniedReason: null, + requiresInlineEvalApproval: false, + requiresAutoReviewHumanApproval: true, + } satisfies Parameters[0] & { + requiresAutoReviewHumanApproval: true; + }; + + expect(enforceStrictInlineEvalApprovalBoundary(params)).toEqual({ + approvedByAsk: false, + deniedReason: "approval-timeout", + }); + }); + it("keeps explicit approvals intact for strict inline-eval commands", () => { expect( enforceStrictInlineEvalApprovalBoundary({ diff --git a/src/agents/bash-tools.exec-host-shared.ts b/src/agents/bash-tools.exec-host-shared.ts index 8633822f0a6..653734a518b 100644 --- a/src/agents/bash-tools.exec-host-shared.ts +++ b/src/agents/bash-tools.exec-host-shared.ts @@ -353,15 +353,14 @@ export function enforceStrictInlineEvalApprovalBoundary(params: { approvedByAsk: boolean; deniedReason: string | null; requiresInlineEvalApproval: boolean; + requiresAutoReviewHumanApproval?: boolean; }): { approvedByAsk: boolean; deniedReason: string | null; } { - if ( - !params.baseDecision.timedOut || - !params.requiresInlineEvalApproval || - !params.approvedByAsk - ) { + const requiresRealApproval = + params.requiresInlineEvalApproval || params.requiresAutoReviewHumanApproval === true; + if (!params.baseDecision.timedOut || !requiresRealApproval || !params.approvedByAsk) { return { approvedByAsk: params.approvedByAsk, deniedReason: params.deniedReason, diff --git a/src/agents/bash-tools.exec-types.ts b/src/agents/bash-tools.exec-types.ts index 2d1eca235c0..95a0367a446 100644 --- a/src/agents/bash-tools.exec-types.ts +++ b/src/agents/bash-tools.exec-types.ts @@ -1,13 +1,23 @@ +import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { EventSessionRoutingPolicy } from "../infra/event-session-routing.js"; import type { ExecApprovalDecision } from "../infra/exec-approvals.js"; -import type { ExecAsk, ExecHost, ExecSecurity, ExecTarget } from "../infra/exec-approvals.js"; +import type { + ExecAsk, + ExecHost, + ExecMode, + ExecSecurity, + ExecTarget, +} from "../infra/exec-approvals.js"; +import type { ExecAutoReviewer } from "../infra/exec-auto-review.js"; import type { SafeBinProfileFixture } from "../infra/exec-safe-bin-policy.js"; import type { BashSandboxConfig } from "./bash-tools.shared.js"; import type { EmbeddedFullAccessBlockedReason } from "./embedded-agent-runner/types.js"; +import type { ExecReviewerConfig } from "./exec-auto-reviewer.js"; export type ExecToolDefaults = { hasCronTool?: boolean; host?: ExecTarget; + mode?: ExecMode; security?: ExecSecurity; ask?: ExecAsk; trigger?: string; @@ -18,6 +28,9 @@ export type ExecToolDefaults = { commandHighlighting?: boolean; safeBinTrustedDirs?: string[]; safeBinProfiles?: Record; + reviewer?: ExecReviewerConfig; + config?: OpenClawConfig; + autoReviewer?: ExecAutoReviewer; agentId?: string; backgroundMs?: number; timeoutSec?: number; diff --git a/src/agents/bash-tools.exec.security-floor.test.ts b/src/agents/bash-tools.exec.security-floor.test.ts index 57bdeebb4fe..e5fde3c5abd 100644 --- a/src/agents/bash-tools.exec.security-floor.test.ts +++ b/src/agents/bash-tools.exec.security-floor.test.ts @@ -1,10 +1,17 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { ExecAutoReviewer } from "../infra/exec-auto-review.js"; import { captureEnv } from "../test-utils/env.js"; import { resetProcessRegistryForTests } from "./bash-process-registry.js"; import { createExecTool } from "./bash-tools.exec.js"; +import { callGatewayTool } from "./tools/gateway.js"; + +vi.mock("./tools/gateway.js", () => ({ + callGatewayTool: vi.fn(), + readGatewayCallOptions: vi.fn(() => ({})), +})); describe("exec security floor", () => { let envSnapshot: ReturnType; @@ -34,6 +41,7 @@ describe("exec security floor", () => { delete process.env.HOMEPATH; } resetProcessRegistryForTests(); + vi.mocked(callGatewayTool).mockReset(); }); afterEach(() => { @@ -110,4 +118,260 @@ describe("exec security floor", () => { }), ).rejects.toThrow(/exec denied/i); }); + + it("does not let host approval defaults deny implicit sandbox execution", async () => { + const openclawDir = path.join(tempRoot ?? os.tmpdir(), ".openclaw"); + fs.mkdirSync(openclawDir, { recursive: true }); + fs.writeFileSync( + path.join(openclawDir, "exec-approvals.json"), + `${JSON.stringify({ version: 1, defaults: { security: "deny", ask: "off" }, agents: {} })}\n`, + ); + const buildExecSpec = vi.fn(async () => ({ + argv: ["/bin/sh", "-lc", "printf sandbox-ok"], + env: process.env, + stdinMode: "pipe-closed" as const, + })); + const tool = createExecTool({ + host: "auto", + sandbox: { + containerName: "sandbox-host-approval-defaults-test", + workspaceDir: tempRoot ?? "/tmp", + containerWorkdir: "/workspace", + buildExecSpec, + }, + }); + + const result = await tool.execute("call-sandbox-host-defaults", { + command: "echo sandbox-ok", + }); + + expect(buildExecSpec).toHaveBeenCalledTimes(1); + expect(result.content[0]?.type).toBe("text"); + const text = (result.content[0] as { text?: string }).text ?? ""; + expect(text).toContain("sandbox-ok"); + }); + + it("honors configured deny mode before implicit sandbox execution", async () => { + const buildExecSpec = vi.fn(async () => ({ + argv: ["/bin/sh", "-lc", "printf leaked"], + env: process.env, + stdinMode: "pipe-closed" as const, + })); + const tool = createExecTool({ + host: "auto", + mode: "deny", + sandbox: { + containerName: "sandbox-deny-test", + workspaceDir: tempRoot ?? "/tmp", + containerWorkdir: "/workspace", + buildExecSpec, + }, + }); + + await expect( + tool.execute("call-mode-deny-sandbox", { + command: "echo blocked", + }), + ).rejects.toThrow(/security=deny|exec denied/i); + expect(buildExecSpec).not.toHaveBeenCalled(); + }); + + it("lets normalized auto mode run implicit sandbox execution", async () => { + const buildExecSpec = vi.fn(async () => ({ + argv: ["/bin/sh", "-lc", "printf sandbox-auto-ok"], + env: process.env, + stdinMode: "pipe-closed" as const, + })); + const tool = createExecTool({ + host: "auto", + mode: "auto", + sandbox: { + containerName: "sandbox-auto-mode-test", + workspaceDir: tempRoot ?? "/tmp", + containerWorkdir: "/workspace", + buildExecSpec, + }, + }); + + const result = await tool.execute("call-mode-auto-sandbox", { + command: "echo sandbox-auto-ok", + }); + + expect(buildExecSpec).toHaveBeenCalledTimes(1); + expect(result.content[0]?.type).toBe("text"); + const text = (result.content[0] as { text?: string }).text ?? ""; + expect(text).toContain("sandbox-auto-ok"); + }); + + it("intersects normalized gateway auto mode with host approval deny defaults", async () => { + const openclawDir = path.join(tempRoot ?? os.tmpdir(), ".openclaw"); + fs.mkdirSync(openclawDir, { recursive: true }); + fs.writeFileSync( + path.join(openclawDir, "exec-approvals.json"), + `${JSON.stringify({ version: 1, defaults: { security: "deny", ask: "off" }, agents: {} })}\n`, + ); + const autoReviewer = vi.fn(async () => ({ + decision: "allow-once", + risk: "low", + rationale: "would otherwise run", + })); + const tool = createExecTool({ + host: "gateway", + mode: "auto", + safeBins: [], + autoReviewer, + }); + + await expect( + tool.execute("call-auto-mode-host-deny", { + command: "echo blocked", + }), + ).rejects.toThrow(/security=deny|exec denied/i); + expect(autoReviewer).not.toHaveBeenCalled(); + }); + + it("uses agent-scoped host policy when clamping normalized modes", async () => { + const openclawDir = path.join(tempRoot ?? os.tmpdir(), ".openclaw"); + fs.mkdirSync(openclawDir, { recursive: true }); + fs.writeFileSync( + path.join(openclawDir, "exec-approvals.json"), + `${JSON.stringify({ + version: 1, + defaults: { security: "deny", ask: "off" }, + agents: { main: { security: "full", ask: "off" } }, + })}\n`, + ); + const tool = createExecTool({ + host: "gateway", + mode: "full", + agentId: "main", + }); + + const result = await tool.execute("call-agent-host-policy", { + command: "echo agent-ok", + }); + + expect(result.content[0]?.type).toBe("text"); + const text = (result.content[0] as { text?: string }).text ?? ""; + expect(text.trim()).toContain("agent-ok"); + }); + + it("preserves host ask floors for elevated full gateway exec", async () => { + const openclawDir = path.join(tempRoot ?? os.tmpdir(), ".openclaw"); + fs.mkdirSync(openclawDir, { recursive: true }); + fs.writeFileSync( + path.join(openclawDir, "exec-approvals.json"), + `${JSON.stringify({ version: 1, defaults: { security: "full", ask: "always" }, agents: {} })}\n`, + ); + const calls: string[] = []; + vi.mocked(callGatewayTool).mockImplementation(async (method) => { + calls.push(method); + if (method === "exec.approval.request") { + return { status: "accepted", id: "approval-id" }; + } + if (method === "exec.approval.waitDecision") { + return { decision: null }; + } + return { ok: true }; + }); + const tool = createExecTool({ + host: "gateway", + security: "full", + ask: "off", + approvalRunningNoticeMs: 0, + elevated: { enabled: true, allowed: true, defaultLevel: "full" }, + }); + + const result = await tool.execute("call-elevated-full-host-ask-floor", { + command: "echo ok", + elevated: true, + }); + + expect(result.details.status).toBe("approval-pending"); + expect(calls).toContain("exec.approval.request"); + }); + + it("honors normalized auto mode before elevated full bypass", async () => { + const calls: string[] = []; + vi.mocked(callGatewayTool).mockImplementation(async (method) => { + calls.push(method); + if (method === "exec.approval.request") { + return { status: "accepted", id: "approval-id" }; + } + if (method === "exec.approval.waitDecision") { + return { decision: null }; + } + return { ok: true }; + }); + const autoReviewer = vi.fn(async () => ({ + decision: "ask", + risk: "high", + rationale: "test reviewer asks for approval", + })); + const tool = createExecTool({ + host: "gateway", + mode: "auto", + safeBins: [], + autoReviewer, + elevated: { enabled: true, allowed: true, defaultLevel: "full" }, + }); + + const result = await tool.execute("call-elevated-full-auto-mode", { + command: "pwd", + elevated: true, + }); + + expect(autoReviewer).toHaveBeenCalledWith( + expect.objectContaining({ + command: "pwd", + host: "gateway", + reason: "approval-required", + }), + ); + expect(result.details.status).toBe("approval-pending"); + expect(calls).toContain("exec.approval.request"); + }); + + it.each(["on-miss", "off"] as const)( + "keeps auto review enabled when legacy ask=%s does not strengthen auto mode", + async (ask) => { + const calls: string[] = []; + vi.mocked(callGatewayTool).mockImplementation(async (method) => { + calls.push(method); + if (method === "exec.approval.request") { + return { status: "accepted", id: "approval-id" }; + } + if (method === "exec.approval.waitDecision") { + return { decision: null }; + } + return { ok: true }; + }); + const autoReviewer = vi.fn(async () => ({ + decision: "ask", + risk: "high", + rationale: "test reviewer asks for approval", + })); + const tool = createExecTool({ + host: "gateway", + mode: "auto", + safeBins: [], + autoReviewer, + }); + + const result = await tool.execute(`call-auto-review-${ask}`, { + command: "pwd", + ask, + }); + + expect(autoReviewer).toHaveBeenCalledWith( + expect.objectContaining({ + command: "pwd", + host: "gateway", + reason: "approval-required", + }), + ); + expect(result.details.status).toBe("approval-pending"); + expect(calls).toContain("exec.approval.request"); + }, + ); }); diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index ee828a87a20..5d9460f9633 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -9,7 +9,10 @@ import { type ExecSecurity, loadExecApprovals, maxAsk, + minSecurity, requireValidExecTarget, + resolveExecApprovalsFromFile, + resolveExecModePolicy, } from "../infra/exec-approvals.js"; import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js"; import { sanitizeHostExecEnvWithDiagnostics } from "../infra/host-env-security.js"; @@ -18,7 +21,11 @@ import { resolveShellEnvFallbackTimeoutMs, } from "../infra/shell-env.js"; import { logInfo } from "../logger.js"; -import { parseAgentSessionKey, resolveAgentIdFromSessionKey } from "../routing/session-key.js"; +import { + normalizeAgentId, + parseAgentSessionKey, + resolveAgentIdFromSessionKey, +} from "../routing/session-key.js"; import { createLazyImportLoader } from "../shared/lazy-promise.js"; import { normalizeLowercaseStringOrEmpty, @@ -57,6 +64,7 @@ import { resolveWorkdir, truncateMiddle, } from "./bash-tools.shared.js"; +import { createModelExecAutoReviewer } from "./exec-auto-reviewer.js"; import type { AgentToolResult } from "./runtime/index.js"; import { EXEC_TOOL_DISPLAY_SUMMARY } from "./tool-description-presets.js"; import { type AgentToolWithMeta, failedTextResult, textResult } from "./tools/common.js"; @@ -1215,6 +1223,18 @@ function rejectUnsafeControlShellCommand(command: string): void { } } +function resolveExecReviewerDefaults(params: { defaults?: ExecToolDefaults; agentId?: string }) { + if (params.defaults?.reviewer) { + return params.defaults.reviewer; + } + const cfg = params.defaults?.config; + const agentId = params.agentId ? normalizeAgentId(params.agentId) : undefined; + const agentExec = agentId + ? cfg?.agents?.list?.find((entry) => normalizeAgentId(entry.id) === agentId)?.tools?.exec + : undefined; + return agentExec?.reviewer ?? cfg?.tools?.exec?.reviewer; +} + export function createExecTool( defaults?: ExecToolDefaults, ): AgentToolWithMeta { @@ -1271,6 +1291,13 @@ export function createExecTool( const agentId = defaults?.agentId ?? (parsedAgentSession ? resolveAgentIdFromSessionKey(defaults?.sessionKey) : undefined); + const autoReviewer = + defaults?.autoReviewer ?? + createModelExecAutoReviewer({ + cfg: defaults?.config, + agentId, + reviewer: resolveExecReviewerDefaults({ defaults, agentId }), + }); return { name: "exec", @@ -1391,21 +1418,58 @@ export function createExecTool( }); const host: ExecHost = target.effectiveHost; - const approvalDefaults = loadExecApprovals().defaults; - const configuredSecurity = - defaults?.security ?? approvalDefaults?.security ?? (host === "sandbox" ? "deny" : "full"); - let security = configuredSecurity; - if (elevatedRequested && elevatedMode === "full") { + const explicitSecurity = defaults?.security; + const configuredSecurity = explicitSecurity ?? (host === "sandbox" ? "deny" : "full"); + const modePolicy = resolveExecModePolicy({ + mode: defaults?.mode, + security: configuredSecurity, + ask: defaults?.ask ?? "off", + }); + const approvalPolicy = + host === "sandbox" + ? undefined + : resolveExecApprovalsFromFile({ + file: loadExecApprovals(), + agentId, + overrides: { + security: "full", + ask: "off", + }, + }).agent; + let security = minSecurity( + modePolicy.security, + approvalPolicy?.security ?? modePolicy.security, + ); + if ( + security === "deny" && + (host !== "sandbox" || defaults?.mode === "deny" || explicitSecurity === "deny") + ) { + throw new Error(`exec denied: host=${host} security=deny`); + } + const hostPolicyAllowsFullBypass = + (approvalPolicy?.security ?? "full") === "full" && (approvalPolicy?.ask ?? "off") === "off"; + const modePolicyAllowsFullBypass = modePolicy.security === "full" && modePolicy.ask === "off"; + if ( + elevatedRequested && + elevatedMode === "full" && + modePolicyAllowsFullBypass && + hostPolicyAllowsFullBypass + ) { security = "full"; } // Keep local exec defaults in sync with exec-approvals.json when tools.exec.* is unset. - const configuredAsk = defaults?.ask ?? approvalDefaults?.ask ?? "off"; const requestedAsk = normalizeExecAsk(params.ask); - let ask = maxAsk(configuredAsk, requestedAsk ?? configuredAsk); - const bypassApprovals = elevatedRequested && elevatedMode === "full"; + const hostAsk = maxAsk(modePolicy.ask, approvalPolicy?.ask ?? modePolicy.ask); + let ask = maxAsk(hostAsk, requestedAsk ?? hostAsk); + const bypassApprovals = + elevatedRequested && + elevatedMode === "full" && + modePolicyAllowsFullBypass && + hostPolicyAllowsFullBypass; if (bypassApprovals) { ask = "off"; } + const autoReview = modePolicy.autoReview && ask === modePolicy.ask && !bypassApprovals; const sandbox = host === "sandbox" ? defaults?.sandbox : undefined; if (target.selectedTarget === "sandbox" && !sandbox) { @@ -1535,6 +1599,8 @@ export function createExecTool( agentId, security, ask, + autoReview, + autoReviewer, strictInlineEval: defaults?.strictInlineEval, commandHighlighting: defaults?.commandHighlighting, trigger: defaults?.trigger, @@ -1564,6 +1630,8 @@ export function createExecTool( defaultTimeoutSec, security, ask, + autoReview, + autoReviewer, safeBins, safeBinProfiles, strictInlineEval: defaults?.strictInlineEval, diff --git a/src/agents/embedded-agent-runner.buildembeddedsandboxinfo.test.ts b/src/agents/embedded-agent-runner.buildembeddedsandboxinfo.test.ts index 9d615b6ea93..d776980d086 100644 --- a/src/agents/embedded-agent-runner.buildembeddedsandboxinfo.test.ts +++ b/src/agents/embedded-agent-runner.buildembeddedsandboxinfo.test.ts @@ -1,6 +1,10 @@ -import { describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as execApprovals from "../infra/exec-approvals.js"; import { buildEmbeddedSandboxInfo } from "./embedded-agent-runner.js"; -import { resolveEmbeddedFullAccessState } from "./embedded-agent-runner/sandbox-info.js"; +import { + resolveEmbeddedFullAccessState, + resolveEmbeddedSandboxInfoExecPolicy, +} from "./embedded-agent-runner/sandbox-info.js"; import type { SandboxContext } from "./sandbox.js"; function createSandboxContext(overrides?: Partial): SandboxContext { @@ -41,6 +45,14 @@ function createSandboxContext(overrides?: Partial): SandboxConte } describe("buildEmbeddedSandboxInfo", () => { + beforeEach(() => { + vi.restoreAllMocks(); + vi.spyOn(execApprovals, "loadExecApprovals").mockReturnValue({ + version: 1, + agents: {}, + }); + }); + it("returns undefined when sandbox is missing", () => { expect(buildEmbeddedSandboxInfo()).toBeUndefined(); }); @@ -113,6 +125,169 @@ describe("buildEmbeddedSandboxInfo", () => { }, }); }); + + it("marks full access unavailable when exec policy denies execution", () => { + const sandbox = createSandboxContext(); + + expect( + buildEmbeddedSandboxInfo( + sandbox, + { + enabled: true, + allowed: true, + defaultLevel: "full", + }, + { mode: "deny" }, + )?.elevated, + ).toEqual({ + allowed: true, + defaultLevel: "full", + fullAccessAvailable: false, + fullAccessBlockedReason: "host-policy", + }); + }); + + it("uses config exec mode when building prompt full-access state", () => { + const sandbox = createSandboxContext(); + const execPolicy = resolveEmbeddedSandboxInfoExecPolicy({ + config: { + tools: { + exec: { + mode: "auto", + }, + }, + }, + agentId: "main", + sandboxAvailable: true, + }); + + expect( + buildEmbeddedSandboxInfo( + sandbox, + { + enabled: true, + allowed: true, + defaultLevel: "full", + }, + execPolicy, + )?.elevated, + ).toEqual({ + allowed: true, + defaultLevel: "full", + fullAccessAvailable: false, + fullAccessBlockedReason: "host-policy", + }); + }); + + it("uses elevated host policy when sandbox is active and exec policy is unset", () => { + const sandbox = createSandboxContext(); + const execPolicy = resolveEmbeddedSandboxInfoExecPolicy({ + config: { + tools: { + exec: { + host: "auto", + }, + }, + }, + agentId: "main", + sandboxAvailable: true, + }); + + expect( + buildEmbeddedSandboxInfo( + sandbox, + { + enabled: true, + allowed: true, + defaultLevel: "full", + }, + execPolicy, + )?.elevated, + ).toEqual({ + allowed: true, + defaultLevel: "full", + fullAccessAvailable: true, + }); + }); + + it("marks full access unavailable when host approval defaults deny execution", () => { + const sandbox = createSandboxContext(); + + expect( + buildEmbeddedSandboxInfo( + sandbox, + { + enabled: true, + allowed: true, + defaultLevel: "full", + }, + { mode: "full", security: "full" }, + { security: "deny" }, + )?.elevated, + ).toEqual({ + allowed: true, + defaultLevel: "full", + fullAccessAvailable: false, + fullAccessBlockedReason: "host-policy", + }); + }); + + it("marks full access unavailable when host approval floors still require review", () => { + const sandbox = createSandboxContext(); + + expect( + buildEmbeddedSandboxInfo( + sandbox, + { + enabled: true, + allowed: true, + defaultLevel: "full", + }, + { mode: "full", security: "full", ask: "off" }, + { security: "allowlist", ask: "off" }, + )?.elevated, + ).toEqual({ + allowed: true, + defaultLevel: "full", + fullAccessAvailable: false, + fullAccessBlockedReason: "host-policy", + }); + + expect( + buildEmbeddedSandboxInfo( + sandbox, + { + enabled: true, + allowed: true, + defaultLevel: "full", + }, + { mode: "full", security: "full", ask: "off" }, + { security: "full", ask: "always" }, + )?.elevated, + ).toEqual({ + allowed: true, + defaultLevel: "full", + fullAccessAvailable: false, + fullAccessBlockedReason: "host-policy", + }); + + expect( + buildEmbeddedSandboxInfo( + sandbox, + { + enabled: true, + allowed: true, + defaultLevel: "full", + }, + { mode: "full", security: "full", ask: "on-miss" }, + { security: "full", ask: "on-miss" }, + )?.elevated, + ).toEqual({ + allowed: true, + defaultLevel: "full", + fullAccessAvailable: true, + }); + }); }); describe("resolveEmbeddedFullAccessState", () => { diff --git a/src/agents/embedded-agent-runner/compact.ts b/src/agents/embedded-agent-runner/compact.ts index 749f820e534..3e7b4681cdf 100644 --- a/src/agents/embedded-agent-runner/compact.ts +++ b/src/agents/embedded-agent-runner/compact.ts @@ -148,7 +148,7 @@ import { resolveModelAsync } from "./model.js"; import { sanitizeSessionHistory, validateReplayTurns } from "./replay-history.js"; import { createEmbeddedAgentResourceLoader } from "./resource-loader.js"; import { resolveAttemptSpawnWorkspaceDir } from "./run/attempt.thread-helpers.js"; -import { buildEmbeddedSandboxInfo } from "./sandbox-info.js"; +import { buildEmbeddedSandboxInfo, resolveEmbeddedSandboxInfoExecPolicy } from "./sandbox-info.js"; import { prewarmSessionFile, trackSessionManagerAccess } from "./session-manager-cache.js"; import { resolveEmbeddedRunSkillEntries } from "./skills-runtime.js"; import { @@ -741,6 +741,8 @@ async function compactEmbeddedAgentSessionDirectOnce( const runAbortController = new AbortController(); const toolsRaw = createOpenClawCodingTools({ exec: { + ...params.execOverrides, + config: params.config, elevated: params.bashElevated, }, sandbox, @@ -914,7 +916,18 @@ async function compactEmbeddedAgentSessionDirectOnce( }), }), }; - const sandboxInfo = buildEmbeddedSandboxInfo(sandbox, params.bashElevated); + const sandboxInfoExecPolicy = resolveEmbeddedSandboxInfoExecPolicy({ + config: params.config, + agentId: sessionAgentId, + sessionKey: params.sessionKey, + sandboxAvailable: sandbox?.enabled === true, + execOverrides: params.execOverrides, + }); + const sandboxInfo = buildEmbeddedSandboxInfo( + sandbox, + params.bashElevated, + sandboxInfoExecPolicy, + ); const reasoningTagHint = isReasoningTagProvider(provider, { config: params.config, workspaceDir: effectiveWorkspace, diff --git a/src/agents/embedded-agent-runner/compact.types.ts b/src/agents/embedded-agent-runner/compact.types.ts index 820e730b1a7..4b379f30334 100644 --- a/src/agents/embedded-agent-runner/compact.types.ts +++ b/src/agents/embedded-agent-runner/compact.types.ts @@ -3,7 +3,7 @@ import type { ReasoningLevel, ThinkLevel } from "../../auto-reply/thinking.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import type { ContextEngine, ContextEngineRuntimeContext } from "../../context-engine/types.js"; import type { CommandQueueEnqueueFn } from "../../process/command-queue.types.js"; -import type { ExecElevatedDefaults } from "../bash-tools.exec-types.js"; +import type { ExecElevatedDefaults, ExecToolDefaults } from "../bash-tools.exec-types.js"; import type { AgentRuntimePlan } from "../runtime-plan/types.js"; import type { SkillSnapshot } from "../skills.js"; @@ -59,6 +59,7 @@ export type CompactEmbeddedAgentSessionParams = { runtimePlan?: AgentRuntimePlan; thinkLevel?: ThinkLevel; reasoningLevel?: ReasoningLevel; + execOverrides?: Pick; bashElevated?: ExecElevatedDefaults; customInstructions?: string; tokenBudget?: number; diff --git a/src/agents/embedded-agent-runner/run/attempt.ts b/src/agents/embedded-agent-runner/run/attempt.ts index 6617bc1de9d..924304b3fd0 100644 --- a/src/agents/embedded-agent-runner/run/attempt.ts +++ b/src/agents/embedded-agent-runner/run/attempt.ts @@ -234,7 +234,7 @@ import { updateActiveEmbeddedRunSessionFile, updateActiveEmbeddedRunSnapshot, } from "../runs.js"; -import { buildEmbeddedSandboxInfo } from "../sandbox-info.js"; +import { buildEmbeddedSandboxInfo, resolveEmbeddedSandboxInfoExecPolicy } from "../sandbox-info.js"; import { prewarmSessionFile, trackSessionManagerAccess } from "../session-manager-cache.js"; import { prepareSessionManagerForRun } from "../session-manager-init.js"; import { resolveEmbeddedRunSkillEntries } from "../skills-runtime.js"; @@ -1061,6 +1061,7 @@ export async function runEmbeddedAttempt( ...buildEmbeddedAttemptToolRunContext({ ...params, trace: runTrace }), exec: { ...params.execOverrides, + config: params.config, elevated: params.bashElevated, }, sandbox, @@ -1557,7 +1558,18 @@ export async function runEmbeddedAttempt( accountId: params.agentAccountId, }) : undefined; - const sandboxInfo = buildEmbeddedSandboxInfo(sandbox, params.bashElevated); + const sandboxInfoExecPolicy = resolveEmbeddedSandboxInfoExecPolicy({ + config: params.config, + agentId: sessionAgentId, + sessionKey: params.sessionKey, + sandboxAvailable: sandbox?.enabled === true, + execOverrides: params.execOverrides, + }); + const sandboxInfo = buildEmbeddedSandboxInfo( + sandbox, + params.bashElevated, + sandboxInfoExecPolicy, + ); const reasoningTagHint = isReasoningTagProvider(params.provider, { config: params.config, workspaceDir: effectiveWorkspace, diff --git a/src/agents/embedded-agent-runner/sandbox-info.ts b/src/agents/embedded-agent-runner/sandbox-info.ts index c6d9d3db72e..9f60e402567 100644 --- a/src/agents/embedded-agent-runner/sandbox-info.ts +++ b/src/agents/embedded-agent-runner/sandbox-info.ts @@ -1,11 +1,43 @@ -import type { ExecElevatedDefaults } from "../bash-tools.js"; +import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import type { ExecElevatedDefaults, ExecToolDefaults } from "../bash-tools.js"; +import { resolveExecDefaults } from "../exec-defaults.js"; import type { resolveSandboxContext } from "../sandbox.js"; import type { EmbeddedFullAccessBlockedReason, EmbeddedSandboxInfo } from "./types.js"; -export function resolveEmbeddedFullAccessState(params: { execElevated?: ExecElevatedDefaults }): { +type EmbeddedFullAccessExecPolicy = Pick; +type EmbeddedFullAccessHostPolicy = Pick; +type EmbeddedSandboxInfoExecOverrides = Pick< + ExecToolDefaults, + "host" | "security" | "ask" | "node" +>; + +function execPolicyBlocksFullAccess(params: { + execPolicy?: EmbeddedFullAccessExecPolicy; + hostPolicy?: EmbeddedFullAccessHostPolicy; +}): boolean { + return ( + (params.execPolicy?.mode !== undefined && params.execPolicy.mode !== "full") || + (params.execPolicy?.security !== undefined && params.execPolicy.security !== "full") || + (params.execPolicy?.ask !== undefined && params.execPolicy.ask === "always") || + (params.hostPolicy?.security !== undefined && params.hostPolicy.security !== "full") || + (params.hostPolicy?.ask !== undefined && params.hostPolicy.ask === "always") + ); +} + +export function resolveEmbeddedFullAccessState(params: { + execElevated?: ExecElevatedDefaults; + execPolicy?: EmbeddedFullAccessExecPolicy; + hostPolicy?: EmbeddedFullAccessHostPolicy; +}): { available: boolean; blockedReason?: EmbeddedFullAccessBlockedReason; } { + if (execPolicyBlocksFullAccess(params)) { + return { + available: false, + blockedReason: "host-policy", + }; + } if (params.execElevated?.fullAccessAvailable === true) { return { available: true }; } @@ -24,9 +56,33 @@ export function resolveEmbeddedFullAccessState(params: { execElevated?: ExecElev return { available: true }; } +export function resolveEmbeddedSandboxInfoExecPolicy(params: { + config?: OpenClawConfig; + agentId?: string; + sessionKey?: string; + sandboxAvailable?: boolean; + execOverrides?: EmbeddedSandboxInfoExecOverrides; +}): EmbeddedFullAccessExecPolicy { + const defaults = resolveExecDefaults({ + cfg: params.config, + agentId: params.agentId, + sessionKey: params.sessionKey, + sandboxAvailable: params.sandboxAvailable, + elevatedRequested: true, + execOverrides: params.execOverrides, + }); + return { + mode: defaults.mode, + security: defaults.security, + ask: defaults.ask, + }; +} + export function buildEmbeddedSandboxInfo( sandbox?: Awaited>, execElevated?: ExecElevatedDefaults, + execPolicy?: EmbeddedFullAccessExecPolicy, + hostPolicy?: EmbeddedFullAccessHostPolicy, ): EmbeddedSandboxInfo | undefined { if (!sandbox?.enabled) { return undefined; @@ -35,6 +91,8 @@ export function buildEmbeddedSandboxInfo( const elevatedAllowed = Boolean(execElevated?.enabled && execElevated.allowed); const fullAccess = resolveEmbeddedFullAccessState({ execElevated, + execPolicy, + hostPolicy, }); return { enabled: true, diff --git a/src/agents/exec-auto-reviewer.prompt.ts b/src/agents/exec-auto-reviewer.prompt.ts new file mode 100644 index 00000000000..a9d87e36fef --- /dev/null +++ b/src/agents/exec-auto-reviewer.prompt.ts @@ -0,0 +1,11 @@ +export const DEFAULT_EXEC_REVIEWER_SYSTEM_PROMPT = `You are OpenClaw's exec safety reviewer. +Review exactly one pending shell command before it runs. +Return exactly one JSON object and no other text. + +Decision rules: +- Use "allow" only when the command is clearly low-risk for this single execution. +- Use "ask" when intent, path safety or command parsing, seem dangerous. This will prompt the user for confirmation. +- Treat internal network access, package publishing, chmod/chown, rm/mv sensitive paths, sudo, ssh/scp/rsync, and secret paths as high security risk. +- "ask" should be high fidelity, only "ask" when you are genuinely unsure. Ideally the user does not get prompted often as to reduce fatigue. + +Output schema: {"decision":"allow|ask","risk":"low|medium|high|unknown","rationale":"one short sentence"}`; diff --git a/src/agents/exec-auto-reviewer.test.ts b/src/agents/exec-auto-reviewer.test.ts new file mode 100644 index 00000000000..df32d32c72a --- /dev/null +++ b/src/agents/exec-auto-reviewer.test.ts @@ -0,0 +1,311 @@ +import { describe, expect, it, vi } from "vitest"; +import { createModelExecAutoReviewer, parseExecAutoReviewResponse } from "./exec-auto-reviewer.js"; + +const input = { + command: "git status", + argv: ["git", "status"], + cwd: "/repo", + envKeys: [], + host: "gateway" as const, + reason: "approval-required" as const, + analysis: { + parsed: true, + allowlistMatched: false, + inlineEval: false, + }, +}; + +describe("parseExecAutoReviewResponse", () => { + it("maps model allow decisions to single-use approvals", () => { + expect( + parseExecAutoReviewResponse( + JSON.stringify({ + decision: "allow", + risk: "low", + rationale: "read-only inspection", + }), + ), + ).toEqual({ + decision: "allow-once", + risk: "low", + rationale: "read-only inspection", + }); + }); + + it("maps model ask decisions to human approval", () => { + expect( + parseExecAutoReviewResponse( + JSON.stringify({ + decision: "ask", + risk: "medium", + rationale: "side effects need a human", + }), + ), + ).toEqual({ + decision: "ask", + risk: "medium", + rationale: "side effects need a human", + }); + }); + + it("normalizes unsupported or malformed decisions to human review", () => { + expect(parseExecAutoReviewResponse("sure, run it")).toMatchObject({ + decision: "ask", + }); + expect( + parseExecAutoReviewResponse( + JSON.stringify({ + decision: "allow-once", + risk: "low", + rationale: "legacy internal decision", + }), + ), + ).toMatchObject({ + decision: "ask", + rationale: "exec reviewer returned an unsupported response", + }); + expect( + parseExecAutoReviewResponse( + JSON.stringify({ + decision: "deny", + risk: "high", + rationale: "dangerous command", + }), + ), + ).toMatchObject({ + decision: "ask", + rationale: "exec reviewer returned an unsupported response", + }); + }); + + it("requires allow decisions to carry low risk", () => { + for (const risk of ["medium", "high", "unknown"] as const) { + expect( + parseExecAutoReviewResponse( + JSON.stringify({ + decision: "allow", + risk, + rationale: "looks fine", + }), + ), + ).toEqual({ + decision: "ask", + risk, + rationale: "exec reviewer returned a non-low allow decision", + }); + } + }); +}); + +describe("createModelExecAutoReviewer", () => { + it("uses the configured exec reviewer model for review calls", async () => { + const prepare = vi.fn(async () => ({ + selection: { + provider: "openrouter", + modelId: "anthropic/claude-sonnet-4-6", + agentDir: "/agent", + }, + model: { provider: "openrouter", id: "anthropic/claude-sonnet-4-6", api: "openai" }, + auth: { apiKey: "key", mode: "env" }, + })); + const complete = vi.fn(async () => ({ + content: [ + { + type: "text", + text: JSON.stringify({ + decision: "ask", + risk: "high", + rationale: "network side effect", + }), + }, + ], + })); + const reviewer = createModelExecAutoReviewer({ + cfg: {}, + agentId: "ops", + reviewer: { model: { primary: "openrouter/anthropic/claude-sonnet-4-6" } }, + deps: { + prepareSimpleCompletionModelForAgent: + prepare as unknown as typeof import("./simple-completion-runtime.js").prepareSimpleCompletionModelForAgent, + completeWithPreparedSimpleCompletionModel: + complete as unknown as typeof import("./simple-completion-runtime.js").completeWithPreparedSimpleCompletionModel, + }, + }); + + await expect(reviewer(input)).resolves.toEqual({ + decision: "ask", + risk: "high", + rationale: "network side effect", + }); + expect(prepare).toHaveBeenCalledWith( + expect.objectContaining({ + agentId: "ops", + modelRef: "openrouter/anthropic/claude-sonnet-4-6", + }), + ); + expect(complete).toHaveBeenCalledWith( + expect.objectContaining({ + context: expect.objectContaining({ + systemPrompt: expect.stringContaining('"decision":"allow|ask"'), + }), + options: expect.objectContaining({ + temperature: 0, + }), + }), + ); + }); + + it("falls back to human approval when the model is unavailable", async () => { + const reviewer = createModelExecAutoReviewer({ + cfg: {}, + deps: { + prepareSimpleCompletionModelForAgent: vi.fn(async () => ({ + error: "missing API key", + })) as unknown as typeof import("./simple-completion-runtime.js").prepareSimpleCompletionModelForAgent, + }, + }); + + await expect(reviewer(input)).resolves.toMatchObject({ + decision: "ask", + rationale: "exec reviewer model unavailable: missing API key", + }); + }); + + it("falls back to human approval with the model completion error", async () => { + const complete = vi.fn(async () => ({ + role: "assistant", + content: [], + api: "openai-responses", + provider: "atlassian-aigw", + model: "gpt-5.4-nano", + stopReason: "error", + errorMessage: "OpenAI API error (400): 400 Model Id [gpt-5.4-nano] not found", + })); + const reviewer = createModelExecAutoReviewer({ + cfg: {}, + deps: { + prepareSimpleCompletionModelForAgent: vi.fn(async () => ({ + selection: { + provider: "atlassian-aigw", + modelId: "gpt-5.4-nano", + agentDir: "/agent", + }, + model: { provider: "atlassian-aigw", id: "gpt-5.4-nano", api: "openai-responses" }, + auth: { apiKey: "key", mode: "env" }, + })) as unknown as typeof import("./simple-completion-runtime.js").prepareSimpleCompletionModelForAgent, + completeWithPreparedSimpleCompletionModel: + complete as unknown as typeof import("./simple-completion-runtime.js").completeWithPreparedSimpleCompletionModel, + }, + }); + + await expect(reviewer(input)).resolves.toEqual({ + decision: "ask", + risk: "unknown", + rationale: + "exec reviewer completion failed: OpenAI API error (400): 400 Model Id [gpt-5.4-nano] not found", + }); + }); + + it("applies the reviewer timeout while preparing the model", async () => { + vi.useFakeTimers(); + try { + const prepare = vi.fn( + () => + new Promise(() => { + // Keep model preparation pending until the reviewer timeout wins. + }), + ); + const reviewer = createModelExecAutoReviewer({ + cfg: {}, + reviewer: { timeoutMs: 5_000 }, + deps: { + prepareSimpleCompletionModelForAgent: + prepare as unknown as typeof import("./simple-completion-runtime.js").prepareSimpleCompletionModelForAgent, + }, + }); + + let settled = false; + const result = Promise.resolve(reviewer(input)).then((decision) => { + settled = true; + return decision; + }); + await vi.advanceTimersByTimeAsync(5_001); + + expect(settled).toBe(true); + await expect(result).resolves.toMatchObject({ + decision: "ask", + rationale: "exec reviewer timed out after 5000ms", + }); + } finally { + vi.useRealTimers(); + } + }); + + it("gives reviewer completion a fresh timeout after slow model preparation", async () => { + vi.useFakeTimers(); + try { + const prepare = vi.fn( + () => + new Promise<{ + selection: { provider: string; modelId: string; agentDir: string }; + model: { provider: string; id: string; api: "openai" }; + auth: { apiKey: string; mode: "env" }; + }>((resolve) => { + setTimeout(() => { + resolve({ + selection: { + provider: "openrouter", + modelId: "anthropic/claude-sonnet-4-6", + agentDir: "/agent", + }, + model: { provider: "openrouter", id: "anthropic/claude-sonnet-4-6", api: "openai" }, + auth: { apiKey: "key", mode: "env" }, + }); + }, 4_900); + }), + ); + const complete = vi.fn( + () => + new Promise<{ content: Array<{ type: "text"; text: string }> }>((resolve) => { + setTimeout(() => { + resolve({ + content: [ + { + type: "text", + text: JSON.stringify({ + decision: "allow", + risk: "low", + rationale: "read-only inspection", + }), + }, + ], + }); + }, 2_000); + }), + ); + const reviewer = createModelExecAutoReviewer({ + cfg: {}, + reviewer: { timeoutMs: 5_000 }, + deps: { + prepareSimpleCompletionModelForAgent: + prepare as unknown as typeof import("./simple-completion-runtime.js").prepareSimpleCompletionModelForAgent, + completeWithPreparedSimpleCompletionModel: + complete as unknown as typeof import("./simple-completion-runtime.js").completeWithPreparedSimpleCompletionModel, + }, + }); + + const result = reviewer(input); + await vi.advanceTimersByTimeAsync(4_900); + expect(complete).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(2_000); + + await expect(result).resolves.toEqual({ + decision: "allow-once", + risk: "low", + rationale: "read-only inspection", + }); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/src/agents/exec-auto-reviewer.ts b/src/agents/exec-auto-reviewer.ts new file mode 100644 index 00000000000..fabbd5b8cb2 --- /dev/null +++ b/src/agents/exec-auto-reviewer.ts @@ -0,0 +1,291 @@ +import { z } from "zod"; +import type { AgentModelConfig } from "../config/types.agents-shared.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { formatErrorMessage } from "../infra/errors.js"; +import { + defaultExecAutoReviewer, + type ExecAutoReviewDecision, + type ExecAutoReviewInput, + type ExecAutoReviewer, +} from "../infra/exec-auto-review.js"; +import { normalizeOptionalString } from "../shared/string-coerce.js"; +import { DEFAULT_EXEC_REVIEWER_SYSTEM_PROMPT } from "./exec-auto-reviewer.prompt.js"; +import { + completeWithPreparedSimpleCompletionModel, + prepareSimpleCompletionModelForAgent, +} from "./simple-completion-runtime.js"; +import { coerceToolModelConfig } from "./tools/model-config.helpers.js"; + +const DEFAULT_EXEC_REVIEWER_TIMEOUT_MS = 30_000; +const EXEC_REVIEWER_MAX_TOKENS = 360; +const EXEC_REVIEWER_TIMEOUT = Symbol("exec-reviewer-timeout"); + +const execAutoReviewResponseSchema = z.object({ + decision: z.enum(["allow", "ask"]), + risk: z.enum(["low", "medium", "high", "unknown"]), + rationale: z.string().optional(), +}); + +export type ExecReviewerConfig = { + model?: AgentModelConfig; + timeoutMs?: number; +}; + +type ExecReviewerDeps = { + prepareSimpleCompletionModelForAgent?: typeof prepareSimpleCompletionModelForAgent; + completeWithPreparedSimpleCompletionModel?: typeof completeWithPreparedSimpleCompletionModel; +}; + +function stringifyInput(input: ExecAutoReviewInput): string { + return JSON.stringify( + { + command: input.command, + argv: input.argv, + cwd: input.cwd, + envKeys: input.envKeys, + host: input.host, + reason: input.reason, + analysis: input.analysis, + agent: input.agent, + }, + null, + 2, + ); +} + +function normalizeRationale(value: unknown, fallback: string): string { + const text = normalizeOptionalString(typeof value === "string" ? value : undefined); + return (text ?? fallback).slice(0, 500); +} + +function stripJsonFence(text: string): string { + const trimmed = text.trim(); + const fenced = /^```(?:json)?\s*([\s\S]*?)\s*```$/iu.exec(trimmed); + return fenced?.[1]?.trim() ?? trimmed; +} + +function extractJsonObject(text: string): string | null { + const stripped = stripJsonFence(text); + if (stripped.startsWith("{") && stripped.endsWith("}")) { + return stripped; + } + const start = stripped.indexOf("{"); + const end = stripped.lastIndexOf("}"); + if (start >= 0 && end > start) { + return stripped.slice(start, end + 1); + } + return null; +} + +export function parseExecAutoReviewResponse(text: string): ExecAutoReviewDecision { + const objectText = extractJsonObject(text); + if (!objectText) { + return { + decision: "ask", + risk: "unknown", + rationale: "exec reviewer returned no parseable JSON", + }; + } + let parsed: unknown; + try { + parsed = JSON.parse(objectText); + } catch { + return { + decision: "ask", + risk: "unknown", + rationale: "exec reviewer returned malformed JSON", + }; + } + const response = execAutoReviewResponseSchema.safeParse(parsed); + if (!response.success) { + return { + decision: "ask", + risk: "unknown", + rationale: "exec reviewer returned an unsupported response", + }; + } + + const { decision, risk } = response.data; + const rationale = normalizeRationale( + response.data.rationale, + "exec reviewer did not explain decision", + ); + if (decision === "ask") { + return { + decision: "ask", + risk, + rationale, + }; + } + + if (risk !== "low") { + return { + decision: "ask", + risk, + rationale: "exec reviewer returned a non-low allow decision", + }; + } + + return { + decision: "allow-once", + risk, + rationale, + }; +} + +function extractTextContent( + result: Awaited>, +) { + return result.content + .filter((block): block is { type: "text"; text: string } => block.type === "text") + .map((block) => block.text) + .join("") + .trim(); +} + +function extractCompletionError( + result: Awaited>, +): string | undefined { + if (!("stopReason" in result) || result.stopReason !== "error") { + return undefined; + } + const message = + "errorMessage" in result && typeof result.errorMessage === "string" + ? result.errorMessage + : undefined; + return normalizeRationale(message, "model returned an error"); +} + +function resolveReviewerModelRef(config?: ExecReviewerConfig): string | undefined { + return coerceToolModelConfig(config?.model).primary; +} + +function resolveReviewerTimeoutMs(config?: ExecReviewerConfig): number { + return typeof config?.timeoutMs === "number" && Number.isFinite(config.timeoutMs) + ? Math.max(1_000, Math.floor(config.timeoutMs)) + : DEFAULT_EXEC_REVIEWER_TIMEOUT_MS; +} + +function buildReviewerTimeoutDecision(timeoutMs: number): ExecAutoReviewDecision { + return { + decision: "ask", + risk: "unknown", + rationale: `exec reviewer timed out after ${timeoutMs}ms`, + }; +} + +async function raceWithReviewerTimeout( + promise: Promise, + params: { + timeoutMs: number; + onTimeout?: () => void; + }, +): Promise { + let timer: ReturnType | undefined; + const timeout = new Promise((resolve) => { + timer = setTimeout(() => { + params.onTimeout?.(); + resolve(EXEC_REVIEWER_TIMEOUT); + }, params.timeoutMs); + }); + try { + return await Promise.race([promise, timeout]); + } finally { + if (timer) { + clearTimeout(timer); + } + } +} + +export function createModelExecAutoReviewer(params: { + cfg?: OpenClawConfig; + agentId?: string; + reviewer?: ExecReviewerConfig; + deps?: ExecReviewerDeps; +}): ExecAutoReviewer { + const cfg = params.cfg; + const agentId = params.agentId ?? "main"; + if (!cfg) { + return defaultExecAutoReviewer; + } + const prepareModel = + params.deps?.prepareSimpleCompletionModelForAgent ?? prepareSimpleCompletionModelForAgent; + const complete = + params.deps?.completeWithPreparedSimpleCompletionModel ?? + completeWithPreparedSimpleCompletionModel; + const modelRef = resolveReviewerModelRef(params.reviewer); + const timeoutMs = resolveReviewerTimeoutMs(params.reviewer); + return async (input) => { + let completionController: AbortController | undefined; + try { + const prepared = await raceWithReviewerTimeout( + prepareModel({ + cfg, + agentId, + modelRef, + allowMissingApiKeyModes: ["aws-sdk"], + }), + { timeoutMs }, + ); + if (prepared === EXEC_REVIEWER_TIMEOUT) { + return buildReviewerTimeoutDecision(timeoutMs); + } + if ("error" in prepared) { + return { + decision: "ask", + risk: "unknown", + rationale: `exec reviewer model unavailable: ${prepared.error}`, + }; + } + + completionController = new AbortController(); + const result = await raceWithReviewerTimeout( + complete({ + model: prepared.model, + auth: prepared.auth, + cfg, + context: { + systemPrompt: DEFAULT_EXEC_REVIEWER_SYSTEM_PROMPT, + messages: [ + { + role: "user", + content: `Review this pending exec request:\n\n${stringifyInput(input)}`, + timestamp: Date.now(), + }, + ], + }, + options: { + maxTokens: EXEC_REVIEWER_MAX_TOKENS, + temperature: 0, + signal: completionController.signal, + }, + }), + { + timeoutMs, + onTimeout: () => completionController?.abort(), + }, + ); + if (result === EXEC_REVIEWER_TIMEOUT) { + return buildReviewerTimeoutDecision(timeoutMs); + } + const completionError = extractCompletionError(result); + if (completionError) { + return { + decision: "ask", + risk: "unknown", + rationale: `exec reviewer completion failed: ${completionError}`, + }; + } + return parseExecAutoReviewResponse(extractTextContent(result)); + } catch (err) { + if (completionController?.signal.aborted) { + return buildReviewerTimeoutDecision(timeoutMs); + } + return { + decision: "ask", + risk: "unknown", + rationale: `exec reviewer failed: ${formatErrorMessage(err)}`, + }; + } + }; +} diff --git a/src/agents/exec-defaults.test.ts b/src/agents/exec-defaults.test.ts index 3bd1346e17c..efa7ca83b24 100644 --- a/src/agents/exec-defaults.test.ts +++ b/src/agents/exec-defaults.test.ts @@ -94,6 +94,7 @@ describe("resolveExecDefaults", () => { expect(defaults.host).toBe("auto"); expect(defaults.effectiveHost).toBe("gateway"); + expect(defaults.mode).toBe("full"); expect(defaults.security).toBe("full"); expect(defaults.ask).toBe("off"); }); @@ -112,10 +113,180 @@ describe("resolveExecDefaults", () => { expect(defaults.host).toBe("auto"); expect(defaults.effectiveHost).toBe("sandbox"); + expect(defaults.mode).toBe("deny"); expect(defaults.security).toBe("deny"); expect(defaults.ask).toBe("off"); }); + it("ignores host approval defaults when auto resolves to sandbox", () => { + vi.mocked(execApprovals.loadExecApprovals).mockReturnValue({ + version: 1, + defaults: { + security: "full", + ask: "always", + }, + agents: {}, + }); + + const defaults = resolveExecDefaults({ + cfg: { + tools: { + exec: { + host: "auto", + }, + }, + }, + sandboxAvailable: true, + }); + + expect(defaults.effectiveHost).toBe("sandbox"); + expect(defaults.security).toBe("deny"); + expect(defaults.ask).toBe("off"); + expect(execApprovals.loadExecApprovals).not.toHaveBeenCalled(); + }); + + it("maps normalized auto mode to allowlist plus on-miss approvals", () => { + expect( + resolveExecDefaults({ + cfg: { + tools: { + exec: { + mode: "auto", + }, + }, + }, + sandboxAvailable: false, + }), + ).toMatchObject({ + mode: "auto", + security: "allowlist", + ask: "on-miss", + }); + }); + + it("reports host approval floors after normalized exec modes", () => { + vi.mocked(execApprovals.loadExecApprovals).mockReturnValue({ + version: 1, + defaults: { + security: "deny", + ask: "off", + }, + agents: {}, + }); + + expect( + resolveExecDefaults({ + cfg: { + tools: { + exec: { + mode: "auto", + }, + }, + }, + sandboxAvailable: false, + }), + ).toMatchObject({ + mode: "deny", + security: "deny", + ask: "on-miss", + }); + }); + + it("reports agent-scoped host approval floors", () => { + vi.mocked(execApprovals.loadExecApprovals).mockReturnValue({ + version: 1, + agents: { + "agent-a": { + security: "full", + ask: "always", + }, + }, + }); + + expect( + resolveExecDefaults({ + cfg: { + tools: { + exec: { + mode: "full", + }, + }, + }, + agentId: "agent-a", + sandboxAvailable: false, + }), + ).toMatchObject({ + mode: "ask", + security: "full", + ask: "always", + }); + }); + + it("keeps legacy security overrides ahead of higher-scope normalized mode", () => { + expect( + resolveExecDefaults({ + cfg: { + tools: { + exec: { + mode: "auto", + }, + }, + agents: { + list: [ + { + id: "agent-a", + tools: { + exec: { + security: "full", + ask: "off", + }, + }, + }, + ], + }, + }, + agentId: "agent-a", + sandboxAvailable: false, + }), + ).toMatchObject({ + mode: "full", + security: "full", + ask: "off", + }); + }); + + it("preserves mode-derived security for partial legacy agent overrides", () => { + expect( + resolveExecDefaults({ + cfg: { + tools: { + exec: { + mode: "auto", + }, + }, + agents: { + list: [ + { + id: "agent-a", + tools: { + exec: { + ask: "off", + }, + }, + }, + ], + }, + }, + agentId: "agent-a", + sandboxAvailable: false, + }), + ).toMatchObject({ + mode: "allowlist", + security: "allowlist", + ask: "off", + }); + }); + it("blocks node advertising in helper calls when sandbox is available", () => { expect( canExecRequestNode({ diff --git a/src/agents/exec-defaults.ts b/src/agents/exec-defaults.ts index b8ee1fa1da4..6101aeaf5c4 100644 --- a/src/agents/exec-defaults.ts +++ b/src/agents/exec-defaults.ts @@ -4,8 +4,18 @@ import { loadExecApprovals, type ExecAsk, type ExecHost, + type ExecMode, type ExecSecurity, type ExecTarget, + maxAsk, + minSecurity, + normalizeExecAsk, + normalizeExecSecurity, + normalizeExecTarget, + resolveExecApprovalsFromFile, + resolveExecModeFromPolicy, + resolveExecModePolicy, + resolveExecPolicyForMode, } from "../infra/exec-approvals.js"; import { resolveAgentConfig, resolveSessionAgentId } from "./agent-scope.js"; import { isRequestedExecTargetAllowed, resolveExecTarget } from "./bash-tools.exec-runtime.js"; @@ -13,19 +23,71 @@ import { resolveSandboxRuntimeStatus } from "./sandbox/runtime-status.js"; type ResolvedExecConfig = { host?: ExecTarget; + mode?: ExecMode; security?: ExecSecurity; ask?: ExecAsk; node?: string; }; +type ExecOverridesConfig = Omit; + +function hasLegacyExecPolicyOverride(exec?: ResolvedExecConfig): boolean { + return exec?.security !== undefined || exec?.ask !== undefined; +} + +type LayeredExecPolicy = { + mode?: ExecMode; + security: ExecSecurity; + ask: ExecAsk; +}; + +function applyExecPolicyLayer( + base: LayeredExecPolicy, + layer?: ResolvedExecConfig, +): LayeredExecPolicy { + if (!layer) { + return base; + } + if (layer.mode) { + return { + mode: layer.mode, + ...resolveExecPolicyForMode(layer.mode), + }; + } + if (hasLegacyExecPolicyOverride(layer)) { + return { + security: layer.security ?? base.security, + ask: layer.ask ?? base.ask, + }; + } + return base; +} + +function applySessionLegacyExecPolicyLayer( + base: LayeredExecPolicy, + sessionEntry?: SessionEntry, +): LayeredExecPolicy { + const security = normalizeExecSecurity(sessionEntry?.execSecurity); + const ask = normalizeExecAsk(sessionEntry?.execAsk); + if (security !== null || ask !== null) { + return { + security: security ?? base.security, + ask: ask ?? base.ask, + }; + } + return base; +} + function resolveExecConfigState(params: { cfg?: OpenClawConfig; sessionEntry?: SessionEntry; + execOverrides?: ExecOverridesConfig; agentId?: string; sessionKey?: string; }): { cfg: OpenClawConfig; host: ExecTarget; + agentId: string | undefined; agentExec?: ResolvedExecConfig; globalExec?: ResolvedExecConfig; } { @@ -41,13 +103,15 @@ function resolveExecConfigState(params: { ? resolveAgentConfig(cfg, resolvedAgentId)?.tools?.exec : undefined; const host = - (params.sessionEntry?.execHost as ExecTarget | undefined) ?? + params.execOverrides?.host ?? + normalizeExecTarget(params.sessionEntry?.execHost) ?? (agentExec?.host as ExecTarget | undefined) ?? (globalExec?.host as ExecTarget | undefined) ?? "auto"; return { cfg, host, + agentId: resolvedAgentId, agentExec, globalExec, }; @@ -72,6 +136,7 @@ function resolveExecSandboxAvailability(params: { export function canExecRequestNode(params: { cfg?: OpenClawConfig; sessionEntry?: SessionEntry; + execOverrides?: ExecOverridesConfig; agentId?: string; sessionKey?: string; sandboxAvailable?: boolean; @@ -91,18 +156,27 @@ export function canExecRequestNode(params: { export function resolveExecDefaults(params: { cfg?: OpenClawConfig; sessionEntry?: SessionEntry; + execOverrides?: ExecOverridesConfig; agentId?: string; sessionKey?: string; sandboxAvailable?: boolean; + elevatedRequested?: boolean; }): { host: ExecTarget; effectiveHost: ExecHost; + mode: ExecMode; security: ExecSecurity; ask: ExecAsk; node?: string; canRequestNode: boolean; } { - const { cfg, host, agentExec, globalExec } = resolveExecConfigState(params); + const { + cfg, + host, + agentId: resolvedAgentId, + agentExec, + globalExec, + } = resolveExecConfigState(params); const sandboxAvailable = resolveExecSandboxAvailability({ cfg, sessionKey: params.sessionKey, @@ -110,27 +184,56 @@ export function resolveExecDefaults(params: { }); const resolved = resolveExecTarget({ configuredTarget: host, - elevatedRequested: false, + elevatedRequested: params.elevatedRequested === true, sandboxAvailable, }); - const approvalDefaults = loadExecApprovals().defaults; const defaultSecurity = resolved.effectiveHost === "sandbox" ? "deny" : "full"; + const approvalDefaults = + resolved.effectiveHost === "sandbox" + ? undefined + : resolveExecApprovalsFromFile({ + file: loadExecApprovals(), + agentId: resolvedAgentId, + overrides: { + security: defaultSecurity, + ask: "off", + }, + }).agent; + const basePolicy: LayeredExecPolicy = { + security: approvalDefaults?.security ?? defaultSecurity, + ask: approvalDefaults?.ask ?? "off", + }; + const layeredPolicy = applyExecPolicyLayer( + applySessionLegacyExecPolicyLayer( + applyExecPolicyLayer(applyExecPolicyLayer(basePolicy, globalExec), agentExec), + params.sessionEntry, + ), + params.execOverrides, + ); + const modePolicy = resolveExecModePolicy(layeredPolicy); + const security = + approvalDefaults?.security !== undefined + ? minSecurity(modePolicy.security, approvalDefaults.security) + : modePolicy.security; + const ask = + approvalDefaults?.ask !== undefined + ? maxAsk(modePolicy.ask, approvalDefaults.ask) + : modePolicy.ask; + const mode = + security === modePolicy.security && ask === modePolicy.ask + ? modePolicy.mode + : resolveExecModeFromPolicy({ security, ask }); return { host, effectiveHost: resolved.effectiveHost, - security: - (params.sessionEntry?.execSecurity as ExecSecurity | undefined) ?? - agentExec?.security ?? - globalExec?.security ?? - approvalDefaults?.security ?? - defaultSecurity, - ask: - (params.sessionEntry?.execAsk as ExecAsk | undefined) ?? - agentExec?.ask ?? - globalExec?.ask ?? - approvalDefaults?.ask ?? - "off", - node: params.sessionEntry?.execNode ?? agentExec?.node ?? globalExec?.node, + mode, + security, + ask, + node: + params.execOverrides?.node ?? + params.sessionEntry?.execNode ?? + agentExec?.node ?? + globalExec?.node, canRequestNode: isRequestedExecTargetAllowed({ configuredTarget: host, requestedTarget: "node", diff --git a/src/config/schema.help.quality.test.ts b/src/config/schema.help.quality.test.ts index 6af861c0d45..0b57eb8d106 100644 --- a/src/config/schema.help.quality.test.ts +++ b/src/config/schema.help.quality.test.ts @@ -303,6 +303,7 @@ const TARGET_KEYS = [ "tools.deny", "tools.exec", "tools.exec.host", + "tools.exec.mode", "tools.exec.security", "tools.exec.ask", "tools.exec.node", diff --git a/src/config/schema.help.ts b/src/config/schema.help.ts index 89d0652cd19..d5e57477c72 100644 --- a/src/config/schema.help.ts +++ b/src/config/schema.help.ts @@ -413,6 +413,14 @@ export const FIELD_HELP: Record = { "Exec-tool policy grouping for shell execution host, security mode, approval behavior, and runtime bindings. Keep conservative defaults in production and tighten elevated execution paths.", "tools.exec.host": 'Selects execution target strategy for shell commands. Use "auto" for runtime-aware behavior (sandbox when available, otherwise gateway), or pin sandbox/gateway/node explicitly when you need a fixed surface.', + "tools.exec.mode": + 'Normalized exec policy selector. Use "auto" for classifier-reviewed approval misses, "ask" for human-reviewed misses, "allowlist" for deterministic safe commands only, or "full" for trusted local operation.', + "tools.exec.reviewer": + "Model-backed exec reviewer used by auto mode before human approval fallback. Configure a narrow model override here when you want exec review isolated from the main agent model.", + "tools.exec.reviewer.model": + "Optional provider/model override for the exec reviewer agent. Omit to reuse the current agent model.", + "tools.exec.reviewer.timeoutMs": + "Exec reviewer timeout in milliseconds before falling back to human approval (default: 30000).", "tools.exec.security": "Execution security posture selector controlling sandbox/approval expectations for command execution. Keep strict security mode for untrusted prompts and relax only for trusted operator workflows.", "tools.exec.ask": diff --git a/src/config/schema.labels.ts b/src/config/schema.labels.ts index 8dc21b3cc3d..e702cf83507 100644 --- a/src/config/schema.labels.ts +++ b/src/config/schema.labels.ts @@ -237,6 +237,10 @@ export const FIELD_LABELS: Record = { "tools.exec.notifyOnExitEmptySuccess": "Exec Notify On Empty Success", "tools.exec.approvalRunningNoticeMs": "Exec Approval Running Notice (ms)", "tools.exec.host": "Exec Target", + "tools.exec.mode": "Exec Mode", + "tools.exec.reviewer": "Exec Reviewer", + "tools.exec.reviewer.model": "Exec Reviewer Model", + "tools.exec.reviewer.timeoutMs": "Exec Reviewer Timeout (ms)", "tools.exec.security": "Exec Security", "tools.exec.ask": "Exec Ask", "tools.exec.node": "Exec Node Binding", diff --git a/src/config/schema.tags.ts b/src/config/schema.tags.ts index 9fdfdd01c2a..62ab7fdc457 100644 --- a/src/config/schema.tags.ts +++ b/src/config/schema.tags.ts @@ -57,6 +57,7 @@ const TAG_OVERRIDES: Record = { "gateway.nodes.pairing.autoApproveCidrs": ["security", "access", "network", "advanced"], "proxy.tls.caFile": ["security", "network", "storage", "advanced"], "tools.exec.applyPatch.workspaceOnly": ["tools", "security", "access", "advanced"], + "tools.exec.mode": ["tools", "security", "access"], }; const PREFIX_RULES: Array<{ prefix: string; tags: ConfigTag[] }> = [ diff --git a/src/config/schema.test.ts b/src/config/schema.test.ts index 7a8c1419a5c..ed9f4edb1fb 100644 --- a/src/config/schema.test.ts +++ b/src/config/schema.test.ts @@ -493,6 +493,69 @@ describe("config schema", () => { expect(config.agents?.list?.[0]?.tools?.exec?.commandHighlighting).toBe(false); }); + it("accepts exec reviewer model config in global and agent scopes", () => { + const tools = ToolsSchema.parse({ + exec: { + reviewer: { + model: { + primary: "openrouter/anthropic/claude-sonnet-4-6", + }, + timeoutMs: 15_000, + }, + }, + }); + expect(tools?.exec?.reviewer?.model).toEqual({ + primary: "openrouter/anthropic/claude-sonnet-4-6", + }); + + const config = OpenClawSchema.parse({ + agents: { + list: [ + { + id: "main", + tools: { + exec: { + reviewer: { + model: "openai/gpt-5.5", + }, + }, + }, + }, + ], + }, + }); + expect(config.agents?.list?.[0]?.tools?.exec?.reviewer?.model).toBe("openai/gpt-5.5"); + }); + + it("rejects mixed normalized and legacy exec policy config", () => { + expect( + ToolsSchema.safeParse({ + exec: { + mode: "auto", + ask: "always", + }, + }).success, + ).toBe(false); + + expect( + OpenClawSchema.safeParse({ + agents: { + list: [ + { + id: "main", + tools: { + exec: { + mode: "full", + security: "deny", + }, + }, + }, + ], + }, + }).success, + ).toBe(false); + }); + it("accepts experimental tool flags in the runtime zod schema", () => { const parsed = ToolsSchema.parse({ experimental: { diff --git a/src/config/types.tools.ts b/src/config/types.tools.ts index 450cd777033..0cf0e67bfa9 100644 --- a/src/config/types.tools.ts +++ b/src/config/types.tools.ts @@ -1,6 +1,7 @@ import type { ChatType } from "../channels/chat-type.js"; import type { SafeBinProfileFixture } from "../infra/exec-safe-bin-policy.js"; import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js"; +import type { AgentModelConfig } from "./types.agents-shared.js"; import type { AgentElevatedAllowFromConfig, SessionSendPolicyAction } from "./types.base.js"; import type { MemoryQmdIndexPath } from "./types.memory.js"; import type { ConfiguredProviderRequest } from "./types.provider-request.js"; @@ -298,6 +299,8 @@ export type GroupToolPolicyBySenderConfig = Record; + /** Model-backed reviewer used by tools.exec.mode=auto before falling back to human approval. */ + reviewer?: { + /** Optional reviewer model override (provider/model or agent model config). */ + model?: AgentModelConfig; + /** Reviewer timeout in milliseconds (default: 30000). */ + timeoutMs?: number; + }; /** Default time (ms) before an exec command auto-backgrounds. */ backgroundMs?: number; /** Default timeout (seconds) before auto-killing exec commands. */ diff --git a/src/config/zod-schema.agent-runtime.ts b/src/config/zod-schema.agent-runtime.ts index 5605621371f..1b5ce783312 100644 --- a/src/config/zod-schema.agent-runtime.ts +++ b/src/config/zod-schema.agent-runtime.ts @@ -553,6 +553,7 @@ const ToolExecSafeBinProfileSchema = z const ToolExecBaseShape = { host: z.enum(["auto", "sandbox", "gateway", "node"]).optional(), + mode: z.enum(["deny", "allowlist", "ask", "auto", "full"]).optional(), security: z.enum(["deny", "allowlist", "full"]).optional(), ask: z.enum(["off", "on-miss", "always"]).optional(), node: z.string().optional(), @@ -562,6 +563,13 @@ const ToolExecBaseShape = { commandHighlighting: z.boolean().optional(), safeBinTrustedDirs: z.array(z.string()).optional(), safeBinProfiles: z.record(z.string(), ToolExecSafeBinProfileSchema).optional(), + reviewer: z + .object({ + model: AgentModelSchema.optional(), + timeoutMs: z.number().int().positive().optional(), + }) + .strict() + .optional(), backgroundMs: z.number().int().positive().optional(), timeoutSec: z.number().int().positive().optional(), cleanupMs: z.number().int().positive().optional(), @@ -570,15 +578,34 @@ const ToolExecBaseShape = { applyPatch: ToolExecApplyPatchSchema, } as const; +function addExecPolicyModeConflictIssue( + value: { mode?: unknown; security?: unknown; ask?: unknown }, + ctx: z.RefinementCtx, +): void { + if (value.mode === undefined || (value.security === undefined && value.ask === undefined)) { + return; + } + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["mode"], + message: "tools.exec.mode cannot be combined with tools.exec.security or tools.exec.ask", + }); +} + const AgentToolExecSchema = z .object({ ...ToolExecBaseShape, approvalRunningNoticeMs: z.number().int().nonnegative().optional(), }) .strict() + .superRefine(addExecPolicyModeConflictIssue) .optional(); -const ToolExecSchema = z.object(ToolExecBaseShape).strict().optional(); +const ToolExecSchema = z + .object(ToolExecBaseShape) + .strict() + .superRefine(addExecPolicyModeConflictIssue) + .optional(); const ToolFsSchema = z .object({ diff --git a/src/gateway/operator-approvals-client.test.ts b/src/gateway/operator-approvals-client.test.ts index 9267cc1d8b0..38e8366b65e 100644 --- a/src/gateway/operator-approvals-client.test.ts +++ b/src/gateway/operator-approvals-client.test.ts @@ -195,9 +195,10 @@ describe("withOperatorApprovalsGatewayClient", () => { ); expect(typeof clientState.options?.approvalRuntimeToken).toBe("string"); + expect(clientState.options?.deviceIdentity).toBeNull(); }); - it("keeps device identity for loopback approval clients without shared auth", async () => { + it("omits stored device identity for local runtime-token approval clients without shared auth", async () => { bootstrapState.auth = { token: undefined, password: undefined }; await withOperatorApprovalsGatewayClient( @@ -208,7 +209,8 @@ describe("withOperatorApprovalsGatewayClient", () => { async () => undefined, ); - expect(clientState.options?.deviceIdentity).toBeUndefined(); + expect(typeof clientState.options?.approvalRuntimeToken).toBe("string"); + expect(clientState.options?.deviceIdentity).toBeNull(); }); it("surfaces close failures before hello", async () => { diff --git a/src/gateway/operator-approvals-client.ts b/src/gateway/operator-approvals-client.ts index 38196502904..9c565e6a481 100644 --- a/src/gateway/operator-approvals-client.ts +++ b/src/gateway/operator-approvals-client.ts @@ -35,6 +35,18 @@ function shouldSendApprovalRuntimeToken(urlSource: string): boolean { ); } +function shouldOmitApprovalRuntimeDeviceIdentity(params: { + url: string; + token?: string; + password?: string; + sendsApprovalRuntimeToken: boolean; +}): boolean { + if (params.sendsApprovalRuntimeToken) { + return true; + } + return shouldOmitOperatorApprovalDeviceIdentity(params); +} + export async function createOperatorApprovalsGatewayClient( params: Pick< GatewayClientOptions, @@ -54,12 +66,13 @@ export async function createOperatorApprovalsGatewayClient( gatewayUrl: params.gatewayUrl, env: process.env, }); + const sendsApprovalRuntimeToken = shouldSendApprovalRuntimeToken(bootstrap.urlSource); return new GatewayClient({ url: bootstrap.url, token: bootstrap.auth.token, password: bootstrap.auth.password, - ...(shouldSendApprovalRuntimeToken(bootstrap.urlSource) + ...(sendsApprovalRuntimeToken ? { approvalRuntimeToken: getOperatorApprovalRuntimeToken() } : {}), preauthHandshakeTimeoutMs: bootstrap.preauthHandshakeTimeoutMs, @@ -67,10 +80,11 @@ export async function createOperatorApprovalsGatewayClient( clientDisplayName: params.clientDisplayName, mode: GATEWAY_CLIENT_MODES.BACKEND, scopes: ["operator.approvals"], - deviceIdentity: shouldOmitOperatorApprovalDeviceIdentity({ + deviceIdentity: shouldOmitApprovalRuntimeDeviceIdentity({ url: bootstrap.url, token: bootstrap.auth.token, password: bootstrap.auth.password, + sendsApprovalRuntimeToken, }) ? null : undefined, diff --git a/src/gateway/server-methods/approval-shared.test.ts b/src/gateway/server-methods/approval-shared.test.ts index 3c0c320eb3d..6ea8d1c5519 100644 --- a/src/gateway/server-methods/approval-shared.test.ts +++ b/src/gateway/server-methods/approval-shared.test.ts @@ -528,6 +528,57 @@ describe("handlePendingApprovalRequest", () => { ); }); + it("keeps register-only approval requests pending without a delivery route", async () => { + hasApprovalTurnSourceRouteMock.mockReturnValueOnce(false); + const manager = new ExecApprovalManager(); + const record = manager.create( + { + command: "echo ok", + }, + 60_000, + "approval-register-only", + ); + const decisionPromise = manager.register(record, 60_000); + const respond = vi.fn(); + const requestPromise = handlePendingApprovalRequest({ + manager, + record, + decisionPromise, + respond, + context: { + broadcast: vi.fn(), + hasExecApprovalClients: () => false, + } as unknown as GatewayRequestContext, + requestEventName: "exec.approval.requested", + requestEvent: { + id: record.id, + request: record.request, + createdAtMs: record.createdAtMs, + expiresAtMs: record.expiresAtMs, + }, + twoPhase: true, + requireDeliveryRoute: false, + deliverRequest: () => false, + }); + + await Promise.resolve(); + expect(manager.getSnapshot(record.id)?.resolvedAtMs).toBeUndefined(); + expect(respond).toHaveBeenCalledWith( + true, + expect.objectContaining({ id: "approval-register-only", status: "accepted" }), + undefined, + ); + + expect(manager.resolve(record.id, "allow-once")).toBe(true); + await requestPromise; + expect(manager.getSnapshot(record.id)?.resolvedBy).not.toBe("no-approval-route"); + expect(respond).toHaveBeenLastCalledWith( + true, + expect.objectContaining({ id: "approval-register-only", decision: "allow-once" }), + undefined, + ); + }); + it("does not target no-device browser UI approvals to unrelated approval-scoped clients", async () => { hasApprovalTurnSourceRouteMock.mockReturnValueOnce(false); const manager = new ExecApprovalManager(); diff --git a/src/gateway/server-methods/approval-shared.ts b/src/gateway/server-methods/approval-shared.ts index af5da9cd5ef..a7909d92fbb 100644 --- a/src/gateway/server-methods/approval-shared.ts +++ b/src/gateway/server-methods/approval-shared.ts @@ -287,30 +287,39 @@ export async function handlePendingApprovalRequest< requestEvent: RequestedApprovalEvent, ) => Promise | void; afterDecisionErrorLabel?: string; + keepPendingWithoutRoute?: boolean; + requireDeliveryRoute?: boolean; + suppressDelivery?: boolean; }): Promise { - const approvalClientConnIds = resolveApprovalRequestRecipientConnIds({ - context: params.context, - record: params.record, - excludeConnId: params.clientConnId, - }); - if (approvalClientConnIds) { - params.context.broadcastToConnIds( - params.requestEventName, - params.requestEvent, - approvalClientConnIds, - { - dropIfSlow: true, - }, - ); - } else { - params.context.broadcast(params.requestEventName, params.requestEvent, { dropIfSlow: true }); + const suppressDelivery = params.suppressDelivery === true; + const approvalClientConnIds = suppressDelivery + ? null + : resolveApprovalRequestRecipientConnIds({ + context: params.context, + record: params.record, + excludeConnId: params.clientConnId, + }); + if (!suppressDelivery) { + if (approvalClientConnIds) { + params.context.broadcastToConnIds( + params.requestEventName, + params.requestEvent, + approvalClientConnIds, + { + dropIfSlow: true, + }, + ); + } else { + params.context.broadcast(params.requestEventName, params.requestEvent, { dropIfSlow: true }); + } } - const hasApprovalClients = - approvalClientConnIds !== null + const hasApprovalClients = suppressDelivery + ? false + : approvalClientConnIds !== null ? approvalClientConnIds.size > 0 : (params.context.hasExecApprovalClients?.(params.clientConnId) ?? false); - const deliveredResult = params.deliverRequest(); + const deliveredResult = suppressDelivery ? false : params.deliverRequest(); const delivered = isPromiseLike(deliveredResult) ? await deliveredResult : deliveredResult; const hasTurnSourceRoute = !hasApprovalClients && @@ -321,7 +330,13 @@ export async function handlePendingApprovalRequest< approvalKind: params.approvalKind ?? "exec", }); - if (!hasApprovalClients && !hasTurnSourceRoute && !delivered) { + if ( + params.requireDeliveryRoute !== false && + !params.keepPendingWithoutRoute && + !hasApprovalClients && + !hasTurnSourceRoute && + !delivered + ) { params.manager.expire(params.record.id, "no-approval-route"); params.respond( true, diff --git a/src/gateway/server-methods/exec-approval.ts b/src/gateway/server-methods/exec-approval.ts index 5b0de81a9aa..9f5f4a3c4ce 100644 --- a/src/gateway/server-methods/exec-approval.ts +++ b/src/gateway/server-methods/exec-approval.ts @@ -187,6 +187,8 @@ export function createExecApprovalHandlers( turnSourceTo?: string; turnSourceAccountId?: string; turnSourceThreadId?: string | number; + requireDeliveryRoute?: boolean; + suppressDelivery?: boolean; timeoutMs?: number; twoPhase?: boolean; }; @@ -368,6 +370,8 @@ export function createExecApprovalHandlers( requestEvent, twoPhase, approvalKind: "exec", + requireDeliveryRoute: p.requireDeliveryRoute, + suppressDelivery: p.suppressDelivery, deliverRequest: () => { const deliveryTasks: Array> = []; if (opts?.forwarder) { diff --git a/src/infra/exec-approvals-effective.ts b/src/infra/exec-approvals-effective.ts index 33af2690ee2..71b6ee000fe 100644 --- a/src/infra/exec-approvals-effective.ts +++ b/src/infra/exec-approvals-effective.ts @@ -8,8 +8,11 @@ import { maxAsk, minSecurity, resolveExecApprovalsFromFile, + resolveExecModeFromPolicy, + resolveExecModePolicy, type ExecApprovalsFile, type ExecAsk, + type ExecMode, type ExecSecurity, type ExecTarget, } from "./exec-approvals.js"; @@ -23,6 +26,7 @@ const REQUESTED_DEFAULT_LABEL = { } as const; type ExecPolicyConfig = { host?: ExecTarget; + mode?: ExecMode; security?: ExecSecurity; ask?: ExecAsk; }; @@ -46,6 +50,12 @@ export type ExecPolicyScopeSnapshot = { configPath: string; agentId?: string; host: ExecPolicyHostSummary; + mode: { + requested: ExecMode; + requestedSource: string; + effective: ExecMode; + note: string; + }; security: ExecPolicyFieldSummary; ask: ExecPolicyFieldSummary; askFallback: { @@ -93,6 +103,13 @@ function formatRequestedSource(params: { : `${params.sourcePath}.${params.field}`; } +function formatModeSource(params: { sourcePath: string; configPath: string }): string { + if (params.sourcePath === "__default__") { + return "derived from OpenClaw defaults"; + } + return `${params.sourcePath === "scope" ? params.configPath : params.sourcePath}.mode`; +} + type ExecPolicyField = "security" | "ask" | "askFallback"; function resolveRequestedField< @@ -124,6 +141,125 @@ function resolveRequestedField< }; } +function hasLegacyExecPolicyOverride(exec?: ExecPolicyConfig): boolean { + return exec?.security !== undefined || exec?.ask !== undefined; +} + +function resolveRequestedPolicy(params: { + scopeExecConfig?: ExecPolicyConfig; + globalExecConfig?: ExecPolicyConfig; + configPath: string; +}): { + mode: ExecMode; + modeSource: string; + security: ExecSecurity; + securitySource: string; + ask: ExecAsk; + askSource: string; +} { + if (params.scopeExecConfig?.mode) { + const policy = resolveExecModePolicy({ + mode: params.scopeExecConfig.mode, + security: DEFAULT_REQUESTED_SECURITY, + ask: DEFAULT_REQUESTED_ASK, + }); + const source = formatModeSource({ sourcePath: "scope", configPath: params.configPath }); + return { + mode: policy.mode, + modeSource: source, + security: policy.security, + securitySource: source, + ask: policy.ask, + askSource: source, + }; + } + if (!hasLegacyExecPolicyOverride(params.scopeExecConfig) && params.globalExecConfig?.mode) { + const policy = resolveExecModePolicy({ + mode: params.globalExecConfig.mode, + security: DEFAULT_REQUESTED_SECURITY, + ask: DEFAULT_REQUESTED_ASK, + }); + const source = formatModeSource({ sourcePath: "tools.exec", configPath: params.configPath }); + return { + mode: policy.mode, + modeSource: source, + security: policy.security, + securitySource: source, + ask: policy.ask, + askSource: source, + }; + } + if (hasLegacyExecPolicyOverride(params.scopeExecConfig) && params.globalExecConfig?.mode) { + const inherited = resolveExecModePolicy({ + mode: params.globalExecConfig.mode, + security: DEFAULT_REQUESTED_SECURITY, + ask: DEFAULT_REQUESTED_ASK, + }); + const inheritedSource = formatModeSource({ + sourcePath: "tools.exec", + configPath: params.configPath, + }); + const scopeSecuritySource = formatRequestedSource({ + sourcePath: params.configPath, + field: "security", + defaultValue: DEFAULT_REQUESTED_SECURITY, + }); + const scopeAskSource = formatRequestedSource({ + sourcePath: params.configPath, + field: "ask", + defaultValue: DEFAULT_REQUESTED_ASK, + }); + const security = params.scopeExecConfig?.security ?? inherited.security; + const ask = params.scopeExecConfig?.ask ?? inherited.ask; + const securitySource = + params.scopeExecConfig?.security !== undefined ? scopeSecuritySource : inheritedSource; + const askSource = params.scopeExecConfig?.ask !== undefined ? scopeAskSource : inheritedSource; + return { + mode: resolveExecModeFromPolicy({ security, ask }), + modeSource: + securitySource === askSource + ? `derived from ${securitySource}` + : `derived from ${securitySource} and ${askSource}`, + security, + securitySource, + ask, + askSource, + }; + } + + const security = resolveRequestedField({ + field: "security", + scopeExecConfig: params.scopeExecConfig, + globalExecConfig: params.globalExecConfig, + }); + const ask = resolveRequestedField({ + field: "ask", + scopeExecConfig: params.scopeExecConfig, + globalExecConfig: params.globalExecConfig, + }); + const securitySource = formatRequestedSource({ + sourcePath: security.sourcePath === "scope" ? params.configPath : security.sourcePath, + field: "security", + defaultValue: DEFAULT_REQUESTED_SECURITY, + }); + const askSource = formatRequestedSource({ + sourcePath: ask.sourcePath === "scope" ? params.configPath : ask.sourcePath, + field: "ask", + defaultValue: DEFAULT_REQUESTED_ASK, + }); + return { + mode: resolveExecModeFromPolicy({ security: security.value, ask: ask.value }), + modeSource: + securitySource === askSource + ? `derived from ${securitySource}` + : `derived from ${securitySource} and ${askSource}`, + security: security.value, + securitySource, + ask: ask.value, + askSource, + }; +} + function formatHostFieldSource(params: { hostPath: string; field: ExecPolicyField; @@ -213,32 +349,34 @@ export function resolveExecPolicyScopeSnapshot(params: { agentId?: string; hostPath?: string; }): ExecPolicyScopeSnapshot { - const requestedSecurity = resolveRequestedField({ - field: "security", - scopeExecConfig: params.scopeExecConfig, - globalExecConfig: params.globalExecConfig, - }); const requestedHost = resolveRequestedHost({ scopeExecConfig: params.scopeExecConfig, globalExecConfig: params.globalExecConfig, }); - const requestedAsk = resolveRequestedField({ - field: "ask", + const requestedPolicy = resolveRequestedPolicy({ scopeExecConfig: params.scopeExecConfig, globalExecConfig: params.globalExecConfig, + configPath: params.configPath, }); const resolved = resolveExecApprovalsFromFile({ file: params.approvals, agentId: params.agentId, overrides: { - security: requestedSecurity.value, - ask: requestedAsk.value, + security: requestedPolicy.security, + ask: requestedPolicy.ask, }, }); const hostPath = params.hostPath ?? DEFAULT_HOST_PATH; - const effectiveSecurity = minSecurity(requestedSecurity.value, resolved.agent.security); - const effectiveAsk = maxAsk(requestedAsk.value, resolved.agent.ask); + const effectiveSecurity = minSecurity(requestedPolicy.security, resolved.agent.security); + const effectiveAsk = maxAsk(requestedPolicy.ask, resolved.agent.ask); const effectiveAskFallback = minSecurity(effectiveSecurity, resolved.agent.askFallback); + const effectiveMode = + effectiveSecurity === requestedPolicy.security && effectiveAsk === requestedPolicy.ask + ? requestedPolicy.mode + : resolveExecModeFromPolicy({ + security: effectiveSecurity, + ask: effectiveAsk, + }); return { scopeLabel: params.scopeLabel, configPath: params.configPath, @@ -250,16 +388,18 @@ export function resolveExecPolicyScopeSnapshot(params: { ? "OpenClaw default (auto)" : `${requestedHost.sourcePath === "scope" ? params.configPath : requestedHost.sourcePath}.host`, }, + mode: { + requested: requestedPolicy.mode, + requestedSource: requestedPolicy.modeSource, + effective: effectiveMode, + note: + effectiveMode === requestedPolicy.mode + ? "requested mode applies" + : "host policy changes effective mode", + }, security: { - requested: requestedSecurity.value, - requestedSource: formatRequestedSource({ - sourcePath: - requestedSecurity.sourcePath === "scope" - ? params.configPath - : requestedSecurity.sourcePath, - field: "security", - defaultValue: DEFAULT_REQUESTED_SECURITY, - }), + requested: requestedPolicy.security, + requestedSource: requestedPolicy.securitySource, host: resolved.agent.security, hostSource: formatHostFieldSource({ hostPath, @@ -268,18 +408,13 @@ export function resolveExecPolicyScopeSnapshot(params: { }), effective: effectiveSecurity, note: - effectiveSecurity === requestedSecurity.value + effectiveSecurity === requestedPolicy.security ? "requested security applies" : "stricter host security wins", }, ask: { - requested: requestedAsk.value, - requestedSource: formatRequestedSource({ - sourcePath: - requestedAsk.sourcePath === "scope" ? params.configPath : requestedAsk.sourcePath, - field: "ask", - defaultValue: DEFAULT_REQUESTED_ASK, - }), + requested: requestedPolicy.ask, + requestedSource: requestedPolicy.askSource, host: resolved.agent.ask, hostSource: formatHostFieldSource({ hostPath, @@ -288,7 +423,7 @@ export function resolveExecPolicyScopeSnapshot(params: { }), effective: effectiveAsk, note: resolveAskNote({ - requestedAsk: requestedAsk.value, + requestedAsk: requestedPolicy.ask, hostAsk: resolved.agent.ask, effectiveAsk, }), diff --git a/src/infra/exec-approvals-policy.test.ts b/src/infra/exec-approvals-policy.test.ts index 6111bb143a3..edcdae4ed0f 100644 --- a/src/infra/exec-approvals-policy.test.ts +++ b/src/infra/exec-approvals-policy.test.ts @@ -19,9 +19,13 @@ let minSecurity: typeof import("./exec-approvals.js").minSecurity; let requireValidExecTarget: typeof import("./exec-approvals.js").requireValidExecTarget; let normalizeExecAsk: typeof import("./exec-approvals.js").normalizeExecAsk; let normalizeExecHost: typeof import("./exec-approvals.js").normalizeExecHost; +let normalizeExecMode: typeof import("./exec-approvals.js").normalizeExecMode; let normalizeExecTarget: typeof import("./exec-approvals.js").normalizeExecTarget; let normalizeExecSecurity: typeof import("./exec-approvals.js").normalizeExecSecurity; let requiresExecApproval: typeof import("./exec-approvals.js").requiresExecApproval; +let resolveExecModeFromPolicy: typeof import("./exec-approvals.js").resolveExecModeFromPolicy; +let resolveExecModePolicy: typeof import("./exec-approvals.js").resolveExecModePolicy; +let resolveExecPolicyForMode: typeof import("./exec-approvals.js").resolveExecPolicyForMode; async function loadActualExecApprovalModules(): Promise { vi.resetModules(); @@ -39,9 +43,13 @@ async function loadActualExecApprovalModules(): Promise { requireValidExecTarget = execApprovals.requireValidExecTarget; normalizeExecAsk = execApprovals.normalizeExecAsk; normalizeExecHost = execApprovals.normalizeExecHost; + normalizeExecMode = execApprovals.normalizeExecMode; normalizeExecTarget = execApprovals.normalizeExecTarget; normalizeExecSecurity = execApprovals.normalizeExecSecurity; requiresExecApproval = execApprovals.requiresExecApproval; + resolveExecModeFromPolicy = execApprovals.resolveExecModeFromPolicy; + resolveExecModePolicy = execApprovals.resolveExecModePolicy; + resolveExecPolicyForMode = execApprovals.resolveExecPolicyForMode; } function expectFields(value: unknown, expected: Record): void { @@ -137,6 +145,73 @@ describe("exec approvals policy helpers", () => { expect(normalizeExecAsk(raw)).toBe(expected); }); + it.each([ + { raw: " auto ", expected: "auto" }, + { raw: "ASK", expected: "ask" }, + { raw: "allowlist", expected: "allowlist" }, + { raw: "maybe", expected: null }, + ])("normalizes exec mode value %j", ({ raw, expected }) => { + expect(normalizeExecMode(raw)).toBe(expected); + }); + + it.each([ + { security: "deny" as const, ask: "off" as const, expected: "deny" as const }, + { + security: "allowlist" as const, + ask: "off" as const, + expected: "allowlist" as const, + }, + { + security: "allowlist" as const, + ask: "on-miss" as const, + expected: "ask" as const, + }, + { security: "full" as const, ask: "off" as const, expected: "full" as const }, + { security: "full" as const, ask: "on-miss" as const, expected: "full" as const }, + { security: "full" as const, ask: "always" as const, expected: "ask" as const }, + ])("derives normalized exec mode from legacy policy %j", ({ security, ask, expected }) => { + expect(resolveExecModeFromPolicy({ security, ask })).toBe(expected); + }); + + it.each([ + { + mode: "deny" as const, + expected: { security: "deny" as const, ask: "off" as const, autoReview: false }, + }, + { + mode: "allowlist" as const, + expected: { security: "allowlist" as const, ask: "off" as const, autoReview: false }, + }, + { + mode: "ask" as const, + expected: { security: "allowlist" as const, ask: "on-miss" as const, autoReview: false }, + }, + { + mode: "auto" as const, + expected: { security: "allowlist" as const, ask: "on-miss" as const, autoReview: true }, + }, + { + mode: "full" as const, + expected: { security: "full" as const, ask: "off" as const, autoReview: false }, + }, + ])("maps explicit exec mode to effective policy %j", ({ mode, expected }) => { + expect(resolveExecPolicyForMode(mode)).toEqual(expected); + }); + + it("preserves legacy security and ask when no explicit mode is set", () => { + expect( + resolveExecModePolicy({ + security: "full", + ask: "always", + }), + ).toEqual({ + mode: "ask", + security: "full", + ask: "always", + autoReview: false, + }); + }); + it.each([ { left: "deny" as const, right: "full" as const, expected: "deny" as const }, { @@ -327,6 +402,126 @@ describe("exec approvals policy helpers", () => { }); }); + it("maps normalized requested mode into policy snapshots", () => { + const summary = resolveExecPolicyScopeSummary({ + approvals: { + version: 1, + }, + scopeExecConfig: { + mode: "auto", + }, + configPath: "tools.exec", + scopeLabel: "tools.exec", + }); + + expectFields(summary.mode, { + requested: "auto", + requestedSource: "tools.exec.mode", + effective: "auto", + note: "requested mode applies", + }); + expectFields(summary.security, { + requested: "allowlist", + requestedSource: "tools.exec.mode", + effective: "allowlist", + }); + expectFields(summary.ask, { + requested: "on-miss", + requestedSource: "tools.exec.mode", + effective: "on-miss", + }); + }); + + it("lets narrower legacy policy override a global normalized mode in snapshots", () => { + const summary = resolveExecPolicyScopeSummary({ + approvals: { + version: 1, + }, + globalExecConfig: { + mode: "deny", + }, + scopeExecConfig: { + security: "full", + ask: "off", + }, + configPath: "agents.list.runner.tools.exec", + scopeLabel: "agent:runner", + agentId: "runner", + }); + + expectFields(summary.mode, { + requested: "full", + requestedSource: + "derived from agents.list.runner.tools.exec.security and agents.list.runner.tools.exec.ask", + effective: "full", + }); + expectFields(summary.security, { + requested: "full", + requestedSource: "agents.list.runner.tools.exec.security", + effective: "full", + }); + }); + + it("preserves mode-derived siblings for partial narrower legacy policy snapshots", () => { + const summary = resolveExecPolicyScopeSummary({ + approvals: { + version: 1, + }, + globalExecConfig: { + mode: "auto", + }, + scopeExecConfig: { + ask: "off", + }, + configPath: "agents.list.runner.tools.exec", + scopeLabel: "agent:runner", + agentId: "runner", + }); + + expectFields(summary.security, { + requested: "allowlist", + requestedSource: "tools.exec.mode", + }); + expectFields(summary.ask, { + requested: "off", + requestedSource: "agents.list.runner.tools.exec.ask", + }); + expectFields(summary.mode, { + requested: "allowlist", + effective: "allowlist", + }); + }); + + it("reports full plus on-miss as full because on-miss only gates allowlist misses", () => { + const summary = resolveExecPolicyScopeSummary({ + approvals: { + version: 1, + }, + globalExecConfig: { + mode: "auto", + }, + scopeExecConfig: { + security: "full", + }, + configPath: "agents.list.runner.tools.exec", + scopeLabel: "agent:runner", + agentId: "runner", + }); + + expectFields(summary.security, { + requested: "full", + requestedSource: "agents.list.runner.tools.exec.security", + }); + expectFields(summary.ask, { + requested: "on-miss", + requestedSource: "tools.exec.mode", + }); + expectFields(summary.mode, { + requested: "full", + effective: "full", + }); + }); + it("uses the actual approvals path when reporting host sources", () => { const summary = resolveExecPolicyScopeSummary({ approvals: { diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index 8b30c6d1a7c..d5246db8ae4 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -10,7 +10,7 @@ import { } from "../shared/string-coerce.js"; import type { CommandExplanationSummary } from "./command-analysis/explain.js"; import { resolveAllowAlwaysPatternEntries } from "./exec-approvals-allowlist.js"; -import type { ExecCommandSegment } from "./exec-approvals-analysis.js"; +import { analyzeShellCommand, type ExecCommandSegment } from "./exec-approvals-analysis.js"; import type { ExecAllowlistEntry } from "./exec-approvals.types.js"; import { assertNoSymlinkParentsSync } from "./fs-safe-advanced.js"; import { expandHomePrefix, resolveRequiredHomeDir } from "./home-dir.js"; @@ -23,6 +23,7 @@ export type ExecHost = "sandbox" | "gateway" | "node"; export type ExecTarget = "auto" | ExecHost; export type ExecSecurity = "deny" | "allowlist" | "full"; export type ExecAsk = "off" | "on-miss" | "always"; +export type ExecMode = "deny" | "allowlist" | "ask" | "auto" | "full"; export const EXEC_TARGET_VALUES: readonly ExecTarget[] = ["auto", "sandbox", "gateway", "node"]; @@ -85,6 +86,82 @@ export function normalizeExecAsk(value?: string | null): ExecAsk | null { return null; } +export function normalizeExecMode(value?: string | null): ExecMode | null { + const normalized = normalizeOptionalLowercaseString(value); + if ( + normalized === "deny" || + normalized === "allowlist" || + normalized === "ask" || + normalized === "auto" || + normalized === "full" + ) { + return normalized; + } + return null; +} + +export function resolveExecModeFromPolicy(params: { + security: ExecSecurity; + ask: ExecAsk; +}): ExecMode { + if (params.security === "deny") { + return "deny"; + } + if (params.security === "allowlist" && params.ask === "off") { + return "allowlist"; + } + if (params.security === "full" && params.ask !== "always") { + return "full"; + } + return "ask"; +} + +export function resolveExecPolicyForMode(mode: ExecMode): { + security: ExecSecurity; + ask: ExecAsk; + autoReview: boolean; +} { + switch (mode) { + case "deny": + return { security: "deny", ask: "off", autoReview: false }; + case "allowlist": + return { security: "allowlist", ask: "off", autoReview: false }; + case "ask": + return { security: "allowlist", ask: "on-miss", autoReview: false }; + case "auto": + return { security: "allowlist", ask: "on-miss", autoReview: true }; + case "full": + return { security: "full", ask: "off", autoReview: false }; + } + const _exhaustive: never = mode; + void _exhaustive; + throw new Error("Unsupported exec mode"); +} + +export function resolveExecModePolicy(params: { + mode?: ExecMode | null; + security: ExecSecurity; + ask: ExecAsk; +}): { + mode: ExecMode; + security: ExecSecurity; + ask: ExecAsk; + autoReview: boolean; +} { + if (!params.mode) { + return { + mode: resolveExecModeFromPolicy({ security: params.security, ask: params.ask }), + security: params.security, + ask: params.ask, + autoReview: false, + }; + } + return { + mode: params.mode, + ...resolveExecPolicyForMode(params.mode), + }; +} + export type SystemRunApprovalBinding = { argv: string[]; cwd: string | null; @@ -1059,6 +1136,111 @@ export function requiresExecApproval(params: { ); } +function normalizeCommandName(value: string | undefined): string { + return (value ?? "").split(/[\\/]/).pop()?.toLowerCase() ?? ""; +} + +function textMentionsSecurityAuditSuppressions(value: string): boolean { + const normalized = value.toLowerCase(); + return ( + normalized.includes("security.audit.suppressions") || + /["']?security["']?[\s\S]{0,200}["']?audit["']?[\s\S]{0,200}["']?suppressions["']?/.test( + normalized, + ) + ); +} + +function isReadOnlySecurityAuditSuppressionInspection(argv: string[]): boolean { + const command = normalizeCommandName(argv[0]); + let offset = command === "pnpm" && argv[1] === "openclaw" ? 1 : 0; + if (normalizeCommandName(argv[offset]) !== "openclaw") { + return false; + } + offset += 1; + while (offset < argv.length) { + const arg = argv[offset]; + if (["--dev", "--no-color"].includes(arg ?? "")) { + offset += 1; + continue; + } + if (["--profile", "--container", "--log-level"].includes(arg ?? "")) { + offset += 2; + continue; + } + if ( + arg?.startsWith("--profile=") || + arg?.startsWith("--container=") || + arg?.startsWith("--log-level=") + ) { + offset += 1; + continue; + } + break; + } + return ( + argv[offset] === "config" && ["get", "schema", "validate"].includes(argv[offset + 1] ?? "") + ); +} + +function removeParsedSegmentText(command: string, segments: Array<{ raw?: string }>): string { + let remaining = command; + for (const segment of segments) { + const raw = segment.raw?.trim(); + if (!raw) { + continue; + } + remaining = remaining.replace(raw, " "); + } + return remaining; +} + +export function commandRequiresSecurityAuditSuppressionApproval(params: { + command: string; + cwd?: string; + env?: NodeJS.ProcessEnv; + segments: Array<{ argv: string[]; raw?: string }>; +}): boolean { + let sawSegmentMention = false; + for (const segment of params.segments) { + const segmentText = `${segment.raw ?? ""} ${segment.argv.join(" ")}`; + if (!textMentionsSecurityAuditSuppressions(segmentText)) { + continue; + } + sawSegmentMention = true; + if (!isReadOnlySecurityAuditSuppressionInspection(segment.argv)) { + return true; + } + } + if (sawSegmentMention) { + const rawAnalysis = analyzeShellCommand({ + command: params.command, + cwd: params.cwd, + env: params.env, + platform: process.platform, + }); + if (!rawAnalysis.ok) { + return textMentionsSecurityAuditSuppressions(params.command); + } + for (const segment of rawAnalysis.segments) { + if ( + textMentionsSecurityAuditSuppressions(`${segment.raw} ${segment.argv.join(" ")}`) && + !isReadOnlySecurityAuditSuppressionInspection(segment.argv) + ) { + return true; + } + } + if ( + textMentionsSecurityAuditSuppressions( + removeParsedSegmentText(params.command, rawAnalysis.segments), + ) + ) { + return true; + } + return false; + } + return textMentionsSecurityAuditSuppressions(params.command); +} + export function hasDurableExecApproval(params: { analysisOk: boolean; segmentAllowlistEntries: Array; diff --git a/src/infra/exec-auto-review.test.ts b/src/infra/exec-auto-review.test.ts new file mode 100644 index 00000000000..ab2633508c7 --- /dev/null +++ b/src/infra/exec-auto-review.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vitest"; +import { defaultExecAutoReviewer, type ExecAutoReviewInput } from "./exec-auto-review.js"; + +function reviewCommand(command: string, argv: string[]) { + return defaultExecAutoReviewer({ + command, + argv, + host: "gateway", + reason: "approval-required", + analysis: { + parsed: true, + allowlistMatched: false, + inlineEval: false, + }, + } satisfies ExecAutoReviewInput); +} + +describe("default exec auto reviewer", () => { + it("falls back to human approval instead of maintaining a static allowlist", () => { + expect(reviewCommand("pwd", ["pwd"])).toMatchObject({ + decision: "ask", + }); + }); + + it.each([ + ["./pwd", ["./pwd"]], + ["/tmp/pwd", ["/tmp/pwd"]], + ])("does not auto-approve path-qualified pwd lookalikes: %s", (_command, argv) => { + expect(reviewCommand(_command, argv)).toMatchObject({ + decision: "ask", + }); + }); + + it.each([ + ["cat ~/.openclaw/credentials/model.json", ["cat", "~/.openclaw/credentials/model.json"]], + ["rg token ~/.ssh", ["rg", "token", "~/.ssh"]], + ["sed -i s/foo/bar/g file.txt", ["sed", "-i", "s/foo/bar/g", "file.txt"]], + ["git status", ["git", "status"]], + ["git branch scratch", ["git", "branch", "scratch"]], + ["git diff --output=/tmp/diff.patch", ["git", "diff", "--output=/tmp/diff.patch"]], + ["git status\nnode -e 'console.log(1)'", ["git", "status"]], + ])( + "asks for human review on sensitive or externally influenced commands: %s", + (_command, argv) => { + expect(reviewCommand(_command, argv)).toMatchObject({ + decision: "ask", + }); + }, + ); +}); diff --git a/src/infra/exec-auto-review.ts b/src/infra/exec-auto-review.ts new file mode 100644 index 00000000000..bcf67eaf851 --- /dev/null +++ b/src/infra/exec-auto-review.ts @@ -0,0 +1,59 @@ +export type ExecAutoReviewRisk = "unknown" | "low" | "medium" | "high"; + +export type ExecAutoReviewDecision = + | { + decision: "allow-once"; + rationale: string; + risk: "low" | "medium" | "high"; + } + | { + decision: "ask"; + rationale: string; + risk: ExecAutoReviewRisk; + }; + +export type ExecAutoReviewHost = "gateway" | "node"; + +export type ExecAutoReviewInput = { + command: string; + argv?: readonly string[]; + cwd?: string | null; + envKeys?: readonly string[]; + host: ExecAutoReviewHost; + reason: + | "approval-required" + | "allowlist-miss" + | "strict-inline-eval" + | "heredoc" + | "execution-plan-miss"; + analysis: { + parsed: boolean; + allowlistMatched: boolean; + safeBinMatched?: boolean; + durableApprovalMatched?: boolean; + inlineEval: boolean; + heredoc?: boolean; + shellWrapper?: boolean; + }; + agent?: { + id?: string | null; + sessionKey?: string | null; + }; +}; + +export type ExecAutoReviewer = ( + input: ExecAutoReviewInput, +) => Promise | ExecAutoReviewDecision; + +/** + * Conservative fallback used when no model-backed reviewer is available. + * Auto mode must never become a static allowlist; without a reviewer, defer to + * the normal human approval route. + */ +export const defaultExecAutoReviewer: ExecAutoReviewer = (input) => { + return { + decision: "ask", + rationale: `no model-backed exec reviewer is configured for ${input.host}`, + risk: input.analysis.inlineEval ? "medium" : "unknown", + }; +}; diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index 23f15f2a522..63b02da47d8 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -24,6 +24,7 @@ import { resolveExecApprovalsPath, saveExecApprovals, } from "../infra/exec-approvals.js"; +import type { ExecAutoReviewer } from "../infra/exec-auto-review.js"; import type { ExecHostResponse } from "../infra/exec-host.js"; import { buildSystemRunApprovalPlan } from "./invoke-system-run-plan.js"; import { handleSystemRunInvoke } from "./invoke-system-run.js"; @@ -290,6 +291,14 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { }; } + function resolveProductionExecSecurity(value?: string): "deny" | "allowlist" | "full" { + return value === "deny" || value === "allowlist" || value === "full" ? value : "allowlist"; + } + + function resolveProductionExecAsk(value?: string): "off" | "on-miss" | "always" { + return value === "off" || value === "on-miss" || value === "always" ? value : "on-miss"; + } + function createInvokeSpies(params?: { runCommand?: MockedRunCommand }): { runCommand: MockedRunCommand; sendInvokeResult: MockedSendInvokeResult; @@ -450,6 +459,9 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { skillBinsCurrent?: () => Promise>; isCmdExeInvocation?: HandleSystemRunInvokeOptions["isCmdExeInvocation"]; sanitizeEnv?: HandleSystemRunInvokeOptions["sanitizeEnv"]; + resolveExecSecurity?: HandleSystemRunInvokeOptions["resolveExecSecurity"]; + resolveExecAsk?: HandleSystemRunInvokeOptions["resolveExecAsk"]; + autoReviewer?: ExecAutoReviewer; }): Promise<{ runCommand: MockedRunCommand; runViaMacAppExecHost: MockedRunViaMacAppExecHost; @@ -506,8 +518,8 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { }, execHostEnforced: false, execHostFallbackAllowed: true, - resolveExecSecurity: () => params.security ?? "full", - resolveExecAsk: () => params.ask ?? "off", + resolveExecSecurity: params.resolveExecSecurity ?? (() => params.security ?? "full"), + resolveExecAsk: params.resolveExecAsk ?? (() => params.ask ?? "off"), isCmdExeInvocation: params.isCmdExeInvocation ?? (() => false), sanitizeEnv: params.sanitizeEnv ?? (() => undefined), runCommand, @@ -518,6 +530,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { sendExecFinishedEvent, preferMacAppExecHost: params.preferMacAppExecHost, getRuntimeConfig: () => getRuntimeConfigSnapshot() ?? {}, + autoReviewer: params.autoReviewer, }); return { @@ -572,6 +585,192 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { ); }); + it("uses auto reviewer for system.run approval misses when exec mode is auto", async () => { + const tmp = createFixtureDir("openclaw-system-run-auto-review-"); + const executablePath = createTempExecutable({ dir: tmp, name: "read-info" }); + setRuntimeConfigSnapshot({ + tools: { + exec: { + mode: "auto", + }, + }, + }); + try { + const autoReviewer = vi.fn(() => ({ + decision: "allow-once", + rationale: "reads fixture metadata only", + risk: "low", + })); + const runCommand = vi.fn(async () => createLocalRunResult("auto-reviewed")); + const prepared = buildSystemRunApprovalPlan({ + command: [executablePath], + cwd: tmp, + }); + expect(prepared.ok).toBe(true); + if (!prepared.ok) { + throw new Error("unreachable"); + } + const invoke = await runSystemInvoke({ + preferMacAppExecHost: false, + command: prepared.plan.argv, + cwd: prepared.plan.cwd ?? tmp, + systemRunPlan: prepared.plan, + runCommand, + resolveExecSecurity: resolveProductionExecSecurity, + resolveExecAsk: resolveProductionExecAsk, + autoReviewer, + }); + + expect(autoReviewer).toHaveBeenCalledTimes(1); + expect(autoReviewer).toHaveBeenCalledWith( + expect.objectContaining({ + command: executablePath, + argv: [executablePath], + cwd: tmp, + host: "node", + reason: "approval-required", + analysis: expect.objectContaining({ + parsed: true, + allowlistMatched: false, + inlineEval: false, + }), + }), + ); + expect(runCommand).toHaveBeenCalledTimes(1); + expectInvokeOk(invoke.sendInvokeResult, { payloadContains: "auto-reviewed" }); + } finally { + clearRuntimeConfigSnapshot(); + } + }); + + it("does not auto-review direct system.run approval misses without an approval plan", async () => { + const tmp = createFixtureDir("openclaw-system-run-auto-review-no-plan-"); + const executablePath = createTempExecutable({ dir: tmp, name: "read-info" }); + setRuntimeConfigSnapshot({ + tools: { + exec: { + mode: "auto", + }, + }, + }); + try { + const autoReviewer = vi.fn(() => ({ + decision: "allow-once", + rationale: "reads fixture metadata only", + risk: "low", + })); + const runCommand = vi.fn(async () => createLocalRunResult("should-not-run")); + const invoke = await runSystemInvoke({ + preferMacAppExecHost: false, + command: [executablePath], + cwd: tmp, + runCommand, + resolveExecSecurity: resolveProductionExecSecurity, + resolveExecAsk: resolveProductionExecAsk, + autoReviewer, + }); + + expect(autoReviewer).not.toHaveBeenCalled(); + expect(runCommand).not.toHaveBeenCalled(); + expectInvokeErrorMessage(invoke.sendInvokeResult, { + message: "SYSTEM_RUN_DENIED: approval required", + }); + } finally { + clearRuntimeConfigSnapshot(); + } + }); + + it("does not auto-review direct system.run security audit suppression edits", async () => { + const tmp = createFixtureDir("openclaw-system-run-auto-review-suppression-"); + setRuntimeConfigSnapshot({ + tools: { + exec: { + mode: "auto", + }, + }, + }); + try { + const autoReviewer = vi.fn(() => ({ + decision: "allow-once", + rationale: "test reviewer would allow it", + risk: "low", + })); + const runCommand = vi.fn(async () => createLocalRunResult("should-not-run")); + const prepared = buildSystemRunApprovalPlan({ + command: ["openclaw", "config", "set", "security.audit.suppressions", "[]"], + cwd: tmp, + }); + expect(prepared.ok).toBe(true); + if (!prepared.ok) { + throw new Error("unreachable"); + } + const invoke = await runSystemInvoke({ + preferMacAppExecHost: false, + command: prepared.plan.argv, + cwd: prepared.plan.cwd ?? tmp, + systemRunPlan: prepared.plan, + runCommand, + resolveExecSecurity: resolveProductionExecSecurity, + resolveExecAsk: resolveProductionExecAsk, + autoReviewer, + }); + + expect(autoReviewer).not.toHaveBeenCalled(); + expect(runCommand).not.toHaveBeenCalled(); + expectInvokeErrorMessage(invoke.sendInvokeResult, { + message: "SYSTEM_RUN_DENIED: approval required", + }); + } finally { + clearRuntimeConfigSnapshot(); + } + }); + + it("defers to human approval when system.run auto reviewer asks", async () => { + const tmp = createFixtureDir("openclaw-system-run-auto-review-ask-"); + const executablePath = createTempExecutable({ dir: tmp, name: "read-info" }); + setRuntimeConfigSnapshot({ + tools: { + exec: { + mode: "auto", + }, + }, + }); + try { + const autoReviewer = vi.fn(() => ({ + decision: "ask", + rationale: "needs a person", + risk: "medium", + })); + const runCommand = vi.fn(async () => createLocalRunResult("should-not-run")); + const prepared = buildSystemRunApprovalPlan({ + command: [executablePath], + cwd: tmp, + }); + expect(prepared.ok).toBe(true); + if (!prepared.ok) { + throw new Error("unreachable"); + } + const invoke = await runSystemInvoke({ + preferMacAppExecHost: false, + command: prepared.plan.argv, + cwd: prepared.plan.cwd ?? tmp, + systemRunPlan: prepared.plan, + runCommand, + resolveExecSecurity: resolveProductionExecSecurity, + resolveExecAsk: resolveProductionExecAsk, + autoReviewer, + }); + + expect(autoReviewer).toHaveBeenCalledTimes(1); + expect(runCommand).not.toHaveBeenCalled(); + expectInvokeErrorMessage(invoke.sendInvokeResult, { + message: "exec auto-review deferred to human approval", + }); + } finally { + clearRuntimeConfigSnapshot(); + } + }); + const approvedEnvShellWrapperCases = [ { name: "preserves wrapper argv for approved env shell commands in local execution", diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index 67e840841ab..49cd3352719 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -8,18 +8,25 @@ import { import { detectPolicyInlineEval } from "../infra/command-analysis/policy.js"; import { addDurableCommandApproval, + commandRequiresSecurityAuditSuppressionApproval, hasDurableExecApproval, + maxAsk, + minSecurity, persistAllowAlwaysPatterns, recordAllowlistMatchesUse, resolveApprovalAuditTrustPath, resolveExecApprovals, + resolveExecModePolicy, + resolveExecPolicyForMode, type ExecAllowlistEntry, type ExecAsk, type ExecCommandSegment, + type ExecMode, type ExecSegmentSatisfiedBy, type ExecSecurity, type SkillBinTrustEntry, } from "../infra/exec-approvals.js"; +import type { ExecAutoReviewer } from "../infra/exec-auto-review.js"; import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../infra/exec-host.js"; import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js"; import { @@ -106,6 +113,7 @@ type SystemRunParsePhase = { type SystemRunPolicyPhase = SystemRunParsePhase & { approvals: ResolvedExecApprovals; security: ExecSecurity; + ask: ExecAsk; policy: ReturnType; durableApprovalSatisfied: boolean; strictInlineEval: boolean; @@ -134,6 +142,35 @@ const APPROVAL_SCRIPT_OPERAND_DRIFT_DENIED_MESSAGE = "SYSTEM_RUN_DENIED: approval script operand changed before execution"; type ExecToolConfig = NonNullable["exec"]>; +type LayeredExecPolicy = { + mode?: ExecMode; + security: ExecSecurity; + ask: ExecAsk; +}; + +function hasLegacyExecPolicyOverride(exec?: ExecToolConfig): boolean { + return exec?.security !== undefined || exec?.ask !== undefined; +} + +function applyExecPolicyLayer(base: LayeredExecPolicy, layer?: ExecToolConfig): LayeredExecPolicy { + if (!layer) { + return base; + } + if (layer.mode) { + return { + mode: layer.mode, + ...resolveExecPolicyForMode(layer.mode), + }; + } + if (hasLegacyExecPolicyOverride(layer)) { + return { + security: layer.security ?? base.security, + ask: layer.ask ?? base.ask, + }; + } + return base; +} + function warnWritableTrustedDirOnce(message: string): void { if (safeBinTrustedDirWarningCache.has(message)) { return; @@ -173,6 +210,24 @@ function resolveAgentExecConfig( return entry?.tools?.exec; } +async function resolveSystemRunAutoReviewer(params: { + opts: HandleSystemRunInvokeOptions; + cfg: OpenClawConfig; + agentId: string | undefined; + agentExec: ExecToolConfig | undefined; + globalExec: ExecToolConfig | undefined; +}): Promise { + if (params.opts.autoReviewer) { + return params.opts.autoReviewer; + } + const { createModelExecAutoReviewer } = await import("../agents/exec-auto-reviewer.js"); + return createModelExecAutoReviewer({ + cfg: params.cfg, + agentId: params.agentId, + reviewer: params.agentExec?.reviewer ?? params.globalExec?.reviewer, + }); +} + export type HandleSystemRunInvokeOptions = { client: GatewayClient; params: SystemRunParams; @@ -199,6 +254,7 @@ export type HandleSystemRunInvokeOptions = { sendExecFinishedEvent: (params: ExecFinishedEventParams) => Promise; preferMacAppExecHost: boolean; getRuntimeConfig?: () => OpenClawConfig; + autoReviewer?: ExecAutoReviewer; }; async function loadSystemRunConfig(opts: HandleSystemRunInvokeOptions): Promise { @@ -257,6 +313,14 @@ async function sendSystemRunCompleted( }); } +function argvArraysMatch(left: readonly string[] | undefined, right: readonly string[]): boolean { + return ( + left !== undefined && + left.length === right.length && + left.every((entry, index) => entry === right[index]) + ); +} + export { buildSystemRunApprovalPlan } from "./invoke-system-run-plan.js"; async function parseSystemRunPhase( @@ -380,17 +444,31 @@ async function evaluateSystemRunPolicyPhase( ): Promise { const cfg = await loadSystemRunConfig(opts); const agentExec = resolveAgentExecConfig(cfg, parsed.agentId); - const configuredSecurity = opts.resolveExecSecurity( - agentExec?.security ?? cfg.tools?.exec?.security, + const globalExec = cfg.tools?.exec; + const layeredPolicy = applyExecPolicyLayer( + applyExecPolicyLayer( + { + security: opts.resolveExecSecurity(undefined), + ask: opts.resolveExecAsk(undefined), + }, + globalExec, + ), + agentExec, ); - const configuredAsk = opts.resolveExecAsk(agentExec?.ask ?? cfg.tools?.exec?.ask); + const modePolicy = resolveExecModePolicy({ + mode: layeredPolicy.mode, + security: layeredPolicy.security, + ask: layeredPolicy.ask, + }); + const configuredSecurity = modePolicy.security; + const configuredAsk = modePolicy.ask; const approvals = resolveExecApprovals(parsed.agentId, { security: configuredSecurity, ask: configuredAsk, requireSocket: opts.preferMacAppExecHost, }); - const security = approvals.agent.security; - const ask = approvals.agent.ask; + const security = minSecurity(configuredSecurity, approvals.agent.security); + const ask = maxAsk(configuredAsk, approvals.agent.ask); const autoAllowSkills = approvals.agent.autoAllowSkills; const { safeBins, safeBinProfiles, trustedSafeBinDirs } = resolveExecSafeBinRuntimePolicy({ global: cfg.tools?.exec, @@ -436,13 +514,14 @@ async function evaluateSystemRunPolicyPhase( const inlineEvalExecutableTrusted = inlineEvalHit !== null && segmentAllowlistEntries.some((entry) => entry?.source === "allow-always"); - const policy = evaluateSystemRunPolicy({ + let approvalDecision = parsed.approvalDecision; + let policy = evaluateSystemRunPolicy({ security, ask, analysisOk, allowlistSatisfied, durableApprovalSatisfied: durableApprovalSatisfied || inlineEvalExecutableTrusted, - approvalDecision: parsed.approvalDecision, + approvalDecision, approved: parsed.approved, isWindows, cmdInvocation, @@ -450,6 +529,28 @@ async function evaluateSystemRunPolicyPhase( // Env sanitization uses broader shell-wrapper detection in parse phase. shellWrapperInvocation: parsed.shellPayload !== null, }); + const requiresSecurityAuditSuppressionApproval = + commandRequiresSecurityAuditSuppressionApproval({ + command: parsed.commandText, + cwd: parsed.cwd, + env: parsed.env, + segments, + }) && !(security === "full" && ask === "off"); + if (requiresSecurityAuditSuppressionApproval && !policy.approvedByAsk) { + policy = { + allowed: false, + eventReason: "approval-required", + errorMessage: "SYSTEM_RUN_DENIED: approval required", + analysisOk: policy.analysisOk, + allowlistSatisfied: policy.allowlistSatisfied, + shellWrapperBlocked: policy.shellWrapperBlocked, + windowsShellWrapperBlocked: policy.windowsShellWrapperBlocked, + requiresAsk: true, + approvalDecision: policy.approvalDecision, + approvedByAsk: policy.approvedByAsk, + }; + } + let autoReviewDeferredMessage: string | undefined; analysisOk = policy.analysisOk; allowlistSatisfied = policy.allowlistSatisfied; const strictInlineEvalRequiresApproval = @@ -466,10 +567,78 @@ async function evaluateSystemRunPolicyPhase( return null; } + if (!policy.allowed) { + const [autoReviewSegment] = segments; + const directAutoReviewArgvMatchesRequest = + parsed.shellPayload !== null || argvArraysMatch(autoReviewSegment?.argv, parsed.argv); + const autoReviewArgv = + segments.length === 1 && + directAutoReviewArgvMatchesRequest && + (parsed.shellPayload === null || + (autoReviewSegment?.raw !== undefined && + autoReviewSegment.raw.trim() === parsed.shellPayload.trim())) + ? autoReviewSegment?.argv + : undefined; + const canAutoReviewApprovalMiss = + modePolicy.autoReview && + ask !== "always" && + analysisOk && + autoReviewArgv !== undefined && + parsed.approvalPlan !== null && + inlineEvalHit === null && + !requiresSecurityAuditSuppressionApproval && + policy.eventReason !== "security=deny"; + if (canAutoReviewApprovalMiss) { + const reviewer = await resolveSystemRunAutoReviewer({ + opts, + cfg, + agentId: parsed.agentId, + agentExec, + globalExec, + }); + const decision = await reviewer({ + command: parsed.commandText, + argv: autoReviewArgv, + cwd: parsed.cwd, + envKeys: Object.keys(parsed.envOverrides ?? {}).toSorted(), + host: "node", + reason: policy.eventReason === "allowlist-miss" ? "allowlist-miss" : "approval-required", + analysis: { + parsed: analysisOk, + allowlistMatched: allowlistSatisfied, + durableApprovalMatched: durableApprovalSatisfied, + inlineEval: false, + shellWrapper: parsed.shellWrapperInvocation, + }, + agent: { + id: parsed.agentId, + sessionKey: parsed.sessionKey, + }, + }); + if (decision.decision === "allow-once") { + approvalDecision = "allow-once"; + policy = evaluateSystemRunPolicy({ + security, + ask, + analysisOk, + allowlistSatisfied, + durableApprovalSatisfied: durableApprovalSatisfied || inlineEvalExecutableTrusted, + approvalDecision, + approved: true, + isWindows, + cmdInvocation, + shellWrapperInvocation: parsed.shellPayload !== null, + }); + } else { + autoReviewDeferredMessage = `${policy.errorMessage} (exec auto-review deferred to human approval: ${decision.rationale})`; + } + } + } + if (!policy.allowed) { await sendSystemRunDenied(opts, parsed.execution, { reason: policy.eventReason, - message: policy.errorMessage, + message: autoReviewDeferredMessage ?? policy.errorMessage, }); return null; } @@ -520,10 +689,12 @@ async function evaluateSystemRunPolicyPhase( } return { ...parsed, + approvalDecision, argv: hardenedPaths.argv, cwd: hardenedPaths.cwd, approvals, security, + ask, policy, durableApprovalSatisfied, strictInlineEval, diff --git a/src/plugin-sdk/exec-approvals-runtime.ts b/src/plugin-sdk/exec-approvals-runtime.ts new file mode 100644 index 00000000000..07720e246ce --- /dev/null +++ b/src/plugin-sdk/exec-approvals-runtime.ts @@ -0,0 +1,7 @@ +// Exec approval policy file helpers without the broad infra-runtime barrel. + +export { + loadExecApprovals, + resolveExecApprovalsFromFile, + type ExecApprovalsFile, +} from "../infra/exec-approvals.js"; diff --git a/src/plugins/contracts/extension-package-project-boundaries.test.ts b/src/plugins/contracts/extension-package-project-boundaries.test.ts index 0de4bb45778..105595d3d1b 100644 --- a/src/plugins/contracts/extension-package-project-boundaries.test.ts +++ b/src/plugins/contracts/extension-package-project-boundaries.test.ts @@ -221,6 +221,9 @@ describe("opt-in extension package boundaries", () => { expect(packageJson.exports?.["./error-runtime"]?.types).toBe( "./dist/src/plugin-sdk/error-runtime.d.ts", ); + expect(packageJson.exports?.["./exec-approvals-runtime"]?.types).toBe( + "./dist/src/plugin-sdk/exec-approvals-runtime.d.ts", + ); expect(packageJson.exports?.["./plugin-entry"]?.types).toBe( "./dist/src/plugin-sdk/plugin-entry.d.ts", );