mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string, unknown> })?.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);
|
||||
|
||||
@@ -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),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -294,6 +294,24 @@ describe("exec approval forwarder", () => {
|
||||
expect(text).toContain("Reply with: /approve <id> 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 });
|
||||
|
||||
Reference in New Issue
Block a user