feat(sharing): record who acted when it was not the account's owner

Adds AuditLog.actorUserId (migration 0294) and threads it centrally, out
of the wide-event context the acting-account resolver already stamps. No
call site passes an actor, so no route can forget one.

NULL keeps meaning "acted for themselves" — the reading every existing
row carries. The threading fills the column only when a request resolved
an acting account AND the row is being filed under that account, so a
self-action, a refusal, and a delegate's row about themselves all stay
NULL rather than acquiring an id that would rewrite what the history says.

Delegated reads coalesce to one row per owner, delegate and day, through
a partial unique index and an insert that expects to lose the race. The
alternative is a row per request on the busiest table in the schema and
an activity view made of identical lines.

The column carries no foreign key, against the grain of user_id beside
it: SET NULL there erases an account from its own history, while here it
would turn "the delegate opened this record" into "the owner opened this
record". An id that no longer resolves reads as a deleted account; an id
that silently became NULL reads as a lie.
This commit is contained in:
Marc-André Bombeck committed 2026-08-02 13:04:25 +02:00
1 parent 42f9e754e7
commit 843c74c98c
8 files changed
+570 -4

No files matched your search

@@ -0,0 +1,66 @@
-- v1.36.0 — who acted, when it was not the person the row is filed under.
--
-- The audit trail has answered one question until now: what happened to this
-- account. Delegation splits it in two — what happened to this account, and
-- who was at the keyboard — and a single `user_id` cannot hold both answers.
--
-- NULL is the whole design of this column. Every row written before today
-- means "this account acted for itself", and that reading has to survive: a
-- backfill, a default, or a threading rule that helpfully stamps the caller's
-- own id would rewrite the meaning of every historical row at once, silently,
-- with nothing to compare against afterwards. So the column arrives nullable,
-- unbackfilled, and the code that fills it (`src/lib/auth/audit.ts`) fills it
-- only when the request actually resolved an acting account.
--
-- A delegated action files under the OWNER, with the delegate here. The owner's
-- record was the thing touched, and "what happened to my data" is the owner's
-- question to ask.
--
-- No foreign key, deliberately, and against the grain of `user_id` right above
-- it. `user_id` is ON DELETE SET NULL because an erased account should drop out
-- of its own history. Applying that rule here would not erase, it would falsify:
-- nulling the actor turns "the delegate opened this record" into "the owner
-- opened this record", which is a sentence the trail would then assert about a
-- day it did not happen. An id that no longer resolves to a user reads as a
-- deleted account and is honest about it. The 365-day retention purge
-- (`src/lib/jobs/audit-log-cleanup.ts`) bounds how long the id lives either way.
ALTER TABLE "audit_logs" ADD COLUMN "actor_user_id" TEXT;
-- The owner's "who touched my record" view: their rows, delegated ones only,
-- newest first. Every existing index on this table leads with `user_id` and
-- then `action` or `created_at`, so without this one that read scans an
-- account's entire history — on the table the schema itself calls the
-- highest-churn one — to surface the few rows that carry an actor.
CREATE INDEX "audit_logs_user_id_actor_user_id_created_at_idx"
ON "audit_logs" ("user_id", "actor_user_id", "created_at" DESC);
-- Delegated reads coalesce to one row per (owner, delegate, day).
--
-- Without this, a delegate paging through a record writes an audit row per
-- request, on the table that is already the busiest one here, and the owner's
-- activity view becomes a wall of identical lines nobody can read. One row a
-- day answers the question the owner is actually asking — "was she in my record
-- yesterday" — and the counter inside it answers "how much".
--
-- The uniqueness is what makes the coalescing atomic rather than a read
-- followed by a hopeful write: the writer INSERTs and lets the index send it
-- into the ON CONFLICT arm, so two concurrent delegated reads cannot both
-- decide the day's row is missing.
--
-- The day is a bare cast because `created_at` on this table is TIMESTAMP
-- WITHOUT TIME ZONE holding UTC — the whole application writes it that way, so
-- `::date` is already the UTC day and needs no conversion. It also has to be
-- bare: an index expression must be IMMUTABLE, and any form that converts a
-- zone at read time (`AT TIME ZONE`, casting through timestamptz) is only
-- STABLE, because its answer depends on a session setting an operator can
-- change. Postgres refuses to index it, which is the right refusal: a day
-- boundary that moves with a container's TZ would split or merge days the
-- trail has already recorded.
--
-- Partial on the action so it constrains exactly the rows the writer emits and
-- nothing else: `auth.login` rows carry no actor, and two of them on one day
-- for one account are entirely ordinary.
CREATE UNIQUE INDEX "audit_logs_delegated_access_day_key"
ON "audit_logs" ("user_id", "actor_user_id", (("created_at")::date))
WHERE "action" = 'sharing.record.accessed';
+31 -2
View File
@@ -4483,8 +4483,31 @@ model AppSettings {
// ─── Audit Log ─────────────────────────────────────────────
model AuditLog {
id String @id @default(cuid())
userId String? @map("user_id")
id String @id @default(cuid())
userId String? @map("user_id")
/// v1.36.0 — who acted, when that is not the person the row is filed under.
///
/// NULL means the account acted for itself, which is what every row written
/// before this column existed means and what almost every row written after
/// it will mean. The threading in `src/lib/auth/audit.ts` fills it only when
/// the request resolved an acting account, so a self-action can never grow
/// an actor id by accident — if it could, the reading of every historical
/// row would change under us.
///
/// A delegated action files under the OWNER (`userId`) with the delegate
/// here: it is the owner's record that was touched, and the owner's view of
/// "what happened to my data" has to contain it.
///
/// Deliberately NOT a foreign key. `userId` above is ON DELETE SET NULL, and
/// the same rule here would rewrite history rather than erase it — nulling
/// the actor turns "your daughter opened your record" into "you opened your
/// record" the moment her account is deleted. An id that no longer resolves
/// to a person renders as a deleted account; an id that silently became NULL
/// renders as a lie. The 365-day retention purge bounds how long either
/// lives.
actorUserId String? @map("actor_user_id")
action String // e.g. "auth.login", "measurement.create", "export.csv"
details String? // JSON string, no sensitive data
ipAddress String? @map("ip_address")
@@ -4512,6 +4535,12 @@ model AuditLog {
// scan and could not finish inside statement_timeout on an instance with
// enough history. See migration 0276.
@@index([createdAt])
// v1.36.0 — the owner's "who touched my record" view reads
// `{ userId, actorUserId: { not: null } }` newest first. Every index above
// leads with `userId` and then `action` or `createdAt`, so that read would
// have to filter the whole of an account's history to find the handful of
// delegated rows in it. See migration 0294.
@@index([userId, actorUserId, createdAt(sort: Desc)])
@@map("audit_logs")
}
@@ -86,6 +86,10 @@ vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
vi.mock("@/lib/auth/hmac", () => ({ hashToken: vi.fn(() => "hash") }));
vi.mock("@/lib/auth/audit", () => ({
auditLog: vi.fn().mockResolvedValue(undefined),
// v1.36.0 — the resolver also writes the owner's day-coalesced access row.
// Stubbed here because what it writes is a question about rows, answered in
// `tests/integration/sharing-audit-actor.test.ts` against a real database.
recordDelegatedAccess: vi.fn().mockResolvedValue(undefined),
}));
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
+7 -1
View File
@@ -7,7 +7,7 @@ import { annotate, eventStorage, getEvent } from "./logging/context";
import { emitIfSampled } from "./logging/transports";
import { redactOptional, redactSecrets } from "./logging/redact";
import { getSession } from "./auth/session";
import { auditLog } from "./auth/audit";
import { auditLog, recordDelegatedAccess } from "./auth/audit";
import {
resolveBearerToken,
BearerAuthError,
@@ -879,6 +879,12 @@ export async function requireRecordAuth(
// Fire-and-forget, the `ApiToken.lastUsedAt` posture: the read must not wait
// on the bookkeeping, and the bookkeeping failing must not fail the read.
void touchGrantUsage(grant.id);
// The owner's copy of the same fact, and the one they can read: a day-
// coalesced row on their own record saying who opened it. It lands here
// rather than in the handlers because a route cannot forget what it never
// had to remember — every delegated request in the product passes through
// this line, and only requests that got past the grant check reach it.
void recordDelegatedAccess(owner.id, auth.user.id);
return {
session: auth.session,
+4
View File
@@ -66,6 +66,9 @@ describe("auditLog", () => {
userId: "user-7",
details: JSON.stringify({ reason: "passkey", browser: "firefox" }),
ipAddress: "203.0.113.5",
// v1.36.0 — no acting account on this request, so the row says the
// account acted for itself. NULL is the statement, not a gap.
actorUserId: null,
});
// Location is intentionally not set on insert — it's filled later by the
// geo lookup. The schema column itself is not in the create payload.
@@ -81,6 +84,7 @@ describe("auditLog", () => {
userId: null,
details: null,
ipAddress: null,
actorUserId: null,
});
// No IP → no geo lookup enqueued.
expect(lookupIpGeo).not.toHaveBeenCalled();
+106 -1
View File
@@ -1,5 +1,49 @@
import { prisma } from "@/lib/db";
import { lookupIpGeo } from "@/lib/geo";
import { getEvent } from "@/lib/logging/context";
/**
* v1.36.0 — the action a delegated request records against the record it read.
*
* One row per (owner, delegate, day), enforced by the partial unique index in
* migration 0294. The name is part of the contract: the index is partial on
* exactly this string, so changing it here without changing it there turns the
* coalescing off and nothing fails.
*/
export const DELEGATED_ACCESS_ACTION = "sharing.record.accessed";
/**
* Who is acting, when the row is not theirs.
*
* Read from the wide-event context rather than from an argument, because an
* argument is something ~600 audit call sites can forget and this one cannot
* be forgotten safely: a delegated action recorded with no actor is
* indistinguishable from the owner having done it themselves.
*
* Returns null — "acted for themselves" — unless all three hold:
*
* * the request resolved an acting account (`acting_as` is stamped, which
* only `requireRecordAuth` does and only after a live grant);
* * the actor is not that account (a switch into one's own record is not a
* delegation and has nothing to attribute);
* * the row is being filed under the account being ACTED ON. A delegable
* route that deliberately writes a row about the delegate — under the
* delegate's own id — is describing the delegate's own act, and stamping
* the actor there would say "somebody else did this to you" about a person
* acting on themselves.
*
* Every other path — every self-action, every refusal, every background job —
* falls through to null, which is what every row written before this column
* existed already means.
*/
function actingActorFor(filedUnder: string | null): string | null {
const auth = getEvent()?.getAuth();
const owner = auth?.acting_as ?? null;
const actor = auth?.user_id ?? null;
if (owner === null || actor === null) return null;
if (owner === actor) return null;
return filedUnder === owner ? actor : null;
}
export async function auditLog(
action: string,
@@ -9,10 +53,12 @@ export async function auditLog(
ipAddress?: string | null;
} = {},
) {
const userId = opts.userId ?? null;
const entry = await prisma.auditLog.create({
data: {
action,
userId: opts.userId ?? null,
userId,
actorUserId: actingActorFor(userId),
details: opts.details ? JSON.stringify(opts.details) : null,
ipAddress: opts.ipAddress ?? null,
},
@@ -53,3 +99,62 @@ export async function auditLog(
});
}
}
/**
* v1.36.0 — record that a delegate opened an owner's record today.
*
* One row per (owner, delegate, day), with a counter inside it. The alternative
* — a row per delegated request — would put a new hot stripe on the table this
* schema already calls the highest-churn one, and would hand the owner an
* activity view made of thousands of identical lines, which is a way of
* answering nothing. "She was in my record on Tuesday, 40 times" is the answer
* somebody actually wants.
*
* The coalescing is an INSERT that expects to lose. Two delegated reads racing
* inside the same second both try to create the day's row and the unique index
* sends the loser into the ON CONFLICT arm; a read-then-write would let both
* decide the row was missing and leave a duplicate that the index would then
* refuse anyway. `created_at` is written as an explicit UTC instant because
* the column is TIMESTAMP WITHOUT TIME ZONE and a bare `NOW()` would be
* converted through whatever timezone the connection happens to carry — the
* day key in the index is a plain cast of this column, so a row landing an
* hour off would land on the wrong day of the owner's trail.
*
* The action string appears twice in the statement below — once as the row's
* value, once as a literal in the conflict predicate, because Postgres has to
* prove the predicate matches the partial index and cannot do that through a
* bind parameter. They must agree, and the integration test that asserts a
* second read produces no second row is what fails when they stop agreeing.
*
* Fire-and-forget by the `ApiToken.lastUsedAt` posture: a delegated read must
* not wait on its own bookkeeping, and bookkeeping that fails must not turn a
* successful read into a 500. The promise is returned so a test can await it.
*/
export async function recordDelegatedAccess(
ownerId: string,
actorId: string,
): Promise<void> {
try {
await prisma.$executeRaw`
INSERT INTO audit_logs (id, user_id, actor_user_id, action, details, created_at)
VALUES (
gen_random_uuid()::text,
${ownerId},
${actorId},
${DELEGATED_ACCESS_ACTION},
'{"accesses":1}',
(NOW() AT TIME ZONE 'UTC')
)
ON CONFLICT ("user_id", "actor_user_id", (("created_at")::date))
WHERE "action" = 'sharing.record.accessed'
DO UPDATE SET details = jsonb_set(
COALESCE(audit_logs.details::jsonb, '{}'::jsonb),
'{accesses}',
to_jsonb(COALESCE((audit_logs.details::jsonb ->> 'accesses')::int, 0) + 1)
)::text
`;
} catch {
// Bookkeeping. The read it describes has already succeeded, and a failure
// to describe it must not undo that.
}
}
+15
View File
@@ -223,6 +223,21 @@ export class WideEventBuilder {
return this.event.http?.method;
}
/**
* Who authenticated, and whose record they are acting on.
*
* v1.36.0 — read by `auditLog()`, which needs both halves and must not be
* handed either one by a call site. The event is already the one place that
* knows them: `setAuth` stamps the actor at authentication, `setActingAs`
* stamps the owner when the resolver substitutes one, and both run before any
* handler body can write an audit row. A copy of this fact threaded through
* function arguments would be a second answer to "who is doing this", and the
* routes that forgot to pass it would be the ones nobody reviewed.
*/
getAuth(): WideEvent["auth"] {
return this.event.auth;
}
getRequestId(): string {
return this.event.request_id!;
}
@@ -0,0 +1,337 @@
/**
* What the audit trail records for a delegated action, and for a self action.
*
* The whole feature rests on one reading: `actorUserId` NULL means the account
* acted for itself. Every row written before the column existed means that,
* and every row written after it will mostly mean it too — so a threading rule
* that stamps an actor where none was acting does not add information, it
* changes what several years of history says. That reading is what these tests
* hold, against real rows.
*
* Round trips through the real routes, for the reason that cost this project a
* release: a unit stub proves the code ran, not what landed. Here the question
* is only ever "which row exists afterwards, and what is in its columns".
*/
import { beforeEach, describe, expect, it, vi } from "vitest";
import { NextRequest, NextResponse } from "next/server";
import { cookieJar, headerJar } from "./mock-next-headers";
import { getPrismaClient, truncateAllTables } from "./setup";
vi.mock("next/headers", async () => {
const { cookieJar, headerJar } = await import("./mock-next-headers");
return {
headers: vi.fn(async () => ({
get: (name: string) => headerJar.get(name.toLowerCase()) ?? null,
})),
cookies: vi.fn(async () => ({
get: (name: string) => {
const value = cookieJar.get(name);
return value ? { name, value } : undefined;
},
set: (name: string, value: string) => {
cookieJar.set(name, value);
},
delete: (name: string) => {
cookieJar.delete(name);
},
})),
};
});
vi.mock("@/lib/db-compat", () => ({
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
}));
import { apiHandler, requireRecordAuth } from "@/lib/api-handler";
import { auditLog, DELEGATED_ACCESS_ACTION } from "@/lib/auth/audit";
import { acceptGrant, inviteGrant } from "@/lib/sharing/grants";
let counter = 0;
async function makeUser(label: string) {
const suffix = `${label}-${counter++}`;
return getPrismaClient().user.create({
data: {
username: `audit-${suffix}`,
email: `audit-${suffix}@example.test`,
role: "USER",
},
});
}
async function signIn(userId: string) {
const session = await getPrismaClient().session.create({
data: { userId, expiresAt: new Date(Date.now() + 60_000) },
});
cookieJar.set("healthlog_session", session.id);
return session;
}
/**
* A delegable route that writes an audit row the way a migrated route does:
* `userId` from the resolved data scope, and nothing about the actor, because
* the actor is not the handler's business.
*/
const delegableAction: (request: NextRequest) => Promise<Response> = apiHandler(
async () => {
const { user } = await requireRecordAuth("read");
await auditLog("measurement.read", {
userId: user.id,
details: { via: "test" },
});
return NextResponse.json({ data: { ok: true }, error: null });
},
);
/** The same handler's self-action case: no switch, same code path. */
function act(): Promise<Response> {
return delegableAction(
new NextRequest("http://localhost/api/test/delegable", { method: "GET" }),
);
}
async function household() {
const owner = await makeUser("owner");
const delegate = await makeUser("delegate");
const invited = await inviteGrant({
grantorId: owner.id,
granteeId: delegate.id,
});
await acceptGrant({ grantId: invited.id, granteeId: delegate.id });
const session = await signIn(delegate.id);
await getPrismaClient().session.update({
where: { id: session.id },
data: { actingAsUserId: owner.id },
});
return { owner, delegate, session };
}
async function rowsFor(action: string) {
return getPrismaClient().auditLog.findMany({
where: { action },
orderBy: { createdAt: "asc" },
});
}
/** The resolver's access row is fire-and-forget; give it a tick to land. */
async function settle() {
await new Promise((resolve) => setTimeout(resolve, 80));
}
beforeEach(async () => {
await truncateAllTables(getPrismaClient());
cookieJar.clear();
headerJar.clear();
});
describe("who the trail says did it", () => {
it("files a delegated action under the owner, naming the delegate", async () => {
const { owner, delegate } = await household();
expect((await act()).status).toBe(200);
const rows = await rowsFor("measurement.read");
expect(rows).toHaveLength(1);
expect(rows[0].userId).toBe(owner.id);
expect(rows[0].actorUserId).toBe(delegate.id);
});
it("leaves the actor NULL when somebody acts on their own record", async () => {
// The load-bearing one. NULL is not "we did not know" — it is the
// statement every historical row already makes, and the same handler,
// unswitched, has to keep making it.
const solo = await makeUser("solo");
await signIn(solo.id);
expect((await act()).status).toBe(200);
const rows = await rowsFor("measurement.read");
expect(rows).toHaveLength(1);
expect(rows[0].userId).toBe(solo.id);
expect(rows[0].actorUserId).toBeNull();
});
it("leaves the actor NULL on a row a delegate writes about themselves", async () => {
// A delegated request that files under the DELEGATE describes the
// delegate's own act. Stamping the actor there would say "somebody else
// did this to you" about a person acting on themselves.
const { delegate } = await household();
const handler: (request: NextRequest) => Promise<Response> = apiHandler(
async () => {
const { actor } = await requireRecordAuth("read");
await auditLog("delegate.own.act", { userId: actor.id });
return NextResponse.json({ data: { ok: true }, error: null });
},
);
await handler(
new NextRequest("http://localhost/api/test/delegable", { method: "GET" }),
);
const rows = await rowsFor("delegate.own.act");
expect(rows).toHaveLength(1);
expect(rows[0].userId).toBe(delegate.id);
expect(rows[0].actorUserId).toBeNull();
});
it("leaves the actor NULL on a refused delegation", async () => {
// The refusal row already exists (the resolver writes it). It is the
// caller's own act — an attempt they made as themselves — and it must not
// acquire an actor just because the request mentioned another account.
const delegate = await makeUser("delegate");
const stranger = await makeUser("stranger");
const session = await signIn(delegate.id);
await getPrismaClient().session.update({
where: { id: session.id },
data: { actingAsUserId: stranger.id },
});
expect((await act()).status).toBe(403);
await settle();
const rows = await rowsFor("sharing.access.denied");
expect(rows).toHaveLength(1);
expect(rows[0].userId).toBe(delegate.id);
expect(rows[0].actorUserId).toBeNull();
});
});
describe("delegated reads coalesce to one row a day", () => {
it("writes one row for many reads, and counts them", async () => {
const { owner, delegate } = await household();
await act();
await act();
await act();
await settle();
const rows = await rowsFor(DELEGATED_ACCESS_ACTION);
expect(rows).toHaveLength(1);
expect(rows[0].userId).toBe(owner.id);
expect(rows[0].actorUserId).toBe(delegate.id);
expect(JSON.parse(rows[0].details!)).toEqual({ accesses: 3 });
});
it("keeps a second delegate's reads on their own row", async () => {
// Coalescing is per (owner, delegate, day). Two delegates in one record on
// one day are two rows — merging them would answer "somebody was in your
// record" when the owner asked "who".
const { owner, delegate } = await household();
const second = await makeUser("second");
const invited = await inviteGrant({
grantorId: owner.id,
granteeId: second.id,
});
await acceptGrant({ grantId: invited.id, granteeId: second.id });
await act();
const secondSession = await signIn(second.id);
await getPrismaClient().session.update({
where: { id: secondSession.id },
data: { actingAsUserId: owner.id },
});
await act();
await settle();
const rows = await rowsFor(DELEGATED_ACCESS_ACTION);
expect(rows).toHaveLength(2);
expect(rows.map((r) => r.actorUserId).sort()).toEqual(
[delegate.id, second.id].sort(),
);
expect(rows.every((r) => r.userId === owner.id)).toBe(true);
});
it("starts a new row on the next day", async () => {
const { owner, delegate } = await household();
await act();
await settle();
// Age today's row by a day. The coalescing key is the row's own date, so
// this is exactly what tomorrow looks like to the next read — and if the
// key were "the latest row" rather than the day, the next read would still
// find this one and the trail would grow one row per delegate forever.
await getPrismaClient().$executeRaw`
UPDATE audit_logs
SET created_at = created_at - INTERVAL '1 day'
WHERE action = ${DELEGATED_ACCESS_ACTION}
`;
await act();
await settle();
const rows = await rowsFor(DELEGATED_ACCESS_ACTION);
expect(rows).toHaveLength(2);
expect(rows.every((r) => r.userId === owner.id)).toBe(true);
expect(rows.every((r) => r.actorUserId === delegate.id)).toBe(true);
});
it("writes nothing at all when nobody is delegating", async () => {
const solo = await makeUser("solo");
await signIn(solo.id);
await act();
await settle();
expect(await rowsFor(DELEGATED_ACCESS_ACTION)).toHaveLength(0);
});
});
describe("the lifecycle verbs leave their own trail", () => {
it("records the invitation, the acceptance and the revocation", async () => {
const owner = await makeUser("owner");
const delegate = await makeUser("delegate");
await signIn(owner.id);
const { POST } = await import("@/app/api/account/grants/route");
const invited = await POST(
new NextRequest("http://localhost/api/account/grants", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify({ identifier: delegate.username }),
}),
);
const grantId = (await invited.json()).data.id;
await signIn(delegate.id);
const accept = await import("@/app/api/account/grants/[id]/accept/route");
await accept.POST(
new NextRequest(`http://localhost/api/account/grants/${grantId}/accept`, {
method: "POST",
headers: { "content-type": "application/json" },
}),
{ params: Promise.resolve({ id: grantId }) },
);
await signIn(owner.id);
const revokeRoute = await import("@/app/api/account/grants/[id]/route");
await revokeRoute.DELETE(
new NextRequest(`http://localhost/api/account/grants/${grantId}`, {
method: "DELETE",
headers: { "content-type": "application/json" },
}),
{ params: Promise.resolve({ id: grantId }) },
);
const trail = await getPrismaClient().auditLog.findMany({
where: { action: { startsWith: "sharing.grant." } },
orderBy: { createdAt: "asc" },
});
expect(trail.map((r) => r.action)).toEqual([
"sharing.grant.invited",
"sharing.grant.accepted",
"sharing.grant.revoked",
]);
// Each row files under whoever performed it, acting as themselves. The
// owner's side of the acceptance is not an audit row at all — it is
// `acceptedAt` on the grant, which is the durable consent record.
expect(trail.map((r) => r.userId)).toEqual([
owner.id,
delegate.id,
owner.id,
]);
expect(trail.every((r) => r.actorUserId === null)).toBe(true);
});
});