diff --git a/packages/db/src/migrations/0452_secret_proposal_secret_cascade.sql b/packages/db/src/migrations/0452_secret_proposal_secret_cascade.sql new file mode 100644 index 00000000000..2d85e90ed77 --- /dev/null +++ b/packages/db/src/migrations/0452_secret_proposal_secret_cascade.sql @@ -0,0 +1,3 @@ +ALTER TABLE "company_secret_proposals" DROP CONSTRAINT IF EXISTS "company_secret_proposals_secret_id_company_secrets_id_fk"; +--> statement-breakpoint +ALTER TABLE "company_secret_proposals" ADD CONSTRAINT "company_secret_proposals_secret_id_company_secrets_id_fk" FOREIGN KEY ("secret_id") REFERENCES "public"."company_secrets"("id") ON DELETE cascade ON UPDATE no action; \ No newline at end of file diff --git a/packages/db/src/migrations/meta/0447_snapshot.json b/packages/db/src/migrations/meta/0452_snapshot.json similarity index 99% rename from packages/db/src/migrations/meta/0447_snapshot.json rename to packages/db/src/migrations/meta/0452_snapshot.json index 4e82c37d4bb..082e8419e84 100644 --- a/packages/db/src/migrations/meta/0447_snapshot.json +++ b/packages/db/src/migrations/meta/0452_snapshot.json @@ -1,6 +1,6 @@ { - "id": "76f06e29-53a8-4ecc-809a-d52b6bbb6520", - "prevId": "782688f1-7843-4a93-8ea3-2cec362d269e", + "id": "64e8be1e-e0cb-4cd8-85e0-0fdcce5eafde", + "prevId": "b5699366-77c2-48c8-b7dc-658a75de7658", "version": "7", "dialect": "postgresql", "tables": { @@ -4102,6 +4102,28 @@ "method": "btree", "with": {} }, + "agent_wakeup_requests_issue_blocker_stalled_uq": { + "name": "agent_wakeup_requests_issue_blocker_stalled_uq", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "idempotency_key", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "where": "\"agent_wakeup_requests\".\"idempotency_key\" LIKE 'issue_blocker_stalled:%' AND \"agent_wakeup_requests\".\"status\" IN ('queued', 'deferred_issue_execution', 'claimed', 'coalesced', 'completed')", + "concurrently": true, + "method": "btree", + "with": {} + }, "agent_wakeup_requests_company_payload_issue_idx": { "name": "agent_wakeup_requests_company_payload_issue_idx", "columns": [ @@ -4252,6 +4274,18 @@ "primaryKey": false, "notNull": false }, + "harness_verified_digest": { + "name": "harness_verified_digest", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "harness_verified_at": { + "name": "harness_verified_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, "reports_to": { "name": "reports_to", "type": "uuid", @@ -7848,6 +7882,150 @@ }, "isRLSEnabled": false }, + "public.budget_resume_claims": { + "name": "budget_resume_claims", + "schema": "", + "columns": { + "run_id": { + "name": "run_id", + "type": "uuid", + "primaryKey": true, + "notNull": true + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "incident_id": { + "name": "incident_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "actor_user_id": { + "name": "actor_user_id", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "actor_source": { + "name": "actor_source", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "state": { + "name": "state", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "'pending'" + }, + "claimed_at": { + "name": "claimed_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "settled_at": { + "name": "settled_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "reason": { + "name": "reason", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "request_id": { + "name": "request_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "budget_resume_claims_pending_idx": { + "name": "budget_resume_claims_pending_idx", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "run_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "where": "\"budget_resume_claims\".\"state\" = 'pending'", + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "budget_resume_claims_run_id_heartbeat_runs_id_fk": { + "name": "budget_resume_claims_run_id_heartbeat_runs_id_fk", + "tableFrom": "budget_resume_claims", + "tableTo": "heartbeat_runs", + "columnsFrom": [ + "run_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "budget_resume_claims_company_id_companies_id_fk": { + "name": "budget_resume_claims_company_id_companies_id_fk", + "tableFrom": "budget_resume_claims", + "tableTo": "companies", + "columnsFrom": [ + "company_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "budget_resume_claims_incident_id_budget_incidents_id_fk": { + "name": "budget_resume_claims_incident_id_budget_incidents_id_fk", + "tableFrom": "budget_resume_claims", + "tableTo": "budget_incidents", + "columnsFrom": [ + "incident_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, "public.built_in_managed_resources": { "name": "built_in_managed_resources", "schema": "", @@ -17221,7 +17399,7 @@ "columnsTo": [ "id" ], - "onDelete": "set null", + "onDelete": "cascade", "onUpdate": "no action" }, "company_secret_proposals_secret_proposal_id_company_secret_proposals_id_fk": { @@ -21965,6 +22143,112 @@ "checkConstraints": {}, "isRLSEnabled": false }, + "public.controller_instances": { + "name": "controller_instances", + "schema": "", + "columns": { + "boot_id": { + "name": "boot_id", + "type": "text", + "primaryKey": true, + "notNull": true + }, + "epoch": { + "name": "epoch", + "type": "bigserial", + "primaryKey": false, + "notNull": true + }, + "process_space": { + "name": "process_space", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "pid": { + "name": "pid", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "started_at": { + "name": "started_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "renewed_at": { + "name": "renewed_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "lease_expires_at": { + "name": "lease_expires_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true + }, + "fenced_at": { + "name": "fenced_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "fenced_by_epoch": { + "name": "fenced_by_epoch", + "type": "bigint", + "primaryKey": false, + "notNull": false + }, + "stopped_at": { + "name": "stopped_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + } + }, + "indexes": { + "controller_instances_epoch_idx": { + "name": "controller_instances_epoch_idx", + "columns": [ + { + "expression": "epoch", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + }, + "controller_instances_lease_idx": { + "name": "controller_instances_lease_idx", + "columns": [ + { + "expression": "lease_expires_at", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, "public.cost_events": { "name": "cost_events", "schema": "", diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index cbd48bace5c..8d913de1669 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -3151,6 +3151,13 @@ "when": 1794185816322, "tag": "0451_budget_resume_claims", "breakpoints": true + }, + { + "idx": 452, + "version": "7", + "when": 1794205579544, + "tag": "0452_secret_proposal_secret_cascade", + "breakpoints": true } ] } \ No newline at end of file diff --git a/packages/db/src/schema/company_secret_proposals.ts b/packages/db/src/schema/company_secret_proposals.ts index 7bcfd3c1e48..476179a1e1e 100644 --- a/packages/db/src/schema/company_secret_proposals.ts +++ b/packages/db/src/schema/company_secret_proposals.ts @@ -21,7 +21,9 @@ export const companySecretProposals = pgTable( valueCiphertext: jsonb("value_ciphertext").$type | null>(), valueFingerprintSha256: text("value_fingerprint_sha256"), valueLength: integer("value_length"), - secretId: uuid("secret_id").references(() => companySecrets.id, { onDelete: "set null" }), + // Binding proposals require exactly one source reference (shape check), so + // they go with the secret they reference, like secretProposalId below. + secretId: uuid("secret_id").references(() => companySecrets.id, { onDelete: "cascade" }), secretProposalId: uuid("secret_proposal_id").references((): AnyPgColumn => companySecretProposals.id, { onDelete: "cascade" }), targetType: text("target_type"), targetId: uuid("target_id").references(() => agents.id, { onDelete: "cascade" }), diff --git a/server/src/__tests__/secret-proposals-routes.test.ts b/server/src/__tests__/secret-proposals-routes.test.ts index 940b1c09a12..250132f6a46 100644 --- a/server/src/__tests__/secret-proposals-routes.test.ts +++ b/server/src/__tests__/secret-proposals-routes.test.ts @@ -865,6 +865,73 @@ describeEmbeddedPostgres("secret proposal routes", () => { ]); }); + it.each(["rejected", "approved", "pending"] as const)("deletes a secret whose %s binding proposal referenced it", async (outcome) => { + const fixture = await seedRun(); + const secret = await secretService(db).create(fixture.companyId, { + name: "dev/deploy/publishing", + key: "DEPLOY_PUBLISHING", + provider: "local_encrypted", + value: "publishing-secret", + }); + const proposals = createSecretProposalsService(db); + const proposal = await proposals.createBinding({ companyId: fixture.companyId, heartbeatRunId: fixture.heartbeatRunId }, { + secretId: secret.id, + configPath: "env.PUBLISHING_API_KEY", + justification: "Publish the site", + bindingTargetPolicy: "self_and_reports", + }); + if (outcome === "rejected") await proposals.transition(fixture.companyId, proposal.id, "rejected", { resolvedByUserId: "board-user" }); + if (outcome === "approved") { + await proposals.approve(fixture.companyId, proposal.id, { resolvedByUserId: "board-user" }); + await db.delete(companySecretBindings).where(eq(companySecretBindings.secretId, secret.id)); + } + + // The proposal's secret reference used ON DELETE SET NULL, which the binding + // shape check rejects, so deleting the secret failed with a 500. + await db.update(issues).set({ assigneeAgentId: fixture.agentId }).where(eq(issues.id, fixture.issueId)); + const wakeup = vi.fn().mockResolvedValue(null); + const persistedIssues = issueService(db); + const addComment = vi.fn().mockResolvedValue(undefined); + const deleted = await request(createBoardApp(fixture, { + heartbeat: { wakeup }, + issues: { getById: persistedIssues.getById, addComment }, + })).delete(`/api/secrets/${secret.id}`); + expect(deleted.status).toBe(200); + // Only a pending proposal is rejected by the delete; it gets the normal rejection signal. + expect(addComment).toHaveBeenCalledTimes(outcome === "pending" ? 1 : 0); + if (outcome === "pending") { + expect(addComment.mock.calls[0]?.[1]).toContain("Status: **rejected**"); + expect(addComment.mock.calls[0]?.[1]).toContain("The referenced secret was deleted."); + } + expect(wakeup).toHaveBeenCalledTimes(outcome === "pending" ? 1 : 0); + expect(await db.select().from(companySecrets).where(eq(companySecrets.id, secret.id))).toEqual([]); + expect(await db.select().from(companySecretProposals).where(eq(companySecretProposals.id, proposal.id))).toEqual([]); + // A pending review card closes as rejected instead of pointing at a deleted proposal. + const [card] = await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, proposal.interactionId!)); + expect(card?.status).toBe(outcome === "approved" ? "accepted" : "rejected"); + }); + + it("refuses a new binding proposal for a secret that a delete already marked deleted", async () => { + const fixture = await seedRun(); + const secret = await secretService(db).create(fixture.companyId, { + name: "dev/deploy/deleting", + key: "DEPLOY_DELETING", + provider: "local_encrypted", + value: "deleting-secret", + }); + let proposalDuringDelete: unknown; + await secretService(db).remove(secret.id, { + beforeDelete: async () => { + proposalDuringDelete = await createSecretProposalsService(db).createBinding( + { companyId: fixture.companyId, heartbeatRunId: fixture.heartbeatRunId }, + { secretId: secret.id, configPath: "env.LATE_KEY", justification: "Late request", bindingTargetPolicy: "self_and_reports" }, + ).catch((error: unknown) => error); + }, + }); + expect(proposalDuringDelete).toMatchObject({ status: 404 }); + expect(await db.select().from(companySecretProposals)).toEqual([]); + }); + it("proposes and approves a binding by resolving the proposer's own sourceConfigPath", async () => { const fixture = await seedRun(); const sourceSecret = await secretService(db).create(fixture.companyId, { diff --git a/server/src/__tests__/secrets-routes.test.ts b/server/src/__tests__/secrets-routes.test.ts index 73d37996bdf..d684c7c16d6 100644 --- a/server/src/__tests__/secrets-routes.test.ts +++ b/server/src/__tests__/secrets-routes.test.ts @@ -1039,6 +1039,7 @@ describe("secret routes", () => { expect(res.body).toEqual({ ok: true }); expect(mockSecretService.remove).toHaveBeenCalledWith( "33333333-3333-4333-8333-333333333333", + { beforeDelete: expect.any(Function) }, ); expect(mockLogActivity).toHaveBeenCalledWith( expect.anything(), diff --git a/server/src/routes/secrets.ts b/server/src/routes/secrets.ts index 3d3ca863f22..0747738e2b4 100644 --- a/server/src/routes/secrets.ts +++ b/server/src/routes/secrets.ts @@ -1115,7 +1115,17 @@ export function secretRoutes(db: Db, deps: SecretRoutesDeps = {}) { ); if (!existing) return; - const removed = await svc.remove(id); + const resolvedByUserId = req.actor.userId ?? "board"; + const reason = "The referenced secret was deleted."; + const removed = await svc.remove(id, { + beforeDelete: async (secret) => { + const rejected = await proposals.rejectPendingBindingsForSecret(secret.companyId, secret.id, { resolvedByUserId, reason }); + // Same signal as an ordinary rejection, so the proposing agent continues. + for (const proposal of rejected) { + await notifySecretProposalResolution({ proposal, status: "rejected", userId: resolvedByUserId, reason, issues, heartbeat }); + } + }, + }); if (!removed) { res.status(404).json({ error: "Secret not found" }); return; diff --git a/server/src/services/secret-proposals.ts b/server/src/services/secret-proposals.ts index 870749d1513..7de0ad8302e 100644 --- a/server/src/services/secret-proposals.ts +++ b/server/src/services/secret-proposals.ts @@ -446,6 +446,13 @@ export function createSecretProposalsService(db: Db) { return createWithinQuota( { companyId: context.companyId, agentId: run.agentId, runId: run.id, issueId: originIssueId }, async (txDb) => { + if (resolvedSecretId) { + // Hold the secret against a concurrent delete: the delete marks it + // deleted first, then settles the proposals that reference it. + const live = await txDb.select({ status: companySecrets.status }).from(companySecrets) + .where(eq(companySecrets.id, resolvedSecretId)).for("share").then((rows) => rows[0] ?? null); + if (!live || live.status === "deleted") throw notFound("Secret not found"); + } if (input.secretProposalId) { const dependency = await txDb.select().from(companySecretProposals).where(and( eq(companySecretProposals.id, input.secretProposalId), @@ -915,5 +922,35 @@ export function createSecretProposalsService(db: Db) { return row?.at == null ? null : new Date(row.at).getTime(); } - return { getById, view: enrich, createSecret, createBinding, listForAgent, listForBoard, assertBindingSnapshotCurrent, approve, transition, sweepExpired, sweepExpiredForCompany, expiryDeadlines, nextExpiryAt }; + /** + * Deleting a secret also deletes the binding proposals that reference it. + * Settle pending ones first, so their review cards close as rejected with + * the reason instead of pointing at a proposal that no longer exists. Call + * it after the secret is marked deleted: createBinding holds the secret row + * and refuses a deleted one, so no new pending proposal can follow. + */ + async function rejectPendingBindingsForSecret(companyId: string, secretId: string, input: { resolvedByUserId?: string | null; reason: string }) { + const pending = await db.select().from(companySecretProposals).where(and( + eq(companySecretProposals.companyId, companyId), + eq(companySecretProposals.secretId, secretId), + eq(companySecretProposals.kind, "binding"), + eq(companySecretProposals.status, "pending"), + )); + const rejected: Proposal[] = []; + for (const row of pending) { + try { + await transition(companyId, row.id, "rejected", { + resolvedByUserId: input.resolvedByUserId ?? null, + reason: input.reason, + }); + rejected.push(row); + } catch (error) { + // Resolved concurrently; the secret deletion still removes the row. + if (!(error instanceof HttpError) || error.status !== 409) throw error; + } + } + return rejected; + } + + return { getById, view: enrich, createSecret, createBinding, listForAgent, listForBoard, assertBindingSnapshotCurrent, approve, transition, rejectPendingBindingsForSecret, sweepExpired, sweepExpiredForCompany, expiryDeadlines, nextExpiryAt }; } diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index 730c15c9059..deb48d52673 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -970,6 +970,11 @@ function assertSelectableProviderConfig(config: { } } +export type RemoveSecretOptions = { + /** Runs after the secret is marked deleted and before its row is removed. */ + beforeDelete?: (secret: typeof companySecrets.$inferSelect) => Promise; +}; + export function secretService(db: Db | DbTransaction) { const authorization = authorizationService(db); @@ -3401,18 +3406,18 @@ export function secretService(db: Db | DbTransaction) { // `removeSecretUnlocked` re-reads the secret under the lock, so a delete // that lost a race between the pre-check and the lock still sees current // state, not the pre-check's stale read. - async function removeSecretInternal(secretId: string) { + async function removeSecretInternal(secretId: string, options: RemoveSecretOptions = {}) { const preCheckSecret = await getById(secretId); if (!preCheckSecret) return null; return withAccountHomeSecretMutationLock(undefined, preCheckSecret.companyId, () => - removeSecretUnlocked(secretId), + removeSecretUnlocked(secretId, options), ); } // The body of `removeSecretInternal` above, unchanged. Extracted to a named // function so that wrapper can hold the lock across this whole delete // sequence without duplicating it. - async function removeSecretUnlocked(secretId: string) { + async function removeSecretUnlocked(secretId: string, options: RemoveSecretOptions = {}) { const secret = await getById(secretId); if (!secret) return null; const versionRow = await getSecretVersion(secret.id, secret.latestVersion); @@ -3430,6 +3435,9 @@ export function secretService(db: Db | DbTransaction) { }) .where(eq(companySecrets.id, secretId)); } + // New references now see the deleted status, so the caller can settle + // every reference that exists before the row and its cascades go. + await options.beforeDelete?.(secret); const providerConfig = secret.providerConfigId ? await getProviderConfigById(secret.providerConfigId) : null;