From df0074768c97326b582b3431655e047dd65fe9e5 Mon Sep 17 00:00:00 2001 From: Vyctor Huggo Przozwski da Silva <51521767+vyctorbrzezowski@users.noreply.github.com> Date: Wed, 29 Apr 2026 13:25:45 -0300 Subject: [PATCH] fix(exec): reject invalid host targets (#74468) * fix(exec): reject invalid host targets * docs(changelog): credit exec host validation contributor --------- Co-authored-by: Peter Steinberger --- CHANGELOG.md | 1 + docs/tools/exec.md | 1 + ...ash-tools.exec-foreground-failures.test.ts | 28 +++++++++++++++++++ src/agents/bash-tools.exec.ts | 5 ++-- src/agents/bash-tools.schemas.ts | 11 ++++---- src/infra/exec-approvals-policy.test.ts | 13 +++++++++ src/infra/exec-approvals.ts | 26 +++++++++++++++++ 7 files changed, 78 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40d25a6836f..bfd86674940 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Exec: reject invalid per-call `host` values instead of silently falling back to the default target, so hostname-like values fail before commands run. Fixes #74426. Thanks @scr00ge-00 and @vyctorbrzezowski. - Build/Gateway: route restart, shutdown, respawn, diagnostics, command-queue cleanup, and runtime cleanup through one stable gateway lifecycle runtime entry so rebuilt packages do not strand long-running gateways on stale hashed chunks. Carries forward #73964. Thanks @pashpashpash. - Memory/wiki: keep broad shared-source and generated related-link blocks from turning every page into a search hit, cap noisy backlinks, support all-term searches such as people-routing queries, and prefer readable page body snippets over generated metadata. Thanks @vincentkoc. - Cron/Gateway: abort and bounded-clean up timed-out isolated agent turns before recording the timeout, so stale cron sessions cannot leave Discord or other chat lanes stuck in `processing` after a timeout. Thanks @vincentkoc. diff --git a/docs/tools/exec.md b/docs/tools/exec.md index d8fcafcd1d5..6d449f69754 100644 --- a/docs/tools/exec.md +++ b/docs/tools/exec.md @@ -63,6 +63,7 @@ Request elevated mode — escape the sandbox onto the configured host path. `sec 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. - 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. diff --git a/src/agents/bash-tools.exec-foreground-failures.test.ts b/src/agents/bash-tools.exec-foreground-failures.test.ts index de7bce9dd07..4ca357b67de 100644 --- a/src/agents/bash-tools.exec-foreground-failures.test.ts +++ b/src/agents/bash-tools.exec-foreground-failures.test.ts @@ -50,4 +50,32 @@ describe("exec foreground failures", () => { }); expect((result.details as { durationMs?: number }).durationMs).toEqual(expect.any(Number)); }); + + it("rejects invalid host values before launching a command", async () => { + const tool = createExecTool({ + security: "full", + ask: "off", + allowBackground: false, + }); + for (const testCase of [ + { + host: "spark-ff13", + message: 'Invalid exec host "spark-ff13". Allowed values: auto, sandbox, gateway, node.', + }, + { + host: 42, + message: + "Invalid exec host value type number. Allowed values: auto, sandbox, gateway, node.", + }, + ]) { + const malformedArgs = { + command: "echo should-not-run", + host: testCase.host, + } as unknown as Parameters[1]; + + await expect(tool.execute("call-invalid-host", malformedArgs)).rejects.toThrow( + testCase.message, + ); + } + }); }); diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index b7d9b9493d9..1a3dfd1c31d 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -8,6 +8,7 @@ import { loadExecApprovals, maxAsk, minSecurity, + requireValidExecTarget, } from "../infra/exec-approvals.js"; import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js"; import { sanitizeHostExecEnvWithDiagnostics } from "../infra/host-env-security.js"; @@ -38,7 +39,6 @@ import { applyShellPath, normalizeExecAsk, normalizeExecSecurity, - normalizeExecTarget, normalizePathPrepend, resolveExecTarget, resolveApprovalRunningNoticeMs, @@ -1543,9 +1543,10 @@ export function createExecTool( if (elevatedRequested) { logInfo(`exec: elevated command ${truncateMiddle(params.command, 120)}`); } + const requestedTarget = requireValidExecTarget(params.host); const target = resolveExecTarget({ configuredTarget: defaults?.host, - requestedTarget: normalizeExecTarget(params.host), + requestedTarget, elevatedRequested, sandboxAvailable: Boolean(defaults?.sandbox), }); diff --git a/src/agents/bash-tools.schemas.ts b/src/agents/bash-tools.schemas.ts index a4ededcbc29..0251736b6b5 100644 --- a/src/agents/bash-tools.schemas.ts +++ b/src/agents/bash-tools.schemas.ts @@ -1,4 +1,7 @@ import { Type } from "typebox"; +import { optionalStringEnum } from "./schema/typebox.js"; + +const EXEC_TOOL_HOST_VALUES = ["auto", "sandbox", "gateway", "node"] as const; export const execSchema = Type.Object({ command: Type.String({ description: "Shell command to execute" }), @@ -26,11 +29,9 @@ export const execSchema = Type.Object({ description: "Run on the host with elevated permissions (if allowed)", }), ), - host: Type.Optional( - Type.String({ - description: "Exec host/target (auto|sandbox|gateway|node).", - }), - ), + host: optionalStringEnum(EXEC_TOOL_HOST_VALUES, { + description: "Exec host/target (auto|sandbox|gateway|node).", + }), security: Type.Optional( Type.String({ description: "Exec security mode (deny|allowlist|full).", diff --git a/src/infra/exec-approvals-policy.test.ts b/src/infra/exec-approvals-policy.test.ts index b6787d7f4a9..bae43151ca6 100644 --- a/src/infra/exec-approvals-policy.test.ts +++ b/src/infra/exec-approvals-policy.test.ts @@ -14,6 +14,7 @@ import { hasDurableExecApproval, maxAsk, minSecurity, + requireValidExecTarget, type ExecApprovalsFile, normalizeExecAsk, normalizeExecHost, @@ -73,6 +74,18 @@ describe("exec approvals policy helpers", () => { expect(normalizeExecTarget(raw)).toBe(expected); }); + it("requires direct exec target requests to use the closed host set", () => { + expect(requireValidExecTarget(" gateway ")).toBe("gateway"); + expect(requireValidExecTarget("")).toBe(null); + expect(requireValidExecTarget(undefined)).toBe(null); + expect(() => requireValidExecTarget("spark-ff13")).toThrow( + 'Invalid exec host "spark-ff13". Allowed values: auto, sandbox, gateway, node.', + ); + expect(() => requireValidExecTarget(42)).toThrow( + "Invalid exec host value type number. Allowed values: auto, sandbox, gateway, node.", + ); + }); + it.each([ { raw: " allowlist ", expected: "allowlist" }, { raw: "FULL", expected: "full" }, diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index 2de6bbc741a..e74d406acf4 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -22,6 +22,8 @@ export type ExecTarget = "auto" | ExecHost; export type ExecSecurity = "deny" | "allowlist" | "full"; export type ExecAsk = "off" | "on-miss" | "always"; +export const EXEC_TARGET_VALUES: readonly ExecTarget[] = ["auto", "sandbox", "gateway", "node"]; + export function normalizeExecHost(value?: string | null): ExecHost | null { const normalized = normalizeOptionalLowercaseString(value); if (normalized === "sandbox" || normalized === "gateway" || normalized === "node") { @@ -38,6 +40,30 @@ export function normalizeExecTarget(value?: string | null): ExecTarget | null { return normalizeExecHost(normalized); } +export function requireValidExecTarget(value?: unknown): ExecTarget | null { + if (value == null) { + return null; + } + if (typeof value !== "string") { + throw new Error( + `Invalid exec host value type ${typeof value}. Allowed values: ${EXEC_TARGET_VALUES.join( + ", ", + )}.`, + ); + } + const normalized = normalizeOptionalLowercaseString(value); + if (!normalized) { + return null; + } + const target = normalizeExecTarget(normalized); + if (target) { + return target; + } + throw new Error( + `Invalid exec host "${value}". Allowed values: ${EXEC_TARGET_VALUES.join(", ")}.`, + ); +} + /** Coerce a raw JSON field to string, returning undefined for non-string types. */ const toStringOrUndefined = readStringValue;