From b1dad5c9befc8a2dda07f538bdd7ab1855a246ec Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 12 Aug 2026 08:38:35 -0700 Subject: [PATCH] FN-9003: reconcile approval audit events with project schema Align approval audit event declarations and reads with PostgreSQL project ownership. - Model composite project-scoped audit event identities in the schema - Scope public audit history reads and cover project isolation behavior - Document the storage contract and add a patch changeset Files changed: .changeset/fn-9003-approval-audit-project-scope.md | 7 + docs/storage.md | 2 +- ...oval-request-audit-project-isolation.pg.test.ts | 183 +++++++++++++++++++++ .../async-stores/async-approval-request-store.ts | 5 +- packages/core/src/postgres/schema/project.ts | 9 +- 5 files changed, 202 insertions(+), 4 deletions(-) Fusion-Task-Id: FN-9003 Fusion-Task-Lineage: 5a9caac3-7c2f-4584-8844-4ac711cc48bb Co-authored-by: Fusion (runfusion.ai) --- .../fn-9003-approval-audit-project-scope.md | 7 + docs/storage.md | 2 +- ...request-audit-project-isolation.pg.test.ts | 183 ++++++++++++++++++ .../async-approval-request-store.ts | 5 +- packages/core/src/postgres/schema/project.ts | 9 +- 5 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 .changeset/fn-9003-approval-audit-project-scope.md create mode 100644 packages/core/src/__tests__/postgres/approval-request-audit-project-isolation.pg.test.ts diff --git a/.changeset/fn-9003-approval-audit-project-scope.md b/.changeset/fn-9003-approval-audit-project-scope.md new file mode 100644 index 0000000000..162597f28d --- /dev/null +++ b/.changeset/fn-9003-approval-audit-project-scope.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Keep approval audit history isolated to the active project. +category: fix +dev: Reconciles approval audit event ownership declarations and threads getApprovalAuditHistory projectId from ApprovalRequestStore. diff --git a/docs/storage.md b/docs/storage.md index 9f713ba15f..945e85228f 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -510,7 +510,7 @@ The `tasks.cumulativeActiveMs` and `tasks.executionCompletedAt` columns are the | `agents` | Agent registry/state/task assignment metadata. | | `agentHeartbeats` | Heartbeat run events linked to agents (`agentId` FK cascade). | | `approval_requests` | Durable approval request records: requester actor snapshot, target action payload (category/action/resource/context), lifecycle status (`pending`/`approved`/`denied`/`completed`), optional task/run context, and requested/decided/completed timestamps. | -| `approval_request_audit_events` | Append-only audit trail for approval requests. Each row stores event type (`created`/`approved`/`denied`/`completed`), immutable actor snapshot, optional note, and deterministic per-request ordering by `(createdAt, rowid)`. | +| `approval_request_audit_events` | Append-only audit trail for approval requests. PostgreSQL uses the physical `(project_id, id)` identity, while public `ApprovalRequestStore.getAuditHistory` scopes bound audit-history reads; Command Center analytics remains intentionally unbound-tolerant. The ownership trigger normalizes only NULL/exact `''`, so a whitespace-only binding is stored literally. Rows store event type (`created`/`approved`/`denied`/`completed`), immutable actor snapshot, optional note, and deterministic per-request ordering by `(createdAt, rowid)`. | | `secrets` | Encrypted secret KV rows (`key` unique) with raw BLOB `value_ciphertext` + per-row random `nonce` (AES-256-GCM), per-secret `access_policy` CHECK (`auto`/`prompt`/`deny`), env-materialization metadata (`env_exportable`, `env_export_key`), and read-audit fields (`last_read_at`, `last_read_by`). Plaintext is never written to the database. | | `task_documents` | Task-scoped document metadata/content keyed by `(taskId, key)` with current revision pointer. | | `task_document_revisions` | Immutable revision history for task documents (content snapshots by revision). | diff --git a/packages/core/src/__tests__/postgres/approval-request-audit-project-isolation.pg.test.ts b/packages/core/src/__tests__/postgres/approval-request-audit-project-isolation.pg.test.ts new file mode 100644 index 0000000000..9da3a6a8c4 --- /dev/null +++ b/packages/core/src/__tests__/postgres/approval-request-audit-project-isolation.pg.test.ts @@ -0,0 +1,183 @@ +import { afterAll, afterEach, beforeAll, beforeEach, expect, it } from "vitest"; +import { and, eq } from "drizzle-orm"; +import { + createSharedPgTaskStoreTestHarness, + pgDescribe, + type SharedPgTaskStoreHarness, +} from "../../__test-utils__/pg-test-harness.js"; +import { + createApprovalRequest, + getApprovalAuditHistory, +} from "../../async-stores/async-approval-request-store.js"; +import { ApprovalRequestStore } from "../../agents/approval-request-store.js"; +import type { AsyncDataLayer } from "../../postgres/data-layer.js"; +import * as schema from "../../postgres/schema/index.js"; + +pgDescribe("approval request audit project isolation", () => { + const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({ + prefix: "fusion_approval_audit_isolation", + }); + const bind = (projectId: string): AsyncDataLayer => ({ ...h.layer(), projectId }); + const input = { + requester: { actorId: "agent", actorType: "agent" as const, actorName: "Agent" }, + targetAction: { + category: "other" as const, + action: "test", + summary: "Test approval audit ownership", + resourceType: "task", + resourceId: "FN-9003", + }, + }; + + beforeAll(h.beforeAll); + afterAll(h.afterAll); + beforeEach(h.beforeEach); + afterEach(h.afterEach); + + it("models the live 0006 ownership shape exactly", async () => { + const columns = await h.adminSql()>` + SELECT column_name, data_type, is_nullable, column_default + FROM information_schema.columns + WHERE table_schema = 'project' + AND table_name = 'approval_request_audit_events' + AND column_name = 'project_id' + `; + expect(columns).toEqual([expect.objectContaining({ + column_name: "project_id", + data_type: "text", + is_nullable: "NO", + column_default: expect.stringContaining("current_setting"), + })]); + + const keys = await h.adminSql()>` + SELECT c.conname, array_agg(a.attname ORDER BY k.ordinality) AS columns + FROM pg_constraint c + CROSS JOIN LATERAL unnest(c.conkey) WITH ORDINALITY k(attnum, ordinality) + JOIN pg_attribute a ON a.attrelid = c.conrelid AND a.attnum = k.attnum + WHERE c.conrelid = 'project.approval_request_audit_events'::regclass + AND c.contype = 'p' + GROUP BY c.conname + `; + expect(keys).toEqual([{ + conname: "approval_request_audit_events_pkey", + columns: ["project_id", "id"], + }]); + expect(await h.adminSql()>` + SELECT polname FROM pg_policy + WHERE polrelid = 'project.approval_request_audit_events'::regclass + `).toEqual([{ polname: "fusion_project_isolation" }]); + expect(await h.adminSql()>` + SELECT tgname FROM pg_trigger + WHERE tgrelid = 'project.approval_request_audit_events'::regclass + AND NOT tgisinternal + `).toEqual([{ tgname: "fusion_assign_project_id" }]); + expect(await h.adminSql()>` + SELECT indexname FROM pg_indexes + WHERE schemaname = 'project' + AND tablename = 'approval_request_audit_events' + AND indexname IN ('idxApprovalRequestAuditRequestCreatedAt', 'idxApprovalRequestAuditProjectCreatedAt') + ORDER BY indexname + `).toEqual([ + { indexname: "idxApprovalRequestAuditProjectCreatedAt" }, + { indexname: "idxApprovalRequestAuditRequestCreatedAt" }, + ]); + }); + + it("isolates colliding audit identities through helper and public store paths", async () => { + /* + FNXC:ApprovalAuditProjectIsolation 2026-08-12-15:37: + Owner connections can bypass RLS. Deterministic event IDs can collide when requests share + a creation instant, so make the two generated rows share an ID after creation. This proves + both the physical composite key and public-store binding prevent an authorization-trail leak. + */ + const projectA = bind("approval-audit-a"); + const projectB = bind("approval-audit-b"); + const sharedRequestId = "apr-audit-shared"; + await createApprovalRequest(projectA, { ...input, id: sharedRequestId }); + await createApprovalRequest(projectB, { ...input, id: sharedRequestId }); + + const events = await h.adminDb().select({ + projectId: schema.project.approvalRequestAuditEvents.projectId, + id: schema.project.approvalRequestAuditEvents.id, + }).from(schema.project.approvalRequestAuditEvents) + .where(eq(schema.project.approvalRequestAuditEvents.requestId, sharedRequestId)); + const aEvent = events.find((event) => event.projectId === projectA.projectId)!; + const bEvent = events.find((event) => event.projectId === projectB.projectId)!; + await h.adminDb().update(schema.project.approvalRequestAuditEvents).set({ id: aEvent.id }) + .where(and( + eq(schema.project.approvalRequestAuditEvents.projectId, projectB.projectId!), + eq(schema.project.approvalRequestAuditEvents.id, bEvent.id), + )); + const collidingEvents = await h.adminDb().select({ + projectId: schema.project.approvalRequestAuditEvents.projectId, + id: schema.project.approvalRequestAuditEvents.id, + }).from(schema.project.approvalRequestAuditEvents) + .where(eq(schema.project.approvalRequestAuditEvents.requestId, sharedRequestId)); + expect(collidingEvents).toEqual([ + { projectId: "approval-audit-a", id: aEvent.id }, + { projectId: "approval-audit-b", id: aEvent.id }, + ]); + + expect((await getApprovalAuditHistory(projectA.db, sharedRequestId, projectA.projectId)).map((event) => event.id)) + .toEqual([aEvent.id]); + expect((await getApprovalAuditHistory(projectB.db, sharedRequestId, projectB.projectId)).map((event) => event.id)) + .toEqual([aEvent.id]); + expect(await getApprovalAuditHistory(bind("approval-audit-empty").db, sharedRequestId, "approval-audit-empty")) + .toEqual([]); + + const storeA = new ApprovalRequestStore(null, { asyncLayer: projectA }); + const storeB = new ApprovalRequestStore(null, { asyncLayer: projectB }); + const unboundStore = new ApprovalRequestStore(null, { + asyncLayer: { ...h.layer(), projectId: undefined }, + }); + expect((await storeA.getAuditHistory(sharedRequestId)).map((event) => event.actor.actorName)) + .toEqual(["Agent"]); + expect((await storeB.getAuditHistory(sharedRequestId)).map((event) => event.actor.actorName)) + .toEqual(["Agent"]); + const unboundEvents = await unboundStore.getAuditHistory(sharedRequestId); + expect(unboundEvents).toHaveLength(2); + expect(Object.keys(unboundEvents[0]!).sort()).toEqual([ + "actor", "createdAt", "eventType", "id", "note", "requestId", + ]); + }); + + it("preserves unbound, whitespace, and ordering behavior", async () => { + const unbound = { ...h.layer(), projectId: undefined }; + const empty = { ...h.layer(), projectId: "" }; + const whitespace = { ...h.layer(), projectId: " " }; + await createApprovalRequest(unbound, { ...input, id: "apr-unbound" }); + await createApprovalRequest(empty, { ...input, id: "apr-empty" }); + await createApprovalRequest(whitespace, { ...input, id: "apr-whitespace" }); + + const stored = await h.adminDb().select({ + requestId: schema.project.approvalRequestAuditEvents.requestId, + projectId: schema.project.approvalRequestAuditEvents.projectId, + }).from(schema.project.approvalRequestAuditEvents) + .where(and( + eq(schema.project.approvalRequestAuditEvents.eventType, "created"), + eq(schema.project.approvalRequestAuditEvents.actorId, "agent"), + )); + expect(stored.find((row) => row.requestId === "apr-unbound")?.projectId).toBe("__legacy_unscoped__"); + expect(stored.find((row) => row.requestId === "apr-empty")?.projectId).toBe("__legacy_unscoped__"); + // The read helper trims, but the trigger's exact-'' NULLIF deliberately stores whitespace. + expect(stored.find((row) => row.requestId === "apr-whitespace")?.projectId).toBe(" "); + expect(stored.find((row) => row.requestId === "apr-whitespace")?.projectId).not.toBe("__legacy_unscoped__"); + + expect(await getApprovalAuditHistory(unbound.db, "apr-unbound", unbound.projectId)).toHaveLength(1); + expect(await getApprovalAuditHistory(empty.db, "apr-unbound", empty.projectId)).toHaveLength(1); + expect(await getApprovalAuditHistory(whitespace.db, "apr-unbound", whitespace.projectId)).toHaveLength(1); + + await h.adminDb().insert(schema.project.approvalRequestAuditEvents).values([ + { projectId: "order-project", id: "b", requestId: "apr-order", eventType: "created", actorId: "agent", actorType: "agent", actorName: "Agent", createdAt: "2026-08-12T15:00:00.000Z" }, + { projectId: "order-project", id: "a", requestId: "apr-order", eventType: "approved", actorId: "agent", actorType: "agent", actorName: "Agent", createdAt: "2026-08-12T15:00:00.000Z" }, + { projectId: "order-project", id: "c", requestId: "apr-order", eventType: "completed", actorId: "agent", actorType: "agent", actorName: "Agent", createdAt: "2026-08-12T16:00:00.000Z" }, + ]); + expect((await getApprovalAuditHistory(h.layer().db, "apr-order", "order-project")).map((event) => event.id)) + .toEqual(["a", "b", "c"]); + }); +}); diff --git a/packages/core/src/async-stores/async-approval-request-store.ts b/packages/core/src/async-stores/async-approval-request-store.ts index 323a0b8cd0..1b966c7d76 100644 --- a/packages/core/src/async-stores/async-approval-request-store.ts +++ b/packages/core/src/async-stores/async-approval-request-store.ts @@ -120,7 +120,7 @@ function rowToAuditEvent(row: ApprovalRequestAuditEventRow): ApprovalRequestAudi * Append an audit event row inside the given transaction handle. * * FNXC:ApprovalAnalyticsIsolation 2026-07-14-01:04: - * Audit events must carry the bound layer's project ID at write time because request IDs alone do not provide a reliable tenant ownership join for Command Center intervention analytics. + * Audit events must carry the bound layer's project ID at write time because request IDs alone do not provide a reliable tenant ownership join for Command Center intervention analytics. The live `(project_id, id)` key also permits deterministic audit IDs to collide safely across partitions; preserve the explicit value so the ownership trigger observes blank writes unchanged. */ async function appendAuditEvent( tx: DbTransaction, @@ -358,6 +358,9 @@ export async function markApprovalRequestCompleted( /** * Get the audit history for a request, ordered by createdAt ASC. + * + * FNXC:ApprovalAuditProjectIsolation 2026-08-12-15:37: + * Owner and superuser connections can enable `fusion.project_bypass`, so RLS cannot backstop this bare request-id lookup. Scope in SQL from the public ApprovalRequestStore layer binding; projectScopeFor intentionally treats blank and whitespace-only bindings as unbound even though fusion_assign_project_id preserves whitespace writes literally. */ export async function getApprovalAuditHistory( handle: QueryHandle, diff --git a/packages/core/src/postgres/schema/project.ts b/packages/core/src/postgres/schema/project.ts index 1014bd60aa..ee8223ab65 100644 --- a/packages/core/src/postgres/schema/project.ts +++ b/packages/core/src/postgres/schema/project.ts @@ -2433,9 +2433,13 @@ export const approvalRequests = projectSchema.table("approval_requests", { index("idxApprovalRequestsTaskCreatedAt").on(t.taskId, t.createdAt), ]); +/* +FNXC:MultiProjectIsolation 2026-08-12-15:37: +Migrations 0000 and 0003 created approval audit events with an empty-string default and id-only identity. Migration 0006 rewrote the live table to the trigger/GUC-owned partition default and `(project_id, id)` key; this declaration mirrors that physical ownership shape. +*/ export const approvalRequestAuditEvents = projectSchema.table("approval_request_audit_events", { - projectId: text("project_id").notNull().default(""), - id: text("id").primaryKey(), + projectId: text("project_id").notNull().default(sql`current_setting('fusion.project_id', true)`), + id: text("id").notNull(), requestId: text("request_id").notNull(), eventType: text("event_type").notNull(), actorId: text("actor_id").notNull(), @@ -2444,6 +2448,7 @@ export const approvalRequestAuditEvents = projectSchema.table("approval_request_ note: text("note"), createdAt: text("created_at").notNull(), }, (t) => [ + primaryKey({ columns: [t.projectId, t.id] }), index("idxApprovalRequestAuditRequestCreatedAt").on(t.requestId, t.createdAt, t.id), index("idxApprovalRequestAuditProjectCreatedAt").on(t.projectId, t.createdAt), ]);