Skip to content

Expose structured permission changes in ACP metadata #342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Ololoshechkin merged 1 commit into from
Jul 30, 2026
+362 −1
Merged

Expose structured permission changes in ACP metadata #342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 158 additions & 1 deletion src/CodexApprovalHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,28 @@ type FileChangeDecisionOption = {
decision: FileChangeApprovalDecision;
};

type PermissionMetadata = {
version: 1;
changes: Array<Record<string, unknown>>;
};

function permissionOption(
optionId: string,
name: string,
kind: acp.PermissionOptionKind,
codexMeta?: Record<string, unknown>,
permission?: PermissionMetadata,
): acp.PermissionOption {
return {
optionId,
name,
kind,
...(codexMeta ? { _meta: { codex: codexMeta } } : {}),
...((codexMeta || permission) ? {
_meta: {
...(permission ? {permission} : {}),
...(codexMeta ? {codex: codexMeta} : {}),
},
} : {}),
};
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
},
Expand All @@ -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: {
Expand All @@ -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: {
Expand Down Expand Up @@ -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",
},
Expand All @@ -353,6 +431,85 @@ export class CodexApprovalHandler implements ApprovalHandler {
};
}

private permissionGrantMetadata(
permissions: RequestPermissionProfile,
scope: "turn" | "session",
): PermissionMetadata | undefined {
const changes: Array<Record<string, unknown>> = [];
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<string, unknown> {
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}`;
}
Expand Down
82 changes: 82 additions & 0 deletions src/__tests__/CodexACPAgent/approval-events.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}),
},
})
);

Expand Down Expand Up @@ -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,
}),
},
})
);

Expand Down Expand Up @@ -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',
Expand Down
Loading