vultr/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-05-06 16:40:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(codex): sanitize approval preview text (#70569)

Browse Source 
Harden Codex app-server approval preview text sanitization and truncation handling.

Thanks @Lucenx9.

Co-authored-by: Lucenx9 <185146821+Lucenx9@users.noreply.github.com>
This commit is contained in:
Simone
2026-04-24 06:23:29 +02:00
committed by GitHub
parent d3dd61f2c5
commit ed6094b301
2 changed files with 392 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

249
extensions/codex/src/app-server/approval-bridge.test.ts
View File

@@ -119,6 +119,185 @@ describe("Codex app-server approval bridge", () => {
);
});
it("sanitizes command previews before forwarding approval text and events", async () => {
const params = createParams();
mockCallGatewayTool.mockResolvedValueOnce({
id: "plugin:approval-sanitized-command",
decision: "allow-once",
});
await handleCodexAppServerApprovalRequest({
method: "item/commandExecution/requestApproval",
requestParams: {
threadId: "thread-1",
turnId: "turn-1",
itemId: "cmd-sanitized",
command: ["pnpm", "test\n--watch", "\u001b[31mextensions/codex/src/app-server\u001b[0m"],
},
paramsForRun: params,
threadId: "thread-1",
turnId: "turn-1",
});
const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? [];
expect(requestPayload).toEqual(
expect.objectContaining({
description:
"Command: pnpm test --watch extensions/codex/src/app-server\nSession: agent:main:session-1",
}),
);
expect(params.onAgentEvent).toHaveBeenCalledWith(
expect.objectContaining({
stream: "approval",
data: expect.objectContaining({
status: "pending",
command: "pnpm test --watch extensions/codex/src/app-server",
}),
}),
);
});
it("preserves visible OSC-8 link labels in command previews", async () => {
const params = createParams();
mockCallGatewayTool.mockResolvedValueOnce({
id: "plugin:approval-osc",
decision: "allow-once",
});
const esc = "\u001b";
await handleCodexAppServerApprovalRequest({
method: "item/commandExecution/requestApproval",
requestParams: {
threadId: "thread-1",
turnId: "turn-1",
itemId: "cmd-osc",
command: `prefix ${esc}]8;;https://example.com${esc}\\VISIBLE${esc}]8;;${esc}\\ suffix`,
},
paramsForRun: params,
threadId: "thread-1",
turnId: "turn-1",
});
const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? [];
expect(requestPayload).toEqual(
expect.objectContaining({
description: "Command: prefix VISIBLE suffix\nSession: agent:main:session-1",
}),
);
expect(params.onAgentEvent).toHaveBeenCalledWith(
expect.objectContaining({
stream: "approval",
data: expect.objectContaining({ command: "prefix VISIBLE suffix" }),
}),
);
});
it("strips bidi and invisible formatting controls from command previews", async () => {
const params = createParams();
mockCallGatewayTool.mockResolvedValueOnce({
id: "plugin:approval-bidi",
decision: "allow-once",
});
await handleCodexAppServerApprovalRequest({
method: "item/commandExecution/requestApproval",
requestParams: {
threadId: "thread-1",
turnId: "turn-1",
itemId: "cmd-bidi",
command: "echo safe\u202e cod.exe\u2066 hidden\u2069 \ufeffdone\u{e0100}",
},
paramsForRun: params,
threadId: "thread-1",
turnId: "turn-1",
});
const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? [];
expect(requestPayload).toEqual(
expect.objectContaining({
description: "Command: echo safe cod.exe hidden done\nSession: agent:main:session-1",
}),
);
expect(params.onAgentEvent).toHaveBeenCalledWith(
expect.objectContaining({
stream: "approval",
data: expect.objectContaining({ command: "echo safe cod.exe hidden done" }),
}),
);
});
it("marks oversized unsafe command previews as omitted", async () => {
const params = createParams();
mockCallGatewayTool.mockResolvedValueOnce({
id: "plugin:approval-omitted-command",
decision: "allow-once",
});
const esc = "\u001b";
const oversizedPrefix = `${esc}]8;;https://example.com${esc}\\`.repeat(300);
await handleCodexAppServerApprovalRequest({
method: "item/commandExecution/requestApproval",
requestParams: {
threadId: "thread-1",
turnId: "turn-1",
itemId: "cmd-omitted",
command: [oversizedPrefix, "TAIL"],
},
paramsForRun: params,
threadId: "thread-1",
turnId: "turn-1",
});
const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? [];
expect(requestPayload).toEqual(
expect.objectContaining({
description:
"Command: [preview truncated or unsafe content omitted]\nSession: agent:main:session-1",
}),
);
expect(params.onAgentEvent).toHaveBeenCalledWith(
expect.objectContaining({
stream: "approval",
data: expect.objectContaining({
commandPreviewOmitted: true,
}),
}),
);
});
it("marks clipped command previews even when a safe prefix remains", async () => {
const params = createParams();
mockCallGatewayTool.mockResolvedValueOnce({
id: "plugin:approval-clipped-command",
decision: "allow-once",
});
await handleCodexAppServerApprovalRequest({
method: "item/commandExecution/requestApproval",
requestParams: {
threadId: "thread-1",
turnId: "turn-1",
itemId: "cmd-clipped",
command: `${"a".repeat(5000)} tail`,
},
paramsForRun: params,
threadId: "thread-1",
turnId: "turn-1",
});
const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? [];
const description = (requestPayload as { description: string }).description;
expect(description).toContain("[preview truncated or unsafe content omitted]");
expect(params.onAgentEvent).toHaveBeenCalledWith(
expect.objectContaining({
stream: "approval",
data: expect.objectContaining({
commandPreviewOmitted: true,
}),
}),
);
});
it("fails closed when no approval route is available", async () => {
const params = createParams();
mockCallGatewayTool.mockResolvedValueOnce({
@@ -149,6 +328,43 @@ describe("Codex app-server approval bridge", () => {
);
});
it("sanitizes reason previews before forwarding approval text and events", async () => {
const params = createParams();
mockCallGatewayTool.mockResolvedValueOnce({
id: "plugin:approval-sanitized-reason",
decision: null,
});
await handleCodexAppServerApprovalRequest({
method: "item/fileChange/requestApproval",
requestParams: {
threadId: "thread-1",
turnId: "turn-1",
itemId: "patch-sanitized",
reason: "needs write access\nfor \u001b[31m/tmp\u001b[0m\tplease",
},
paramsForRun: params,
threadId: "thread-1",
turnId: "turn-1",
});
const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? [];
expect(requestPayload).toEqual(
expect.objectContaining({
description: "Reason: needs write access for /tmp please\nSession: agent:main:session-1",
}),
);
expect(params.onAgentEvent).toHaveBeenCalledWith(
expect.objectContaining({
stream: "approval",
data: expect.objectContaining({
status: "unavailable",
reason: "needs write access for /tmp please",
}),
}),
);
});
it("fails closed for unsupported native approval methods without requesting plugin approval", async () => {
const params = createParams();
@@ -277,6 +493,39 @@ describe("Codex app-server approval bridge", () => {
expect(description).toContain("High-risk targets:");
});
it("strips terminal and invisible controls from permission descriptions", async () => {
const params = createParams();
mockCallGatewayTool.mockResolvedValueOnce({
id: "plugin:approval-permission-controls",
decision: "allow-once",
});
await handleCodexAppServerApprovalRequest({
method: "item/permissions/requestApproval",
requestParams: {
threadId: "thread-1",
turnId: "turn-1",
itemId: "perm-controls",
permissions: {
network: { allowHosts: ["exa\u009b31mmple.com", "safe\u202e.example.com"] },
fileSystem: { roots: ["/tmp/\u001b[31mproject\u001b[0m"] },
},
},
paramsForRun: params,
threadId: "thread-1",
turnId: "turn-1",
});
const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? [];
const description = (requestPayload as { description: string }).description;
expect(description).toContain("example.com");
expect(description).toContain("safe .example.com");
expect(description).toContain("/tmp/project");
expect(description).not.toContain("\u009b");
expect(description).not.toContain("\u202e");
expect(description).not.toContain("\u001b");
});
it("ignores approval requests that are missing explicit thread or turn ids", async () => {
const params = createParams();

168
extensions/codex/src/app-server/approval-bridge.ts
View File

@@ -13,6 +13,35 @@ import { isJsonObject, type JsonObject, type JsonValue } from "./protocol.js";
const PERMISSION_DESCRIPTION_MAX_LENGTH = 700;
const PERMISSION_SAMPLE_LIMIT = 2;
const PERMISSION_VALUE_MAX_LENGTH = 48;
const APPROVAL_PREVIEW_SCAN_MAX_LENGTH = 4096;
const APPROVAL_PREVIEW_OMITTED = "[preview truncated or unsafe content omitted]";
const ANSI_OSC_SEQUENCE_RE = new RegExp(
String.raw`(?:\u001b]|\u009d)[^\u001b\u009c\u0007]*(?:\u0007|\u001b\\|\u009c)`,
"g",
);
const ANSI_CONTROL_SEQUENCE_RE = new RegExp(
String.raw`(?:\u001b\[[0-?]*[ -/]*[@-~]|\u009b[0-?]*[ -/]*[@-~]|\u001b[@-Z\\-_])`,
"g",
);
const CONTROL_CHARACTER_RE = new RegExp(String.raw`[\u0000-\u001f\u007f-\u009f]+`, "g");
const INVISIBLE_FORMATTING_CONTROL_RE = new RegExp(
String.raw`[\u00ad\u034f\u061c\u200b-\u200f\u202a-\u202e\u2060-\u206f\ufeff\ufe00-\ufe0f\u{e0100}-\u{e01ef}]`,
"gu",
);
const DANGLING_TERMINAL_SEQUENCE_SUFFIX_RE = new RegExp(
String.raw`(?:\u001b\][^\u001b\u009c\u0007]*|\u009d[^\u001b\u009c\u0007]*|\u001b\[[0-?]*[ -/]*|\u009b[0-?]*[ -/]*|\u001b)$`,
);
type ApprovalPreviewSource = {
value: string;
clipped: boolean;
};
type SanitizedApprovalPreview = {
text?: string;
omitted: boolean;
};
export async function handleCodexAppServerApprovalRequest(params: {
method: string;
requestParams: JsonValue | undefined;
@@ -161,8 +190,16 @@ function buildApprovalContext(params: {
readString(params.requestParams, "itemId") ??
readString(params.requestParams, "callId") ??
readString(params.requestParams, "approvalId");
const command = readDisplayCommand(params.requestParams);
const reason = readString(params.requestParams, "reason");
const commandPreview = sanitizeApprovalPreview(
readDisplayCommandPreview(params.requestParams),
180,
);
const reasonPreview = sanitizeApprovalPreview(
readStringPreview(params.requestParams, "reason"),
180,
);
const command = commandPreview.text;
const reason = reasonPreview.text;
const kind = approvalKindForMethod(params.method);
const permissionLines =
params.method === "item/permissions/requestApproval"
@@ -179,10 +216,14 @@ function buildApprovalContext(params: {
const subject =
permissionLines[0] ??
(command
? `Command: ${truncate(command, 180)}`
: reason
? `Reason: ${truncate(reason, 180)}`
: `Request method: ${params.method}`);
? `Command: ${formatApprovalPreviewSubject(command, commandPreview.omitted)}`
: commandPreview.omitted
? `Command: ${APPROVAL_PREVIEW_OMITTED}`
: reason
? `Reason: ${formatApprovalPreviewSubject(reason, reasonPreview.omitted)}`
: reasonPreview.omitted
? `Reason: ${APPROVAL_PREVIEW_OMITTED}`
: `Request method: ${params.method}`);
const description =
permissionLines.length > 0
? joinDescriptionLinesWithinLimit(permissionLines, PERMISSION_DESCRIPTION_MAX_LENGTH)
@@ -205,7 +246,9 @@ function buildApprovalContext(params: {
eventDetails: {
...(itemId ? { itemId } : {}),
...(command ? { command } : {}),
...(commandPreview.omitted ? { commandPreviewOmitted: true } : {}),
...(reason ? { reason } : {}),
...(reasonPreview.omitted ? { reasonPreviewOmitted: true } : {}),
},
};
}
@@ -394,12 +437,7 @@ function sanitizePermissionPathValue(value: string): string {
}
function sanitizePermissionScalar(value: string): string {
let sanitized = "";
for (let index = 0; index < value.length; index += 1) {
const code = value.charCodeAt(index);
sanitized += code < 32 || code === 127 ? " " : value[index];
}
return sanitized.replace(/\s+/g, " ").trim();
return sanitizeVisibleScalar(value);
}
function permissionHostRisks(value: string): string[] {
@@ -531,34 +569,64 @@ function emitApprovalEvent(params: EmbeddedRunAttemptParams, data: AgentApproval
params.onAgentEvent?.({ stream: "approval", data: data as unknown as Record<string, unknown> });
}
function readDisplayCommand(record: JsonObject | undefined): string | undefined {
const actionCommand = readCommandActions(record);
function readDisplayCommandPreview(
record: JsonObject | undefined,
): ApprovalPreviewSource | undefined {
const actionCommand = readCommandActionsPreview(record);
if (actionCommand) {
return actionCommand;
}
return readCommand(record);
return readCommandPreview(record);
}
function readCommandActions(record: JsonObject | undefined): string | undefined {
function readCommandActionsPreview(
record: JsonObject | undefined,
): ApprovalPreviewSource | undefined {
const actions = record?.commandActions;
if (!Array.isArray(actions)) {
return undefined;
}
const commands = actions
.map((action) => (isJsonObject(action) ? readString(action, "command") : undefined))
.filter((command): command is string => Boolean(command));
return commands.length > 0 ? commands.join(" && ") : undefined;
let source: ApprovalPreviewSource | undefined;
for (const action of actions) {
const command = isJsonObject(action) ? readString(action, "command") : undefined;
if (!command) {
continue;
}
source = appendPreviewPart(source, command, " && ");
if (source.clipped) {
break;
}
}
return source;
}
function readCommand(record: JsonObject | undefined): string | undefined {
function readCommandPreview(record: JsonObject | undefined): ApprovalPreviewSource | undefined {
const command = record?.command;
if (typeof command === "string") {
return command;
return previewSource(command);
}
if (Array.isArray(command) && command.every((part) => typeof part === "string")) {
return command.join(" ");
if (!Array.isArray(command)) {
return undefined;
}
return undefined;
let source: ApprovalPreviewSource | undefined;
for (const part of command) {
if (typeof part !== "string") {
return undefined;
}
source = appendPreviewPart(source, part, " ");
if (source.clipped) {
break;
}
}
return source;
}
function readStringPreview(
record: JsonObject | undefined,
key: string,
): ApprovalPreviewSource | undefined {
const value = readString(record, key);
return value === undefined ? undefined : previewSource(value);
}
function readString(record: JsonObject | undefined, key: string): string | undefined {
@@ -570,6 +638,56 @@ function truncate(value: string, maxLength: number): string {
return value.length <= maxLength ? value : `${value.slice(0, Math.max(0, maxLength - 3))}...`;
}
function previewSource(value: string): ApprovalPreviewSource {
return {
value: value.slice(0, APPROVAL_PREVIEW_SCAN_MAX_LENGTH),
clipped: value.length > APPROVAL_PREVIEW_SCAN_MAX_LENGTH,
};
}
function appendPreviewPart(
source: ApprovalPreviewSource | undefined,
part: string,
separator: string,
): ApprovalPreviewSource {
const prefix = source?.value ? `${source.value}${separator}` : "";
const value = `${prefix}${part}`;
const clipped = source?.clipped === true || value.length > APPROVAL_PREVIEW_SCAN_MAX_LENGTH;
return {
value: value.slice(0, APPROVAL_PREVIEW_SCAN_MAX_LENGTH),
clipped,
};
}
function sanitizeApprovalPreview(
source: ApprovalPreviewSource | undefined,
maxLength: number,
): SanitizedApprovalPreview {
if (!source || !source.value) {
return { omitted: false };
}
const rawPreview = source.value.replace(DANGLING_TERMINAL_SEQUENCE_SUFFIX_RE, "");
const sanitized = sanitizeVisibleScalar(rawPreview);
if (!sanitized) {
return { omitted: true };
}
return { text: truncate(sanitized, maxLength), omitted: source.clipped };
}
function sanitizeVisibleScalar(value: string): string {
return value
.replace(ANSI_OSC_SEQUENCE_RE, "")
.replace(ANSI_CONTROL_SEQUENCE_RE, "")
.replace(INVISIBLE_FORMATTING_CONTROL_RE, " ")
.replace(CONTROL_CHARACTER_RE, " ")
.replace(/\s+/g, " ")
.trim();
}
function formatApprovalPreviewSubject(text: string, omitted: boolean): string {
return omitted ? `${text} ${APPROVAL_PREVIEW_OMITTED}` : text;
}
function joinDescriptionLinesWithinLimit(lines: string[], maxLength: number): string {
let description = "";
for (const line of lines) {