fix(portal-bff): audit writes use raw INSERT (audit_writer has no SELECT for RETURNING) #121

Merged

julien merged 2 commits from fix/portal-bff/audit-insert-without-returning into main 2026-05-13 19:48:34 +02:00

Conversation 0 Commits 2 Files Changed 4

+191 -107

julien commented 2026-05-13 19:46:05 +02:00

Owner

Copy Link

Copy Source

Summary

#120 shipped the audit pipeline but the end-to-end path was never smoke-tested against a running Postgres. First click on /auth/logout returned 500 with the Pino log:

PostgresError code 42501 — permission denied for table events

Despite:

• ACL on audit.events showing audit_writer=a/audit_owner (INSERT granted).
• has_table_privilege('audit_writer', 'audit.events', 'INSERT') returning t.
• has_schema_privilege / has_type_privilege all t.
• A direct psql INSERT INTO audit.events ... after SET LOCAL ROLE audit_writer succeeding.
• A psql INSERT ... RETURNING id after the same SET LOCAL ROLE failing with the exact same error.

Root cause: Prisma's ORM tx.auditEvent.create(...) issues INSERT ... RETURNING * to hydrate the returned entity. Postgres requires SELECT on every column listed in RETURNING. audit_writer has INSERT only by ADR-0013 design — RETURNING fails with code 42501 and the error message reads "permission denied for table events" (no mention of SELECT or RETURNING, which is what made it deeply non-obvious to diagnose).

Fix

AuditWriter.recordEvent now issues a parameterised raw INSERT via tx.$executeRawUnsafe instead of the ORM create():

await tx.$executeRawUnsafe(
  `INSERT INTO "audit"."events"
     (id, event_type, audience, outcome, subject, actor_id_hash, trace_id, payload)
   VALUES (gen_random_uuid(), $1, $2::"audit"."AuditAudience", $3::"audit"."AuditOutcome",
           $4, $5, $6, $7::jsonb)`,
  input.eventType, input.audience, input.outcome,
  input.subject ?? null, actorIdHash, traceId, payloadJson,
);

The role contract per ADR-0013 stays strict: audit_writer keeps INSERT only, no SELECT/UPDATE/DELETE/TRUNCATE. The other natural fix (GRANT SELECT to audit_writer) would have weakened the writer/reader role separation, so we deliberately went the other way.

Notable choices

gen_random_uuid() server-side instead of Prisma's @default(uuid()) client-side. The model still declares @default(uuid()) for any future ORM read or audit_reader-side query, but the write path uses the built-in Postgres function. No extension required (Postgres 13+).

Explicit enum and jsonb casts. Parameters travel as TEXT over the wire; the SQL casts ($2::"audit"."AuditAudience", $7::jsonb) ensure Postgres parses them as the right type. Without the casts, the type system rejects the INSERT before privilege check even fires.

Parameterised, not interpolated. $executeRawUnsafe accepts a SQL template with $1, $2, … placeholders and a vararg of values — same wire-level parameter binding as a prepared statement, so SQL injection isn't possible even on caller-controlled inputs like eventType. The spec pins this with a malicious-input test.

Also fixes an env-sensitivity bug in auth.controller.spec.ts. The test that asserts session.absoluteExpiresAt == createdAt + 43200000 was reading the default via readSessionTimeouts() but didn't override SESSION_ABSOLUTE_TIMEOUT_SECONDS. If apps/portal-bff/.env has a custom value (as it did during the manual audit debugging), the test failed non-deterministically. Now the test deletes the env var before running and restores it after — same pattern as the other env-touching tests in this file.

ADR amendment

ADR-0013 §"Writer" now carries an Implementation trap callout explaining why Prisma's ORM create() cannot be used for audit writes (RETURNING requires SELECT, audit_writer has INSERT only). The corresponding Confirmation entry cross-references the callout. Two-commit shape on this PR (code + docs) — the squash-merge will fold them.

Test plan

• pnpm nx test portal-bff (clean env) → 144/144 pass.
• pnpm nx lint portal-bff → clean.
• pnpm nx build portal-bff → clean.
• Prettier-clean.
• Manual smoke against running BFF + Postgres:
  • Sign in → row in audit.events with event_type = 'auth.sign_in'.
  • Sign out → row with event_type = 'auth.sign_out'. The 500 from before is gone.
  • Verify the role contract is still strict :

    SET ROLE audit_writer;
    SELECT * FROM audit.events LIMIT 1;  -- should fail "permission denied"
    UPDATE audit.events SET event_type = 'x';  -- should fail
    DELETE FROM audit.events;  -- should fail
    

