From 2504cb6a1e0dc8db9a52428945b9294174b69232 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 12 Mar 2026 01:20:04 -0400 Subject: [PATCH] Security: escape invisible exec approval format chars (#43687) * Infra: escape invisible exec approval chars * Gateway: sanitize exec approval display text * Tests: cover sanitized exec approval payloads * Tests: cover sanitized exec approval forwarding * Changelog: note exec approval prompt hardening --- CHANGELOG.md | 3 ++ src/gateway/server-methods/exec-approval.ts | 8 +++- .../server-methods/server-methods.test.ts | 28 ++++++++++++++ src/infra/exec-approval-command-display.ts | 38 ++++++++++++------- src/infra/exec-approval-forwarder.test.ts | 18 +++++++++ 5 files changed, 80 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14e3e890270..7bc48b65841 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ Docs: https://docs.openclaw.ai ## Unreleased +### Security +- Security/exec approvals: escape invisible Unicode format characters in approval prompts so zero-width command text renders as visible `\u{...}` escapes instead of spoofing the reviewed command. (#43687) Thanks @EkiXu and @vincentkoc. + ### Changes ### Fixes diff --git a/src/gateway/server-methods/exec-approval.ts b/src/gateway/server-methods/exec-approval.ts index 07dd8546c3f..81d479cbbd6 100644 --- a/src/gateway/server-methods/exec-approval.ts +++ b/src/gateway/server-methods/exec-approval.ts @@ -1,3 +1,4 @@ +import { sanitizeExecApprovalDisplayText } from "../../infra/exec-approval-command-display.js"; import type { ExecApprovalForwarder } from "../../infra/exec-approval-forwarder.js"; import { DEFAULT_EXEC_APPROVAL_TIMEOUT_MS, @@ -125,8 +126,11 @@ export function createExecApprovalHandlers( return; } const request = { - command: effectiveCommandText, - commandPreview: host === "node" ? undefined : approvalContext.commandPreview, + command: sanitizeExecApprovalDisplayText(effectiveCommandText), + commandPreview: + host === "node" || !approvalContext.commandPreview + ? undefined + : sanitizeExecApprovalDisplayText(approvalContext.commandPreview), commandArgv: host === "node" ? undefined : effectiveCommandArgv, envKeys: systemRunBinding?.envKeys?.length ? systemRunBinding.envKeys : undefined, systemRunBinding: systemRunBinding?.binding ?? null, diff --git a/src/gateway/server-methods/server-methods.test.ts b/src/gateway/server-methods/server-methods.test.ts index 51da6927f5e..424511370cd 100644 --- a/src/gateway/server-methods/server-methods.test.ts +++ b/src/gateway/server-methods/server-methods.test.ts @@ -641,6 +641,34 @@ describe("exec approval handlers", () => { ); }); + it("sanitizes invisible Unicode format chars in approval display text without changing node bindings", async () => { + const { handlers, broadcasts, respond, context } = createExecApprovalFixture(); + await requestExecApproval({ + handlers, + respond, + context, + params: { + timeoutMs: 10, + command: "bash safe\u200B.sh", + commandArgv: ["bash", "safe\u200B.sh"], + systemRunPlan: { + argv: ["bash", "safe\u200B.sh"], + cwd: "/real/cwd", + commandText: "bash safe\u200B.sh", + agentId: "main", + sessionKey: "agent:main:main", + }, + }, + }); + const requested = broadcasts.find((entry) => entry.event === "exec.approval.requested"); + expect(requested).toBeTruthy(); + const request = (requested?.payload as { request?: Record })?.request ?? {}; + expect(request["command"]).toBe("bash safe\\u{200B}.sh"); + expect((request["systemRunPlan"] as { commandText?: string }).commandText).toBe( + "bash safe\u200B.sh", + ); + }); + it("accepts resolve during broadcast", async () => { const manager = new ExecApprovalManager(); const handlers = createExecApprovalHandlers(manager); diff --git a/src/infra/exec-approval-command-display.ts b/src/infra/exec-approval-command-display.ts index b5b00625ef2..9ab62e55669 100644 --- a/src/infra/exec-approval-command-display.ts +++ b/src/infra/exec-approval-command-display.ts @@ -1,8 +1,22 @@ import type { ExecApprovalRequestPayload } from "./exec-approvals.js"; +const UNICODE_FORMAT_CHAR_REGEX = /\p{Cf}/gu; + +function formatCodePointEscape(char: string): string { + return `\\u{${char.codePointAt(0)?.toString(16).toUpperCase() ?? "FFFD"}}`; +} + +export function sanitizeExecApprovalDisplayText(commandText: string): string { + return commandText.replace(UNICODE_FORMAT_CHAR_REGEX, formatCodePointEscape); +} + function normalizePreview(commandText: string, commandPreview?: string | null): string | null { - const preview = commandPreview?.trim() ?? ""; - if (!preview || preview === commandText) { + const previewRaw = commandPreview?.trim() ?? ""; + if (!previewRaw) { + return null; + } + const preview = sanitizeExecApprovalDisplayText(previewRaw); + if (preview === commandText) { return null; } return preview; @@ -12,17 +26,15 @@ export function resolveExecApprovalCommandDisplay(request: ExecApprovalRequestPa commandText: string; commandPreview: string | null; } { - if (request.host === "node" && request.systemRunPlan) { - return { - commandText: request.systemRunPlan.commandText, - commandPreview: normalizePreview( - request.systemRunPlan.commandText, - request.systemRunPlan.commandPreview, - ), - }; - } + const commandTextSource = + request.command || + (request.host === "node" && request.systemRunPlan ? request.systemRunPlan.commandText : ""); + const commandText = sanitizeExecApprovalDisplayText(commandTextSource); + const previewSource = + request.commandPreview ?? + (request.host === "node" ? (request.systemRunPlan?.commandPreview ?? null) : null); return { - commandText: request.command, - commandPreview: normalizePreview(request.command, request.commandPreview), + commandText, + commandPreview: normalizePreview(commandText, previewSource), }; } diff --git a/src/infra/exec-approval-forwarder.test.ts b/src/infra/exec-approval-forwarder.test.ts index 8ae1b53cc57..ca4d81e012e 100644 --- a/src/infra/exec-approval-forwarder.test.ts +++ b/src/infra/exec-approval-forwarder.test.ts @@ -294,6 +294,24 @@ describe("exec approval forwarder", () => { expect(text).toContain("Reply with: /approve allow-once|allow-always|deny"); }); + it("renders invisible Unicode format chars as visible escapes", async () => { + vi.useFakeTimers(); + const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG }); + + await expect( + forwarder.handleRequested({ + ...baseRequest, + request: { + ...baseRequest.request, + command: "bash safe\u200B.sh", + }, + }), + ).resolves.toBe(true); + await Promise.resolve(); + + expect(getFirstDeliveryText(deliver)).toContain("Command: `bash safe\\u{200B}.sh`"); + }); + it("formats complex commands as fenced code blocks", async () => { vi.useFakeTimers(); const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG });