fix(codex): preserve approval permission paths

This commit is contained in:
Peter Steinberger
2026-04-25 02:18:06 +01:00
parent 2cacd2097b
commit 0d3a5c3101
2 changed files with 56 additions and 22 deletions

View File

@@ -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 () => {

View File

@@ -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;