## Summary #120 shipped the audit pipeline but the end-to-end path was never smoke-tested against a running Postgres. First click on `/auth/logout` returned 500 with the Pino log: ``` PostgresError code 42501 — permission denied for table events ``` Despite: - ACL on `audit.events` showing `audit_writer=a/audit_owner` (INSERT granted). - `has_table_privilege('audit_writer', 'audit.events', 'INSERT')` returning `t`. - `has_schema_privilege` / `has_type_privilege` all `t`. - A direct psql `INSERT INTO audit.events ...` after `SET LOCAL ROLE audit_writer` **succeeding**. - A psql `INSERT ... RETURNING id` after the same `SET LOCAL ROLE` **failing** with the exact same error. Root cause: Prisma's ORM `tx.auditEvent.create(...)` issues `INSERT ... RETURNING *` to hydrate the returned entity. Postgres requires **SELECT** on every column listed in `RETURNING`. `audit_writer` has INSERT only by ADR-0013 design — RETURNING fails with `code 42501` and the error message reads "permission denied for table events" (no mention of SELECT or RETURNING, which is what made it deeply non-obvious to diagnose). ## Fix `AuditWriter.recordEvent` now issues a parameterised raw INSERT via `tx.$executeRawUnsafe` instead of the ORM `create()`: ```ts await tx.$executeRawUnsafe( `INSERT INTO "audit"."events" (id, event_type, audience, outcome, subject, actor_id_hash, trace_id, payload) VALUES (gen_random_uuid(), $1, $2::"audit"."AuditAudience", $3::"audit"."AuditOutcome", $4, $5, $6, $7::jsonb)`, input.eventType, input.audience, input.outcome, input.subject ?? null, actorIdHash, traceId, payloadJson, ); ``` The role contract per ADR-0013 stays strict: `audit_writer` keeps INSERT only, no SELECT/UPDATE/DELETE/TRUNCATE. The other natural fix (`GRANT SELECT` to `audit_writer`) would have weakened the writer/reader role separation, so we deliberately went the other way. ## Notable choices **`gen_random_uuid()` server-side instead of Prisma's `@default(uuid())` client-side.** The model still declares `@default(uuid())` for any future ORM read or `audit_reader`-side query, but the write path uses the built-in Postgres function. No extension required (Postgres 13+). **Explicit enum and jsonb casts.** Parameters travel as TEXT over the wire; the SQL casts (`$2::"audit"."AuditAudience"`, `$7::jsonb`) ensure Postgres parses them as the right type. Without the casts, the type system rejects the INSERT before privilege check even fires. **Parameterised, not interpolated.** `$executeRawUnsafe` accepts a SQL template with `$1, $2, …` placeholders and a vararg of values — same wire-level parameter binding as a prepared statement, so SQL injection isn't possible even on caller-controlled inputs like `eventType`. The spec pins this with a malicious-input test. **Also fixes an env-sensitivity bug in `auth.controller.spec.ts`.** The test that asserts `session.absoluteExpiresAt == createdAt + 43200000` was reading the default via `readSessionTimeouts()` but didn't override `SESSION_ABSOLUTE_TIMEOUT_SECONDS`. If `apps/portal-bff/.env` has a custom value (as it did during the manual audit debugging), the test failed non-deterministically. Now the test deletes the env var before running and restores it after — same pattern as the other env-touching tests in this file. ## ADR amendment [ADR-0013](docs/decisions/0013-audit-trail-separated-postgres-append-only.md) §"Writer" now carries an **Implementation trap** callout explaining why Prisma's ORM `create()` cannot be used for audit writes (RETURNING requires SELECT, audit_writer has INSERT only). The corresponding Confirmation entry cross-references the callout. Two-commit shape on this PR (code + docs) — the squash-merge will fold them. ## Test plan - [x] `pnpm nx test portal-bff` (clean env) → **144/144 pass**. - [x] `pnpm nx lint portal-bff` → clean. - [x] `pnpm nx build portal-bff` → clean. - [x] Prettier-clean. - [x] Manual smoke against running BFF + Postgres: - [x] Sign in → row in `audit.events` with `event_type = 'auth.sign_in'`. - [x] Sign out → row with `event_type = 'auth.sign_out'`. **The 500 from before is gone.** - [x] Verify the role contract is still strict : ```sql SET ROLE audit_writer; SELECT * FROM audit.events LIMIT 1; -- should fail "permission denied" UPDATE audit.events SET event_type = 'x'; -- should fail DELETE FROM audit.events; -- should fail ```

