feat(runner): bind ACPX permission policy (#12391)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - ACP agents can request permission before reads, process execution, and workspace mutation. > - The runner must apply the configured policy without allowing provider display text to grant authority. > - Runner-owned semantic tools already have a separate run-scoped authorization catalog. > - This pull request defines the local permission decision and the narrow metadata needed to recognize those authorized tools. > - The benefit is a fail-closed permission boundary before an ACPX process can use it. ## Linked Issues or Issue Description **Agent or provider** Qualified Pi, Claude, and Codex ACP servers through the internal ACPX driver. **Why this adapter is useful** ACP providers use permission requests for both ordinary provider operations and runner-owned semantic operations. Paperclip must apply `approve-all`, `approve-reads`, or `deny-all` consistently while keeping semantic authorization bound to structural MCP metadata. **How the agent is invoked** A later pull request will install this policy in the private ACPX runtime host. This pull request does not launch a provider, add a dependency, register an adapter, or change runtime selection. **Additional context** This pull request is stacked on #12390. Pi uses a different bridge and never receives semantic auto-approval through this ACP permission path. ## What Changed - Map each ACPX permission mode to a closed runtime policy. - Decide local allow, reject, or coordinator delegation outcomes. - Auto-approve only runner-owned semantic MCP calls identified by structural metadata. - Ignore provider display titles when determining semantic authority. - Limit Codex blanket MCP approval to sessions where every configured MCP server is runner-owned. - Add table-driven tests for all modes, agents, metadata shapes, spoofed titles, and non-runner servers. ## Verification - Runner TypeScript typecheck — passed. - Runner TypeScript tests — passed, including 10 new permission-policy assertions. - `pnpm -r typecheck` — passed for all applicable workspaces. - `pnpm build` — passed, including runner binary, server, UI, and workspace packages. - Prettier and `git diff --check` — passed. - The diff contains 2 files and does not change `pnpm-lock.yaml`, a workflow, a dependency, a public export, server selection, or UI behavior. ## Risks The main risk is mistaking a provider-controlled label for an authorized semantic tool. The implementation ignores display titles and requires a runner-owned MCP server name, a transport tool name, or provider metadata. All other `approve-reads` mutations delegate to the coordinator, and the caller must reject them when no delegate exists. ## Model Used OpenAI Codex with GPT-5 and repository tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either linked an existing public item or described the issue in this PR - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal task identifier - [x] I have run the affected tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have documented the permission and semantic-authorization boundary - [ ] All applicable GitHub Actions are green - [ ] Greptile is 5/5 with every actionable comment resolved - [x] I will address all review findings before requesting merge
This commit is contained in:
parent
da6124adf2
commit
07a80cda41
|
|
@ -0,0 +1,119 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
acpxRuntimePermissionPolicy,
|
||||
decideAcpxPermission,
|
||||
} from "./permission-policy.js";
|
||||
|
||||
describe("ACPX permission policy", () => {
|
||||
it("maps each configured mode to a closed ACP runtime policy", () => {
|
||||
expect(acpxRuntimePermissionPolicy("approve-all")).toEqual({
|
||||
defaultAction: "approve",
|
||||
});
|
||||
expect(acpxRuntimePermissionPolicy("deny-all")).toEqual({
|
||||
defaultAction: "deny",
|
||||
});
|
||||
expect(acpxRuntimePermissionPolicy("approve-reads")).toEqual({
|
||||
defaultAction: "escalate",
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
["approve-all", "execute", "allow_once"],
|
||||
["approve-reads", "read", "delegate"],
|
||||
["approve-reads", "search", "delegate"],
|
||||
["approve-reads", "execute", "delegate"],
|
||||
["deny-all", "read", "reject_once"],
|
||||
] as const)("%s maps %s to %s", (mode, inferredKind, expected) => {
|
||||
expect(
|
||||
decideAcpxPermission("claude", mode, { inferredKind, raw: {} }),
|
||||
).toBe(expected);
|
||||
});
|
||||
|
||||
it("keeps deny-all closed against provider-supplied semantic metadata", () => {
|
||||
for (const [agent, raw] of [
|
||||
["claude", { toolCall: { name: "mcp__paperclip__paperclip_finish" } }],
|
||||
["claude", { toolCall: { rawInput: { serverName: "paperclip" } } }],
|
||||
[
|
||||
"codex",
|
||||
{
|
||||
_meta: { is_mcp_tool_approval: true },
|
||||
toolCall: { title: "MCP approval" },
|
||||
},
|
||||
],
|
||||
] as const) {
|
||||
expect(
|
||||
decideAcpxPermission(
|
||||
agent,
|
||||
"deny-all",
|
||||
{ inferredKind: "execute", raw },
|
||||
{ allConfiguredMcpServersAreRunnerOwned: true },
|
||||
),
|
||||
).toBe("reject_once");
|
||||
}
|
||||
});
|
||||
|
||||
it("does not let provider metadata widen approve-reads", () => {
|
||||
for (const [agent, inferredKind, raw, options] of [
|
||||
[
|
||||
"codex",
|
||||
"execute",
|
||||
{
|
||||
_meta: { is_mcp_tool_approval: true },
|
||||
toolCall: { title: "MCP approval" },
|
||||
},
|
||||
{ allConfiguredMcpServersAreRunnerOwned: true },
|
||||
],
|
||||
[
|
||||
"claude",
|
||||
"write",
|
||||
{ toolCall: { rawInput: { serverName: "paperclip" } } },
|
||||
{},
|
||||
],
|
||||
[
|
||||
"claude",
|
||||
"execute",
|
||||
{ toolCall: { name: "mcp__paperclip__paperclip_finish" } },
|
||||
{},
|
||||
],
|
||||
[
|
||||
"claude",
|
||||
"write",
|
||||
{
|
||||
toolCall: {
|
||||
_meta: {
|
||||
claudeCode: { toolName: "mcp.paperclip.get_task_context" },
|
||||
},
|
||||
},
|
||||
},
|
||||
{},
|
||||
],
|
||||
] as const) {
|
||||
expect(
|
||||
decideAcpxPermission(
|
||||
agent,
|
||||
"approve-reads",
|
||||
{ inferredKind, raw },
|
||||
options,
|
||||
),
|
||||
).toBe("delegate");
|
||||
}
|
||||
});
|
||||
|
||||
it("does not trust provider-originated read classifications", () => {
|
||||
for (const inferredKind of ["read", "search", "list", "READ"]) {
|
||||
expect(
|
||||
decideAcpxPermission("codex", "approve-reads", {
|
||||
inferredKind,
|
||||
raw: {
|
||||
_meta: { is_mcp_tool_approval: true },
|
||||
toolCall: {
|
||||
name: "mcp__paperclip__get_task_context",
|
||||
rawInput: { serverName: "paperclip" },
|
||||
},
|
||||
},
|
||||
}),
|
||||
).toBe("delegate");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,54 @@
|
|||
import type { NativeAcpxPermissionMode } from "../../contracts/native-execution.js";
|
||||
import type { QualifiedAcpxAgent } from "./qualified-profiles.js";
|
||||
|
||||
export interface AcpxPermissionRequestLike {
|
||||
inferredKind?: unknown;
|
||||
raw?: unknown;
|
||||
}
|
||||
|
||||
export type AcpxPermissionDisposition =
|
||||
"allow_once" | "reject_once" | "delegate";
|
||||
|
||||
export interface AcpxPermissionPolicyOptions {
|
||||
/** Descriptive configuration only; never proof that a request is authorized. */
|
||||
runnerOwnedMcpServerNames?: ReadonlySet<string>;
|
||||
/** Descriptive configuration only; never proof that a request is authorized. */
|
||||
allConfiguredMcpServersAreRunnerOwned?: boolean;
|
||||
}
|
||||
|
||||
export interface AcpxRuntimePermissionPolicy {
|
||||
autoApprove?: readonly string[];
|
||||
escalate?: readonly string[];
|
||||
defaultAction: "approve" | "deny" | "escalate";
|
||||
}
|
||||
|
||||
export function acpxRuntimePermissionPolicy(
|
||||
mode: NativeAcpxPermissionMode,
|
||||
): AcpxRuntimePermissionPolicy {
|
||||
if (mode === "approve-all") return { defaultAction: "approve" };
|
||||
if (mode === "deny-all") return { defaultAction: "deny" };
|
||||
// ACPX derives permission kinds from provider-originated requests. Until the
|
||||
// host can bind a request to independent authority, no kind is safe to
|
||||
// auto-approve. Escalation delegates to the coordinator and otherwise fails
|
||||
// closed through the non-interactive permission policy.
|
||||
return { defaultAction: "escalate" };
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide the local part of an ACP permission request. `delegate` means the
|
||||
* caller must ask the coordinator and fail closed when no delegate exists.
|
||||
*/
|
||||
export function decideAcpxPermission(
|
||||
_agent: QualifiedAcpxAgent,
|
||||
mode: NativeAcpxPermissionMode,
|
||||
_request: AcpxPermissionRequestLike,
|
||||
_options: AcpxPermissionPolicyOptions = {},
|
||||
): AcpxPermissionDisposition {
|
||||
if (mode === "deny-all") return "reject_once";
|
||||
if (mode === "approve-all") return "allow_once";
|
||||
// inferredKind and raw semantic/MCP metadata both originate outside the
|
||||
// runner trust boundary. Neither can grant local read or semantic authority.
|
||||
// A future verified decision must carry runner-issued call identity bound to
|
||||
// the run-scoped catalog; absent that proof, the coordinator decides.
|
||||
return "delegate";
|
||||
}
|
||||
Loading…
Reference in New Issue