fix(secrets): include skill metadata references in usages, route, and delete dialog

secretsSvc.usages() previously only scanned agent env bindings. Skills can
reference a secret via metadata.sourceAuthSecretId (set by the PAT auth
feature), so removing a secret without checking those references could orphan
a sibling skill's PAT pointer.

- Extend usages() to return { agents, skills } with both reference kinds.
- Update remove() to block when either array is non-empty.
- /secrets/:id/usages now responds with { agents, skills }.
- The delete dialog displays both kinds inline so the operator knows which
  references to detach first.
This commit is contained in:
2026-05-03 11:02:16 -04:00
parent 6cb333b986
commit 3ea3020a76
4 changed files with 65 additions and 24 deletions
+2 -2
View File
@@ -141,8 +141,8 @@ export function secretRoutes(db: Db) {
return; return;
} }
assertCompanyAccess(req, existing.companyId); assertCompanyAccess(req, existing.companyId);
const agents = await svc.usages(existing.companyId, id); const used = await svc.usages(existing.companyId, id);
res.json({ agents }); res.json({ agents: used.agents, skills: used.skills });
}); });
router.delete("/secrets/:id", async (req, res) => { router.delete("/secrets/:id", async (req, res) => {
+32 -10
View File
@@ -1,6 +1,6 @@
import { and, desc, eq } from "drizzle-orm"; import { and, desc, eq, sql } from "drizzle-orm";
import type { Db } from "@paperclipai/db"; import type { Db } from "@paperclipai/db";
import { companySecrets, companySecretVersions } from "@paperclipai/db"; import { companySecrets, companySecretVersions, companySkills } from "@paperclipai/db";
import type { AgentEnvConfig, EnvBinding, SecretProvider } from "@paperclipai/shared"; import type { AgentEnvConfig, EnvBinding, SecretProvider } from "@paperclipai/shared";
import { envBindingSchema } from "@paperclipai/shared"; import { envBindingSchema } from "@paperclipai/shared";
import { conflict, notFound, unprocessable } from "../errors.js"; import { conflict, notFound, unprocessable } from "../errors.js";
@@ -46,6 +46,8 @@ export function secretService(db: Db) {
}; };
type SecretUsageAgent = { id: string; name: string; envKeys: string[] }; type SecretUsageAgent = { id: string; name: string; envKeys: string[] };
type SecretUsageSkill = { id: string; name: string; slug: string };
type SecretUsages = { agents: SecretUsageAgent[]; skills: SecretUsageSkill[] };
async function getById(id: string) { async function getById(id: string) {
return db return db
@@ -55,10 +57,10 @@ export function secretService(db: Db) {
.then((rows) => rows[0] ?? null); .then((rows) => rows[0] ?? null);
} }
async function usages(companyId: string, secretId: string): Promise<SecretUsageAgent[]> { async function usages(companyId: string, secretId: string): Promise<SecretUsages> {
const agents = agentService(db); const agents = agentService(db);
const allAgents = await agents.list(companyId); const allAgents = await agents.list(companyId);
const out: SecretUsageAgent[] = []; const agentRefs: SecretUsageAgent[] = [];
for (const agent of allAgents) { for (const agent of allAgents) {
const config = asRecord(agent.adapterConfig); const config = asRecord(agent.adapterConfig);
if (!config) continue; if (!config) continue;
@@ -73,10 +75,28 @@ export function secretService(db: Db) {
} }
} }
if (matchingKeys.length > 0) { if (matchingKeys.length > 0) {
out.push({ id: agent.id, name: agent.name, envKeys: matchingKeys }); agentRefs.push({ id: agent.id, name: agent.name, envKeys: matchingKeys });
} }
} }
return out;
const skillRows = await db
.select({
id: companySkills.id,
name: companySkills.name,
slug: companySkills.slug,
})
.from(companySkills)
.where(and(
eq(companySkills.companyId, companyId),
sql`${companySkills.metadata} ->> 'sourceAuthSecretId' = ${secretId}`,
));
const skillRefs: SecretUsageSkill[] = skillRows.map((row) => ({
id: row.id,
name: row.name,
slug: row.slug,
}));
return { agents: agentRefs, skills: skillRefs };
} }
async function getByName(companyId: string, name: string) { async function getByName(companyId: string, name: string) {
@@ -315,11 +335,13 @@ export function secretService(db: Db) {
const secret = await getById(secretId); const secret = await getById(secretId);
if (!secret) return null; if (!secret) return null;
const used = await usages(secret.companyId, secretId); const used = await usages(secret.companyId, secretId);
if (used.length > 0) { if (used.agents.length > 0 || used.skills.length > 0) {
const names = used.map((agent) => agent.name).sort((left, right) => left.localeCompare(right)); const agentNames = used.agents.map((agent) => agent.name);
const skillNames = used.skills.map((skill) => skill.name);
const names = [...agentNames, ...skillNames].sort((left, right) => left.localeCompare(right));
throw unprocessable( throw unprocessable(
`Cannot delete secret "${secret.name}" while it is still used by ${names.join(", ")}. Detach it from those agents first.`, `Cannot delete secret "${secret.name}" while it is still used by ${names.join(", ")}. Detach it from those references first.`,
{ secretId, usedByAgents: used }, { secretId, usedByAgents: used.agents, usedBySkills: used.skills },
); );
} }
await db.delete(companySecrets).where(eq(companySecrets.id, secretId)); await db.delete(companySecrets).where(eq(companySecrets.id, secretId));
+4 -1
View File
@@ -23,5 +23,8 @@ export const secretsApi = {
) => api.patch<CompanySecret>(`/secrets/${id}`, data), ) => api.patch<CompanySecret>(`/secrets/${id}`, data),
remove: (id: string) => api.delete<{ ok: true }>(`/secrets/${id}`), remove: (id: string) => api.delete<{ ok: true }>(`/secrets/${id}`),
usages: (id: string) => usages: (id: string) =>
api.get<{ agents: { id: string; name: string; envKeys: string[] }[] }>(`/secrets/${id}/usages`), api.get<{
agents: { id: string; name: string; envKeys: string[] }[];
skills: { id: string; name: string; slug: string }[];
}>(`/secrets/${id}/usages`),
}; };
+27 -11
View File
@@ -421,7 +421,9 @@ function EditSecretDialog({
); );
} }
type DeleteUsage = { id: string; name: string; envKeys: string[] }; type DeleteUsageAgent = { id: string; name: string; envKeys: string[] };
type DeleteUsageSkill = { id: string; name: string; slug: string };
type DeleteBlock = { agents: DeleteUsageAgent[]; skills: DeleteUsageSkill[] };
function DeleteSecretDialog({ function DeleteSecretDialog({
secret, secret,
@@ -432,7 +434,7 @@ function DeleteSecretDialog({
onClose: () => void; onClose: () => void;
onDeleted: () => void; onDeleted: () => void;
}) { }) {
const [blockedBy, setBlockedBy] = useState<DeleteUsage[] | null>(null); const [blockedBy, setBlockedBy] = useState<DeleteBlock | null>(null);
const [errorMessage, setErrorMessage] = useState<string | null>(null); const [errorMessage, setErrorMessage] = useState<string | null>(null);
const remove = useMutation({ const remove = useMutation({
@@ -441,12 +443,16 @@ function DeleteSecretDialog({
onError: (error: unknown) => { onError: (error: unknown) => {
if (error instanceof ApiError) { if (error instanceof ApiError) {
const body = error.body as const body = error.body as
| { details?: { usedByAgents?: DeleteUsage[] } } | { details?: { usedByAgents?: DeleteUsageAgent[]; usedBySkills?: DeleteUsageSkill[] } }
| null | null
| undefined; | undefined;
const usedByAgents = body?.details?.usedByAgents; const usedByAgents = body?.details?.usedByAgents ?? [];
if (Array.isArray(usedByAgents) && usedByAgents.length > 0) { const usedBySkills = body?.details?.usedBySkills ?? [];
setBlockedBy(usedByAgents); if (
(Array.isArray(usedByAgents) && usedByAgents.length > 0)
|| (Array.isArray(usedBySkills) && usedBySkills.length > 0)
) {
setBlockedBy({ agents: usedByAgents, skills: usedBySkills });
setErrorMessage(error.message); setErrorMessage(error.message);
return; return;
} }
@@ -457,6 +463,8 @@ function DeleteSecretDialog({
}, },
}); });
const totalBlockers = blockedBy ? blockedBy.agents.length + blockedBy.skills.length : 0;
return ( return (
<Dialog open onOpenChange={(open) => !open && onClose()}> <Dialog open onOpenChange={(open) => !open && onClose()}>
<DialogContent className="sm:max-w-md"> <DialogContent className="sm:max-w-md">
@@ -464,25 +472,33 @@ function DeleteSecretDialog({
<DialogTitle>Delete "{secret.name}"</DialogTitle> <DialogTitle>Delete "{secret.name}"</DialogTitle>
<DialogDescription> <DialogDescription>
This permanently deletes the secret and all of its versions. Agents This permanently deletes the secret and all of its versions. Agents
that reference it will fail until the binding is removed. and skills that reference it will fail until the binding is removed.
</DialogDescription> </DialogDescription>
</DialogHeader> </DialogHeader>
{blockedBy && blockedBy.length > 0 ? ( {blockedBy && totalBlockers > 0 ? (
<div className="space-y-2 rounded-md border border-destructive/50 bg-destructive/5 px-3 py-3 text-xs"> <div className="space-y-2 rounded-md border border-destructive/50 bg-destructive/5 px-3 py-3 text-xs">
<p className="font-medium text-destructive"> <p className="font-medium text-destructive">
Cannot delete still referenced by: Cannot delete still referenced by:
</p> </p>
<ul className="space-y-1 text-muted-foreground"> <ul className="space-y-1 text-muted-foreground">
{blockedBy.map((agent) => ( {blockedBy.agents.map((agent) => (
<li key={agent.id}> <li key={`agent:${agent.id}`}>
<span className="font-medium text-foreground">{agent.name}</span>{" "} <span className="font-medium text-foreground">{agent.name}</span>{" "}
<span className="text-muted-foreground/70">agent</span>{" "}
({agent.envKeys.join(", ")}) ({agent.envKeys.join(", ")})
</li> </li>
))} ))}
{blockedBy.skills.map((skill) => (
<li key={`skill:${skill.id}`}>
<span className="font-medium text-foreground">{skill.name}</span>{" "}
<span className="text-muted-foreground/70">skill</span>{" "}
({skill.slug})
</li>
))}
</ul> </ul>
<p className="text-muted-foreground"> <p className="text-muted-foreground">
Detach this secret from those agents' environment variables first. Detach this secret from those references first.
</p> </p>
</div> </div>
) : errorMessage ? ( ) : errorMessage ? (