From 7516b64d8c3814d193bf9760b1b7b0b4deaa05e7 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 1 Aug 2026 18:41:44 +0000 Subject: [PATCH] fix: retain fail-closed run tombstones Co-Authored-By: Paperclip --- doc/DEVELOPING.md | 9 +++--- .../0199_heartbeat_run_issue_privacy.sql | 14 +++++---- packages/db/src/schema/heartbeat_runs.ts | 4 +-- .../db/src/schema/workspace_operations.ts | 4 +-- .../heartbeat-run-privacy-routes.test.ts | 29 +++++++++++++++---- 5 files changed, 42 insertions(+), 18 deletions(-) diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index b008d82d26c..23abfd65ec8 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -341,10 +341,11 @@ No Docker or external database is required for this mode. Grant lookups use a five-second cache by default; override it with `PAPERCLIP_ISSUE_PRIVACY_CACHE_TTL_MS` when testing revocation timing. Issue-bound heartbeat runs inherit the same read predicate through the stored -`heartbeat_runs.scope_kind = 'issue'` and `issue_id` binding. The database -rejects incomplete bindings and restricts deletion of referenced issues and -workspace-operation history, so confidential runs cannot become company-level -maintenance runs through foreign-key nulling. In enforce mode, direct run detail, transcript, +`heartbeat_runs.scope_kind = 'issue'` and `issue_id` binding. Issue deletion +nulls the foreign key but preserves the explicit issue scope as a fail-closed +tombstone. Workspace-operation history is either deleted with its direct issue +or run binding, or remains bound to that fail-closed run tombstone, so it cannot +become company-level maintenance data through foreign-key nulling. In enforce mode, direct run detail, transcript, event, log, and workspace-operation reads return `404` to non-members. Company run-history and live-run lists retain a metadata-only row with timing, status, token usage, and cost for budget oversight; issue identifiers, summaries, and diff --git a/packages/db/src/migrations/0199_heartbeat_run_issue_privacy.sql b/packages/db/src/migrations/0199_heartbeat_run_issue_privacy.sql index eef95517a6c..4fba80a906b 100644 --- a/packages/db/src/migrations/0199_heartbeat_run_issue_privacy.sql +++ b/packages/db/src/migrations/0199_heartbeat_run_issue_privacy.sql @@ -91,7 +91,7 @@ ALTER TABLE "heartbeat_runs" DROP CONSTRAINT IF EXISTS "heartbeat_runs_issue_id_ ALTER TABLE "heartbeat_runs" ADD CONSTRAINT "heartbeat_runs_issue_id_issues_id_fk" FOREIGN KEY ("issue_id") REFERENCES "public"."issues"("id") - ON DELETE RESTRICT ON UPDATE NO ACTION NOT VALID; + ON DELETE SET NULL ON UPDATE NO ACTION NOT VALID; --> statement-breakpoint ALTER TABLE "heartbeat_runs" VALIDATE CONSTRAINT "heartbeat_runs_issue_id_issues_id_fk"; --> statement-breakpoint @@ -100,7 +100,11 @@ RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN - NEW."scope_kind" := CASE WHEN NEW."issue_id" IS NULL THEN 'company' ELSE 'issue' END; + NEW."scope_kind" := CASE + WHEN NEW."issue_id" IS NOT NULL THEN 'issue' + WHEN TG_OP = 'UPDATE' AND OLD."scope_kind" = 'issue' THEN 'issue' + ELSE 'company' + END; RETURN NEW; END $$; --> statement-breakpoint @@ -116,7 +120,7 @@ ALTER TABLE "heartbeat_runs" ADD CONSTRAINT "heartbeat_runs_scope_binding_check" CHECK ( ("scope_kind" = 'company' AND "issue_id" IS NULL) - OR ("scope_kind" = 'issue' AND "issue_id" IS NOT NULL) + OR "scope_kind" = 'issue' ) NOT VALID; --> statement-breakpoint ALTER TABLE "heartbeat_runs" VALIDATE CONSTRAINT "heartbeat_runs_scope_binding_check"; @@ -126,7 +130,7 @@ ALTER TABLE "workspace_operations" DROP CONSTRAINT IF EXISTS "workspace_operatio ALTER TABLE "workspace_operations" ADD CONSTRAINT "workspace_operations_heartbeat_run_id_heartbeat_runs_id_fk" FOREIGN KEY ("heartbeat_run_id") REFERENCES "public"."heartbeat_runs"("id") - ON DELETE RESTRICT ON UPDATE NO ACTION NOT VALID; + ON DELETE CASCADE ON UPDATE NO ACTION NOT VALID; --> statement-breakpoint ALTER TABLE "workspace_operations" VALIDATE CONSTRAINT "workspace_operations_heartbeat_run_id_heartbeat_runs_id_fk"; --> statement-breakpoint @@ -135,7 +139,7 @@ ALTER TABLE "workspace_operations" DROP CONSTRAINT IF EXISTS "workspace_operatio ALTER TABLE "workspace_operations" ADD CONSTRAINT "workspace_operations_issue_id_issues_id_fk" FOREIGN KEY ("issue_id") REFERENCES "public"."issues"("id") - ON DELETE RESTRICT ON UPDATE NO ACTION NOT VALID; + ON DELETE CASCADE ON UPDATE NO ACTION NOT VALID; --> statement-breakpoint ALTER TABLE "workspace_operations" VALIDATE CONSTRAINT "workspace_operations_issue_id_issues_id_fk"; --> statement-breakpoint diff --git a/packages/db/src/schema/heartbeat_runs.ts b/packages/db/src/schema/heartbeat_runs.ts index b8c4e2f5a34..e9febfece93 100644 --- a/packages/db/src/schema/heartbeat_runs.ts +++ b/packages/db/src/schema/heartbeat_runs.ts @@ -27,7 +27,7 @@ export const heartbeatRuns = pgTable( .$type<"company" | "issue">() .notNull() .default("company"), - issueId: uuid("issue_id").references((): AnyPgColumn => issues.id, { onDelete: "restrict" }), + issueId: uuid("issue_id").references((): AnyPgColumn => issues.id, { onDelete: "set null" }), invocationSource: text("invocation_source").notNull().default("on_demand"), triggerDetail: text("trigger_detail"), status: text("status").notNull().default("queued"), @@ -81,7 +81,7 @@ export const heartbeatRuns = pgTable( scopeBindingCheck: check( "heartbeat_runs_scope_binding_check", sql`(${table.scopeKind} = 'company' AND ${table.issueId} IS NULL) - OR (${table.scopeKind} = 'issue' AND ${table.issueId} IS NOT NULL)`, + OR ${table.scopeKind} = 'issue'`, ), companyAgentStartedIdx: index("heartbeat_runs_company_agent_started_idx").on( table.companyId, diff --git a/packages/db/src/schema/workspace_operations.ts b/packages/db/src/schema/workspace_operations.ts index 3c730f74f5b..c81f4aad97f 100644 --- a/packages/db/src/schema/workspace_operations.ts +++ b/packages/db/src/schema/workspace_operations.ts @@ -23,10 +23,10 @@ export const workspaceOperations = pgTable( onDelete: "set null", }), heartbeatRunId: uuid("heartbeat_run_id").references(() => heartbeatRuns.id, { - onDelete: "restrict", + onDelete: "cascade", }), issueId: uuid("issue_id").references(() => issues.id, { - onDelete: "restrict", + onDelete: "cascade", }), phase: text("phase").notNull(), command: text("command"), diff --git a/server/src/__tests__/heartbeat-run-privacy-routes.test.ts b/server/src/__tests__/heartbeat-run-privacy-routes.test.ts index 906138fa907..cc6b1374956 100644 --- a/server/src/__tests__/heartbeat-run-privacy-routes.test.ts +++ b/server/src/__tests__/heartbeat-run-privacy-routes.test.ts @@ -350,15 +350,34 @@ describeEmbeddedPostgres.sequential("heartbeat run privacy routes", () => { expect(allowedCancel.body.id).toBe(fixture.privateRunId); }); - it("preserves issue and run bindings while confidential operation history exists", async () => { + it("keeps deleted-issue runs and operation history bound to a fail-closed tombstone", async () => { const fixture = await seedFixture(); - await expect(db.delete(issues).where(eq(issues.id, fixture.issueId))).rejects.toThrow(); - await expect(db.delete(heartbeatRuns).where(eq(heartbeatRuns.id, fixture.privateRunId))).rejects.toThrow(); + await db.delete(issues).where(eq(issues.id, fixture.issueId)); - const deniedDetail = await request(createApp(fixture.companyId, fixture.otherAgentId)) + const missingDetail = await request(createApp(fixture.companyId, fixture.ownerAgentId)) .get(`/api/heartbeat-runs/${fixture.privateRunId}`); - expect(deniedDetail.status).toBe(404); + expect(missingDetail.status).toBe(404); + + const [tombstonedRun] = await db.select({ + scopeKind: heartbeatRuns.scopeKind, + issueId: heartbeatRuns.issueId, + }) + .from(heartbeatRuns) + .where(eq(heartbeatRuns.id, fixture.privateRunId)); + expect(tombstonedRun).toEqual({ scopeKind: "issue", issueId: null }); + + const [operation] = await db.select({ + heartbeatRunId: workspaceOperations.heartbeatRunId, + issueId: workspaceOperations.issueId, + }) + .from(workspaceOperations) + .where(eq(workspaceOperations.id, fixture.operationId)); + expect(operation).toEqual({ heartbeatRunId: fixture.privateRunId, issueId: null }); + + const missingOperationLog = await request(createApp(fixture.companyId, fixture.ownerAgentId)) + .get(`/api/workspace-operations/${fixture.operationId}/log`); + expect(missingOperationLog.status).toBe(404); }); it("logs would-deny decisions without enforcing them in shadow mode", async () => {