From 5c9befc0ae2aab3e63839d6ab39fb9bf33633aa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 6 Aug 2026 17:52:10 +0200 Subject: [PATCH] feat(02-12.1): stamp a record-context epoch on every session - add sessions.record_epoch, owned by a BEFORE UPDATE trigger so the ON DELETE SET NULL path and raw SQL writes cannot skip it - normalise pre-existing rows so epoch zero means never switched - project the epoch off the row getSession already loads --- .../0302_session_record_epoch/migration.sql | 64 +++++ prisma/schema.prisma | 18 ++ .../insights/targets/__tests__/route.test.ts | 1 + .../api/mcp/oauth/__tests__/authorize.test.ts | 1 + .../__tests__/acting-account-resolver.test.ts | 12 +- src/lib/auth/session.ts | 16 +- .../record-session-epoch-trigger.test.ts | 266 ++++++++++++++++++ 7 files changed, 376 insertions(+), 2 deletions(-) create mode 100644 prisma/migrations/0302_session_record_epoch/migration.sql create mode 100644 tests/integration/record-session-epoch-trigger.test.ts diff --git a/prisma/migrations/0302_session_record_epoch/migration.sql b/prisma/migrations/0302_session_record_epoch/migration.sql new file mode 100644 index 000000000..322c55fea --- /dev/null +++ b/prisma/migrations/0302_session_record_epoch/migration.sql @@ -0,0 +1,64 @@ +-- v1.37.0 — a monotonic counter for "how many times has this session's record +-- selector moved". +-- +-- The problem it solves is not authorization; `acting_as_user_id` plus the +-- grant join already decides that. It is agreement. A browser holds several +-- tabs on one session, and a request that left a suspended tab before a switch +-- landed arrives after it — carrying an intent formed under the old record and +-- addressed to a server that has since moved. The request is perfectly +-- authorized and answers the wrong question. So every request that resolves a +-- record now asserts which context it believes it is in, and the server refuses +-- when that belief no longer matches the row. +-- +-- The counter has to be the DATABASE's, not the application's, because the +-- application is not the only writer of the column it tracks. Three of the +-- five writers run no application code at all: +-- +-- * `sessions_acting_as_user_id_fkey ON DELETE SET NULL` — deleting an owner +-- account nulls every delegate session pointing at it, by referential +-- action, with nothing to hook; +-- * raw `UPDATE sessions SET acting_as_user_id = NULL` in the end-to-end +-- fixtures; +-- * an operator at `psql`, and whatever writes this column next year. +-- +-- An application-level increment would be a discipline those three break +-- silently, and a fence whose counter can silently fail to move is a fence that +-- passes stale requests. A row trigger cannot be forgotten. +-- +-- BEFORE, not AFTER, and that timing is load-bearing: the trigger overwrites +-- whatever `record_epoch` the incoming statement supplied, so no writer can set +-- the epoch itself even by trying. The counter belongs to the database in the +-- strong sense. +-- +-- `UPDATE OF "acting_as_user_id"` plus the `WHEN` clause keep the two writes +-- that touch a session on nearly every request off the trigger entirely — the +-- throttled `last_active_at` stamp and the sliding `expires_at` extension both +-- name other columns, and neither would satisfy the `IS DISTINCT FROM` test +-- even if it did. +-- +-- Precedent for a trigger on this line: `0298_notification_delivery_identity` +-- ships `push_attempt_attribution_guard` the same way. A trigger is invisible +-- to Prisma's datamodel comparison, which 0298 already proved tolerable. + +ALTER TABLE "sessions" ADD COLUMN "record_epoch" INTEGER NOT NULL DEFAULT 0; + +-- A session already sitting inside somebody else's record is not a session +-- that has never left its own, and the fence's exemption for pre-existing +-- browser tabs is defined as epoch zero. Normalise before the trigger exists, +-- so that from the first request after deploy `record_epoch = 0` means exactly +-- "never pointed at another record, and not pointed at one now". +UPDATE "sessions" SET "record_epoch" = 1 WHERE "acting_as_user_id" IS NOT NULL; + +CREATE OR REPLACE FUNCTION "sessions_bump_record_epoch"() RETURNS trigger +LANGUAGE plpgsql AS $$ +BEGIN + NEW."record_epoch" := OLD."record_epoch" + 1; + RETURN NEW; +END; +$$; + +CREATE TRIGGER "sessions_record_epoch_bump" + BEFORE UPDATE OF "acting_as_user_id" ON "sessions" + FOR EACH ROW + WHEN (OLD."acting_as_user_id" IS DISTINCT FROM NEW."acting_as_user_id") + EXECUTE FUNCTION "sessions_bump_record_epoch"(); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 30290011e..9701fa010 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -1179,6 +1179,24 @@ model Session { /// into its own account. actingAsUserId String? @map("acting_as_user_id") + /// v1.37.0 — how many times this session's record selector has moved. + /// + /// Monotonic, starts at 0, and NEVER written by application code: migration + /// 0302 installs a `BEFORE UPDATE OF acting_as_user_id` trigger that sets it, + /// so the increment also happens on the paths where no application code runs + /// at all — the `ON DELETE SET NULL` referential action above, a raw fixture + /// `UPDATE`, an operator at `psql`. A convention that every writer must + /// remember is a convention those three break silently. + /// + /// Consumer: the record-session fence + /// (`src/lib/sharing/record-session-fence.ts`), which refuses a cookie + /// request whose asserted epoch no longer matches the row. `0` additionally + /// means "this session has never been pointed at another record", which is + /// the fence's exemption for browser tabs that predate the feature; the + /// migration normalises existing rows so that invariant holds from the first + /// request after deploy. + recordEpoch Int @default(0) @map("record_epoch") + user User @relation("SessionOwner", fields: [userId], references: [id], onDelete: Cascade) actingAs User? @relation("SessionActingAs", fields: [actingAsUserId], references: [id], onDelete: SetNull) diff --git a/src/app/api/insights/targets/__tests__/route.test.ts b/src/app/api/insights/targets/__tests__/route.test.ts index 44c48d332..19b0ca3e4 100644 --- a/src/app/api/insights/targets/__tests__/route.test.ts +++ b/src/app/api/insights/targets/__tests__/route.test.ts @@ -100,6 +100,7 @@ const SESSION_OK = { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000), actingAsUserId: null, + recordEpoch: 0, }, user: SESSION_USER as never, }; diff --git a/src/app/api/mcp/oauth/__tests__/authorize.test.ts b/src/app/api/mcp/oauth/__tests__/authorize.test.ts index 5040f996e..9bcc403b3 100644 --- a/src/app/api/mcp/oauth/__tests__/authorize.test.ts +++ b/src/app/api/mcp/oauth/__tests__/authorize.test.ts @@ -81,6 +81,7 @@ function signedIn() { id: "s1", expiresAt: new Date(Date.now() + 1e6), actingAsUserId: null, + recordEpoch: 0, }, user: { id: "user-1" } as never, }); diff --git a/src/lib/__tests__/acting-account-resolver.test.ts b/src/lib/__tests__/acting-account-resolver.test.ts index fac096103..2e6a4e045 100644 --- a/src/lib/__tests__/acting-account-resolver.test.ts +++ b/src/lib/__tests__/acting-account-resolver.test.ts @@ -148,16 +148,25 @@ function user( }; } -/** Sign the delegate in over the cookie transport. */ +/** + * Sign the delegate in over the cookie transport. + * + * `recordEpoch` defaults to 0, which is the record-session fence's exemption: + * a session that has never been pointed at another record is served without a + * fence header, so every pre-fence case in this file keeps meaning what it + * meant. Cases that want the fence live pass an epoch explicitly. + */ function signedInCookie( actingAsUserId: string | null = null, role: "USER" | "ADMIN" = "USER", + recordEpoch = 0, ): void { vi.mocked(getSession).mockResolvedValue({ session: { id: "session-1", expiresAt: new Date(Date.now() + 3_600_000), actingAsUserId, + recordEpoch, }, user: user(DELEGATE, role) as never, }); @@ -649,6 +658,7 @@ describe("the cookie-only helpers stay out of reach of a switch", () => { id: "session-1", expiresAt: new Date(Date.now() + 3_600_000), actingAsUserId: OWNER, + recordEpoch: 0, }, user: { ...user(DELEGATE), diff --git a/src/lib/auth/session.ts b/src/lib/auth/session.ts index 74ef90227..d55186487 100644 --- a/src/lib/auth/session.ts +++ b/src/lib/auth/session.ts @@ -246,7 +246,12 @@ export async function createSession( } export async function getSession(): Promise<{ - session: { id: string; expiresAt: Date; actingAsUserId: string | null }; + session: { + id: string; + expiresAt: Date; + actingAsUserId: string | null; + recordEpoch: number; + }; user: User; } | null> { try { @@ -363,6 +368,15 @@ export async function getSession(): Promise<{ // `src/__tests__/acting-account-boundary-guard.test.ts` freezes the set // of files allowed to touch this field. actingAsUserId: session.actingAsUserId, + // v1.37.0 — the selector's move counter, projected off the same row for + // the same reason. It is what makes the fence's central property true: + // the epoch a request is validated against and the `actingAsUserId` the + // handler is about to serve under are one row read, so they cannot + // straddle a switch that landed between two queries. + // + // Written by a database trigger only (migration 0302); nothing in the + // application assigns it, and the `BEFORE` timing means nothing can. + recordEpoch: session.recordEpoch, }, user: session.user, }; diff --git a/tests/integration/record-session-epoch-trigger.test.ts b/tests/integration/record-session-epoch-trigger.test.ts new file mode 100644 index 000000000..a3ec9cb09 --- /dev/null +++ b/tests/integration/record-session-epoch-trigger.test.ts @@ -0,0 +1,266 @@ +/** + * FENCE-AC-09 — the record-selector epoch, against real Postgres. + * + * `Session.recordEpoch` is the one fact the whole fence rests on, and it is + * owned by a database trigger rather than by application code. That choice only + * pays off if it is true for the writers that run no application code, so this + * file exercises those writers directly rather than through a helper: + * + * * the `ON DELETE SET NULL` referential action on `acting_as_user_id`, + * which fires when an owner account is deleted and executes nothing of ours; + * * a raw `UPDATE sessions SET acting_as_user_id = NULL`, the shape the + * end-to-end fixtures use; + * * a statement that tries to set `record_epoch` itself, which must lose. + * + * And the negative half, which matters just as much: the two writes that touch + * a session on nearly every request — the throttled `last_active_at` stamp and + * the sliding `expires_at` extension — must not move the counter. A counter + * that drifts on ordinary traffic would refuse every request in the app. + * + * Break this file by deleting the `CREATE TRIGGER` statement from migration + * 0302: the referential-action leg and the `clearActingSessions` leg both fail, + * because nothing else increments. + */ +import { beforeEach, describe, expect, it } from "vitest"; + +import { getPrismaClient, truncateAllTables } from "./setup"; +import { clearActingSessions } from "@/lib/sharing/acting-session"; + +let counter = 0; + +async function makeUser(label: string) { + const suffix = `${label}-${counter++}`; + return getPrismaClient().user.create({ + data: { + username: `epoch-${suffix}`, + email: `epoch-${suffix}@example.test`, + role: "USER", + }, + }); +} + +async function makeSession(userId: string, actingAsUserId: string | null) { + return getPrismaClient().session.create({ + data: { + userId, + actingAsUserId, + expiresAt: new Date(Date.now() + 60_000), + }, + }); +} + +async function epochOf(sessionId: string): Promise { + const row = await getPrismaClient().session.findUniqueOrThrow({ + where: { id: sessionId }, + }); + return row.recordEpoch; +} + +describe("FENCE-AC-09 record-selector epoch trigger", () => { + beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + }); + + it("FENCE-AC-09 starts every fresh session at zero", async () => { + const user = await makeUser("fresh"); + const session = await makeSession(user.id, null); + expect(session.recordEpoch).toBe(0); + }); + + it("FENCE-AC-09 bumps by exactly one when the selector is pointed at a record", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const session = await makeSession(delegate.id, null); + + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { actingAsUserId: owner.id }, + }); + + expect(await epochOf(session.id)).toBe(1); + }); + + it("FENCE-AC-09 bumps again on the way back out", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const session = await makeSession(delegate.id, owner.id); + // Seeded already pointed at the owner: the INSERT does not fire an UPDATE + // trigger, so this row starts at 0 and the switch-out is its first bump. + expect(await epochOf(session.id)).toBe(0); + + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { actingAsUserId: null }, + }); + + expect(await epochOf(session.id)).toBe(1); + }); + + it("FENCE-AC-09 does not bump when the selector is rewritten to the same value", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const session = await makeSession(delegate.id, null); + + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { actingAsUserId: owner.id }, + }); + expect(await epochOf(session.id)).toBe(1); + + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { actingAsUserId: owner.id }, + }); + expect(await epochOf(session.id)).toBe(1); + + // And the NULL -> NULL case, which `IS DISTINCT FROM` has to get right + // where a bare `<>` would not. + const untouched = await makeSession(delegate.id, null); + await getPrismaClient().session.update({ + where: { id: untouched.id }, + data: { actingAsUserId: null }, + }); + expect(await epochOf(untouched.id)).toBe(0); + }); + + it("FENCE-AC-09 bumps on the ON DELETE SET NULL referential action, where no application code runs", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const session = await makeSession(delegate.id, null); + + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { actingAsUserId: owner.id }, + }); + const beforeDelete = await epochOf(session.id); + expect(beforeDelete).toBe(1); + + // Deleting the OWNER nulls the delegate's selector by referential action. + // Nothing of ours executes; the trigger is the only thing that can move + // the counter here. + await getPrismaClient().user.delete({ where: { id: owner.id } }); + + const row = await getPrismaClient().session.findUniqueOrThrow({ + where: { id: session.id }, + }); + expect(row.actingAsUserId).toBeNull(); + expect(row.recordEpoch).toBe(beforeDelete + 1); + }); + + it("FENCE-AC-09 bumps every row clearActingSessions clears", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const first = await makeSession(delegate.id, null); + const second = await makeSession(delegate.id, null); + const unrelated = await makeSession(delegate.id, null); + + for (const s of [first, second]) { + await getPrismaClient().session.update({ + where: { id: s.id }, + data: { actingAsUserId: owner.id }, + }); + } + + const cleared = await clearActingSessions({ + grantorId: owner.id, + granteeId: delegate.id, + }); + + expect(cleared).toBe(2); + expect(await epochOf(first.id)).toBe(2); + expect(await epochOf(second.id)).toBe(2); + // The session that was never inside the record is untouched, which is what + // keeps the fence's exemption meaningful. + expect(await epochOf(unrelated.id)).toBe(0); + }); + + it("FENCE-AC-09 bumps on a raw SQL clear, the shape the browser fixtures use", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const session = await makeSession(delegate.id, null); + + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { actingAsUserId: owner.id }, + }); + + await getPrismaClient() + .$executeRaw`UPDATE "sessions" SET "acting_as_user_id" = NULL WHERE "user_id" = ${delegate.id}`; + + expect(await epochOf(session.id)).toBe(2); + }); + + it("FENCE-AC-09 leaves the per-request session writes alone", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const session = await makeSession(delegate.id, null); + + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { actingAsUserId: owner.id }, + }); + expect(await epochOf(session.id)).toBe(1); + + // The throttled "last seen" stamp (`session.ts`). + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { lastActiveAt: new Date() }, + }); + // The sliding-expiry extension (`session.ts`). + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { expiresAt: new Date(Date.now() + 3_600_000) }, + }); + // And both at once, which is the shape a future consolidation would take. + await getPrismaClient().session.update({ + where: { id: session.id }, + data: { lastActiveAt: new Date(), expiresAt: new Date(Date.now() + 1) }, + }); + + expect(await epochOf(session.id)).toBe(1); + }); + + it("FENCE-AC-09 refuses an application-supplied epoch: the trigger's value wins", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const session = await makeSession(delegate.id, null); + + // A statement that sets both the selector and an arbitrary epoch. BEFORE + // timing means the trigger overwrites `NEW.record_epoch` wholesale, so a + // writer cannot steer the counter even deliberately. + await getPrismaClient() + .$executeRaw`UPDATE "sessions" SET "acting_as_user_id" = ${owner.id}, "record_epoch" = 999 WHERE "id" = ${session.id}`; + + expect(await epochOf(session.id)).toBe(1); + + // Nor can a writer freeze it by rewriting the current value. + await getPrismaClient() + .$executeRaw`UPDATE "sessions" SET "acting_as_user_id" = NULL, "record_epoch" = 1 WHERE "id" = ${session.id}`; + + expect(await epochOf(session.id)).toBe(2); + }); + + it("FENCE-AC-09 holds the migration's normalisation invariant: epoch zero iff the selector is null", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + await makeSession(delegate.id, null); + const switched = await makeSession(delegate.id, null); + await getPrismaClient().session.update({ + where: { id: switched.id }, + data: { actingAsUserId: owner.id }, + }); + + // The migration's `UPDATE ... SET record_epoch = 1 WHERE acting_as_user_id + // IS NOT NULL` establishes this over pre-existing rows; the trigger + // maintains one direction of it afterwards. What the fence actually relies + // on is the weaker, always-true half asserted here: a row still pointed at + // another record is never at epoch zero. + const pointed = await getPrismaClient().session.findMany({ + where: { actingAsUserId: { not: null } }, + }); + expect(pointed.length).toBeGreaterThan(0); + for (const row of pointed) { + expect(row.recordEpoch).toBeGreaterThanOrEqual(1); + } + }); +});