mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:10:44 +00:00
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 <steipete@gmail.com>
This commit is contained in:
committed by
GitHub
parent
9a0b43c47e
commit
df0074768c
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<typeof tool.execute>[1];
|
||||
|
||||
await expect(tool.execute("call-invalid-host", malformedArgs)).rejects.toThrow(
|
||||
testCase.message,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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),
|
||||
});
|
||||
|
||||
@@ -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).",
|
||||
|
||||
@@ -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" },
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user