mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:40:44 +00:00
fix(codex): sanitize elicitation approval text
This commit is contained in:
Lucenx9
committed by
Peter Steinberger
Peter Steinberger
parent
972d8fc1cf
commit
e098a439c4
@@ -188,6 +188,184 @@ describe("Codex app-server elicitation bridge", () => {
|
||||
expect(approvalRequest.description).toContain("Repository: openclaw/openclaw");
|
||||
});
|
||||
|
||||
it("strips control and invisible formatting from approval display text", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-sanitized", status: "accepted" })
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-sanitized", decision: "allow-once" });
|
||||
|
||||
await handleCodexAppServerElicitationRequest({
|
||||
requestParams: {
|
||||
...buildCurrentCodexApprovalElicitation(),
|
||||
message: "Approve\u202e hidden",
|
||||
serverName: "codex\u009b31m_apps__github",
|
||||
_meta: {
|
||||
codex_approval_kind: "mcp_tool_call",
|
||||
connector_name: "GitHub\nInjected: approve",
|
||||
tool_title: "\u001b]8;;https://evil.example\u001b\\Visible tool\u001b]8;;\u001b\\",
|
||||
tool_description: "Creates\u0000 a\u202e pull request",
|
||||
tool_params_display: [
|
||||
{
|
||||
name: "repo",
|
||||
display_name: "Repository\u202e",
|
||||
value: "\u001b]8;;https://evil.example\u001b\\openclaw/openclaw\u001b]8;;\u001b\\",
|
||||
},
|
||||
],
|
||||
},
|
||||
requestedSchema: {
|
||||
type: "object",
|
||||
properties: {
|
||||
approve: {
|
||||
type: "boolean",
|
||||
title: "Approve\u202e this tool call",
|
||||
description: "Confirm\u009b31m access",
|
||||
},
|
||||
},
|
||||
required: ["approve"],
|
||||
},
|
||||
},
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
});
|
||||
|
||||
const approvalRequest = mockCallGatewayTool.mock.calls[0]?.[2] as {
|
||||
title: string;
|
||||
description: string;
|
||||
};
|
||||
expect(approvalRequest.title).toBe("Approve hidden");
|
||||
expect(approvalRequest.description).toContain("GitHub Injected: approve");
|
||||
expect(approvalRequest.description).toContain("Tool: Visible tool");
|
||||
expect(approvalRequest.description).toContain("Repository: openclaw/openclaw");
|
||||
expect(approvalRequest.description).toContain("- Approve this tool call: Confirm access");
|
||||
expect(approvalRequest.description).not.toContain("https://evil.example");
|
||||
expect(approvalRequest.description).not.toContain("\u001b");
|
||||
expect(approvalRequest.description).not.toContain("\u009b");
|
||||
expect(approvalRequest.description).not.toContain("\u202e");
|
||||
});
|
||||
|
||||
it("falls back to stable names when display labels sanitize to empty", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-label-fallback", status: "accepted" })
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-label-fallback", decision: "allow-once" });
|
||||
|
||||
await handleCodexAppServerElicitationRequest({
|
||||
requestParams: {
|
||||
...buildCurrentCodexApprovalElicitation(),
|
||||
message: "Approve",
|
||||
_meta: {
|
||||
codex_approval_kind: "mcp_tool_call",
|
||||
connector_name: "App",
|
||||
tool_params_display: [
|
||||
{
|
||||
name: "repo",
|
||||
display_name: "\u202e",
|
||||
value: "openclaw/openclaw",
|
||||
},
|
||||
],
|
||||
},
|
||||
requestedSchema: {
|
||||
type: "object",
|
||||
properties: {
|
||||
approve: {
|
||||
type: "boolean",
|
||||
title: "\u202e",
|
||||
description: "Confirm access",
|
||||
},
|
||||
},
|
||||
required: ["approve"],
|
||||
},
|
||||
},
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
});
|
||||
|
||||
const approvalRequest = mockCallGatewayTool.mock.calls[0]?.[2] as {
|
||||
description: string;
|
||||
};
|
||||
expect(approvalRequest.description).toContain("- repo: openclaw/openclaw");
|
||||
expect(approvalRequest.description).toContain("- approve: Confirm access");
|
||||
expect(approvalRequest.description).not.toContain("- field: Confirm access");
|
||||
});
|
||||
|
||||
it("bounds deep approval display parameter values before forwarding them", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-bounded-params", status: "accepted" })
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-bounded-params", decision: "allow-once" });
|
||||
|
||||
await handleCodexAppServerElicitationRequest({
|
||||
requestParams: {
|
||||
...buildCurrentCodexApprovalElicitation(),
|
||||
message: "Approve",
|
||||
_meta: {
|
||||
codex_approval_kind: "mcp_tool_call",
|
||||
connector_name: "App",
|
||||
tool_title: "Tool",
|
||||
tool_params_display: [
|
||||
{
|
||||
name: "payload",
|
||||
value: {
|
||||
key0: { nested: { deeper: { secret: "hidden" } } },
|
||||
key1: 1,
|
||||
key2: 2,
|
||||
key3: 3,
|
||||
key4: 4,
|
||||
key5: 5,
|
||||
key6: 6,
|
||||
key7: 7,
|
||||
key8: 8,
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
});
|
||||
|
||||
const approvalRequest = mockCallGatewayTool.mock.calls[0]?.[2] as {
|
||||
description: string;
|
||||
};
|
||||
expect(approvalRequest.description).toContain("payload");
|
||||
expect(approvalRequest.description).toContain("key0");
|
||||
expect(approvalRequest.description).not.toContain("key8");
|
||||
expect(approvalRequest.description).not.toContain("hidden");
|
||||
});
|
||||
|
||||
it("caps approval display parameter entries before forwarding them", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-capped-params", status: "accepted" })
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-capped-params", decision: "allow-once" });
|
||||
|
||||
await handleCodexAppServerElicitationRequest({
|
||||
requestParams: {
|
||||
...buildCurrentCodexApprovalElicitation(),
|
||||
message: "Approve",
|
||||
serverName: "",
|
||||
_meta: {
|
||||
codex_approval_kind: "mcp_tool_call",
|
||||
connector_name: "App",
|
||||
tool_params_display: Array.from({ length: 9 }, (_, index) => ({
|
||||
name: `p${index}`,
|
||||
value: index,
|
||||
})),
|
||||
},
|
||||
},
|
||||
paramsForRun: createParams(),
|
||||
threadId: "thread-1",
|
||||
turnId: "turn-1",
|
||||
});
|
||||
|
||||
const approvalRequest = mockCallGatewayTool.mock.calls[0]?.[2] as {
|
||||
description: string;
|
||||
};
|
||||
expect(approvalRequest.description).toContain("p0");
|
||||
expect(approvalRequest.description).toContain("p7");
|
||||
expect(approvalRequest.description).toContain("Additional parameters: 1 more");
|
||||
expect(approvalRequest.description).not.toContain("p8");
|
||||
});
|
||||
|
||||
it("accepts approval elicitations with a null turn id when the thread matches", async () => {
|
||||
mockCallGatewayTool
|
||||
.mockResolvedValueOnce({ id: "plugin:approval-null-turn", status: "accepted" })
|
||||
|
||||
@@ -30,7 +30,28 @@ const MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY = "connector_name";
|
||||
const MCP_TOOL_APPROVAL_TOOL_TITLE_KEY = "tool_title";
|
||||
const MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY = "tool_description";
|
||||
const MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY = "tool_params_display";
|
||||
const MAX_DISPLAY_PARAM_ENTRIES = 8;
|
||||
const MAX_DISPLAY_PARAM_VALUE_LENGTH = 120;
|
||||
const MAX_DISPLAY_VALUE_ARRAY_ITEMS = 8;
|
||||
const MAX_DISPLAY_VALUE_OBJECT_KEYS = 8;
|
||||
const MAX_DISPLAY_VALUE_DEPTH = 3;
|
||||
const DISPLAY_TEXT_SCAN_MAX_LENGTH = 4096;
|
||||
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)$`,
|
||||
);
|
||||
|
||||
export async function handleCodexAppServerElicitationRequest(params: {
|
||||
requestParams: JsonValue | undefined;
|
||||
@@ -97,14 +118,15 @@ function readBridgeableApprovalElicitation(
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const title = readString(requestParams, "message") ?? "Codex MCP tool approval";
|
||||
const title =
|
||||
sanitizeDisplayText(readString(requestParams, "message") ?? "") || "Codex MCP tool approval";
|
||||
return {
|
||||
title,
|
||||
description: buildApprovalDescription({
|
||||
title,
|
||||
meta: requestParams._meta,
|
||||
requestedSchema,
|
||||
serverName: readString(requestParams, "serverName"),
|
||||
serverName: sanitizeOptionalDisplayText(readString(requestParams, "serverName")),
|
||||
}),
|
||||
requestedSchema,
|
||||
meta: requestParams._meta,
|
||||
@@ -117,13 +139,20 @@ function buildApprovalDescription(params: {
|
||||
requestedSchema: JsonObject;
|
||||
serverName: string | undefined;
|
||||
}): string {
|
||||
const summaryLines = [
|
||||
readString(params.meta, MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY) &&
|
||||
`App: ${readString(params.meta, MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY)}`,
|
||||
readString(params.meta, MCP_TOOL_APPROVAL_TOOL_TITLE_KEY) &&
|
||||
`Tool: ${readString(params.meta, MCP_TOOL_APPROVAL_TOOL_TITLE_KEY)}`,
|
||||
params.serverName && `MCP server: ${params.serverName}`,
|
||||
const connectorName = sanitizeOptionalDisplayText(
|
||||
readString(params.meta, MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY),
|
||||
);
|
||||
const toolTitle = sanitizeOptionalDisplayText(
|
||||
readString(params.meta, MCP_TOOL_APPROVAL_TOOL_TITLE_KEY),
|
||||
);
|
||||
const toolDescription = sanitizeOptionalDisplayText(
|
||||
readString(params.meta, MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY),
|
||||
);
|
||||
const summaryLines = [
|
||||
connectorName && `App: ${connectorName}`,
|
||||
toolTitle && `Tool: ${toolTitle}`,
|
||||
params.serverName && `MCP server: ${params.serverName}`,
|
||||
toolDescription,
|
||||
].filter((line): line is string => Boolean(line));
|
||||
const paramLines = readDisplayParamLines(params.meta);
|
||||
const propertyLines = readPropertyDescriptionLines(params.requestedSchema);
|
||||
@@ -145,8 +174,11 @@ function readPropertyDescriptionLines(requestedSchema: JsonObject): string[] {
|
||||
if (!schema) {
|
||||
return undefined;
|
||||
}
|
||||
const propTitle = readString(schema, "title") ?? name;
|
||||
const description = readString(schema, "description");
|
||||
const propTitle =
|
||||
sanitizeDisplayText(readString(schema, "title") ?? "") ||
|
||||
sanitizeDisplayText(name) ||
|
||||
"field";
|
||||
const description = sanitizeOptionalDisplayText(readString(schema, "description"));
|
||||
return description ? `- ${propTitle}: ${description}` : `- ${propTitle}`;
|
||||
})
|
||||
.filter((line): line is string => Boolean(line));
|
||||
@@ -157,26 +189,105 @@ function readDisplayParamLines(meta: JsonObject): string[] {
|
||||
if (!Array.isArray(displayParams)) {
|
||||
return [];
|
||||
}
|
||||
return displayParams
|
||||
const lines = displayParams
|
||||
.slice(0, MAX_DISPLAY_PARAM_ENTRIES)
|
||||
.map((entry) => {
|
||||
const param = isJsonObject(entry) ? entry : undefined;
|
||||
if (!param) {
|
||||
return undefined;
|
||||
}
|
||||
const name = readString(param, "display_name") ?? readString(param, "name");
|
||||
const name =
|
||||
sanitizeOptionalDisplayText(readString(param, "display_name")) ??
|
||||
sanitizeOptionalDisplayText(readString(param, "name"));
|
||||
if (!name) {
|
||||
return undefined;
|
||||
}
|
||||
return `- ${name}: ${formatDisplayParamValue(param.value)}`;
|
||||
})
|
||||
.filter((line): line is string => Boolean(line));
|
||||
const remaining = displayParams.length - MAX_DISPLAY_PARAM_ENTRIES;
|
||||
return remaining > 0 ? [...lines, `- Additional parameters: ${remaining} more`] : lines;
|
||||
}
|
||||
|
||||
function formatDisplayParamValue(value: JsonValue | undefined): string {
|
||||
const formatted = typeof value === "string" ? value : JSON.stringify(value ?? null);
|
||||
return formatted.length <= MAX_DISPLAY_PARAM_VALUE_LENGTH
|
||||
? formatted
|
||||
: `${formatted.slice(0, MAX_DISPLAY_PARAM_VALUE_LENGTH - 3)}...`;
|
||||
const formatted = typeof value === "string" ? value : formatDisplayJsonValue(value ?? null);
|
||||
return truncateDisplayText(sanitizeDisplayText(formatted), MAX_DISPLAY_PARAM_VALUE_LENGTH);
|
||||
}
|
||||
|
||||
function formatDisplayJsonValue(value: JsonValue, depth = MAX_DISPLAY_VALUE_DEPTH): string {
|
||||
if (value === null) {
|
||||
return "null";
|
||||
}
|
||||
if (typeof value === "string") {
|
||||
return JSON.stringify(truncateDisplayText(sanitizeDisplayText(value), 80));
|
||||
}
|
||||
if (typeof value === "number" || typeof value === "boolean") {
|
||||
return String(value);
|
||||
}
|
||||
if (Array.isArray(value)) {
|
||||
if (depth <= 0) {
|
||||
return "[truncated]";
|
||||
}
|
||||
const parts: string[] = [];
|
||||
const limit = Math.min(value.length, MAX_DISPLAY_VALUE_ARRAY_ITEMS);
|
||||
for (let i = 0; i < limit; i += 1) {
|
||||
parts.push(formatDisplayJsonValue(value[i] ?? null, depth - 1));
|
||||
}
|
||||
if (value.length > MAX_DISPLAY_VALUE_ARRAY_ITEMS) {
|
||||
parts.push("...");
|
||||
}
|
||||
return `[${parts.join(",")}]`;
|
||||
}
|
||||
if (typeof value === "object") {
|
||||
if (depth <= 0) {
|
||||
return "{truncated}";
|
||||
}
|
||||
const parts: string[] = [];
|
||||
let count = 0;
|
||||
let truncated = false;
|
||||
for (const key in value) {
|
||||
if (!Object.prototype.hasOwnProperty.call(value, key)) {
|
||||
continue;
|
||||
}
|
||||
if (count >= MAX_DISPLAY_VALUE_OBJECT_KEYS) {
|
||||
truncated = true;
|
||||
break;
|
||||
}
|
||||
const safeKey = truncateDisplayText(sanitizeDisplayText(key), 80);
|
||||
parts.push(
|
||||
`${JSON.stringify(safeKey)}:${formatDisplayJsonValue(value[key] ?? null, depth - 1)}`,
|
||||
);
|
||||
count += 1;
|
||||
}
|
||||
if (truncated) {
|
||||
parts.push("...");
|
||||
}
|
||||
return `{${parts.join(",")}}`;
|
||||
}
|
||||
return "null";
|
||||
}
|
||||
|
||||
function sanitizeOptionalDisplayText(value: string | undefined): string | undefined {
|
||||
const sanitized = value === undefined ? "" : sanitizeDisplayText(value);
|
||||
return sanitized || undefined;
|
||||
}
|
||||
|
||||
function sanitizeDisplayText(value: string): string {
|
||||
const scanned = value.slice(0, DISPLAY_TEXT_SCAN_MAX_LENGTH);
|
||||
const clipped = value.length > DISPLAY_TEXT_SCAN_MAX_LENGTH;
|
||||
const sanitized = scanned
|
||||
.replace(ANSI_OSC_SEQUENCE_RE, "")
|
||||
.replace(ANSI_CONTROL_SEQUENCE_RE, "")
|
||||
.replace(DANGLING_TERMINAL_SEQUENCE_SUFFIX_RE, "")
|
||||
.replace(INVISIBLE_FORMATTING_CONTROL_RE, " ")
|
||||
.replace(CONTROL_CHARACTER_RE, " ")
|
||||
.replace(/\s+/g, " ")
|
||||
.trim();
|
||||
return clipped ? `${sanitized}...` : sanitized;
|
||||
}
|
||||
|
||||
function truncateDisplayText(value: string, maxLength: number): string {
|
||||
return value.length <= maxLength ? value : `${value.slice(0, Math.max(0, maxLength - 3))}...`;
|
||||
}
|
||||
|
||||
async function requestPluginApprovalOutcome(params: {
|
||||
|
||||
Reference in New Issue
Block a user