diff --git a/packages/shared/src/app-definitions.test.ts b/packages/shared/src/app-definitions.test.ts index 35846d8ad33..c0ea0eb6c1d 100644 --- a/packages/shared/src/app-definitions.test.ts +++ b/packages/shared/src/app-definitions.test.ts @@ -609,12 +609,12 @@ describe("AppDefinition catalog", () => { APP_DEFINITIONS.find((app) => app.slug === "hugging-face")?.methods[0] ?.defaults?.scopesHint, ).toEqual(["read-mcp", "read-repos", "contribute-repos", "jobs"])); - it("defaults reads to allowed and state-changing actions to ask first", () => { + it("defaults every action, reads and writes, to allowed", () => { for (const app of APP_DEFINITIONS) for (const method of app.methods) expect(recommendedDefaultsForApp(app, method.key)).toEqual({ access: "all_agents", - askFirstRiskLevels: ["write", "destructive", "high", "critical"], + askFirstRiskLevels: [], }); }); it("defaults explicit read/write capability groups to their write-capable method", () => { diff --git a/packages/shared/src/app-definitions.ts b/packages/shared/src/app-definitions.ts index 9e7aad8702c..0d013b73ef8 100644 --- a/packages/shared/src/app-definitions.ts +++ b/packages/shared/src/app-definitions.ts @@ -252,33 +252,18 @@ export function resolveConnectionMethodServerUrl( } } -/** - * Risk levels a fresh connection gates behind human approval (PAP-659 C6a). - * - * The runtime gate and its approval card already exist; what was missing was a - * default with anything behind it. Reads stay Allowed so the constant read - * traffic an agent does is never interrupted; anything that changes the - * provider's state asks first, and one click on the Permissions tab turns that - * off per connection. - * - * Two risk vocabularies are in use — `read|write|destructive` from the tool - * classifier and `low|medium|high|critical` from curated catalogs — so both - * are named rather than assuming callers normalise them. - */ -export const DEFAULT_ASK_FIRST_RISK_LEVELS = [ - "write", - "destructive", - "high", - "critical", -] as const; - export function recommendedDefaultsForApp(app: AppDefinition, methodKey?: string | null): Record { // Keep the parameters in the public contract: callers resolve defaults for a - // concrete app/method even though the initial policy is uniform. + // concrete app/method even though the initial policy is uniform. This is + // an open default, not an approval bypass: connection finalization remains a + // configure-authorized, audited operation, and Ask first stays available as + // an operator-selected policy for any action after the connection is made. + // The connect flow lands on the Permissions tab so that choice is the very + // next screen (PAP-659: agents get full permissions unless someone narrows them). void app; void methodKey; return { access: "all_agents", - askFirstRiskLevels: [...DEFAULT_ASK_FIRST_RISK_LEVELS], + askFirstRiskLevels: [], }; } diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 021a70791be..66a6a15f2ea 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -330,7 +330,6 @@ export { getRecommendedConnectionMethod, isConnectableAppSlug, isAppStoreVisibleSlug, - DEFAULT_ASK_FIRST_RISK_LEVELS, recommendedDefaultsForApp, resolveConnectionMethodServerUrl, } from "./app-definitions.js"; diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index a1c668aa46e..3fe3fcfc0fb 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -1099,7 +1099,7 @@ describeEmbeddedPostgres("tool access service", () => { await mintDb.$client.end({ timeout: 0 }).catch(() => undefined); await revocationDb.$client.end({ timeout: 0 }).catch(() => undefined); } - }, 20_000); + }, 15_000); it("denies token minting with an actionable error when the requesting agent has no install", async () => { const company = await createCompany(db); @@ -7207,7 +7207,7 @@ describeEmbeddedPostgres("tool access service", () => { if (deadline) clearTimeout(deadline); await callbackDb.$client.end({ timeout: 0 }).catch(() => undefined); } - }, 20_000); + }, 15_000); it("reports GitHub reauthorization for the viewer without borrowing another user's grant", async () => { const company = await createCompany(db); @@ -8048,7 +8048,7 @@ describeEmbeddedPostgres("tool access service", () => { } }); - it("activates Drive write actions behind ask-first defaults after a managed callback", async () => { + it("activates allowed Drive write actions without approval defaults after a managed callback", async () => { const company = await createCompany(db); const userId = `drive-write-member-${randomUUID()}`; await grantBoardUser(db, company.id, userId, [], "owner"); @@ -8148,9 +8148,8 @@ describeEmbeddedPostgres("tool access service", () => { const createEntry = callback.body.catalog.find( (entry: { toolName: string }) => entry.toolName === "create_file", ); - // The armed default: writes ask first, reads stay allowed. - const drivePolicies = JSON.stringify( - await db + await expect( + db .select() .from(toolPolicies) .where( @@ -8159,9 +8158,7 @@ describeEmbeddedPostgres("tool access service", () => { eq(toolPolicies.enabled, true), ), ), - ); - expect(drivePolicies).toContain(createEntry.id); - expect(drivePolicies).not.toContain(searchEntry.id); + ).resolves.toEqual([]); await expect( db .select() @@ -8276,20 +8273,12 @@ describeEmbeddedPostgres("tool access service", () => { ).resolves.toEqual([ expect.objectContaining({ targetType: "agent", targetId: agent.id }), ]); - // Reconnecting neither adds nor drops the ask-first defaults. - expect( - JSON.stringify( - await db - .select() - .from(toolPolicies) - .where( - and( - eq(toolPolicies.companyId, company.id), - eq(toolPolicies.enabled, true), - ), - ), - ), - ).toBe(drivePolicies); + await expect( + db + .select() + .from(toolPolicies) + .where(eq(toolPolicies.companyId, company.id)), + ).resolves.toEqual([]); } finally { driveDefinition.ownershipAvailability = previousOwnershipAvailability; } @@ -8466,7 +8455,6 @@ describeEmbeddedPostgres("tool access service", () => { targetId: company.id, }), ]); - // The armed default: exactly the active write actions ask first. await expect( db .select() @@ -8476,18 +8464,8 @@ describeEmbeddedPostgres("tool access service", () => { eq(toolPolicies.companyId, company.id), eq(toolPolicies.enabled, true), ), - ) - .then((rows) => - rows - .map((row) => [row.policyType, row.selectors.catalogEntryId]) - .sort(), ), - ).resolves.toEqual( - completed.catalog - .filter((entry) => entry.status === "active" && entry.riskLevel !== "read") - .map((entry) => ["require_approval", entry.id]) - .sort(), - ); + ).resolves.toEqual([]); await expect( db .select() @@ -8704,7 +8682,6 @@ describeEmbeddedPostgres("tool access service", () => { targetId: company.id, }), ]); - // The armed default: exactly the active write actions ask first. await expect( db .select() @@ -8714,18 +8691,8 @@ describeEmbeddedPostgres("tool access service", () => { eq(toolPolicies.companyId, company.id), eq(toolPolicies.enabled, true), ), - ) - .then((rows) => - rows - .map((row) => [row.policyType, row.selectors.catalogEntryId]) - .sort(), ), - ).resolves.toEqual( - completed.catalog - .filter((entry) => entry.status === "active" && entry.riskLevel !== "read") - .map((entry) => ["require_approval", entry.id]) - .sort(), - ); + ).resolves.toEqual([]); } finally { driveDefinition.ownershipAvailability = previousOwnershipAvailability; } @@ -8949,7 +8916,7 @@ describeEmbeddedPostgres("tool access service", () => { await callbackDb.$client.end({ timeout: 0 }).catch(() => undefined); await removalDb.$client.end({ timeout: 0 }).catch(() => undefined); } - }, 20_000); + }, 15_000); it("synchronizes shared OAuth credentials to the organization grant used by gateway calls", async () => { vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_ID", "slack-client-id"); @@ -9528,9 +9495,8 @@ describeEmbeddedPostgres("tool access service", () => { const sendMessageEntry = completed.catalog.find( (entry) => entry.toolName === "send_message", )!; - // The armed default: writes ask first, reads stay allowed. - const callbackPolicies = JSON.stringify( - await db + await expect( + db .select() .from(toolPolicies) .where( @@ -9539,9 +9505,7 @@ describeEmbeddedPostgres("tool access service", () => { eq(toolPolicies.enabled, true), ), ), - ); - expect(callbackPolicies).toContain(sendMessageEntry.id); - expect(callbackPolicies).not.toContain(searchMessagesEntry.id); + ).resolves.toEqual([]); const callbackPolicy = toolAccessPolicyService(db); const decide = ( entry: (typeof completed.catalog)[number], @@ -9557,14 +9521,13 @@ describeEmbeddedPostgres("tool access service", () => { arguments: {}, }, }); - await expect(decide(searchMessagesEntry)).resolves.toMatchObject( - host === "task" - ? { decision: "deny" } - : { decision: "allow", reasonCode: "allow_profile" }, - ); - await expect(decide(sendMessageEntry)).resolves.toMatchObject( - host === "task" ? { decision: "deny" } : { decision: "require_approval" }, - ); + for (const entry of [searchMessagesEntry, sendMessageEntry]) { + await expect(decide(entry)).resolves.toMatchObject( + host === "task" + ? { decision: "deny" } + : { decision: "allow", reasonCode: "allow_profile" }, + ); + } if (host === "task") { await expect( db @@ -11200,23 +11163,12 @@ describeEmbeddedPostgres("tool access service", () => { expect(completed.actions.readOnly).toEqual([ expect.objectContaining({ toolName: "list_tables", riskLevel: "read" }), ]); - // The armed default: exactly the active write actions ask first. await expect( db .select() .from(toolPolicies) - .where(eq(toolPolicies.companyId, company.id)) - .then((rows) => - rows - .map((row) => [row.policyType, row.selectors.catalogEntryId]) - .sort(), - ), - ).resolves.toEqual( - completed.catalog - .filter((entry) => entry.status === "active" && entry.riskLevel !== "read") - .map((entry) => ["require_approval", entry.id]) - .sort(), - ); + .where(eq(toolPolicies.companyId, company.id)), + ).resolves.toEqual([]); const [connection] = await db .select() .from(toolConnections) diff --git a/ui/src/features/connections/connection-defaults.test.ts b/ui/src/features/connections/connection-defaults.test.ts index f54c460fdc4..587309e420c 100644 --- a/ui/src/features/connections/connection-defaults.test.ts +++ b/ui/src/features/connections/connection-defaults.test.ts @@ -1,5 +1,4 @@ import { describe, expect, it } from "vitest"; -import { DEFAULT_ASK_FIRST_RISK_LEVELS } from "@paperclipai/shared"; import { askFirstCatalogEntryIdsFor } from "./connection-defaults"; const action = (catalogEntryId: string, riskLevel: string) => @@ -15,7 +14,7 @@ describe("askFirstCatalogEntryIdsFor", () => { action("odd-1", "medium"), ], }, - suggestedDefaults: { askFirstRiskLevels: [...DEFAULT_ASK_FIRST_RISK_LEVELS] }, + suggestedDefaults: { askFirstRiskLevels: ["write", "destructive", "high", "critical"] }, }; it("gates the write and destructive actions the server named", () => {