Harden application delete against concurrent connection creation (PAP-10824)

DELETE /tool-applications/:applicationId pre-checked for connections and
then deleted in a later statement. Because tool_connections.application_id
was ON DELETE CASCADE, a connection created in that gap was silently
cascade-deleted instead of forcing the promised 409.

Switch the FK to ON DELETE NO ACTION so the database fails closed: an
application with connections can never be deleted, and a concurrently
inserted connection (which takes a FOR KEY SHARE lock on the parent row)
forces the delete to raise a foreign_key_violation rather than cascade.
NO ACTION (end-of-statement check) rather than RESTRICT (immediate) keeps
company teardown cascading cleanly.

The service still pre-checks for a friendly 409 with connectionCount, and
now also translates a 23503 violation on the connection FK into the same
409 so the race surfaces the conflict instead of a 500.

Regression tests: the raw delete is rejected and both rows survive (the
old cascade would have silently dropped the connection), and a company
delete still cascades application + connection away.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

packages/db/src/migrations/0103_tool_connection_application_no_cascade.sql

@@ -0,0 +1,18 @@
+-- Harden DELETE /tool-applications/:applicationId against concurrent connection creation.
+-- The tool_connections.application_id FK was ON DELETE CASCADE, so a connection created in
+-- the gap between the endpoint's "any connections?" pre-check and its DELETE could be silently
+-- removed by the cascade instead of forcing the promised 409. Switch the FK to ON DELETE
+-- NO ACTION so the database fails closed: an application with connections can never be deleted,
+-- and a concurrently-inserted connection (which holds a FOR KEY SHARE lock on the parent row)
+-- forces the delete to raise a foreign_key_violation rather than cascade.
+--
+-- NO ACTION (end-of-statement check) rather than RESTRICT (immediate check) is deliberate so a
+-- company delete still cascades cleanly: companies -> tool_applications and companies ->
+-- tool_connections both fire within the one DELETE statement, and the connections are already
+-- gone by the time this constraint is verified.
+DO $$ BEGIN
+  IF EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'tool_connections_application_id_tool_applications_id_fk') THEN
+    ALTER TABLE "tool_connections" DROP CONSTRAINT "tool_connections_application_id_tool_applications_id_fk";
+  END IF;
+  ALTER TABLE "tool_connections" ADD CONSTRAINT "tool_connections_application_id_tool_applications_id_fk" FOREIGN KEY ("application_id") REFERENCES "public"."tool_applications"("id") ON DELETE no action ON UPDATE no action;
+END $$;

packages/db/src/migrations/meta/_journal.json

@@ -722,6 +722,13 @@
       "when": 1780876800000,
       "tag": "0102_tool_gateway_sessions",
       "breakpoints": true
+    },
+    {
+      "idx": 103,
+      "version": "7",
+      "when": 1780963200000,
+      "tag": "0103_tool_connection_application_no_cascade",
+      "breakpoints": true
     }
   ]
 }

packages/db/src/schema/tool_access.ts

