diff --git a/src/CodexApprovalHandler.ts b/src/CodexApprovalHandler.ts index dd73b8ae..7c2e08a9 100644 --- a/src/CodexApprovalHandler.ts +++ b/src/CodexApprovalHandler.ts @@ -29,17 +29,28 @@ type FileChangeDecisionOption = { decision: FileChangeApprovalDecision; }; +type PermissionMetadata = { + version: 1; + changes: Array>; +}; + function permissionOption( optionId: string, name: string, kind: acp.PermissionOptionKind, codexMeta?: Record, + permission?: PermissionMetadata, ): acp.PermissionOption { return { optionId, name, kind, - ...(codexMeta ? { _meta: { codex: codexMeta } } : {}), + ...((codexMeta || permission) ? { + _meta: { + ...(permission ? {permission} : {}), + ...(codexMeta ? {codex: codexMeta} : {}), + }, + } : {}), }; } @@ -175,12 +186,14 @@ export class CodexApprovalHandler implements ApprovalHandler { "Allow for Session", "allow_always", { decision: "allowPermissionsForSession", permissions: params.permissions }, + this.permissionGrantMetadata(params.permissions, "session"), ), permissionOption( ApprovalOptionId.AllowPermissionsForTurn, "Allow Once", "allow_once", { decision: "allowPermissionsForTurn", permissions: params.permissions }, + this.permissionGrantMetadata(params.permissions, "turn"), ), permissionOption( ApprovalOptionId.RejectPermissions, @@ -265,6 +278,23 @@ export class CodexApprovalHandler implements ApprovalHandler { : "Allow for Session", "allow_always", { decision: "acceptForSession" }, + params.networkApprovalContext ? { + version: 1, + changes: [{ + type: "grant", + operation: "grant", + description: `Allow access to ${params.networkApprovalContext.host} for this session`, + lifetime: {scope: "session"}, + targets: [{ + type: "network", + matcher: { + type: "host", + host: params.networkApprovalContext.host, + protocol: params.networkApprovalContext.protocol, + }, + }], + }], + } : undefined, ), decision: "acceptForSession", }, @@ -280,6 +310,22 @@ export class CodexApprovalHandler implements ApprovalHandler { decision: "acceptWithExecpolicyAmendment", execpolicyAmendment: params.proposedExecpolicyAmendment, }, + { + version: 1, + changes: [{ + type: "policy_rule", + operation: "add", + ruleBehavior: "allow", + description: `Allow commands starting with ${params.proposedExecpolicyAmendment.join(" ")}`, + targets: [{ + type: "command", + matcher: { + type: "argv_prefix", + argv: params.proposedExecpolicyAmendment, + }, + }], + }], + }, ), decision: { acceptWithExecpolicyAmendment: { @@ -299,6 +345,24 @@ export class CodexApprovalHandler implements ApprovalHandler { decision: "applyNetworkPolicyAmendment", networkPolicyAmendment: amendment, }, + { + version: 1, + changes: [{ + type: "policy_rule", + operation: "add", + ruleBehavior: amendment.action, + description: amendment.action === "allow" + ? `Allow access to ${amendment.host}` + : `Block access to ${amendment.host}`, + targets: [{ + type: "network", + matcher: { + type: "host", + host: amendment.host, + }, + }], + }], + }, ), decision: { applyNetworkPolicyAmendment: { @@ -328,6 +392,20 @@ export class CodexApprovalHandler implements ApprovalHandler { params.grantRoot ? "Allow Root for Session" : "Allow for Session", "allow_always", { decision: "acceptForSession", grantRoot: params.grantRoot ?? null }, + params.grantRoot ? { + version: 1, + changes: [{ + type: "grant", + operation: "grant", + description: `Allow writes under ${params.grantRoot} for this session`, + lifetime: {scope: "session"}, + targets: [{ + type: "filesystem", + access: ["write"], + matcher: {type: "directory", path: params.grantRoot}, + }], + }], + } : undefined, ), decision: "acceptForSession", }, @@ -353,6 +431,85 @@ export class CodexApprovalHandler implements ApprovalHandler { }; } + private permissionGrantMetadata( + permissions: RequestPermissionProfile, + scope: "turn" | "session", + ): PermissionMetadata | undefined { + const changes: Array> = []; + const lifetime = {scope}; + const suffix = scope === "session" ? " for this session" : " for this turn"; + + if (permissions.network?.enabled !== null && permissions.network?.enabled !== undefined) { + const allowed = permissions.network.enabled; + changes.push({ + type: allowed ? "grant" : "policy_rule", + operation: allowed ? "grant" : "add", + ...(allowed ? {} : {ruleBehavior: "deny"}), + description: `${allowed ? "Allow" : "Deny"} network access${suffix}`, + lifetime, + targets: [{type: "network", matcher: {type: "any"}}], + }); + } + + const fileSystem = permissions.fileSystem; + for (const path of fileSystem?.read ?? []) { + changes.push(this.fileSystemGrantChange(path, "read", lifetime, suffix)); + } + for (const path of fileSystem?.write ?? []) { + changes.push(this.fileSystemGrantChange(path, "write", lifetime, suffix)); + } + for (const entry of fileSystem?.entries ?? []) { + const matcher = (() => { + switch (entry.path.type) { + case "path": + return {type: "exact_path", path: entry.path.path}; + case "glob_pattern": + return {type: "glob", pattern: entry.path.pattern}; + case "special": + return {type: "special", provider: "codex", value: entry.path.value}; + } + })(); + const pathDescription = entry.path.type === "path" + ? entry.path.path + : entry.path.type === "glob_pattern" ? entry.path.pattern : JSON.stringify(entry.path.value); + changes.push({ + type: entry.access === "deny" ? "policy_rule" : "grant", + operation: entry.access === "deny" ? "add" : "grant", + ...(entry.access === "deny" ? {ruleBehavior: "deny"} : {}), + description: entry.access === "deny" + ? `Deny filesystem access to ${pathDescription}${suffix}` + : `Allow ${entry.access} access to ${pathDescription}${suffix}`, + lifetime, + targets: [{ + type: "filesystem", + ...(entry.access === "deny" ? {} : {access: [entry.access]}), + matcher, + }], + }); + } + + return changes.length > 0 ? {version: 1, changes} : undefined; + } + + private fileSystemGrantChange( + path: string, + access: "read" | "write", + lifetime: {scope: "turn" | "session"}, + suffix: string, + ): Record { + return { + type: "grant", + operation: "grant", + description: `Allow ${access} access to ${path}${suffix}`, + lifetime, + targets: [{ + type: "filesystem", + access: [access], + matcher: {type: "exact_path", path}, + }], + }; + } + private networkPolicyAmendmentOptionId(index: number): string { return `${ApprovalOptionId.ApplyNetworkPolicyAmendment}:${index}`; } diff --git a/src/__tests__/CodexACPAgent/approval-events.test.ts b/src/__tests__/CodexACPAgent/approval-events.test.ts index 205a3802..821f23bc 100644 --- a/src/__tests__/CodexACPAgent/approval-events.test.ts +++ b/src/__tests__/CodexACPAgent/approval-events.test.ts @@ -155,6 +155,27 @@ describe('Approval Events', () => { expect.objectContaining({ optionId: ApprovalOptionId.AcceptWithExecpolicyAmendment, kind: 'allow_always', + _meta: { + permission: { + version: 1, + changes: [{ + type: 'policy_rule', + operation: 'add', + ruleBehavior: 'allow', + description: 'Allow commands starting with npm install', + targets: [{ + type: 'command', + matcher: { + type: 'argv_prefix', + argv: proposedExecpolicyAmendment, + }, + }], + }], + }, + codex: expect.objectContaining({ + execpolicyAmendment: proposedExecpolicyAmendment, + }), + }, }) ); @@ -201,6 +222,27 @@ describe('Approval Events', () => { expect.objectContaining({ optionId, kind: 'allow_always', + _meta: { + permission: { + version: 1, + changes: [{ + type: 'policy_rule', + operation: 'add', + ruleBehavior: 'allow', + description: 'Allow access to registry.npmjs.org', + targets: [{ + type: 'network', + matcher: { + type: 'host', + host: 'registry.npmjs.org', + }, + }], + }], + }, + codex: expect.objectContaining({ + networkPolicyAmendment, + }), + }, }) ); @@ -389,6 +431,46 @@ describe('Approval Events', () => { await promptPromise; }); + it('should describe a session write-root grant with common permission metadata', async () => { + const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); + fixture.setPermissionResponse({ + outcome: { outcome: 'selected', optionId: ApprovalOptionId.AllowAlways } + }); + + const params: FileChangeRequestApprovalParams = { + threadId: sessionId, + turnId: 'turn-1', + startedAtMs: 0, + itemId: 'file-change-grant-root', + reason: 'Write generated files', + grantRoot: '/workspace/generated', + }; + + await fixture.sendServerRequest('item/fileChange/requestApproval', params); + + const request = fixture.getAcpConnectionEvents([])[0]!.args[0]; + expect(request.options.find((option: { optionId: string }) => option.optionId === ApprovalOptionId.AllowAlways)?._meta) + .toMatchObject({ + permission: { + version: 1, + changes: [{ + type: 'grant', + operation: 'grant', + description: 'Allow writes under /workspace/generated for this session', + lifetime: {scope: 'session'}, + targets: [{ + type: 'filesystem', + access: ['write'], + matcher: {type: 'directory', path: '/workspace/generated'}, + }], + }], + }, + }); + + completeTurn(); + await promptPromise; + }); + it('should return cancel when no handler registered', async () => { const params: FileChangeRequestApprovalParams = { threadId: 'non-existent-session', diff --git a/src/__tests__/CodexACPAgent/data/approval-permissions-request.json b/src/__tests__/CodexACPAgent/data/approval-permissions-request.json index 99ce7a14..c5bf196b 100644 --- a/src/__tests__/CodexACPAgent/data/approval-permissions-request.json +++ b/src/__tests__/CodexACPAgent/data/approval-permissions-request.json @@ -47,6 +47,67 @@ "name": "Allow for Session", "kind": "allow_always", "_meta": { + "permission": { + "version": 1, + "changes": [ + { + "type": "grant", + "operation": "grant", + "description": "Allow network access for this session", + "lifetime": { + "scope": "session" + }, + "targets": [ + { + "type": "network", + "matcher": { + "type": "any" + } + } + ] + }, + { + "type": "grant", + "operation": "grant", + "description": "Allow read access to /home/user/project for this session", + "lifetime": { + "scope": "session" + }, + "targets": [ + { + "type": "filesystem", + "access": [ + "read" + ], + "matcher": { + "type": "exact_path", + "path": "/home/user/project" + } + } + ] + }, + { + "type": "grant", + "operation": "grant", + "description": "Allow write access to /home/user/project/tmp for this session", + "lifetime": { + "scope": "session" + }, + "targets": [ + { + "type": "filesystem", + "access": [ + "write" + ], + "matcher": { + "type": "exact_path", + "path": "/home/user/project/tmp" + } + } + ] + } + ] + }, "codex": { "decision": "allowPermissionsForSession", "permissions": { @@ -71,6 +132,67 @@ "name": "Allow Once", "kind": "allow_once", "_meta": { + "permission": { + "version": 1, + "changes": [ + { + "type": "grant", + "operation": "grant", + "description": "Allow network access for this turn", + "lifetime": { + "scope": "turn" + }, + "targets": [ + { + "type": "network", + "matcher": { + "type": "any" + } + } + ] + }, + { + "type": "grant", + "operation": "grant", + "description": "Allow read access to /home/user/project for this turn", + "lifetime": { + "scope": "turn" + }, + "targets": [ + { + "type": "filesystem", + "access": [ + "read" + ], + "matcher": { + "type": "exact_path", + "path": "/home/user/project" + } + } + ] + }, + { + "type": "grant", + "operation": "grant", + "description": "Allow write access to /home/user/project/tmp for this turn", + "lifetime": { + "scope": "turn" + }, + "targets": [ + { + "type": "filesystem", + "access": [ + "write" + ], + "matcher": { + "type": "exact_path", + "path": "/home/user/project/tmp" + } + } + ] + } + ] + }, "codex": { "decision": "allowPermissionsForTurn", "permissions": {