mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 01:57:31 +01:00
fix(security): make removing an app revoke its credentials
Removing an app only flipped the connection to `archived`. Everything that
made it work stayed behind: the operator's pasted key or OAuth tokens as live
secrets, the `company_secret_bindings` that project them into runtimes, the
install, the app-managed `app:<connectionId>` profile, gateway tokens minted
against that profile, and any authorization still in flight. Reconnecting
reused the retained credential, so "remove" never actually revoked anything.
Removal is now a teardown, and the order is the security property. Every
database-side access path closes first — grants revoked, bindings and installs
deleted, the app profile removed (archived instead when a gateway's RESTRICT
foreign key forbids deleting it, with that gateway's tokens and sessions
revoked), live local runtimes stopped, catalog entries marked `removed`,
outstanding OAuth state discarded, the connection and its application archived
— so the app is undispatchable before the first call out to a secret provider.
Only then are the credentials revoked, and each secret's ref survives until
that secret is gone: a provider that errors leaves the operation failed closed
and resumable, because the secret row is marked deleted before the provider
call and the surviving refs are what a retry reads to finish the job.
A secret is destroyed only when two independent tests agree: it was minted
into the `tool_app.` namespace by connect/reconnect/OAuth, and nothing outside
this connection references it. Anything ambiguous keeps its row and is
reported as retained — deleting a credential another target still resolves is
an outage, retaining an unreachable row is not.
What stays is deliberate. Connection and application identity, names and
activity survive so a reconnect reuses them, but with no credential, install
or profile, reconnecting has to ask for fresh authentication and rebuild
access. The activity receipt carries cleanup counts only; six of those count
keys name what they counted ("secrets", "token"), so `sanitizeRecord` lets
them through while the value is a finite number and still blanks the same key
carrying anything else.
Refs PAP-17119, PAP-17118, PAP-17089 (QA finding F6).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
DottaandClaude Opus 5 committed
1 parent
ba603abace
commit
1d000bc903
14 files changed
+1329
-46
No files matched your search
@@ -1338,6 +1338,8 @@ export type {
|
||||
ToolConnectionInstall,
|
||||
ToolConnectionInstallSnapshot,
|
||||
ToolConnectionInstallTargetType,
|
||||
ToolConnectionRemovalResult,
|
||||
ToolConnectionRemovalSummary,
|
||||
ConnectionGrant,
|
||||
ConnectionGrantKind,
|
||||
ConnectionGrantStatus,
|
||||
|
||||
@@ -470,6 +470,8 @@ export type {
|
||||
ToolConnectionInstall,
|
||||
ToolConnectionInstallSnapshot,
|
||||
ToolConnectionInstallTargetType,
|
||||
ToolConnectionRemovalResult,
|
||||
ToolConnectionRemovalSummary,
|
||||
ConnectionTokenAttribution,
|
||||
ConnectionRecoverableErrorCode,
|
||||
ConnectionRecoverableErrorPayload,
|
||||
|
||||
@@ -190,6 +190,51 @@ export interface ToolConnectionInstallSnapshot {
|
||||
installs: ToolConnectionInstall[];
|
||||
}
|
||||
|
||||
/**
|
||||
* What a connection removal actually tore down (PAP-17119).
|
||||
*
|
||||
* Removing an app is a revocation boundary, not a cosmetic archive, so the
|
||||
* receipt is counts and outcomes only — never a secret name, key, or value,
|
||||
* because this summary is echoed into the activity log the whole company reads.
|
||||
*/
|
||||
export interface ToolConnectionRemovalSummary {
|
||||
/** Connection-owned secrets revoked at the provider and deleted locally. */
|
||||
secretsRevoked: number;
|
||||
/** Secrets left in place because another consumer still binds them. */
|
||||
secretsRetainedShared: number;
|
||||
/** Credential refs cleared off the connection row. */
|
||||
credentialRefsCleared: number;
|
||||
/** `company_secret_bindings` rows removed for this connection. */
|
||||
secretBindingsRemoved: number;
|
||||
grantsRevoked: number;
|
||||
installsRemoved: number;
|
||||
/**
|
||||
* `deleted` when the app-managed `app:<connectionId>` profile could go away,
|
||||
* `archived` when a gateway still references it (the row survives with no
|
||||
* entries and a non-active status), `absent` when there was never one.
|
||||
*/
|
||||
appProfile: "deleted" | "archived" | "absent";
|
||||
appProfileEntriesRemoved: number;
|
||||
appProfileBindingsRemoved: number;
|
||||
catalogEntriesMarkedRemoved: number;
|
||||
oauthStatesDiscarded: number;
|
||||
/** Token hashes wiped from the retained connection-token issuance ledger. */
|
||||
tokenIssuanceHashesCleared: number;
|
||||
/**
|
||||
* Live local runtimes shut down. A running child process holds the injected
|
||||
* credential in memory, so it is an access path of its own.
|
||||
*/
|
||||
runtimeSlotsStopped: number;
|
||||
gatewayTokensRevoked: number;
|
||||
gatewaySessionsRevoked: number;
|
||||
applicationArchived: boolean;
|
||||
}
|
||||
|
||||
export interface ToolConnectionRemovalResult {
|
||||
connection: ToolConnection;
|
||||
removal: ToolConnectionRemovalSummary;
|
||||
}
|
||||
|
||||
export type ConnectionTokenScope = string | string[];
|
||||
export type ConnectionTokenSubject = { type: "app" } | { type: "user"; userId: string };
|
||||
|
||||
|
||||
@@ -76,6 +76,45 @@ describe("redaction", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* A removal receipt (PAP-17119) has to show what it revoked, so a fixed set of
|
||||
* count keys is exempt from the secret-key guard — but only while the value is
|
||||
* a number. The second half of this test is the point: the same key carrying
|
||||
* anything else is still blanked, so the exemption cannot be used to smuggle
|
||||
* material out under a familiar name.
|
||||
*/
|
||||
it("keeps numeric removal-receipt counts but still redacts non-numeric values on the same keys", () => {
|
||||
expect(sanitizeRecord({
|
||||
secretsRevoked: 2,
|
||||
secretsRetainedShared: 0,
|
||||
credentialRefsCleared: 3,
|
||||
secretBindingsRemoved: 3,
|
||||
tokenIssuanceHashesCleared: 1,
|
||||
gatewayTokensRevoked: 0,
|
||||
appProfile: "deleted",
|
||||
})).toEqual({
|
||||
secretsRevoked: 2,
|
||||
secretsRetainedShared: 0,
|
||||
credentialRefsCleared: 3,
|
||||
secretBindingsRemoved: 3,
|
||||
tokenIssuanceHashesCleared: 1,
|
||||
gatewayTokensRevoked: 0,
|
||||
appProfile: "deleted",
|
||||
});
|
||||
|
||||
expect(sanitizeRecord({
|
||||
secretsRevoked: "pasted-api-key-value",
|
||||
secretBindingsRemoved: { name: "tool_app.abc.headers_authorization" },
|
||||
tokenIssuanceHashesCleared: Number.NaN,
|
||||
gatewayTokensRevoked: ["pcgw_live_token"],
|
||||
})).toEqual({
|
||||
secretsRevoked: REDACTED_EVENT_VALUE,
|
||||
secretBindingsRemoved: REDACTED_EVENT_VALUE,
|
||||
tokenIssuanceHashesCleared: REDACTED_EVENT_VALUE,
|
||||
gatewayTokensRevoked: REDACTED_EVENT_VALUE,
|
||||
});
|
||||
});
|
||||
|
||||
it("redacts common secret shapes from unstructured text", () => {
|
||||
const jwt = "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c";
|
||||
const githubToken = "ghp_1234567890abcdefghijklmnopqrstuvwxyz";
|
||||
|
||||
@@ -0,0 +1,672 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
activityLog,
|
||||
agents,
|
||||
authUsers,
|
||||
companies,
|
||||
companyMemberships,
|
||||
companySecretBindings,
|
||||
companySecrets,
|
||||
companySecretVersions,
|
||||
connectionGrants,
|
||||
connectionTokenIssuances,
|
||||
createDb,
|
||||
heartbeatRuns,
|
||||
issues,
|
||||
secretAccessEvents,
|
||||
toolAccessAuditEvents,
|
||||
toolApplications,
|
||||
toolCatalogEntries,
|
||||
toolConnectionInstalls,
|
||||
toolConnections,
|
||||
toolGatewaySessions,
|
||||
toolMcpGateways,
|
||||
toolMcpGatewayTokens,
|
||||
toolOauthStates,
|
||||
toolProfileBindings,
|
||||
toolProfileEntries,
|
||||
toolProfiles,
|
||||
toolRuntimeSlots,
|
||||
} from "@paperclipai/db";
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { toolAccessService } from "../services/tool-access.js";
|
||||
import { toolAccessPolicyService } from "../services/tool-access-policy.js";
|
||||
import { toolAccessRoutes } from "../routes/tool-access.js";
|
||||
import { secretService } from "../services/secrets.js";
|
||||
import { localEncryptedProvider } from "../secrets/local-encrypted-provider.js";
|
||||
import { errorHandler } from "../middleware/index.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
/**
|
||||
* PAP-17119 — "Remove app" is a credential revocation boundary, not a status
|
||||
* flip. Everything here asks the same question from a different angle: after a
|
||||
* removal, is there any surviving row that still resolves a credential or still
|
||||
* grants an agent a way to call the app?
|
||||
*
|
||||
* The connection is built through the real connect path where that is possible,
|
||||
* so the tests fail if `connectGalleryApp` ever starts writing credential state
|
||||
* somewhere the teardown does not look.
|
||||
*/
|
||||
|
||||
// A public IP literal, matching the generic MCP suite: remote endpoints are
|
||||
// DNS-pinned even in local/private mode (PAP-17098), so a made-up hostname would
|
||||
// fail resolution before it ever reached this fixture.
|
||||
const MCP_ORIGIN = "https://203.0.113.10";
|
||||
const MCP_URL = `${MCP_ORIGIN}/mcp`;
|
||||
|
||||
const FIXTURE_TOOLS = [
|
||||
{ name: "list_things", description: "List things", annotations: { readOnlyHint: true } },
|
||||
{ name: "delete_thing", description: "Delete a thing", annotations: { readOnlyHint: false, destructiveHint: true } },
|
||||
];
|
||||
|
||||
function jsonResponse(payload: unknown, status = 200): Response {
|
||||
const body = JSON.stringify(payload);
|
||||
return {
|
||||
ok: status >= 200 && status < 300,
|
||||
status,
|
||||
headers: {
|
||||
get: (name: string) => (name.toLowerCase() === "content-type" ? "application/json" : null),
|
||||
},
|
||||
text: async () => body,
|
||||
json: async () => payload,
|
||||
} as unknown as Response;
|
||||
}
|
||||
|
||||
function headerRecord(init: RequestInit | undefined): Record<string, string> {
|
||||
const raw = init?.headers;
|
||||
if (!raw) return {};
|
||||
if (raw instanceof Headers) return Object.fromEntries(raw.entries());
|
||||
if (Array.isArray(raw)) return Object.fromEntries(raw as Array<[string, string]>);
|
||||
return Object.fromEntries(
|
||||
Object.entries(raw as Record<string, string>).map(([key, value]) => [key.toLowerCase(), value]),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* The narrowest endpoint that satisfies a header-authenticated connect: it lists
|
||||
* tools when the expected header arrives and 401s otherwise, which is all the
|
||||
* catalog refresh and health check need.
|
||||
*/
|
||||
function installMcpFixture(requiredHeader: { name: string; value: string }) {
|
||||
return vi.spyOn(globalThis, "fetch").mockImplementation(async (url, init) => {
|
||||
const href = String(url);
|
||||
const method = (init?.method ?? "GET").toUpperCase();
|
||||
const headers = headerRecord(init);
|
||||
if (href === MCP_URL && method === "POST") {
|
||||
if (headers[requiredHeader.name.toLowerCase()] !== requiredHeader.value) {
|
||||
return jsonResponse({ error: "unauthorized" }, 401);
|
||||
}
|
||||
return jsonResponse({ jsonrpc: "2.0", id: "paperclip-catalog-refresh", result: { tools: FIXTURE_TOOLS } });
|
||||
}
|
||||
return jsonResponse({ error: "not_found" }, 404);
|
||||
});
|
||||
}
|
||||
|
||||
async function createCompany(db: ReturnType<typeof createDb>) {
|
||||
return db
|
||||
.insert(companies)
|
||||
.values({
|
||||
name: `Removal ${randomUUID()}`,
|
||||
issuePrefix: `RM${randomUUID().slice(0, 6).toUpperCase()}`,
|
||||
})
|
||||
.returning()
|
||||
.then((rows) => rows[0]!);
|
||||
}
|
||||
|
||||
async function createAgent(db: ReturnType<typeof createDb>, companyId: string) {
|
||||
return db
|
||||
.insert(agents)
|
||||
.values({
|
||||
companyId,
|
||||
name: `Removal agent ${randomUUID()}`,
|
||||
role: "engineer",
|
||||
status: "active",
|
||||
adapterType: "process",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
})
|
||||
.returning()
|
||||
.then((rows) => rows[0]!);
|
||||
}
|
||||
|
||||
function createRouteApp(db: ReturnType<typeof createDb>) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => {
|
||||
req.actor = {
|
||||
type: "board",
|
||||
userId: "board-user",
|
||||
userName: "Board User",
|
||||
userEmail: null,
|
||||
isInstanceAdmin: true,
|
||||
source: "local_implicit",
|
||||
};
|
||||
next();
|
||||
});
|
||||
app.use("/api", toolAccessRoutes(db));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
|
||||
describeEmbeddedPostgres("tool connection removal", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
|
||||
const HEADER = { name: "X-Api-Key", value: "fixture-secret-value" };
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-removal-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
vi.unstubAllEnvs();
|
||||
await db.delete(toolGatewaySessions);
|
||||
await db.delete(toolMcpGatewayTokens);
|
||||
await db.delete(toolMcpGateways);
|
||||
await db.delete(connectionTokenIssuances);
|
||||
await db.delete(connectionGrants);
|
||||
await db.delete(toolOauthStates);
|
||||
await db.delete(secretAccessEvents);
|
||||
await db.delete(companySecretBindings);
|
||||
await db.delete(companySecretVersions);
|
||||
await db.delete(companySecrets);
|
||||
await db.delete(activityLog);
|
||||
await db.delete(toolAccessAuditEvents);
|
||||
await db.delete(toolRuntimeSlots);
|
||||
await db.delete(toolConnectionInstalls);
|
||||
await db.delete(toolProfileBindings);
|
||||
await db.delete(toolProfileEntries);
|
||||
await db.delete(toolProfiles);
|
||||
await db.delete(toolCatalogEntries);
|
||||
await db.delete(toolConnections);
|
||||
await db.delete(toolApplications);
|
||||
await db.delete(heartbeatRuns);
|
||||
await db.delete(issues);
|
||||
await db.delete(companyMemberships);
|
||||
await db.delete(agents);
|
||||
await db.delete(companies);
|
||||
await db.delete(authUsers);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
/** Connect a header-authenticated app and finish the wizard for one agent. */
|
||||
async function connectHeaderApp(companyId: string, agentId: string) {
|
||||
const service = toolAccessService(db);
|
||||
const connected = await service.connectGalleryApp(companyId, {
|
||||
link: MCP_URL,
|
||||
name: `Removal fixture ${randomUUID().slice(0, 8)}`,
|
||||
credentialValues: { [`headers.${HEADER.name}`]: HEADER.value },
|
||||
});
|
||||
const readEntry = connected.catalog.find((entry) => entry.toolName === "list_things")!;
|
||||
const finished = await service.finishGalleryAppConnection(companyId, connected.connectionId, {
|
||||
enabledCatalogEntryIds: [readEntry.id],
|
||||
askFirstCatalogEntryIds: [],
|
||||
access: { agentIds: [agentId] },
|
||||
}, { actorType: "user", actorId: "board-user" });
|
||||
await service.putConnectionInstalls(connected.connectionId, {
|
||||
installs: [{ targetType: "agent", targetId: agentId, enabled: true }],
|
||||
}, { actorType: "user", actorId: "board-user" });
|
||||
return { service, connectionId: connected.connectionId, readEntry, profile: finished.profile };
|
||||
}
|
||||
|
||||
it("revokes the custom-header credential and every access path it granted", async () => {
|
||||
installMcpFixture(HEADER);
|
||||
const company = await createCompany(db);
|
||||
const agent = await createAgent(db, company.id);
|
||||
const policy = toolAccessPolicyService(db);
|
||||
const { service, connectionId, readEntry, profile } = await connectHeaderApp(company.id, agent.id);
|
||||
|
||||
const [before] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
// One pasted header value is one secret, referenced twice: once as a header
|
||||
// credential ref and once as a secret ref.
|
||||
const secretIds = [...new Set([
|
||||
...before!.credentialRefs.map((ref) => ref.secretId),
|
||||
...before!.credentialSecretRefs.map((ref) => ref.secretId),
|
||||
])];
|
||||
expect(secretIds).toHaveLength(1);
|
||||
expect(before!.credentialRefs).toHaveLength(1);
|
||||
expect(before!.credentialSecretRefs).toHaveLength(1);
|
||||
// The credential really does resolve before removal, so the assertions after
|
||||
// it are measuring a change rather than a permanent failure.
|
||||
await expect(secretService(db).resolveSecretValue(company.id, secretIds[0]!, "latest", {
|
||||
consumerType: "tool_connection",
|
||||
consumerId: connectionId,
|
||||
configPath: `credentials.headers.${HEADER.name}`,
|
||||
actorType: "system",
|
||||
})).resolves.toBe(HEADER.value);
|
||||
await expect(policy.decide({
|
||||
companyId: company.id,
|
||||
actor: { actorType: "agent", actorId: agent.id, agentId: agent.id },
|
||||
request: { connectionId, catalogEntryId: readEntry.id, toolName: "list_things" },
|
||||
})).resolves.toMatchObject({ allowed: true });
|
||||
|
||||
const removed = await service.archiveConnection(connectionId, company.id, {
|
||||
actorType: "user",
|
||||
actorId: "board-user",
|
||||
});
|
||||
|
||||
expect(removed.connection).toMatchObject({ status: "archived", enabled: false });
|
||||
expect(removed.removal).toMatchObject({
|
||||
secretsRevoked: secretIds.length,
|
||||
secretsRetainedShared: 0,
|
||||
appProfile: "deleted",
|
||||
installsRemoved: 1,
|
||||
applicationArchived: true,
|
||||
});
|
||||
expect(removed.removal.credentialRefsCleared).toBeGreaterThan(0);
|
||||
expect(removed.removal.secretBindingsRemoved).toBeGreaterThan(0);
|
||||
expect(removed.removal.catalogEntriesMarkedRemoved).toBe(FIXTURE_TOOLS.length);
|
||||
|
||||
// No secret row, no version row, nothing resolvable.
|
||||
for (const secretId of secretIds) {
|
||||
expect(await db.select().from(companySecrets).where(eq(companySecrets.id, secretId))).toEqual([]);
|
||||
expect(await db.select().from(companySecretVersions).where(eq(companySecretVersions.secretId, secretId))).toEqual([]);
|
||||
await expect(secretService(db).resolveSecretValue(company.id, secretId, "latest", {
|
||||
consumerType: "tool_connection",
|
||||
consumerId: connectionId,
|
||||
configPath: `credentials.headers.${HEADER.name}`,
|
||||
actorType: "system",
|
||||
})).rejects.toMatchObject({ status: 404 });
|
||||
}
|
||||
expect(await db.select().from(companySecretBindings).where(and(
|
||||
eq(companySecretBindings.targetType, "tool_connection"),
|
||||
eq(companySecretBindings.targetId, connectionId),
|
||||
))).toEqual([]);
|
||||
|
||||
// No ref left on the connection to point at a credential.
|
||||
const [after] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
expect(after!.credentialRefs).toEqual([]);
|
||||
expect(after!.credentialSecretRefs).toEqual([]);
|
||||
|
||||
// No install, no app-managed profile, no profile entries or bindings.
|
||||
expect(await db.select().from(toolConnectionInstalls).where(eq(toolConnectionInstalls.connectionId, connectionId))).toEqual([]);
|
||||
expect(await db.select().from(toolProfiles).where(eq(toolProfiles.profileKey, `app:${connectionId}`))).toEqual([]);
|
||||
expect(await db.select().from(toolProfileEntries).where(eq(toolProfileEntries.profileId, profile.id))).toEqual([]);
|
||||
expect(await db.select().from(toolProfileBindings).where(eq(toolProfileBindings.profileId, profile.id))).toEqual([]);
|
||||
|
||||
// Catalog history survives, marked non-executable.
|
||||
const catalog = await db.select().from(toolCatalogEntries).where(eq(toolCatalogEntries.connectionId, connectionId));
|
||||
expect(catalog.length).toBe(FIXTURE_TOOLS.length);
|
||||
expect(catalog.every((entry) => entry.status === "removed")).toBe(true);
|
||||
|
||||
// And the agent is denied.
|
||||
await expect(policy.decide({
|
||||
companyId: company.id,
|
||||
actor: { actorType: "agent", actorId: agent.id, agentId: agent.id },
|
||||
request: { connectionId, catalogEntryId: readEntry.id, toolName: "list_things" },
|
||||
})).resolves.toMatchObject({ allowed: false });
|
||||
expect(await service.listConnectionInstalls(connectionId, company.id)).toEqual([]);
|
||||
});
|
||||
|
||||
it("revokes OAuth secrets, grant credentials and outstanding authorization state", async () => {
|
||||
installMcpFixture(HEADER);
|
||||
const company = await createCompany(db);
|
||||
const agent = await createAgent(db, company.id);
|
||||
const { service, connectionId } = await connectHeaderApp(company.id, agent.id);
|
||||
const secrets = secretService(db);
|
||||
|
||||
// Stand in for a completed sign-in: access and refresh tokens as dedicated
|
||||
// `tool_app.*` secrets, referenced by the connection and by a user grant.
|
||||
const accessSecret = await secrets.create(company.id, {
|
||||
name: `Removal access ${randomUUID().slice(0, 8)}`,
|
||||
key: `tool_app.${randomUUID()}.oauth_access_token`,
|
||||
provider: "local_encrypted",
|
||||
value: "oauth-access-token",
|
||||
});
|
||||
const refreshSecret = await secrets.create(company.id, {
|
||||
name: `Removal refresh ${randomUUID().slice(0, 8)}`,
|
||||
key: `tool_app.${randomUUID()}.oauth_refresh_token`,
|
||||
provider: "local_encrypted",
|
||||
value: "oauth-refresh-token",
|
||||
});
|
||||
const grantSecret = await secrets.create(company.id, {
|
||||
name: `Removal grant ${randomUUID().slice(0, 8)}`,
|
||||
key: `tool_app.${randomUUID()}.oauth_access_token`,
|
||||
provider: "local_encrypted",
|
||||
value: "grant-access-token",
|
||||
});
|
||||
const [existing] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
const headerSecretIds = existing!.credentialSecretRefs.map((ref) => ref.secretId);
|
||||
await service.updateConnection(connectionId, {
|
||||
credentialSecretRefs: [
|
||||
...existing!.credentialSecretRefs,
|
||||
{ secretId: accessSecret.id, versionSelector: "latest", configPath: "oauth.access_token", required: true, label: "OAuth access token" },
|
||||
{ secretId: refreshSecret.id, versionSelector: "latest", configPath: "oauth.refresh_token", required: false, label: "OAuth refresh token" },
|
||||
],
|
||||
});
|
||||
const [grant] = await db.insert(connectionGrants).values({
|
||||
companyId: company.id,
|
||||
connectionId,
|
||||
kind: "user",
|
||||
subjectUserId: "user-1",
|
||||
credentialSecretRefs: [
|
||||
{ secretId: grantSecret.id, versionSelector: "latest", configPath: "oauth.access_token", required: true, label: "OAuth access token" },
|
||||
],
|
||||
status: "active",
|
||||
}).returning();
|
||||
await db.insert(toolOauthStates).values({
|
||||
state: `state-${randomUUID()}`,
|
||||
companyId: company.id,
|
||||
connectionId,
|
||||
codeVerifier: "verifier",
|
||||
expiresAt: new Date(Date.now() + 600_000),
|
||||
});
|
||||
await db.insert(connectionTokenIssuances).values({
|
||||
companyId: company.id,
|
||||
connectionId,
|
||||
agentId: agent.id,
|
||||
path: "oauth_access",
|
||||
outcome: "success",
|
||||
tokenHash: "a".repeat(64),
|
||||
expiresAt: new Date(Date.now() + 600_000),
|
||||
});
|
||||
|
||||
const removed = await service.archiveConnection(connectionId, company.id);
|
||||
|
||||
expect(removed.removal).toMatchObject({
|
||||
secretsRevoked: headerSecretIds.length + 3,
|
||||
secretsRetainedShared: 0,
|
||||
grantsRevoked: 1,
|
||||
oauthStatesDiscarded: 1,
|
||||
tokenIssuanceHashesCleared: 1,
|
||||
});
|
||||
for (const secretId of [accessSecret.id, refreshSecret.id, grantSecret.id, ...headerSecretIds]) {
|
||||
expect(await db.select().from(companySecrets).where(eq(companySecrets.id, secretId))).toEqual([]);
|
||||
}
|
||||
const [grantAfter] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, grant!.id));
|
||||
expect(grantAfter).toMatchObject({ status: "revoked", isDefault: false });
|
||||
expect(grantAfter!.revokedAt).not.toBeNull();
|
||||
expect(grantAfter!.credentialSecretRefs).toEqual([]);
|
||||
expect(await db.select().from(toolOauthStates).where(eq(toolOauthStates.connectionId, connectionId))).toEqual([]);
|
||||
const [issuance] = await db.select().from(connectionTokenIssuances).where(eq(connectionTokenIssuances.connectionId, connectionId));
|
||||
expect(issuance!.tokenHash).toBeNull();
|
||||
// The ledger row itself is history and stays.
|
||||
expect(issuance!.path).toBe("oauth_access");
|
||||
});
|
||||
|
||||
it("leaves another consumer's credential in place", async () => {
|
||||
installMcpFixture(HEADER);
|
||||
const company = await createCompany(db);
|
||||
const agent = await createAgent(db, company.id);
|
||||
const { service, connectionId } = await connectHeaderApp(company.id, agent.id);
|
||||
const secrets = secretService(db);
|
||||
|
||||
// Two secrets removal must not destroy: one the operator manages by hand
|
||||
// (outside the `tool_app.` namespace), and one dedicated-looking secret a
|
||||
// second connection also binds.
|
||||
const operatorSecret = await secrets.create(company.id, {
|
||||
name: `Operator managed ${randomUUID().slice(0, 8)}`,
|
||||
key: `shared_api_key_${randomUUID().slice(0, 8)}`,
|
||||
provider: "local_encrypted",
|
||||
value: "operator-managed-value",
|
||||
});
|
||||
const alsoUsedSecret = await secrets.create(company.id, {
|
||||
name: `Shared app secret ${randomUUID().slice(0, 8)}`,
|
||||
key: `tool_app.${randomUUID()}.headers_x_shared`,
|
||||
provider: "local_encrypted",
|
||||
value: "shared-app-value",
|
||||
});
|
||||
const [existing] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
await service.updateConnection(connectionId, {
|
||||
credentialSecretRefs: [
|
||||
...existing!.credentialSecretRefs,
|
||||
{ secretId: operatorSecret.id, versionSelector: "latest", configPath: "credentials.operator", required: false, label: "Operator key" },
|
||||
{ secretId: alsoUsedSecret.id, versionSelector: "latest", configPath: "credentials.shared", required: false, label: "Shared key" },
|
||||
],
|
||||
});
|
||||
|
||||
// A second connection referencing the same secret is the other consumer.
|
||||
const [otherApplication] = await db.insert(toolApplications).values({
|
||||
companyId: company.id,
|
||||
applicationKey: `other:${randomUUID()}`,
|
||||
name: `Other app ${randomUUID().slice(0, 8)}`,
|
||||
type: "mcp_http",
|
||||
status: "active",
|
||||
}).returning();
|
||||
const otherConnection = await service.createConnection(company.id, {
|
||||
applicationId: otherApplication!.id,
|
||||
name: `Other connection ${randomUUID().slice(0, 8)}`,
|
||||
transport: "mcp_remote",
|
||||
config: { url: MCP_URL },
|
||||
credentialSecretRefs: [
|
||||
{ secretId: alsoUsedSecret.id, versionSelector: "latest", configPath: "credentials.shared", required: false, label: "Shared key" },
|
||||
],
|
||||
});
|
||||
|
||||
const removed = await service.archiveConnection(connectionId, company.id);
|
||||
|
||||
expect(removed.removal.secretsRetainedShared).toBe(2);
|
||||
for (const secretId of [operatorSecret.id, alsoUsedSecret.id]) {
|
||||
const [row] = await db.select().from(companySecrets).where(eq(companySecrets.id, secretId));
|
||||
expect(row).toBeTruthy();
|
||||
expect(row!.status).not.toBe("deleted");
|
||||
}
|
||||
// The other consumer can still resolve its credential.
|
||||
await expect(secretService(db).resolveSecretValue(company.id, alsoUsedSecret.id, "latest", {
|
||||
consumerType: "tool_connection",
|
||||
consumerId: otherConnection.id,
|
||||
configPath: "credentials.shared",
|
||||
actorType: "system",
|
||||
})).resolves.toBe("shared-app-value");
|
||||
// Its binding survives; the removed connection's does not.
|
||||
expect(await db.select().from(companySecretBindings).where(and(
|
||||
eq(companySecretBindings.targetType, "tool_connection"),
|
||||
eq(companySecretBindings.targetId, connectionId),
|
||||
))).toEqual([]);
|
||||
expect((await db.select().from(companySecretBindings).where(and(
|
||||
eq(companySecretBindings.targetType, "tool_connection"),
|
||||
eq(companySecretBindings.targetId, otherConnection.id),
|
||||
))).length).toBeGreaterThan(0);
|
||||
// And the removed connection no longer points at either of them.
|
||||
const [after] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
expect(after!.credentialSecretRefs).toEqual([]);
|
||||
});
|
||||
|
||||
it("fails closed and stays resumable when the secret provider errors", async () => {
|
||||
installMcpFixture(HEADER);
|
||||
const company = await createCompany(db);
|
||||
const agent = await createAgent(db, company.id);
|
||||
const { service, connectionId, readEntry } = await connectHeaderApp(company.id, agent.id);
|
||||
const policy = toolAccessPolicyService(db);
|
||||
|
||||
const [before] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
const secretIds = before!.credentialSecretRefs.map((ref) => ref.secretId);
|
||||
expect(secretIds.length).toBeGreaterThan(0);
|
||||
|
||||
// Break provider deletion for the first attempt only. The registry hands out
|
||||
// this very object, so patching the method is enough.
|
||||
let failures = 0;
|
||||
const spy = vi.spyOn(localEncryptedProvider, "deleteOrArchive").mockImplementation(async () => {
|
||||
failures += 1;
|
||||
throw new Error("provider unavailable");
|
||||
});
|
||||
|
||||
await expect(service.archiveConnection(connectionId, company.id)).rejects.toThrow("provider unavailable");
|
||||
expect(failures).toBe(1);
|
||||
|
||||
// Failed closed: the app is already undispatchable and the credential is
|
||||
// already unresolvable, even though the provider copy survives.
|
||||
const [midway] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
expect(midway).toMatchObject({ status: "archived", enabled: false });
|
||||
expect(await db.select().from(toolConnectionInstalls).where(eq(toolConnectionInstalls.connectionId, connectionId))).toEqual([]);
|
||||
expect(await db.select().from(toolProfiles).where(eq(toolProfiles.profileKey, `app:${connectionId}`))).toEqual([]);
|
||||
await expect(policy.decide({
|
||||
companyId: company.id,
|
||||
actor: { actorType: "agent", actorId: agent.id, agentId: agent.id },
|
||||
request: { connectionId, catalogEntryId: readEntry.id, toolName: "list_things" },
|
||||
})).resolves.toMatchObject({ allowed: false });
|
||||
await expect(secretService(db).resolveSecretValue(company.id, secretIds[0]!, "latest", {
|
||||
consumerType: "tool_connection",
|
||||
consumerId: connectionId,
|
||||
configPath: `credentials.headers.${HEADER.name}`,
|
||||
actorType: "system",
|
||||
})).rejects.toMatchObject({ status: 404 });
|
||||
// The ref survives the failure on purpose: it is the only pointer a retry
|
||||
// has to the secret it still has to revoke.
|
||||
expect(midway!.credentialSecretRefs.length).toBeGreaterThan(0);
|
||||
|
||||
// Retrying the same removal finishes the job.
|
||||
spy.mockRestore();
|
||||
const removed = await service.archiveConnection(connectionId, company.id);
|
||||
expect(removed.removal.secretsRevoked).toBe(secretIds.length);
|
||||
expect(removed.removal.secretsRetainedShared).toBe(0);
|
||||
for (const secretId of secretIds) {
|
||||
expect(await db.select().from(companySecrets).where(eq(companySecrets.id, secretId))).toEqual([]);
|
||||
}
|
||||
const [after] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
expect(after!.credentialSecretRefs).toEqual([]);
|
||||
|
||||
// A third removal is a no-op that still succeeds.
|
||||
const again = await service.archiveConnection(connectionId, company.id);
|
||||
expect(again.removal).toMatchObject({
|
||||
secretsRevoked: 0,
|
||||
secretsRetainedShared: 0,
|
||||
installsRemoved: 0,
|
||||
appProfile: "absent",
|
||||
grantsRevoked: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it("archives the app profile and revokes gateway tokens when a gateway still points at it", async () => {
|
||||
installMcpFixture(HEADER);
|
||||
const company = await createCompany(db);
|
||||
const agent = await createAgent(db, company.id);
|
||||
const { service, connectionId, profile } = await connectHeaderApp(company.id, agent.id);
|
||||
|
||||
const [gateway] = await db.insert(toolMcpGateways).values({
|
||||
companyId: company.id,
|
||||
name: `Gateway ${randomUUID().slice(0, 8)}`,
|
||||
slug: `gw-${randomUUID().slice(0, 8)}`,
|
||||
profileId: profile.id,
|
||||
status: "active",
|
||||
}).returning();
|
||||
const [token] = await db.insert(toolMcpGatewayTokens).values({
|
||||
companyId: company.id,
|
||||
gatewayId: gateway!.id,
|
||||
name: "client",
|
||||
tokenHash: randomUUID().replace(/-/g, "").padEnd(64, "0"),
|
||||
}).returning();
|
||||
|
||||
const removed = await service.archiveConnection(connectionId, company.id);
|
||||
|
||||
expect(removed.removal).toMatchObject({
|
||||
appProfile: "archived",
|
||||
gatewayTokensRevoked: 1,
|
||||
});
|
||||
const [profileAfter] = await db.select().from(toolProfiles).where(eq(toolProfiles.id, profile.id));
|
||||
// The row survives only because the gateway foreign key forbids deleting it;
|
||||
// a non-active profile with no entries is what the policy engine ignores.
|
||||
expect(profileAfter).toMatchObject({ status: "archived", defaultAction: "deny" });
|
||||
expect(await db.select().from(toolProfileEntries).where(eq(toolProfileEntries.profileId, profile.id))).toEqual([]);
|
||||
const [tokenAfter] = await db.select().from(toolMcpGatewayTokens).where(eq(toolMcpGatewayTokens.id, token!.id));
|
||||
expect(tokenAfter!.revokedAt).not.toBeNull();
|
||||
});
|
||||
|
||||
it("stops a live local runtime so no process keeps serving the revoked credential", async () => {
|
||||
installMcpFixture(HEADER);
|
||||
const company = await createCompany(db);
|
||||
const agent = await createAgent(db, company.id);
|
||||
const { service, connectionId } = await connectHeaderApp(company.id, agent.id);
|
||||
const [connection] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
|
||||
const [slot] = await db.insert(toolRuntimeSlots).values({
|
||||
companyId: company.id,
|
||||
applicationId: connection!.applicationId,
|
||||
connectionId,
|
||||
slotKey: `mcp:${company.id}:${connectionId}`,
|
||||
ownerScopeType: "connection",
|
||||
ownerScopeId: connectionId,
|
||||
runtimeKind: "local_stdio",
|
||||
status: "running",
|
||||
provider: "paperclip",
|
||||
healthStatus: "ok",
|
||||
}).returning();
|
||||
|
||||
const removed = await service.archiveConnection(connectionId, company.id);
|
||||
|
||||
expect(removed.removal.runtimeSlotsStopped).toBe(1);
|
||||
const [slotAfter] = await db.select().from(toolRuntimeSlots).where(eq(toolRuntimeSlots.id, slot!.id));
|
||||
expect(slotAfter!.status).toBe("stopped");
|
||||
expect(slotAfter!.metadata).toMatchObject({ stoppedReason: "connection_removed" });
|
||||
|
||||
// A second removal has nothing left to stop.
|
||||
const again = await service.archiveConnection(connectionId, company.id);
|
||||
expect(again.removal.runtimeSlotsStopped).toBe(0);
|
||||
});
|
||||
|
||||
it("removes over the route with a secret-free receipt, then requires fresh credentials to reconnect", async () => {
|
||||
installMcpFixture(HEADER);
|
||||
const company = await createCompany(db);
|
||||
const agent = await createAgent(db, company.id);
|
||||
const { service, connectionId } = await connectHeaderApp(company.id, agent.id);
|
||||
const app = createRouteApp(db);
|
||||
const [beforeRemoval] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
const originalSecretIds = beforeRemoval!.credentialSecretRefs.map((ref) => ref.secretId);
|
||||
expect(originalSecretIds.length).toBeGreaterThan(0);
|
||||
|
||||
const response = await request(app).delete(`/api/tool-connections/${connectionId}`).expect(200);
|
||||
|
||||
expect(response.body).toMatchObject({ id: connectionId, status: "archived", enabled: false });
|
||||
expect(response.body.removal).toMatchObject({ appProfile: "deleted", secretsRetainedShared: 0 });
|
||||
expect(response.body.removal.secretsRevoked).toBeGreaterThan(0);
|
||||
// Nothing in the response body echoes the pasted value or a secret name.
|
||||
expect(JSON.stringify(response.body)).not.toContain(HEADER.value);
|
||||
|
||||
const [receipt] = await db
|
||||
.select()
|
||||
.from(activityLog)
|
||||
.where(and(eq(activityLog.companyId, company.id), eq(activityLog.action, "tool_connection.archived")));
|
||||
expect(receipt!.details).toMatchObject({
|
||||
transport: "mcp_remote",
|
||||
appProfile: "deleted",
|
||||
secretsRetainedShared: 0,
|
||||
applicationArchived: true,
|
||||
});
|
||||
expect(Number((receipt!.details as Record<string, unknown>).secretsRevoked)).toBeGreaterThan(0);
|
||||
const receiptText = JSON.stringify(receipt!.details);
|
||||
expect(receiptText).not.toContain(HEADER.value);
|
||||
expect(receiptText).not.toContain("tool_app.");
|
||||
|
||||
// Reconnect cannot lean on a retained credential: the archived connection
|
||||
// refuses both reconnect and sign-in, and a fresh connect has to be given
|
||||
// new credential values.
|
||||
await expect(service.reconnectGalleryApp(connectionId, company.id, { credentialValues: {} }))
|
||||
.rejects.toMatchObject({ status: 409 });
|
||||
await expect(service.startOAuth(company.id, connectionId, {
|
||||
redirectUri: "https://paperclip.fixture.test/api/tools/oauth/callback",
|
||||
actor: { actorType: "user", actorId: "board-user" },
|
||||
})).rejects.toMatchObject({ status: 409 });
|
||||
|
||||
const [archived] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
const reconnected = await service.connectGalleryApp(company.id, {
|
||||
applicationId: archived!.applicationId,
|
||||
link: MCP_URL,
|
||||
name: archived!.name,
|
||||
credentialValues: { [`headers.${HEADER.name}`]: HEADER.value },
|
||||
});
|
||||
// Same connection identity, brand new credential.
|
||||
expect(reconnected.connectionId).toBe(connectionId);
|
||||
const [after] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionId));
|
||||
expect(after!.credentialSecretRefs.length).toBeGreaterThan(0);
|
||||
const reconnectedSecretIds = after!.credentialSecretRefs.map((ref) => ref.secretId);
|
||||
// Not one of the revoked secrets came back.
|
||||
for (const secretId of originalSecretIds) expect(reconnectedSecretIds).not.toContain(secretId);
|
||||
// And the operator has to choose access again — the app profile is not back.
|
||||
expect(await db.select().from(toolProfiles).where(eq(toolProfiles.profileKey, `app:${connectionId}`))).toEqual([]);
|
||||
expect(await service.listConnectionInstalls(connectionId, company.id)).toEqual([]);
|
||||
});
|
||||
});
|
||||
+25
-1
@@ -9,6 +9,26 @@ const SECRET_PAYLOAD_KEY_RE = new RegExp(SECRET_FIELD_NAME_PATTERN, "i");
|
||||
// "authorization". JWT-shaped values are still caught by the value guard below.
|
||||
const AUDIT_REASON_PAYLOAD_KEY_RE = /^authorizationReason$/;
|
||||
const AUDIT_SURFACE_PAYLOAD_KEY_RE = /^surface$/;
|
||||
/**
|
||||
* Cleanup counts on a connection-removal receipt (PAP-17119). Their names name
|
||||
* the thing they counted — secrets, bindings, tokens — so the key guard above
|
||||
* would blank the whole receipt and leave the operator unable to see what a
|
||||
* revocation actually tore down. They pass only while the value really is a
|
||||
* finite number, so nothing that could carry material rides through on the
|
||||
* strength of a familiar key name.
|
||||
*/
|
||||
const AUDIT_COUNT_PAYLOAD_KEYS = new Set([
|
||||
"secretsRevoked",
|
||||
"secretsRetainedShared",
|
||||
"credentialRefsCleared",
|
||||
"secretBindingsRemoved",
|
||||
"tokenIssuanceHashesCleared",
|
||||
"gatewayTokensRevoked",
|
||||
]);
|
||||
|
||||
function isAuditCountField(key: string, value: unknown): boolean {
|
||||
return AUDIT_COUNT_PAYLOAD_KEYS.has(key) && typeof value === "number" && Number.isFinite(value);
|
||||
}
|
||||
const COMMAND_PAYLOAD_KEY_RE =
|
||||
/(^command$|^cmd$|command[-_]?line|resolved[-_]?command|PAPERCLIP_RESOLVED_COMMAND)/i;
|
||||
const COMMAND_ARGS_PAYLOAD_KEY_RE = /^(commandArgs|command_?args|argv)$/i;
|
||||
@@ -107,7 +127,11 @@ export function sanitizeRecord(record: Record<string, unknown>): Record<string,
|
||||
redacted[key] = redactSensitiveText(value);
|
||||
continue;
|
||||
}
|
||||
if (SECRET_PAYLOAD_KEY_RE.test(key) && !AUDIT_REASON_PAYLOAD_KEY_RE.test(key)) {
|
||||
if (
|
||||
SECRET_PAYLOAD_KEY_RE.test(key)
|
||||
&& !AUDIT_REASON_PAYLOAD_KEY_RE.test(key)
|
||||
&& !isAuditCountField(key, value)
|
||||
) {
|
||||
if (isSecretRefBinding(value)) {
|
||||
redacted[key] = sanitizeValue(value);
|
||||
continue;
|
||||
|
||||
@@ -832,8 +832,15 @@ export function toolAccessRoutes(
|
||||
if (!existing) return;
|
||||
assertToolAppMutationAccess(req, existing.companyId);
|
||||
const applicationBefore = await svc.getApplication(existing.applicationId);
|
||||
const connection = await svc.archiveConnection(existing.id);
|
||||
const { connection, removal } = await svc.archiveConnection(
|
||||
existing.id,
|
||||
existing.companyId,
|
||||
getActorInfo(req),
|
||||
);
|
||||
const applicationAfter = await svc.getApplication(existing.applicationId);
|
||||
// The receipt is counts and outcomes only. Removal is a revocation boundary
|
||||
// (PAP-17119) and operators need to see what it tore down, but this row is
|
||||
// company-readable activity, so it never carries a secret name or value.
|
||||
await logActivity(db, {
|
||||
companyId: connection.companyId,
|
||||
actorType: "user",
|
||||
@@ -841,7 +848,7 @@ export function toolAccessRoutes(
|
||||
action: "tool_connection.archived",
|
||||
entityType: "tool_connection",
|
||||
entityId: connection.id,
|
||||
details: { transport: connection.transport },
|
||||
details: { transport: connection.transport, ...removal },
|
||||
});
|
||||
if (applicationBefore.status !== "archived" && applicationAfter.status === "archived") {
|
||||
await logActivity(db, {
|
||||
@@ -854,7 +861,7 @@ export function toolAccessRoutes(
|
||||
details: { type: applicationAfter.type, name: applicationAfter.name, reason: "last_connection_removed" },
|
||||
});
|
||||
}
|
||||
res.json(connection);
|
||||
res.json({ ...connection, removal });
|
||||
});
|
||||
|
||||
router.post("/tool-connections/:connectionId/health-check", async (req, res) => {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { createHash, randomBytes, randomUUID } from "node:crypto";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { and, asc, desc, eq, gte, inArray, lt, max, ne, sql } from "drizzle-orm";
|
||||
import { and, asc, desc, eq, gte, inArray, isNull, lt, max, ne, sql } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import {
|
||||
activityLog,
|
||||
@@ -27,7 +27,9 @@ import {
|
||||
toolCallEvents,
|
||||
toolInvocations,
|
||||
toolPolicies,
|
||||
toolGatewaySessions,
|
||||
toolMcpGateways,
|
||||
toolMcpGatewayTokens,
|
||||
toolProfileBindings,
|
||||
toolProfileEntries,
|
||||
toolProfiles,
|
||||
@@ -64,6 +66,8 @@ import type {
|
||||
ToolConnection,
|
||||
ToolConnectionInstall,
|
||||
ToolConnectionInstallSnapshot,
|
||||
ToolConnectionRemovalResult,
|
||||
ToolConnectionRemovalSummary,
|
||||
ToolConnectionHealthCheckResult,
|
||||
ToolConnectionHealthStatus,
|
||||
ToolConnectionAuthKind,
|
||||
@@ -123,6 +127,7 @@ import {
|
||||
type OAuthEndpointUrlRejection,
|
||||
} from "@paperclipai/shared";
|
||||
import { badRequest, conflict, forbidden, HttpError, notFound, unprocessable } from "../errors.js";
|
||||
import { logger } from "../middleware/logger.js";
|
||||
import { logActivity } from "./activity-log.js";
|
||||
import { mcpHttpRequestHeaders, parseMcpHttpResponseBody } from "./mcp-http.js";
|
||||
import { assertPublicRemoteHttpEndpoint, parseRemoteHttpEndpoint } from "./remote-http-endpoint-guard.js";
|
||||
@@ -883,6 +888,48 @@ function connectionUid(namespace: string, name: string, connectionId: string) {
|
||||
return `${normalizeKey(namespace)}/${normalizeKey(name)}-${connectionId.slice(0, 8)}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* The key namespace `connectGalleryApp`, `reconnectGalleryApp` and
|
||||
* `createOrRotateOAuthSecret` mint for credentials a connection owns outright.
|
||||
* Nothing else writes this prefix, which is what lets removal tell a dedicated
|
||||
* app credential apart from a secret the operator manages by hand — see
|
||||
* `classifyConnectionSecrets`. Matched as a prefix on purpose: `secrets.remove`
|
||||
* suffixes `__deleted__<id>` onto the key before it deletes the row, so a
|
||||
* removal that is retried after a provider failure must still recognise it.
|
||||
*/
|
||||
const CONNECTION_OWNED_SECRET_KEY_PREFIX = "tool_app.";
|
||||
|
||||
/** Token fields a legacy row may have inlined into `config.oauth`. */
|
||||
const INLINE_OAUTH_TOKEN_FIELDS = [
|
||||
"access_token",
|
||||
"refresh_token",
|
||||
"accessToken",
|
||||
"refreshToken",
|
||||
"client_secret",
|
||||
"clientSecret",
|
||||
];
|
||||
|
||||
/**
|
||||
* Drop inline OAuth token material from a connection config, keeping the
|
||||
* non-secret identity (client id, issuer, registration source) a later
|
||||
* reconnect reuses. Current code stores tokens as secret refs, never in the
|
||||
* config; this exists so a row written by an older build cannot keep a usable
|
||||
* token after the operator removed the app.
|
||||
*/
|
||||
function withoutInlineOAuthTokens(config: Record<string, unknown>): Record<string, unknown> {
|
||||
const oauth = config.oauth;
|
||||
if (!oauth || typeof oauth !== "object" || Array.isArray(oauth)) return config;
|
||||
const next = { ...(oauth as Record<string, unknown>) };
|
||||
let changed = false;
|
||||
for (const field of INLINE_OAUTH_TOKEN_FIELDS) {
|
||||
if (field in next) {
|
||||
delete next[field];
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
return changed ? { ...config, oauth: next } : config;
|
||||
}
|
||||
|
||||
function actorBinding(actor: ActorInfo | undefined) {
|
||||
return {
|
||||
actorType: actor?.actorType ?? null,
|
||||
@@ -3227,6 +3274,414 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
})));
|
||||
}
|
||||
|
||||
/**
|
||||
* Split the secrets a connection points at into the ones it owns outright and
|
||||
* the ones another consumer still depends on.
|
||||
*
|
||||
* Removing an app is a credential revocation boundary (PAP-17119), but it must
|
||||
* never destroy a secret the operator manages by hand or shares with another
|
||||
* target. Two independent tests have to agree before a secret is destroyed:
|
||||
*
|
||||
* 1. Provenance — the key sits in the `tool_app.` namespace only the
|
||||
* connect/reconnect/OAuth paths mint, and the row is a company-scoped
|
||||
* Paperclip secret rather than a per-user credential.
|
||||
* 2. Exclusivity — nothing outside this connection references it: no
|
||||
* `company_secret_bindings` row from another target, and no other
|
||||
* connection or connection grant naming the same secret id.
|
||||
*
|
||||
* A secret failing either test is reported as retained; removal still drops
|
||||
* this connection's binding and ref, so the connection loses the credential
|
||||
* either way. Retaining a secret nobody can reach is a leak of an unused row;
|
||||
* deleting one another target still resolves is an outage, so the ambiguous
|
||||
* case fails towards retention.
|
||||
*/
|
||||
async function classifyConnectionSecrets(
|
||||
connection: typeof toolConnections.$inferSelect,
|
||||
secretIds: string[],
|
||||
): Promise<{ owned: string[]; retained: string[] }> {
|
||||
const unique = [...new Set(secretIds.filter((id) => typeof id === "string" && id.length > 0))];
|
||||
if (unique.length === 0) return { owned: [], retained: [] };
|
||||
|
||||
const secretRows = await db
|
||||
.select({
|
||||
id: companySecrets.id,
|
||||
key: companySecrets.key,
|
||||
scope: companySecrets.scope,
|
||||
userSecretDefinitionId: companySecrets.userSecretDefinitionId,
|
||||
})
|
||||
.from(companySecrets)
|
||||
.where(and(eq(companySecrets.companyId, connection.companyId), inArray(companySecrets.id, unique)));
|
||||
const byId = new Map(secretRows.map((row) => [row.id, row]));
|
||||
|
||||
const referencedElsewhere = new Set<string>();
|
||||
const foreignBindings = await db
|
||||
.select({ secretId: companySecretBindings.secretId })
|
||||
.from(companySecretBindings)
|
||||
.where(and(
|
||||
eq(companySecretBindings.companyId, connection.companyId),
|
||||
inArray(companySecretBindings.secretId, unique),
|
||||
sql`not (${companySecretBindings.targetType} = 'tool_connection' and ${companySecretBindings.targetId} = ${connection.id})`,
|
||||
));
|
||||
for (const row of foreignBindings) referencedElsewhere.add(row.secretId);
|
||||
|
||||
// Bindings are the authority, but read the sibling refs too: a row written
|
||||
// before `syncCredentialBindings` existed — or by hand — can reference a
|
||||
// secret with no binding to prove it.
|
||||
const siblingConnections = await db
|
||||
.select({
|
||||
credentialRefs: toolConnections.credentialRefs,
|
||||
credentialSecretRefs: toolConnections.credentialSecretRefs,
|
||||
})
|
||||
.from(toolConnections)
|
||||
.where(and(eq(toolConnections.companyId, connection.companyId), ne(toolConnections.id, connection.id)));
|
||||
for (const row of siblingConnections) {
|
||||
for (const ref of row.credentialRefs ?? []) referencedElsewhere.add(ref.secretId);
|
||||
for (const ref of row.credentialSecretRefs ?? []) referencedElsewhere.add(ref.secretId);
|
||||
}
|
||||
const siblingGrants = await db
|
||||
.select({ credentialSecretRefs: connectionGrants.credentialSecretRefs })
|
||||
.from(connectionGrants)
|
||||
.where(and(
|
||||
eq(connectionGrants.companyId, connection.companyId),
|
||||
ne(connectionGrants.connectionId, connection.id),
|
||||
));
|
||||
for (const row of siblingGrants) {
|
||||
for (const ref of row.credentialSecretRefs ?? []) referencedElsewhere.add(ref.secretId);
|
||||
}
|
||||
|
||||
const owned: string[] = [];
|
||||
const retained: string[] = [];
|
||||
for (const secretId of unique) {
|
||||
const row = byId.get(secretId);
|
||||
// No row means an earlier pass of this same removal already deleted it.
|
||||
// Hand it back as owned so a retry re-runs the (idempotent) revocation
|
||||
// instead of reporting a credential this connection never shared.
|
||||
if (!row) {
|
||||
owned.push(secretId);
|
||||
continue;
|
||||
}
|
||||
const dedicated = row.scope === "company"
|
||||
&& row.userSecretDefinitionId === null
|
||||
&& row.key.startsWith(CONNECTION_OWNED_SECRET_KEY_PREFIX);
|
||||
if (dedicated && !referencedElsewhere.has(secretId)) owned.push(secretId);
|
||||
else retained.push(secretId);
|
||||
}
|
||||
return { owned, retained };
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove an app: a credential-revoking teardown, not a status flip (PAP-17119).
|
||||
*
|
||||
* Order is the security property. Every database-side access path closes
|
||||
* first — grants, installs, the app-managed profile, gateway tokens minted
|
||||
* against it, outstanding OAuth state, the catalog, and the connection itself
|
||||
* — so the app is already undispatchable before the first call out to a secret
|
||||
* provider. Secret revocation runs last, and each secret's ref survives until
|
||||
* that secret is gone, so a provider that errors leaves the operation failed
|
||||
* closed and resumable: the credential is already unresolvable (its row is
|
||||
* marked deleted first), and retrying the same removal finishes the job.
|
||||
*
|
||||
* What stays behind is deliberate: the connection and application rows, their
|
||||
* ids, names and activity keep working so a later reconnect reuses the same
|
||||
* identity — but with no credential, no install and no profile, so
|
||||
* reconnecting has to ask for fresh authentication and rebuild access.
|
||||
*/
|
||||
async function removeConnection(
|
||||
connectionId: string,
|
||||
companyId?: string,
|
||||
actor?: ActorInfo,
|
||||
): Promise<ToolConnectionRemovalResult> {
|
||||
const connection = await getConnectionRow(connectionId, companyId);
|
||||
const now = new Date();
|
||||
const binding = actorBinding(actor);
|
||||
|
||||
// Grants are read before they are revoked: a retried removal must still see
|
||||
// the credential refs of a grant an earlier pass already marked revoked.
|
||||
const grantRows = await db
|
||||
.select({
|
||||
id: connectionGrants.id,
|
||||
status: connectionGrants.status,
|
||||
credentialSecretRefs: connectionGrants.credentialSecretRefs,
|
||||
})
|
||||
.from(connectionGrants)
|
||||
.where(and(
|
||||
eq(connectionGrants.companyId, connection.companyId),
|
||||
eq(connectionGrants.connectionId, connection.id),
|
||||
));
|
||||
const grantsToRevoke = grantRows.filter((row) => row.status !== "revoked");
|
||||
if (grantsToRevoke.length > 0) {
|
||||
await db
|
||||
.update(connectionGrants)
|
||||
.set({
|
||||
status: "revoked",
|
||||
isDefault: false,
|
||||
revokedAt: now,
|
||||
revokedByAgentId: binding.actorType === "agent" ? binding.actorId : null,
|
||||
revokedByUserId: binding.actorType === "user" ? binding.actorId : null,
|
||||
updatedAt: now,
|
||||
})
|
||||
.where(inArray(connectionGrants.id, grantsToRevoke.map((row) => row.id)));
|
||||
}
|
||||
|
||||
// Bindings are how a credential reaches a runtime, so they go before the
|
||||
// provider round-trip rather than after it. They are also not needed to
|
||||
// finish the job: the refs on the connection row are what a resumed removal
|
||||
// reads to find the secrets it still owes a revocation.
|
||||
const removedSecretBindings = await db
|
||||
.delete(companySecretBindings)
|
||||
.where(and(
|
||||
eq(companySecretBindings.companyId, connection.companyId),
|
||||
eq(companySecretBindings.targetType, "tool_connection"),
|
||||
eq(companySecretBindings.targetId, connection.id),
|
||||
))
|
||||
.returning({ id: companySecretBindings.id });
|
||||
|
||||
const removedInstalls = await db
|
||||
.delete(toolConnectionInstalls)
|
||||
.where(and(
|
||||
eq(toolConnectionInstalls.companyId, connection.companyId),
|
||||
eq(toolConnectionInstalls.connectionId, connection.id),
|
||||
))
|
||||
.returning({ id: toolConnectionInstalls.id });
|
||||
|
||||
// The app-managed profile exists only to carry this connection's action
|
||||
// selection, so it goes with the connection. Operator-authored profiles that
|
||||
// happen to mention the connection are left alone — the archived connection
|
||||
// is denied by the policy engine regardless.
|
||||
const [appProfile] = await db
|
||||
.select({ id: toolProfiles.id })
|
||||
.from(toolProfiles)
|
||||
.where(and(
|
||||
eq(toolProfiles.companyId, connection.companyId),
|
||||
eq(toolProfiles.profileKey, `app:${connection.id}`),
|
||||
))
|
||||
.limit(1);
|
||||
let appProfileOutcome: ToolConnectionRemovalSummary["appProfile"] = "absent";
|
||||
let appProfileEntriesRemoved = 0;
|
||||
let appProfileBindingsRemoved = 0;
|
||||
let gatewayTokensRevoked = 0;
|
||||
let gatewaySessionsRevoked = 0;
|
||||
if (appProfile) {
|
||||
appProfileEntriesRemoved = (await db
|
||||
.delete(toolProfileEntries)
|
||||
.where(and(
|
||||
eq(toolProfileEntries.companyId, connection.companyId),
|
||||
eq(toolProfileEntries.profileId, appProfile.id),
|
||||
))
|
||||
.returning({ id: toolProfileEntries.id })).length;
|
||||
appProfileBindingsRemoved = (await db
|
||||
.delete(toolProfileBindings)
|
||||
.where(and(
|
||||
eq(toolProfileBindings.companyId, connection.companyId),
|
||||
eq(toolProfileBindings.profileId, appProfile.id),
|
||||
))
|
||||
.returning({ id: toolProfileBindings.id })).length;
|
||||
|
||||
const gatewayRows = await db
|
||||
.select({ id: toolMcpGateways.id })
|
||||
.from(toolMcpGateways)
|
||||
.where(and(
|
||||
eq(toolMcpGateways.companyId, connection.companyId),
|
||||
eq(toolMcpGateways.profileId, appProfile.id),
|
||||
));
|
||||
if (gatewayRows.length === 0) {
|
||||
await db.delete(toolProfiles).where(eq(toolProfiles.id, appProfile.id));
|
||||
appProfileOutcome = "deleted";
|
||||
} else {
|
||||
// `tool_mcp_gateways.profile_id` is ON DELETE RESTRICT, so a gateway
|
||||
// pointing here keeps the row alive. Archive it instead — the policy
|
||||
// engine only consults `active` profiles, and it has no entries left —
|
||||
// and revoke the tokens those gateways already handed out, which are the
|
||||
// one credential a caller could still present.
|
||||
await db
|
||||
.update(toolProfiles)
|
||||
.set({ status: "archived", defaultAction: "deny", updatedAt: now })
|
||||
.where(eq(toolProfiles.id, appProfile.id));
|
||||
appProfileOutcome = "archived";
|
||||
const revokedTokens = await db
|
||||
.update(toolMcpGatewayTokens)
|
||||
.set({ revokedAt: now, updatedAt: now })
|
||||
.where(and(
|
||||
eq(toolMcpGatewayTokens.companyId, connection.companyId),
|
||||
inArray(toolMcpGatewayTokens.gatewayId, gatewayRows.map((row) => row.id)),
|
||||
isNull(toolMcpGatewayTokens.revokedAt),
|
||||
))
|
||||
.returning({ id: toolMcpGatewayTokens.id });
|
||||
gatewayTokensRevoked = revokedTokens.length;
|
||||
if (revokedTokens.length > 0) {
|
||||
gatewaySessionsRevoked = (await db
|
||||
.update(toolGatewaySessions)
|
||||
.set({ revokedAt: now, updatedAt: now })
|
||||
.where(and(
|
||||
eq(toolGatewaySessions.companyId, connection.companyId),
|
||||
inArray(toolGatewaySessions.gatewayTokenId, revokedTokens.map((row) => row.id)),
|
||||
isNull(toolGatewaySessions.revokedAt),
|
||||
))
|
||||
.returning({ id: toolGatewaySessions.id })).length;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A local runtime already holds the injected credential inside a live child
|
||||
// process, so archiving rows is not enough — the process itself is an access
|
||||
// path. Stopping is best effort on purpose: if the supervisor cannot be
|
||||
// reached, revoking the credential anyway (so nothing can start again) beats
|
||||
// abandoning the teardown, and the warning says which slot was left running.
|
||||
let runtimeSlotsStopped = 0;
|
||||
const runtimeSlotRows = await db
|
||||
.select({ id: toolRuntimeSlots.id, status: toolRuntimeSlots.status })
|
||||
.from(toolRuntimeSlots)
|
||||
.where(and(
|
||||
eq(toolRuntimeSlots.companyId, connection.companyId),
|
||||
eq(toolRuntimeSlots.connectionId, connection.id),
|
||||
));
|
||||
for (const slot of runtimeSlotRows) {
|
||||
if (slot.status === "stopped") continue;
|
||||
try {
|
||||
await runtimeSupervisor.stopSlot({
|
||||
companyId: connection.companyId,
|
||||
slotId: slot.id,
|
||||
reason: "connection_removed",
|
||||
});
|
||||
runtimeSlotsStopped += 1;
|
||||
} catch (error) {
|
||||
logger.warn(
|
||||
{ err: error, companyId: connection.companyId, connectionId: connection.id, slotId: slot.id },
|
||||
"tool connection removal could not stop a runtime slot",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// The catalog stays as history, but `removed` is the status that stops a
|
||||
// standing trust rule from auto-allowing one of these actions again.
|
||||
const removedCatalogEntries = await db
|
||||
.update(toolCatalogEntries)
|
||||
.set({ status: "removed", updatedAt: now })
|
||||
.where(and(
|
||||
eq(toolCatalogEntries.companyId, connection.companyId),
|
||||
eq(toolCatalogEntries.connectionId, connection.id),
|
||||
ne(toolCatalogEntries.status, "removed"),
|
||||
))
|
||||
.returning({ id: toolCatalogEntries.id });
|
||||
|
||||
// An authorization already in flight would otherwise come back and mint a
|
||||
// fresh token for an app the operator just removed.
|
||||
const discardedOAuthStates = await db
|
||||
.delete(toolOauthStates)
|
||||
.where(and(
|
||||
eq(toolOauthStates.companyId, connection.companyId),
|
||||
eq(toolOauthStates.connectionId, connection.id),
|
||||
))
|
||||
.returning({ state: toolOauthStates.state });
|
||||
|
||||
// Token-derived material in the issuance ledger is not an access path —
|
||||
// nothing validates against it — but there is no reason to keep a hash of a
|
||||
// credential the operator asked us to revoke. Path, outcome, actor and time
|
||||
// stay, so the usage history survives.
|
||||
const clearedIssuanceHashes = await db
|
||||
.update(connectionTokenIssuances)
|
||||
.set({ tokenHash: null })
|
||||
.where(and(
|
||||
eq(connectionTokenIssuances.companyId, connection.companyId),
|
||||
eq(connectionTokenIssuances.connectionId, connection.id),
|
||||
sql`${connectionTokenIssuances.tokenHash} is not null`,
|
||||
))
|
||||
.returning({ id: connectionTokenIssuances.id });
|
||||
|
||||
const archived = await db.transaction(async (tx) => {
|
||||
const [updatedConnection] = await tx
|
||||
.update(toolConnections)
|
||||
.set({ status: "archived", enabled: false, updatedAt: now })
|
||||
.where(eq(toolConnections.id, connection.id))
|
||||
.returning();
|
||||
if (!updatedConnection) throw notFound("Tool connection not found");
|
||||
|
||||
const remainingConnections = await tx
|
||||
.select({ id: toolConnections.id })
|
||||
.from(toolConnections)
|
||||
.where(and(
|
||||
eq(toolConnections.applicationId, updatedConnection.applicationId),
|
||||
ne(toolConnections.status, "archived"),
|
||||
))
|
||||
.limit(1);
|
||||
|
||||
let applicationArchived = false;
|
||||
if (remainingConnections.length === 0) {
|
||||
const [application] = await tx
|
||||
.update(toolApplications)
|
||||
.set({ status: "archived", archivedAt: now, updatedAt: now })
|
||||
.where(and(
|
||||
eq(toolApplications.id, updatedConnection.applicationId),
|
||||
ne(toolApplications.status, "archived"),
|
||||
))
|
||||
.returning({ id: toolApplications.id });
|
||||
applicationArchived = Boolean(application);
|
||||
}
|
||||
|
||||
return { connection: updatedConnection, applicationArchived };
|
||||
});
|
||||
|
||||
// Only now, with every access path closed, revoke the credentials. Each
|
||||
// `secrets.remove` marks the row deleted before it calls the provider, so a
|
||||
// provider error leaves an unresolvable secret and a resumable removal
|
||||
// rather than a half-open app.
|
||||
const candidateSecretIds = [
|
||||
...connection.credentialRefs.map((ref) => ref.secretId),
|
||||
...connection.credentialSecretRefs.map((ref) => ref.secretId),
|
||||
...grantRows.flatMap((grant) => (grant.credentialSecretRefs ?? []).map((ref) => ref.secretId)),
|
||||
];
|
||||
const { owned, retained } = await classifyConnectionSecrets(connection, candidateSecretIds);
|
||||
let secretsRevoked = 0;
|
||||
for (const secretId of owned) {
|
||||
const removed = await secrets.remove(secretId);
|
||||
if (removed) secretsRevoked += 1;
|
||||
}
|
||||
|
||||
const credentialRefsCleared = connection.credentialRefs.length + connection.credentialSecretRefs.length;
|
||||
const [cleared] = await db
|
||||
.update(toolConnections)
|
||||
.set({
|
||||
credentialRefs: [],
|
||||
credentialSecretRefs: [],
|
||||
config: withoutInlineOAuthTokens(connection.config),
|
||||
transportConfig: withoutInlineOAuthTokens(connection.transportConfig),
|
||||
updatedAt: now,
|
||||
})
|
||||
.where(eq(toolConnections.id, connection.id))
|
||||
.returning();
|
||||
if (grantRows.some((grant) => (grant.credentialSecretRefs ?? []).length > 0)) {
|
||||
await db
|
||||
.update(connectionGrants)
|
||||
.set({ credentialSecretRefs: [], updatedAt: now })
|
||||
.where(and(
|
||||
eq(connectionGrants.companyId, connection.companyId),
|
||||
eq(connectionGrants.connectionId, connection.id),
|
||||
));
|
||||
}
|
||||
|
||||
return {
|
||||
connection: toConnection(cleared ?? archived.connection),
|
||||
removal: {
|
||||
secretsRevoked,
|
||||
secretsRetainedShared: retained.length,
|
||||
credentialRefsCleared,
|
||||
secretBindingsRemoved: removedSecretBindings.length,
|
||||
grantsRevoked: grantsToRevoke.length,
|
||||
installsRemoved: removedInstalls.length,
|
||||
appProfile: appProfileOutcome,
|
||||
appProfileEntriesRemoved,
|
||||
appProfileBindingsRemoved,
|
||||
catalogEntriesMarkedRemoved: removedCatalogEntries.length,
|
||||
oauthStatesDiscarded: discardedOAuthStates.length,
|
||||
tokenIssuanceHashesCleared: clearedIssuanceHashes.length,
|
||||
runtimeSlotsStopped,
|
||||
gatewayTokensRevoked,
|
||||
gatewaySessionsRevoked,
|
||||
applicationArchived: archived.applicationArchived,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
async function ensureRuntimeSlot(connection: typeof toolConnections.$inferSelect): Promise<ToolRuntimeSlot | null> {
|
||||
if (connection.transport !== "local_stdio") return null;
|
||||
const slotKey = `mcp:${connection.companyId}:${connection.id}`;
|
||||
@@ -7661,38 +8116,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
return toConnection(row);
|
||||
},
|
||||
|
||||
archiveConnection: async (connectionId: string): Promise<ToolConnection> => {
|
||||
const row = await db.transaction(async (tx) => {
|
||||
const [updatedConnection] = await tx
|
||||
.update(toolConnections)
|
||||
.set({ status: "archived", enabled: false, updatedAt: new Date() })
|
||||
.where(eq(toolConnections.id, connectionId))
|
||||
.returning();
|
||||
if (!updatedConnection) throw notFound("Tool connection not found");
|
||||
|
||||
const remainingConnections = await tx
|
||||
.select({ id: toolConnections.id })
|
||||
.from(toolConnections)
|
||||
.where(
|
||||
and(
|
||||
eq(toolConnections.applicationId, updatedConnection.applicationId),
|
||||
ne(toolConnections.status, "archived"),
|
||||
),
|
||||
)
|
||||
.limit(1);
|
||||
|
||||
if (remainingConnections.length === 0) {
|
||||
const now = new Date();
|
||||
await tx
|
||||
.update(toolApplications)
|
||||
.set({ status: "archived", archivedAt: now, updatedAt: now })
|
||||
.where(eq(toolApplications.id, updatedConnection.applicationId));
|
||||
}
|
||||
|
||||
return updatedConnection;
|
||||
});
|
||||
return toConnection(row);
|
||||
},
|
||||
archiveConnection: removeConnection,
|
||||
|
||||
checkHealth: checkConnectionHealth,
|
||||
|
||||
|
||||
+6
-1
@@ -3,6 +3,7 @@ import type {
|
||||
ToolConnection,
|
||||
ToolConnectionInstall,
|
||||
ToolConnectionInstallSnapshot,
|
||||
ToolConnectionRemovalSummary,
|
||||
ConnectToolAppResult,
|
||||
FinishToolAppResult,
|
||||
ToolCatalogEntry,
|
||||
@@ -290,8 +291,12 @@ export const toolsApi = {
|
||||
api.post<ToolConnection>(`/companies/${companyId}/tools/connections`, input),
|
||||
updateConnection: (connectionId: string, input: UpdateToolConnectionInput) =>
|
||||
api.patch<ToolConnection>(`/tool-connections/${connectionId}`, input),
|
||||
// Removal is a credential-revoking teardown (PAP-17119), so the response
|
||||
// carries the cleanup receipt alongside the archived connection.
|
||||
archiveConnection: (connectionId: string) =>
|
||||
api.delete<ToolConnection>(`/tool-connections/${connectionId}`),
|
||||
api.delete<ToolConnection & { removal: ToolConnectionRemovalSummary }>(
|
||||
`/tool-connections/${connectionId}`,
|
||||
),
|
||||
checkConnectionHealth: (connectionId: string) =>
|
||||
api.post<ToolConnectionHealthCheckResult>(`/tool-connections/${connectionId}/health-check`, {}),
|
||||
reconnectConnection: (connectionId: string, credentialValues: Record<string, string>) =>
|
||||
|
||||
@@ -273,7 +273,7 @@ export function AppDetail() {
|
||||
queryClient.invalidateQueries({ queryKey: queryKeys.apps.attention(selectedCompanyId!) });
|
||||
pushToast({
|
||||
title: "App removed",
|
||||
body: `${appName} no longer has access. You can connect it again any time.`,
|
||||
body: `${appName} no longer has access and its credentials are deleted. Connecting it again needs a new sign-in or key.`,
|
||||
tone: "success",
|
||||
});
|
||||
navigate("/apps/connections");
|
||||
|
||||
@@ -368,7 +368,7 @@ describe("Connections table (M1b / PAP-13254 door 2)", () => {
|
||||
expect(pushToastMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
title: "Connection deleted",
|
||||
body: "GitHub is no longer available to agents. You can connect it again later.",
|
||||
body: "GitHub is no longer available to agents and its credentials are deleted. Connecting it again needs a new sign-in or key.",
|
||||
tone: "success",
|
||||
}),
|
||||
);
|
||||
@@ -442,7 +442,10 @@ describe("Connections table (M1b / PAP-13254 door 2)", () => {
|
||||
deleteButton?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
|
||||
});
|
||||
|
||||
expect(document.body.textContent).toContain("Agents will lose access immediately.");
|
||||
expect(document.body.textContent).toContain(
|
||||
"The saved credentials are deleted and agents lose access immediately.",
|
||||
);
|
||||
expect(document.body.textContent).toContain("needs a new sign-in or key");
|
||||
expect(document.body.textContent).not.toContain("Agents can still use GitHub");
|
||||
|
||||
const confirmButton = Array.from(document.body.querySelectorAll("button")).find(
|
||||
@@ -455,7 +458,7 @@ describe("Connections table (M1b / PAP-13254 door 2)", () => {
|
||||
|
||||
expect(pushToastMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
body: "GitHub is no longer available to agents. You can connect it again later.",
|
||||
body: "GitHub is no longer available to agents and its credentials are deleted. Connecting it again needs a new sign-in or key.",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -149,7 +149,7 @@ export function Connections() {
|
||||
title: "Connection deleted",
|
||||
body: target.remainingConnectionCount > 0
|
||||
? `${target.appName} still has ${target.remainingConnectionCount} active ${target.remainingConnectionCount === 1 ? "connection" : "connections"} available to agents.`
|
||||
: `${target.appName} is no longer available to agents. You can connect it again later.`,
|
||||
: `${target.appName} is no longer available to agents and its credentials are deleted. Connecting it again needs a new sign-in or key.`,
|
||||
tone: "success",
|
||||
});
|
||||
setConnectionToDelete(null);
|
||||
@@ -456,8 +456,8 @@ export function Connections() {
|
||||
</AlertDialogTitle>
|
||||
<AlertDialogDescription>
|
||||
{connectionToDelete && connectionToDelete.remainingConnectionCount > 0
|
||||
? `This connection will be removed. Agents can still use ${connectionToDelete.appName} through ${connectionToDelete.remainingConnectionCount} other active ${connectionToDelete.remainingConnectionCount === 1 ? "connection" : "connections"}.`
|
||||
: "Agents will lose access immediately. You can connect it again later."}
|
||||
? `This connection's saved credentials are deleted and agents lose access through it immediately. Agents can still use ${connectionToDelete.appName} through ${connectionToDelete.remainingConnectionCount} other active ${connectionToDelete.remainingConnectionCount === 1 ? "connection" : "connections"}.`
|
||||
: "The saved credentials are deleted and agents lose access immediately. Connecting it again later needs a new sign-in or key."}
|
||||
</AlertDialogDescription>
|
||||
</AlertDialogHeader>
|
||||
<AlertDialogFooter>
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
// @vitest-environment jsdom
|
||||
|
||||
import { flushSync } from "react-dom";
|
||||
import { createRoot } from "react-dom/client";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { DangerZone } from "./AdvancedPanel";
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
function act(callback: () => void) {
|
||||
flushSync(callback);
|
||||
}
|
||||
|
||||
let container: HTMLDivElement | null = null;
|
||||
|
||||
afterEach(() => {
|
||||
container?.remove();
|
||||
container = null;
|
||||
});
|
||||
|
||||
function renderDangerZone() {
|
||||
container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
act(() => root.render(<DangerZone appName="PostHog" removing={false} onRemove={vi.fn()} />));
|
||||
return container;
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove app deletes the operator's credentials and revokes agent access
|
||||
* (PAP-17119). The confirmation has to say so before the operator commits — a
|
||||
* "you can connect it again later" reassurance implies the pasted key survives,
|
||||
* so these assertions exist to stop that copy coming back.
|
||||
*/
|
||||
describe("DangerZone", () => {
|
||||
it("promises credential deletion and re-authentication before the operator confirms", () => {
|
||||
const text = renderDangerZone().textContent ?? "";
|
||||
|
||||
expect(text).toContain("Deletes the saved credentials for PostHog");
|
||||
expect(text).toContain("takes agent access away right away");
|
||||
expect(text).toContain("needs a new sign-in or key");
|
||||
expect(text).not.toContain("You can connect it again later");
|
||||
});
|
||||
|
||||
it("keeps the warning visible in the confirming state", () => {
|
||||
const node = renderDangerZone();
|
||||
const trigger = Array.from(node.querySelectorAll("button"))
|
||||
.find((button) => button.textContent?.trim() === "Remove app");
|
||||
expect(trigger).toBeTruthy();
|
||||
|
||||
act(() => trigger!.dispatchEvent(new MouseEvent("click", { bubbles: true })));
|
||||
|
||||
const text = node.textContent ?? "";
|
||||
expect(text).toContain("Yes, remove it");
|
||||
expect(text).toContain("Deletes the saved credentials for PostHog");
|
||||
expect(text).toContain("needs a new sign-in or key");
|
||||
});
|
||||
});
|
||||
@@ -286,7 +286,8 @@ export function DangerZone({
|
||||
<div>
|
||||
<p className="text-sm font-medium text-foreground">Remove this app</p>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Agents lose access to {appName} right away. You can connect it again later.
|
||||
Deletes the saved credentials for {appName} and takes agent access away right away.
|
||||
Connecting it again later needs a new sign-in or key.
|
||||
</p>
|
||||
</div>
|
||||
{confirming ? (
|
||||
|
||||
Reference in new issue
Block a user