diff --git a/extensions/codex/src/app-server/approval-bridge.test.ts b/extensions/codex/src/app-server/approval-bridge.test.ts index 4976a62d1c9..826c0f7bfc2 100644 --- a/extensions/codex/src/app-server/approval-bridge.test.ts +++ b/extensions/codex/src/app-server/approval-bridge.test.ts @@ -512,7 +512,7 @@ describe("Codex app-server approval bridge", () => { expect(mockCallGatewayTool).not.toHaveBeenCalled(); expect(params.onAgentEvent).not.toHaveBeenCalled(); }); - it("labels permission approvals explicitly with sanitized permission detail", async () => { + it("labels permission approvals explicitly with permission detail", async () => { const params = createParams(); mockCallGatewayTool .mockResolvedValueOnce({ id: "plugin:approval-3", status: "accepted" }) @@ -554,7 +554,7 @@ describe("Codex app-server approval bridge", () => { 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("File system roots: /; writePaths: /home/simone"); expect(description).toContain( "High-risk targets: wildcard hosts, private-network wildcards, filesystem root, home directory", ); @@ -565,7 +565,7 @@ describe("Codex app-server approval bridge", () => { ); }); - it("keeps permission detail bounded with truncated and redacted target samples", async () => { + it("keeps permission detail bounded with truncated target samples", async () => { const params = createParams(); mockCallGatewayTool .mockResolvedValueOnce({ id: "plugin:approval-4", status: "accepted" }) @@ -608,12 +608,42 @@ describe("Codex app-server approval bridge", () => { 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("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:"); + expect(description).toContain("readPaths: /Users/simone/.ssh/id_rsa"); + }); + + it("preserves Windows home paths in permission descriptions", async () => { + const params = createParams(); + mockCallGatewayTool + .mockResolvedValueOnce({ id: "plugin:approval-windows-home", status: "accepted" }) + .mockResolvedValueOnce({ id: "plugin:approval-windows-home", decision: "allow-once" }); + + await handleCodexAppServerApprovalRequest({ + method: "item/permissions/requestApproval", + requestParams: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "perm-windows-home", + permissions: { + fileSystem: { + roots: ["C:/Users/alice"], + readPaths: ["C:\\Users\\alice\\.ssh\\id_rsa", "c:/users/bob/project"], + }, + }, + }, + paramsForRun: params, + threadId: "thread-1", + turnId: "turn-1", + }); + + const [, , requestPayload] = mockCallGatewayTool.mock.calls[0] ?? []; + const description = (requestPayload as { description: string }).description; + expect(description).toContain( + "File system roots: C:/Users/alice; readPaths: C:\\Users\\alice\\.ssh\\id_rsa, c:/users/bob/project", + ); + expect(description).toContain("High-risk targets: home directory"); }); it("strips terminal and invisible controls from permission descriptions", async () => { diff --git a/extensions/codex/src/app-server/approval-bridge.ts b/extensions/codex/src/app-server/approval-bridge.ts index d422726b756..f5c66737de7 100644 --- a/extensions/codex/src/app-server/approval-bridge.ts +++ b/extensions/codex/src/app-server/approval-bridge.ts @@ -319,8 +319,9 @@ function describeRequestedPermissions(requestParams: JsonObject | undefined): st if (kinds.length > 0) { lines.push(`Permissions: ${kinds.join(", ")}`); } + let networkSummary: string | undefined; if (isJsonObject(permissions.network)) { - const networkSummary = summarizePermissionRecord(permissions.network, risks, [ + networkSummary = summarizePermissionRecord(permissions.network, risks, [ { key: "allowHosts", label: "allowHosts", @@ -328,12 +329,10 @@ function describeRequestedPermissions(requestParams: JsonObject | undefined): st risksFor: permissionHostRisks, }, ]); - if (networkSummary) { - lines.push(`Network ${networkSummary}`); - } } + let fileSystemSummary: string | undefined; if (isJsonObject(permissions.fileSystem)) { - const fileSystemSummary = summarizePermissionRecord(permissions.fileSystem, risks, [ + fileSystemSummary = summarizePermissionRecord(permissions.fileSystem, risks, [ { key: "roots", label: "roots", @@ -353,13 +352,16 @@ function describeRequestedPermissions(requestParams: JsonObject | undefined): st risksFor: permissionPathRisks, }, ]); - if (fileSystemSummary) { - lines.push(`File system ${fileSystemSummary}`); - } } if (risks.size > 0) { lines.push(`High-risk targets: ${[...risks].join(", ")}`); } + if (networkSummary) { + lines.push(`Network ${networkSummary}`); + } + if (fileSystemSummary) { + lines.push(`File system ${fileSystemSummary}`); + } return lines; } @@ -429,12 +431,7 @@ function sanitizePermissionHostValue(value: string): string { } function sanitizePermissionPathValue(value: string): string { - const normalized = sanitizePermissionScalar(value); - const homeCompacted = normalized - .replace(/^\/home\/[^/]+(?=\/|$)/, "~") - .replace(/^\/Users\/[^/]+(?=\/|$)/, "~") - .replace(/^[A-Za-z]:\\Users\\[^\\]+(?=\\|$)/, "~"); - return truncate(homeCompacted, PERMISSION_VALUE_MAX_LENGTH); + return truncate(sanitizePermissionScalar(value), PERMISSION_VALUE_MAX_LENGTH); } function sanitizePermissionScalar(value: string): string { @@ -454,12 +451,19 @@ function permissionHostRisks(value: string): string[] { } function permissionPathRisks(value: string): string[] { - const normalized = sanitizePermissionPathValue(value); + const normalized = sanitizePermissionScalar(value); const risks: string[] = []; if (normalized === "/" || normalized === "\\" || /^[A-Za-z]:[\\/]*$/.test(normalized)) { risks.push("filesystem root"); } - if (normalized === "~" || normalized === "~/" || normalized === "~\\") { + if ( + normalized === "~" || + normalized === "~/" || + normalized === "~\\" || + /^\/home\/[^/]+\/?$/.test(normalized) || + /^\/Users\/[^/]+\/?$/.test(normalized) || + /^[A-Za-z]:[\\/]Users[\\/][^\\/]+[\\/]?$/i.test(normalized) + ) { risks.push("home directory"); } return risks;