julien added 2 commits 2026-05-13 19:46:05 +02:00

fix(portal-bff): audit writes use raw INSERT (audit_writer has no SELECT for RETURNING) ... f4f9224c68

manual smoke after #120 returned 500 on the first /auth/logout :

  PostgresError code 42501 — permission denied for table events

acl looked correct (audit_writer=a/audit_owner), has_*_privilege
returned t everywhere, and a manual psql `INSERT INTO audit.events`
succeeded after SET LOCAL ROLE audit_writer. but the same INSERT
WITH `RETURNING id` failed with the exact same error.

root cause: tx.auditEvent.create() in prisma emits `INSERT …
RETURNING *` to hydrate the entity it returns, and postgres requires
SELECT on every column in RETURNING. audit_writer has INSERT only
per adr-0013's append-only-by-role contract. RETURNING fails with
"permission denied for table X" — and the message says nothing
about SELECT or RETURNING, which made the bug take a long debug
session to isolate.

fix: AuditWriter.recordEvent now uses tx.$executeRawUnsafe with a
parameterised raw INSERT instead of the orm create(). the role
contract stays strict (audit_writer keeps INSERT only — no SELECT,
UPDATE, DELETE, TRUNCATE). the alternative — GRANT SELECT to
audit_writer — would have collapsed the writer / reader role
separation that adr-0013 hinges on, so we went the other way.

bonus: also fixes an env-sensitivity bug in auth.controller.spec.ts.
the test asserting `session.absoluteExpiresAt == createdAt +
43_200 * 1000` was reading the default via readSessionTimeouts() but
didn't override SESSION_ABSOLUTE_TIMEOUT_SECONDS. apps/portal-bff/
.env may carry a custom value (it did during the audit debugging),
making the test fail non-deterministically. now it deletes the env
var before, restores after — same pattern as the other env-touching
tests in this file.

144/144 specs pass under the clean-env repro:
  env -u REDIS_URL -u SESSION_SECRET -u SESSION_ENCRYPTION_KEY \
      -u SESSION_IDLE_TIMEOUT_SECONDS -u SESSION_ABSOLUTE_TIMEOUT_SECONDS \
      -u LOG_USER_ID_SALT -u DATABASE_URL -u ENTRA_* \
      pnpm exec nx run-many -t test lint build --projects=portal-bff

notable shape choices:
  - gen_random_uuid() server-side instead of prisma's @default(uuid())
    client-side. the model still declares @default for future orm reads
    by audit_reader; the write path now uses postgres's built-in
    (available since 13, target is 17).
  - explicit enum + jsonb casts ($2::"audit"."AuditAudience" etc.)
    because parameters travel as TEXT on the wire.
  - parameterised via $1, $2, …; never interpolated. a spec pins this
    with a sql-injection-shaped eventType input.

unchanged: schema, migration, role grants, ADR-0013 contract.

docs(adr-0013): document the RETURNING-requires-SELECT trap on audit writes ... 199c52247b

the fix in the previous commit (raw INSERT instead of
tx.auditEvent.create) is a non-obvious consequence of the
append-only-by-role contract — adr-0013's writer/reader role
separation forbids audit_writer from holding SELECT, which prisma's
default `INSERT … RETURNING *` requires. without this note in the
adr, a future contributor will likely reach for prisma's orm
create() again and hit the same misleading "permission denied for
table events" 500 we spent a session debugging.

added:
  - an "implementation trap" callout in §"Writer" of the decision
    outcome explaining why orm create() can't be used and what we
    do instead.
  - a cross-reference in the corresponding confirmation entry.
  - last-updated: 2026-05-13 in the frontmatter.

no decision changes — the role contract, schema, and writer
behavior are unchanged; this is purely a footgun-warning to
preserve hard-won context.

julien merged commit a97be121e6 into main 2026-05-13 19:48:34 +02:00

julien deleted branch fix/portal-bff/audit-insert-without-returning 2026-05-13 19:48:36 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

adastra-bot (AdAstra Bot) julien (Julien Gautier)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: julien/apf_portal#121