mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 00:53:38 +02:00
fix(connections): default new connections to allowed for every action
Agents get full permissions on a fresh connection so their tasks are not parked on approval cards. The connect flow lands on the Permissions tab, where any write or destructive action can be set to Ask first or Off. Reverts the C6a ask-first default and the server test assertions that encoded it; the Railway callback interstitial test is kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
8f48b4533a
commit
43a65aabc5
5 files changed
+37
-102
No files matched your search
@@ -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", () => {
|
||||
|
||||
@@ -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<string, unknown> {
|
||||
// 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: [],
|
||||
};
|
||||
}
|
||||
@@ -330,7 +330,6 @@ export {
|
||||
getRecommendedConnectionMethod,
|
||||
isConnectableAppSlug,
|
||||
isAppStoreVisibleSlug,
|
||||
DEFAULT_ASK_FIRST_RISK_LEVELS,
|
||||
recommendedDefaultsForApp,
|
||||
resolveConnectionMethodServerUrl,
|
||||
} from "./app-definitions.js";
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
Reference in new issue
Block a user