@@ -80,7 +80,15 @@ export const toolConnections = pgTable(
   {
     id: uuid("id").primaryKey().defaultRandom(),
     companyId: uuid("company_id").notNull().references(() => companies.id, { onDelete: "cascade" }),
-    applicationId: uuid("application_id").notNull().references(() => toolApplications.id, { onDelete: "cascade" }),
+    // NO ACTION (not CASCADE) so the database itself refuses to delete an application that still
+    // has connections. This closes the delete-vs-create race in DELETE
+    // /tool-applications/:applicationId: a connection inserted concurrently takes a FOR KEY SHARE
+    // lock on the parent row, so the delete fails closed with a foreign-key violation instead of
+    // silently cascading the new connection away. NO ACTION (checked at end-of-statement) rather
+    // than RESTRICT (checked immediately) is deliberate: it lets a company delete still cascade —
+    // companies → tool_applications and companies → tool_connections both fire in one statement,
+    // and the connections are gone by the time this constraint is checked.
+    applicationId: uuid("application_id").notNull().references(() => toolApplications.id, { onDelete: "no action" }),
     name: text("name").notNull(),
     connectionKind: text("connection_kind").$type<ToolConnectionKind>().notNull().default("managed"),
     transport: text("transport").$type<ToolConnectionTransport>().notNull(),

server/src/__tests__/tool-access-service.test.ts

@@ -603,6 +603,63 @@ describeEmbeddedPostgres("tool access service", () => {
     expect(remaining).toHaveLength(1);
   });
 
+  it("fails closed at the database when a connection races an application delete (no silent cascade)", async () => {
+    const company = await createCompany(db);
+    const service = toolAccessService(db);
+    const connection = await service.createConnection(company.id, {
+      name: "Racy connection",
+      transport: "remote_http",
+      config: { url: "https://fixture.example/mcp" },
+    });
+
+    // Simulate the delete-vs-create race: skip the endpoint's "any connections?" pre-check and
+    // issue the raw DELETE it would run afterwards, standing in for a connection created in the
+    // gap. Under the old ON DELETE CASCADE schema this silently removed the linked connection;
+    // the hardened ON DELETE NO ACTION FK must reject it so the delete can never become an
+    // implicit cascade.
+    await expect(
+      db.delete(toolApplications).where(eq(toolApplications.id, connection.applicationId)),
+    ).rejects.toThrow();
+
+    const remainingApp = await db
+      .select()
+      .from(toolApplications)
+      .where(eq(toolApplications.id, connection.applicationId));
+    const remainingConnection = await db
+      .select()
+      .from(toolConnections)
+      .where(eq(toolConnections.id, connection.id));
+    expect(remainingApp).toHaveLength(1);
+    expect(remainingConnection).toHaveLength(1);
+  });
+
+  it("still cascades application + connection deletes when the owning company is removed", async () => {
+    const company = await createCompany(db);
+    const service = toolAccessService(db);
+    const connection = await service.createConnection(company.id, {
+      name: "Company-scoped connection",
+      transport: "remote_http",
+      config: { url: "https://fixture.example/mcp" },
+    });
+
+    // NO ACTION (not RESTRICT) must keep the company teardown cascade intact: deleting the
+    // company cascades to both tool_applications and tool_connections in one statement, and the
+    // end-of-statement FK check passes because the connection is already gone. RESTRICT would
+    // abort this delete mid-cascade.
+    await db.delete(companies).where(eq(companies.id, company.id));
+
+    const remainingApp = await db
+      .select()
+      .from(toolApplications)
+      .where(eq(toolApplications.id, connection.applicationId));
+    const remainingConnection = await db
+      .select()
+      .from(toolConnections)
+      .where(eq(toolConnections.id, connection.id));
+    expect(remainingApp).toHaveLength(0);
+    expect(remainingConnection).toHaveLength(0);
+  });
+
   it("returns 403 for cross-company application deletes and 404 for missing applications", async () => {
     const allowedCompany = await createCompany(db);
     const otherCompany = await createCompany(db);

server/src/services/tool-access.ts

@@ -181,6 +181,36 @@ function asRecord(value: unknown): Record<string, unknown> {
   return {};
 }
 
+// Detects a Postgres foreign_key_violation (SQLSTATE 23503) raised by the
+// tool_connections.application_id constraint — i.e. an application delete that lost the race to
+// a concurrently-created connection now that the FK is ON DELETE RESTRICT. Walks the error and
+// its `cause` since the driver may wrap the original pg error.
+function isToolConnectionForeignKeyViolation(error: unknown): boolean {
+  const records: Record<string, unknown>[] = [];
+  let current: unknown = error;
+  for (let depth = 0; depth < 4 && current && typeof current === "object"; depth += 1) {
+    const record = current as Record<string, unknown>;
+    records.push(record);
+    current = record.cause;
+  }
+  return records.some((record) => {
+    const code = typeof record.code === "string" ? record.code : null;
+    const constraint =
+      typeof record.constraint === "string"
+        ? record.constraint
+        : typeof record.constraint_name === "string"
+          ? record.constraint_name
+          : null;
+    const message = typeof record.message === "string" ? record.message : "";
+    return (
+      code === "23503" &&
+      (constraint === "tool_connections_application_id_tool_applications_id_fk" ||
+        /tool_connections/.test(constraint ?? "") ||
+        /tool_connections/.test(message))
+    );
+  });
+}
+
 function numberValue(value: unknown): number | null {
   const parsed = typeof value === "number" ? value : Number(value);
   return Number.isFinite(parsed) ? parsed : null;
@@ -1901,7 +1931,22 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
           { connectionCount: linkedConnections.length },
         );
       }
-      const [row] = await db.delete(toolApplications).where(eq(toolApplications.id, applicationId)).returning();
+      // The pre-check above gives a friendly 409 in the common case, but it cannot close the
+      // race where a connection is created in the gap before this delete runs. The FK is now
+      // ON DELETE RESTRICT, so such a delete fails closed with a foreign_key_violation instead
+      // of silently cascading the new connection away. Translate that into the same 409 so the
+      // endpoint keeps its contract instead of surfacing a 500.
+      let row: typeof toolApplications.$inferSelect | undefined;
+      try {
+        [row] = await db.delete(toolApplications).where(eq(toolApplications.id, applicationId)).returning();
+      } catch (error) {
+        if (isToolConnectionForeignKeyViolation(error)) {
+          throw conflict(
+            "This application still has connections. Remove its connections or archive the application instead of deleting it.",
+          );
+        }
+        throw error;
+      }
       if (!row) throw notFound("Tool application not found");
       return toApplication(row);
     },