diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bb964d73a3..35424f5f977 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Codex harness: show bounded, sanitized permission target samples in app-server approval prompts, so native permission requests keep their specific hosts, roots, and paths visible without leaking home usernames or URL credentials. (#70340) Thanks @Lucenx9. - Docs/Codex harness: narrow native compaction docs to the current start/completion signals, without promising a readable summary or kept-entry audit list yet. (#69612) Thanks @91wan. - Providers/Amazon Bedrock: use known context-window metadata for discovered models while keeping the unknown-model fallback conservative, so compaction and overflow handling improve for newer Bedrock models without overstating unlisted model limits. Thanks @wirjo. - Providers/Amazon Bedrock Mantle: refresh IAM-backed bearer tokens at runtime instead of baking discovery-time tokens into provider config, so long-lived Mantle sessions keep working after the initial token ages out. Thanks @wirjo. diff --git a/extensions/codex/src/app-server/approval-bridge.test.ts b/extensions/codex/src/app-server/approval-bridge.test.ts index fbc588e799f..282d49b24e2 100644 --- a/extensions/codex/src/app-server/approval-bridge.test.ts +++ b/extensions/codex/src/app-server/approval-bridge.test.ts @@ -168,27 +168,13 @@ describe("Codex app-server approval bridge", () => { }), { expectFinal: false }, ); - expect(mockCallGatewayTool).toHaveBeenCalledWith( - "plugin.approval.request", - expect.any(Object), - expect.objectContaining({ - description: expect.stringContaining( - "Network permission requested (allowHosts: example.com, *.internal; high-risk: wildcard hosts, private-network wildcards)", - ), - }), - { expectFinal: false }, - ); - expect(mockCallGatewayTool).toHaveBeenCalledWith( - "plugin.approval.request", - expect.any(Object), - expect.objectContaining({ - description: expect.stringContaining( - "File system permission requested (roots: /; writePaths: ~; high-risk: filesystem root, home directory)", - ), - }), - { expectFinal: false }, - ); const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? []; + const description = (requestPayload as { description: string }).description; + expect(description).toContain("Network allowHosts: example.com, *.internal"); + expect(description).toContain("File system roots: /; writePaths: ~"); + expect(description).toContain( + "High-risk targets: wildcard hosts, private-network wildcards, filesystem root, home directory", + ); expect(requestPayload).toEqual( expect.objectContaining({ description: expect.not.stringContaining("agent:main:session-1"), @@ -196,7 +182,7 @@ describe("Codex app-server approval bridge", () => { ); }); - it("keeps permission detail bounded and truncated within the approval description cap", async () => { + it("keeps permission detail bounded with truncated and redacted target samples", async () => { const params = createParams(); mockCallGatewayTool.mockResolvedValueOnce({ id: "plugin:approval-4", @@ -212,6 +198,7 @@ describe("Codex app-server approval bridge", () => { permissions: { network: { allowHosts: [ + "https://secret-token@example.com/private", "*.internal", "very-long-service-name.example.corp", "third.example.com", @@ -219,7 +206,8 @@ describe("Codex app-server approval bridge", () => { }, fileSystem: { roots: ["/", "/workspace/project", "/Users/simone/Documents"], - writePaths: ["/tmp/output", "/var/log/app"], + readPaths: ["/Users/simone/.ssh/id_rsa", "/etc/hosts", "/var/log/system.log"], + writePaths: ["/tmp/output", "/var/log/app", "/home/simone/private"], }, }, }, @@ -235,11 +223,15 @@ describe("Codex app-server approval bridge", () => { }), ); const description = (requestPayload as { description: string }).description; - expect(description.length).toBeLessThanOrEqual(256); + expect(description.length).toBeLessThanOrEqual(700); + expect(description).toContain("example.com"); + expect(description).not.toContain("secret-token"); + expect(description).not.toContain("simone"); expect(description).toContain("*.internal"); expect(description).toContain("/workspace/project"); - expect(description).toContain("(+1 more)"); - expect(description).toContain("high-risk:"); + expect(description).toContain("readPaths: ~/.ssh/id_rsa, /etc/hosts (+1 more)"); + expect(description).toContain("writePaths: /tmp/output, /var/log/app (+1 more)"); + expect(description).toContain("High-risk targets:"); }); it("maps app-server approval response families separately", () => { diff --git a/extensions/codex/src/app-server/approval-bridge.ts b/extensions/codex/src/app-server/approval-bridge.ts index be221c9682d..7c7509a2854 100644 --- a/extensions/codex/src/app-server/approval-bridge.ts +++ b/extensions/codex/src/app-server/approval-bridge.ts @@ -7,6 +7,9 @@ import { import { isJsonObject, type JsonObject, type JsonValue } from "./protocol.js"; const DEFAULT_CODEX_APPROVAL_TIMEOUT_MS = 120_000; +const PERMISSION_DESCRIPTION_MAX_LENGTH = 700; +const PERMISSION_SAMPLE_LIMIT = 2; +const PERMISSION_VALUE_MAX_LENGTH = 48; export type AppServerApprovalOutcome = | "approved-once" @@ -208,9 +211,9 @@ function buildApprovalContext(params: { ? "Codex app-server command approval" : params.method === "item/permissions/requestApproval" ? "Codex app-server permission approval" - : kind === "plugin" - ? "Codex app-server file approval" - : "Codex app-server approval"; + : kind === "plugin" + ? "Codex app-server file approval" + : "Codex app-server approval"; const subject = permissionLines[0] ?? (command @@ -218,13 +221,12 @@ function buildApprovalContext(params: { : reason ? `Reason: ${truncate(reason, 180)}` : `Request method: ${params.method}`); - const description = joinDescriptionLinesWithinLimit( - [ - subject, - ...permissionLines.slice(1), - ].filter((line): line is string => Boolean(line)), - 256, - ); + const description = + permissionLines.length > 0 + ? joinDescriptionLinesWithinLimit(permissionLines, PERMISSION_DESCRIPTION_MAX_LENGTH) + : [subject, params.paramsForRun.sessionKey && `Session: ${params.paramsForRun.sessionKey}`] + .filter(Boolean) + .join("\n"); return { kind, title, @@ -326,6 +328,7 @@ function describeRequestedPermissions(requestParams: JsonObject | undefined): st const permissions = requestedPermissions(requestParams); const lines: string[] = []; const kinds: string[] = []; + const risks = new Set(); if (isJsonObject(permissions.network)) { kinds.push("network"); } @@ -336,40 +339,45 @@ function describeRequestedPermissions(requestParams: JsonObject | undefined): st lines.push(`Permissions: ${kinds.join(", ")}`); } if (isJsonObject(permissions.network)) { - lines.push( - summarizePermissionRecord("Network permission requested", permissions.network, [ - { - key: "allowHosts", - label: "allowHosts", - sanitize: sanitizePermissionHostValue, - risksFor: permissionHostRisks, - }, - ]), - ); + const networkSummary = summarizePermissionRecord(permissions.network, risks, [ + { + key: "allowHosts", + label: "allowHosts", + sanitize: sanitizePermissionHostValue, + risksFor: permissionHostRisks, + }, + ]); + if (networkSummary) { + lines.push(`Network ${networkSummary}`); + } } if (isJsonObject(permissions.fileSystem)) { - lines.push( - summarizePermissionRecord("File system permission requested", permissions.fileSystem, [ - { - key: "roots", - label: "roots", - sanitize: sanitizePermissionPathValue, - risksFor: permissionPathRisks, - }, - { - key: "readPaths", - label: "readPaths", - sanitize: sanitizePermissionPathValue, - risksFor: permissionPathRisks, - }, - { - key: "writePaths", - label: "writePaths", - sanitize: sanitizePermissionPathValue, - risksFor: permissionPathRisks, - }, - ]), - ); + const fileSystemSummary = summarizePermissionRecord(permissions.fileSystem, risks, [ + { + key: "roots", + label: "roots", + sanitize: sanitizePermissionPathValue, + risksFor: permissionPathRisks, + }, + { + key: "readPaths", + label: "readPaths", + sanitize: sanitizePermissionPathValue, + risksFor: permissionPathRisks, + }, + { + key: "writePaths", + label: "writePaths", + sanitize: sanitizePermissionPathValue, + risksFor: permissionPathRisks, + }, + ]); + if (fileSystemSummary) { + lines.push(`File system ${fileSystemSummary}`); + } + } + if (risks.size > 0) { + lines.push(`High-risk targets: ${[...risks].join(", ")}`); } return lines; } @@ -382,26 +390,18 @@ type PermissionArrayDescriptor = { }; function summarizePermissionRecord( - label: string, permission: JsonObject, + risks: Set, descriptors: readonly PermissionArrayDescriptor[], -): string { +): string | undefined { const details: string[] = []; - const risks = new Set(); for (const descriptor of descriptors) { const summary = summarizePermissionArray(permission, descriptor, risks); if (summary) { details.push(summary); } } - const suffix: string[] = []; - if (details.length > 0) { - suffix.push(details.join("; ")); - } - if (risks.size > 0) { - suffix.push(`high-risk: ${[...risks].join(", ")}`); - } - return suffix.length > 0 ? `${label} (${suffix.join("; ")})` : label; + return details.length > 0 ? details.join("; ") : undefined; } function summarizePermissionArray( @@ -418,7 +418,10 @@ function summarizePermissionArray( risks.add(risk); } } - const sampleValues = values.slice(0, 2).map(descriptor.sanitize).filter(Boolean); + const sampleValues = values + .slice(0, PERMISSION_SAMPLE_LIMIT) + .map(descriptor.sanitize) + .filter(Boolean); if (sampleValues.length === 0) { return `${descriptor.label}: ${values.length}`; } @@ -435,16 +438,31 @@ function readStringArray(record: JsonObject, key: string): string[] { } function sanitizePermissionHostValue(value: string): string { - return truncate(value.replace(/\s+/g, " ").trim().toLowerCase(), 48); + const compact = sanitizePermissionScalar(value).toLowerCase(); + const withoutScheme = compact.replace(/^[a-z][a-z0-9+.-]*:\/\//, ""); + const authority = withoutScheme.split(/[/?#]/, 1)[0] ?? withoutScheme; + const withoutUserInfo = authority.includes("@") + ? authority.slice(authority.lastIndexOf("@") + 1) + : authority; + return truncate(withoutUserInfo, PERMISSION_VALUE_MAX_LENGTH); } function sanitizePermissionPathValue(value: string): string { - const normalized = value.replace(/\s+/g, " ").trim(); + const normalized = sanitizePermissionScalar(value); const homeCompacted = normalized .replace(/^\/home\/[^/]+(?=\/|$)/, "~") .replace(/^\/Users\/[^/]+(?=\/|$)/, "~") .replace(/^[A-Za-z]:\\Users\\[^\\]+(?=\\|$)/, "~"); - return truncate(homeCompacted, 48); + return truncate(homeCompacted, PERMISSION_VALUE_MAX_LENGTH); +} + +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(); } function permissionHostRisks(value: string): string[] {