diff --git a/.changeset/principal-writes-off-event-loop.md b/.changeset/principal-writes-off-event-loop.md new file mode 100644 index 000000000..7679e3b5a --- /dev/null +++ b/.changeset/principal-writes-off-event-loop.md @@ -0,0 +1,5 @@ +--- +"@schlessera/brain-ui-server": patch +--- + +Changed: the hosted server commits authentication's per-request principal writes (the `last_seen_at` touch, terminal-principal pruning, ambient principal resolution and a login's new principal) on its SQLite write worker at `synchronous=FULL` instead of on the event loop (#2850). A request never waits for its touch; a login answers once its principal has committed. diff --git a/docs/decisions/claude-code-runtime.md b/docs/decisions/claude-code-runtime.md index cc9d9e263..0142e286b 100644 --- a/docs/decisions/claude-code-runtime.md +++ b/docs/decisions/claude-code-runtime.md @@ -64,7 +64,7 @@ next to it. of this binary. The only version probe in the server is for the `brain` CLI (`Probe the selected content CLI`, `packages/ui-server/src/brain/client.ts:142-210`, called at - `probeBrainCliVersion(config.brainPath`, `packages/ui-server/src/app.ts:433`). + `probeBrainCliVersion(config.brainPath`, `packages/ui-server/src/app.ts:434`). `brain doctor` runs `claude mcp list` from `PATH` (`which("claude")`, `packages/core/src/cli/commands/doctor.ts:503-507`) — the user's own Claude Code on their own machine, to check the MCP registration, not the server's @@ -212,7 +212,7 @@ unnecessary. `node_modules/@anthropic-ai/claude-agent-sdk/sdk.mjs`. The boot probe below turns either into a refusal to start, for the same reason a missing backend refuses to boot (`A missing (or unrecognized) agent backend`, - `packages/ui-server/src/app.ts:371-376`). + `packages/ui-server/src/app.ts:372-377`). - **The published range is still a caret, and that bounds what this repo can guarantee.** `@schlessera/brain-backend-claude` depends on `^0.3.241`, and a host resolves it in its own lockfile. A host can bump the SDK — and so the diff --git a/docs/decisions/hardening.md b/docs/decisions/hardening.md index 7afd6f6d8..3bb00760c 100644 --- a/docs/decisions/hardening.md +++ b/docs/decisions/hardening.md @@ -46,7 +46,7 @@ instead, and treat 8 as the question it was answering. epoch. Logout and passkey revocation bump it **and close every open WebSocket** (new `ClientSet.closeAll`). **The bump requires a valid epoch-bearing session**: logout is a public route today - (`/auth/logout`, `packages/ui-server/src/middleware/auth.ts:806`, + (`/auth/logout`, `packages/ui-server/src/middleware/auth.ts:797`, mounted before the guard), and a public epoch bump would let anyone log the owner out of everything in a loop. The epoch accessor is strict, not the fail-open settings getter: first initialization is distinguished from diff --git a/docs/decisions/http-api-boundary.md b/docs/decisions/http-api-boundary.md index f8c1be7bc..dd0367bea 100644 --- a/docs/decisions/http-api-boundary.md +++ b/docs/decisions/http-api-boundary.md @@ -46,11 +46,11 @@ explicit supported rows, not silently excluded by this distinction. The source audit on 2026-09-30 used `e0875269b21b707482545be9e94901acb897893b`. The public health mount precedes auth -(`createHealthRoutes`, `packages/ui-server/src/app.ts:864`), the API guard follows -public ceremonies (`authGuard`, `packages/ui-server/src/app.ts:885`), and the -WebSocket admission is separate (`"/ws"`, `packages/ui-server/src/app.ts:1075`). +(`createHealthRoutes`, `packages/ui-server/src/app.ts:867`), the API guard follows +public ceremonies (`authGuard`, `packages/ui-server/src/app.ts:888`), and the +WebSocket admission is separate (`"/ws"`, `packages/ui-server/src/app.ts:1078`). Static serving is conditional (`if (options.staticRoot)`, -`packages/ui-server/src/app.ts:1095-1116`). That order establishes access; +`packages/ui-server/src/app.ts:1098-1119`). That order establishes access; it does not select compatibility status. A hermetic real-app launch, with temporary brain root, in-memory database, diff --git a/docs/decisions/scheduled-tasks.md b/docs/decisions/scheduled-tasks.md index 4e63d9ac0..49b25e6cf 100644 --- a/docs/decisions/scheduled-tasks.md +++ b/docs/decisions/scheduled-tasks.md @@ -23,7 +23,7 @@ Source inspected at `b5c30c180df5584584f261b446c4d440380d2441`: (`else if (item.status === "scheduled"`, `packages/ui-server/src/inbox/actions.ts:196-197`). The runtime ticks every minute (`INBOX_TICK_MS`, `packages/ui-server/src/inbox/runtime.ts:11-13`). - Production app wiring supplies recovery and budgets, without a dispatcher - (`// Recovery/heartbeat only.`, `packages/ui-server/src/app.ts:491-507`). + (`// Recovery/heartbeat only.`, `packages/ui-server/src/app.ts:494-510`). A scripted nonempty ready-item test still reports no claim, attempt or Activity root. That proves disabled wiring, not safe execution of a schedule. - Headless plumbing checks backend support, a usable principal, lifetime and diff --git a/docs/decisions/session-principals.md b/docs/decisions/session-principals.md index 5450a3373..e1557970c 100644 --- a/docs/decisions/session-principals.md +++ b/docs/decisions/session-principals.md @@ -66,7 +66,7 @@ incident; neither should be the standing answer. - **Scopes / least privilege per route.** The one exception is R9's kind check on the principal-management routes — a single check on a single route family, the same shape as the existing password-disabled guard (`deps.passwordDisabled`, - `packages/ui-server/src/middleware/auth.ts:707`), not a scope system. + `packages/ui-server/src/middleware/auth.ts:698`), not a scope system. - **Multi-user.** One owner, several principals. - **Bearer tokens.** Additive later and cheap (see Q5); not in this plan. - **Containment of a compromised principal.** See the Summary. @@ -96,14 +96,14 @@ Both reviewers verified every row below against the source. | Cookie verify | `auth.ts:265-296` | strict `^(\d+)\.(\d+)$`, safe integers, future/expired rejection, `cookieEpoch === sessionsEpoch(db)` | | Global epoch | `auth.ts:302-318` | `settings` row; missing = 0; non-integer **throws** | | Global revoke | `auth.ts:321-330` | `setSetting` (overflow-guarded) + `clients.closeAll(1008, …)` | -| Auth routes | `authRoutes`, `app.ts:870` | mounted **before** the guard at `authGuard(authMode`, `app.ts:885`; logout guards itself (`resolveCookiePrincipal(c, auth, deps.db)`, `auth.ts:808`) | -| Passkey management | `passkeyManagementRoutes`, `app.ts:895` | mounted **after** the guard ("Mount AFTER the auth guard") | -| WS guard | `"/ws"`, `app.ts:1075-1092` | origin → `isWsAuthorized` (returns a boolean, `auth.ts:221-237`) → capacity → upgrade | +| Auth routes | `authRoutes`, `app.ts:873` | mounted **before** the guard at `authGuard(authMode`, `app.ts:888`; logout guards itself (`resolveCookiePrincipal(c, auth, deps.db)`, `auth.ts:799`) | +| Passkey management | `passkeyManagementRoutes`, `app.ts:898` | mounted **after** the guard ("Mount AFTER the auth guard") | +| WS guard | `"/ws"`, `app.ts:1078-1095` | origin → `isWsAuthorized` (returns a boolean, `auth.ts:221-237`) → capacity → upgrade | | WS upgrade | `ws/connection.ts:262-264` | `createWsUpgrade` ignores the request context | | WS admission | `ws/connection.ts:84` | `clients.add(ws)` — the socket's identity is unknown | | Turn record | `ws/run-session.ts:136-144` | recorder built from `{turnId, sessionId, billing}`; `RunningTurn` (`ws/turns.ts:55-77`) has no actor | | Follow-up queue | `QueuedFollowUp`, `ws/turns.ts:61-103`; `slot.queue.push(entry)`, `run-session.ts:1121` | queued entries re-mint `turnId` in the same slot and may come from another socket | -| Password login | `acquirePasswordVerification(key)`, `auth.ts:749` | argon2id verify, failure counting, in-flight reservation, then `issueSessionCookie` | +| Password login | `acquirePasswordVerification(key)`, `auth.ts:740` | argon2id verify, failure counting, in-flight reservation, then `issueSessionCookie` | | Passkey login | `issueLoginSession`, `packages/ui-server/src/middleware/passkeys.ts:448` | assertion verified, then `issueSessionCookie`; `row.id` is in scope | | Passkey **registration** | `INSERT INTO passkey_credentials`, `passkeys.ts:547` | inserts a credential; calls **neither** helper | | Passkey delete | `passkeys.ts:578-586` | deletes the credential, then a **global** bump | @@ -163,7 +163,7 @@ edited without breaking the signature. must not be able to drive database reads with arbitrary ids. The corrupt-row throw sits after signature verification and outside the cookie-parser catch, exactly as today's code does (`Deliberately after signature`, -`auth.ts:426-428`) — so a corrupt row 500s only that principal's requests +`auth.ts:416-418`) — so a corrupt row 500s only that principal's requests instead of everyone's. **4. Principals carry their lineage, and passkey deletion uses it.** @@ -216,7 +216,7 @@ nothing is backfilled. > example does not describe the current root span. **8. Ambient modes keep the identity they already have.** `proxy` mode reads an -upstream user header (`auth.proxyAuthHeader`, `auth.ts:485`) — that becomes the +upstream user header (`auth.proxyAuthHeader`, `auth.ts:476`) — that becomes the ambient principal's label, bounded like `sanitizeLabel`, `passkeys.ts:273-275`. `tailscale` carries the client IP; `none` is fixed. Minting and revocation are disabled in these modes. Cron is attributed to an explicit system principal, never to the owner. diff --git a/docs/decisions/sqlite-write-worker.md b/docs/decisions/sqlite-write-worker.md index d7a50ec3d..419f69421 100644 --- a/docs/decisions/sqlite-write-worker.md +++ b/docs/decisions/sqlite-write-worker.md @@ -177,10 +177,18 @@ are in [the governed-workers decision](governed-writable-workers.md#admitted-obj still the host fallback afterwards. A type folder translation that cannot be recorded refuses the turn. `holders()`, a diagnostic, still reads on the gate's own connection. +- **Authentication's per-request writes followed** (#2850). A principal's + `last_seen_at` touch, terminal-principal pruning, ambient principal + resolution and a login's new principal commit on the worker + (`packages/ui-server/src/db/principal-writes.ts`). A touch is never waited + for and one pending is not submitted again; an ambient principal that is + current is read at once; a login answers once its row has committed, and + its refusals keep their classes. Revocations and administrative principal + writes still commit on the app connection. - **Other app-connection writers stay on the loop for now**: retained retries and local exchanges, saved worker profiles, policy - proposals, the Activity notifier and push ticks, auth sessions, inbox and - settings. Each commits at + proposals, the Activity notifier and push ticks, principal revocation and + administration, passkeys, inbox and settings. Each commits at FULL on the loop, and while a worker procedure holds its turn, such a write also waits there for the worker's commit to finish, including a journal procedure that writes only `journal.db`, because the ledger's diff --git a/docs/http-api-coverage.md b/docs/http-api-coverage.md index 147139bb5..036612c96 100644 --- a/docs/http-api-coverage.md +++ b/docs/http-api-coverage.md @@ -26,7 +26,7 @@ The four named positive tests are `real WebSocket admission: untrusted proxy accepts a matching HTTPS Origin`, `real WebSocket admission: trusted proxy accepts forwarded HTTPS`, `real WebSocket admission: trusted proxy maps WSS to HTTPS` and `real WebSocket admission: trusted proxy maps WS to HTTP` -(`real WebSocket admission:`, `packages/ui-server/tests/http-websocket-admission.test.ts:44-53`). +(`real WebSocket admission:`, `packages/ui-server/tests/http-websocket-admission.test.ts:44-55`). An HTTP response other than 403 is insufficient evidence of admission. The final column names detailed behavior checks below. Some existing checks @@ -390,7 +390,7 @@ the additive `query_activity` scope carries the supported agent contract. **W — WebSocket admission.** -- `real WebSocket admission rejects origin, authentication and capacity before upgrading` ([source](../packages/ui-server/tests/http-websocket-admission.test.ts)), (`real WebSocket admission rejects origin, authentication and capacity before upgrading`, `packages/ui-server/tests/http-websocket-admission.test.ts:56-87`) +- `real WebSocket admission rejects origin, authentication and capacity before upgrading` ([source](../packages/ui-server/tests/http-websocket-admission.test.ts)), (`real WebSocket admission rejects origin, authentication and capacity before upgrading`, `packages/ui-server/tests/http-websocket-admission.test.ts:58-89`) **Z — Object-only validation and retained defaults.** diff --git a/docs/plans/async-collaboration-requirements.md b/docs/plans/async-collaboration-requirements.md index 57f282d3d..14bb6bfa5 100644 --- a/docs/plans/async-collaboration-requirements.md +++ b/docs/plans/async-collaboration-requirements.md @@ -226,7 +226,7 @@ Stated before the requirements because five of them derive from it. that is the shape to copy, including its `close()` lifecycle. - R19. **The cron backstop has independent authorization before the general guard.** Mount it on the existing listener before - (`app.use("/api/*", authGuard(`, `packages/ui-server/src/app.ts:885`). + (`app.use("/api/*", authGuard(`, `packages/ui-server/src/app.ts:888`). Authorize a boot-minted ephemeral token, rotated each boot and stored in a 0600 runtime file, with the actual socket address as an additional check. Proxy headers cannot authorize it. The poke succeeds in every auth mode diff --git a/docs/plans/async-collaboration.md b/docs/plans/async-collaboration.md index 041b4915b..9b6b9f66c 100644 --- a/docs/plans/async-collaboration.md +++ b/docs/plans/async-collaboration.md @@ -131,8 +131,8 @@ system gate. It is not a prerequisite to rebuild or a new independent package. owns manual triggers/history; it does not tick the inbox. - **Two connections:** (`export function createUiDb(`, `packages/ui-server/src/db/client.ts:27-44`) sets WAL, foreign keys and a 5-second busy timeout. Claims are immediate transactions. -- **Auth mounting:** (`app.use("/api/*", authGuard(`, `packages/ui-server/src/app.ts:885`) follows public routes; - (`export function authGuard(`, `packages/ui-server/src/middleware/auth.ts:189-249`) binds principals in each auth mode. +- **Auth mounting:** (`app.use("/api/*", authGuard(`, `packages/ui-server/src/app.ts:888`) follows public routes; + (`export function authGuard(`, `packages/ui-server/src/middleware/auth.ts:185-245`) binds principals in each auth mode. An internal poke needs independent token authorization before this guard. - **Permission parking:** (`requestPermission: (req) => {`, `packages/ui-server/src/ws/bridge.ts:358-452`). Timeout unwind is (`abortController.abort();`, `packages/ui-server/src/ws/run-session.ts:393-399`). Durable escalation must checkpoint diff --git a/docs/plans/navigation-recovery.md b/docs/plans/navigation-recovery.md index 4d0fdbb60..4b38b1b02 100644 --- a/docs/plans/navigation-recovery.md +++ b/docs/plans/navigation-recovery.md @@ -397,7 +397,7 @@ implementation. Any PR moving these lines owns their citation repairs. - Activity resolution: (`.get("/activity/runs", (c) => {`, `packages/ui-server/src/routes/activity.ts:143-281`). -- API authorization: (`app.use("/api/*", authGuard(authMode, auth, db));`, `packages/ui-server/src/app.ts:885`). +- API authorization: (`app.use("/api/*", authGuard(authMode, auth, db));`, `packages/ui-server/src/app.ts:888`). - Queued acceptance: (`// Queue it as the session's next turn; report queued immediately.`, `packages/ui-server/src/ws/run-session.ts:1098-1143`). diff --git a/docs/plans/storage-accounting.md b/docs/plans/storage-accounting.md index e17891a32..06361d2f6 100644 --- a/docs/plans/storage-accounting.md +++ b/docs/plans/storage-accounting.md @@ -67,7 +67,7 @@ Source anchors, checked against the reviewed commit: The completed #94 and #398 provide existing stats/maintenance foundations. - C: snapshot fields (`snapshotOf`, `packages/core/src/lib/stats-history.ts:132-172`), thinning (`thin`, `packages/core/src/lib/stats-history.ts:232-244`). -- D: effective path/journal resolution (`createApp`, `packages/ui-server/src/app.ts:292-328`), +- D: effective path/journal resolution (`createApp`, `packages/ui-server/src/app.ts:293-329`), WAL store (`createUiDb`, `packages/ui-server/src/db/client.ts:27-44`), and private locations (`statePaths`, `packages/ui-server/src/brain/operational-state.ts:108-114`). - E: live/archive paths (`transcriptPaths`, `packages/ui-backend-claude/src/transcript-archive.ts:30-39`). diff --git a/packages/ui-server/src/app.ts b/packages/ui-server/src/app.ts index b798be84c..4dfc61635 100644 --- a/packages/ui-server/src/app.ts +++ b/packages/ui-server/src/app.ts @@ -91,6 +91,7 @@ import { principalManagementRoutes } from "./middleware/principals.js"; import { createModuleRoutes } from "./routes/modules.js"; import { createUiDb } from "./db/client.js"; import { openSqliteWriter } from "./db/write-worker.js"; +import { registerPrincipalWrites } from "./db/principal-writes.js"; import { JOURNAL_SCHEMA, journalDatabasePath } from "./db/journal-database.js"; import { createInboxBridgeTools } from "./inbox/bridge-tools.js"; import { createInboxStore } from "./inbox/store.js"; @@ -452,6 +453,8 @@ export async function createApp(options: CreateAppOptions = {}): Promise writer.close()); reconciled(hostExecution.attach(db, writer)); // Shared reference source (#2429) runs only where the native manager owns a reference pool. diff --git a/packages/ui-server/src/db/principal-writes.ts b/packages/ui-server/src/db/principal-writes.ts new file mode 100644 index 000000000..cca990a2e --- /dev/null +++ b/packages/ui-server/src/db/principal-writes.ts @@ -0,0 +1,98 @@ +/** + * Principal writes on the request path, off the event loop (#2850). The + * hosted server's authentication touches a principal's `last_seen_at`, + * prunes terminal principals, resolves ambient principals and creates a + * login's principal on every few requests; each of those commits on the + * `ui.db` write worker at `synchronous=FULL` instead of the app connection. + * The procedures are the synchronous writers in `principals.ts`, run on the + * write connection. Revocations and administrative writes still commit on the + * app connection. + */ +import type { Database } from "bun:sqlite"; + +import { procedureCaller, type SqliteWriter, type WireError, type WriteProcedures } from "./write-worker.js"; +import { + createPrincipal, isLastSeenStale, isUsablePrincipal, PRINCIPAL_PRUNE_INTERVAL_MS, PrincipalCreatorNotUsableError, + PrincipalCredentialNotFoundError, PrincipalLimitError, prunePrincipals, resolveAmbientPrincipal, resolvePrincipal, + touchLastSeen, ambientPrincipalId, type AmbientPrincipalSource, type CreatePrincipalInput, type Principal, +} from "./principals.js"; + +export const url = import.meta.url; + +export function setup(db: Database): WriteProcedures { + return { + touch: (id: string, now: number) => touchLastSeen(db, id, now), + prune: (now: number) => prunePrincipals(db, now), + ambient: (source: AmbientPrincipalSource, identity: string, label: string) => resolveAmbientPrincipal(db, source, identity, label), + // A login prunes first, so expired rows never count against its cap. + login(input: CreatePrincipalInput, now: number) { prunePrincipals(db, now); return createPrincipal(db, input); }, + }; +} + +/** A refusal keeps its class across the worker. */ +function revive(error: WireError): Error | undefined { + const as = (made: E) => Object.assign(made, { message: error.message }); + switch (error.name) { + case "PrincipalLimitError": return as(new PrincipalLimitError()); + case "PrincipalCredentialNotFoundError": return as(new PrincipalCredentialNotFoundError("")); + case "PrincipalCreatorNotUsableError": return as(new PrincipalCreatorNotUsableError(undefined)); + default: return undefined; + } +} + +export interface PrincipalWrites { + /** Records that `id` was seen at `now`, at most once a minute; never waits. */ + touch(principal: Principal, now: number): Principal; + /** Prunes terminal principals at most once an interval; never waits. */ + pruneIfDue(now: number): void; + /** The ambient principal for an identity: read at once when it is current, created or relabelled where it commits. */ + ambient(source: AmbientPrincipalSource, identity: string, label: string): Principal | Promise; + /** A verified login's new principal, once it has committed. */ + login(input: CreatePrincipalInput): Promise; +} + +const registered = new WeakMap(); + +/** The principal writes for `db`: the ones the app registered with its writer, else inline on `db`. */ +export function principalWrites(db: Database): PrincipalWrites { + let writes = registered.get(db); + if (!writes) { writes = createPrincipalWrites(db); registered.set(db, writes); } + return writes; +} + +/** Registers `db`'s principal writes on `writer` (#2850). */ +export function registerPrincipalWrites(db: Database, writer: SqliteWriter): PrincipalWrites { + const writes = createPrincipalWrites(db, writer); + registered.set(db, writes); + return writes; +} + +function createPrincipalWrites(db: Database, writer?: SqliteWriter): PrincipalWrites { + const call = procedureCaller(db, { url, setup }, undefined, writer, revive); + // A touch submitted and not yet settled: the next request does not submit another. + const touching = new Set(); + let lastPrunedAt: number | undefined; + const quietly = (done: Promise) => { done.catch(() => { /* Advisory bookkeeping: the next interval tries again. */ }); return done; }; + const touch = (principal: Principal, now: number): Principal => { + if (!isLastSeenStale(principal.lastSeenAt, now) || touching.has(principal.id)) return principal; + touching.add(principal.id); + void quietly(call("touch", principal.id, now)).finally(() => touching.delete(principal.id)); + return { ...principal, lastSeenAt: now }; + }; + return { + touch, + pruneIfDue(now) { + if (lastPrunedAt !== undefined && now >= lastPrunedAt && now - lastPrunedAt < PRINCIPAL_PRUNE_INTERVAL_MS) return; + lastPrunedAt = now; + void quietly(call("prune", now)); + }, + ambient(source, identity, label) { + // The common case writes nothing but the throttled touch. + const existing = resolvePrincipal(db, ambientPrincipalId(source, identity)); + if (existing && existing.label === label && existing.kind === "ambient" && existing.authMethod === "ambient" && isUsablePrincipal(existing, Date.now())) + return touch(existing, Date.now()); + return call("ambient", source, identity, label); + }, + login: input => call("login", input, Date.now()), + }; +} diff --git a/packages/ui-server/src/db/principals.ts b/packages/ui-server/src/db/principals.ts index d1d75ee97..d90b4a239 100644 --- a/packages/ui-server/src/db/principals.ts +++ b/packages/ui-server/src/db/principals.ts @@ -8,7 +8,6 @@ export const PRINCIPAL_PRUNE_INTERVAL_MS = 60 * 60 * 1_000; export const LAST_SEEN_WRITE_INTERVAL_MS = 60_000; const MAX_LABEL_LENGTH = 64; const CONTROL_CHARACTER = /[\u0000-\u001f\u007f-\u009f]/u; -const lastPrincipalPruneByDb = new WeakMap(); export type PrincipalKind = "owner" | "agent" | "ambient" | "system"; export type PrincipalAuthMethod = @@ -122,7 +121,7 @@ function generatePrincipalId(): string { return randomBytes(16).toString("base64url"); } -function ambientPrincipalId( +export function ambientPrincipalId( source: AmbientPrincipalSource, identity: string ): string { @@ -422,24 +421,6 @@ export function prunePrincipals(db: Database, now: number): void { ).run(cutoff, cutoff, cutoff); } -/** - * Run principal retention at most once per database per process interval. - * Authentication paths call this cheaply; only the due call reaches SQLite. - */ -export function prunePrincipalsIfDue(db: Database, now: number): boolean { - const lastPrunedAt = lastPrincipalPruneByDb.get(db); - if ( - lastPrunedAt !== undefined && - now >= lastPrunedAt && - now - lastPrunedAt < PRINCIPAL_PRUNE_INTERVAL_MS - ) { - return false; - } - prunePrincipals(db, now); - lastPrincipalPruneByDb.set(db, now); - return true; -} - export function countLivePrincipals(db: Database, now: number): number { const row = db .prepare( diff --git a/packages/ui-server/src/middleware/auth.ts b/packages/ui-server/src/middleware/auth.ts index eabcc8573..54384a57f 100644 --- a/packages/ui-server/src/middleware/auth.ts +++ b/packages/ui-server/src/middleware/auth.ts @@ -9,20 +9,16 @@ import type { AuthConfig } from "../config/env.js"; import { readJsonBody } from "./body-limit.js"; import { requireJson } from "./origin.js"; import { - createPrincipal, isCookieBearingPrincipal, isUsablePrincipal, PrincipalCredentialNotFoundError, PrincipalLimitError, - prunePrincipals, - prunePrincipalsIfDue, - resolveAmbientPrincipal, resolvePrincipal, revokePrincipal, revokeAllPrincipals, - touchLastSeen, type Principal, } from "../db/principals.js"; +import { principalWrites } from "../db/principal-writes.js"; import type { AppEnv } from "../app-env.js"; import type { WsHost } from "../ws/host.js"; @@ -194,12 +190,12 @@ export function authGuard( switch (mode) { case "tailscale": return async (c, next) => { - prunePrincipalsIfDue(db, Date.now()); + principalWrites(db).pruneIfDue(Date.now()); if (isTailscaleAllowed(c, auth.trustProxy, auth.trustProxyHops)) { const ip = clientIp(c, auth.trustProxy, auth.trustProxyHops); c.set( "principal", - resolveAmbientPrincipal(db, "tailscale", ip, sanitizeAmbientLabel(ip)) + await principalWrites(db).ambient("tailscale", ip, sanitizeAmbientLabel(ip)) ); await next(); } else { @@ -208,13 +204,12 @@ export function authGuard( }; case "proxy": return async (c, next) => { - prunePrincipalsIfDue(db, Date.now()); + principalWrites(db).pruneIfDue(Date.now()); const proxyIdentity = resolveProxyIdentity(c, auth); if (proxyIdentity) { c.set( "principal", - resolveAmbientPrincipal( - db, + await principalWrites(db).ambient( "proxy", proxyIdentity.identity, proxyIdentity.label @@ -230,7 +225,7 @@ export function authGuard( }; case "password": return async (c, next) => { - prunePrincipalsIfDue(db, Date.now()); + principalWrites(db).pruneIfDue(Date.now()); const principal = await resolveCookiePrincipal(c, auth, db); if (principal) { c.set("principal", principal); @@ -244,11 +239,10 @@ export function authGuard( }; case "none": return async (c, next) => { - prunePrincipalsIfDue(db, Date.now()); + principalWrites(db).pruneIfDue(Date.now()); c.set( "principal", - resolveAmbientPrincipal( - db, + await principalWrites(db).ambient( "none", "No authentication", "No authentication" @@ -274,11 +268,10 @@ export async function isWsAuthorized( auth: AuthRuntime, db: Database ): Promise { - prunePrincipalsIfDue(db, Date.now()); + principalWrites(db).pruneIfDue(Date.now()); switch (mode) { case "none": - return resolveAmbientPrincipal( - db, + return await principalWrites(db).ambient( "none", "No authentication", "No authentication" @@ -286,8 +279,7 @@ export async function isWsAuthorized( case "tailscale": { if (!isTailscaleAllowed(c, auth.trustProxy, auth.trustProxyHops)) return null; const ip = clientIp(c, auth.trustProxy, auth.trustProxyHops); - return resolveAmbientPrincipal( - db, + return await principalWrites(db).ambient( "tailscale", ip, sanitizeAmbientLabel(ip) @@ -296,8 +288,7 @@ export async function isWsAuthorized( case "proxy": { const proxyIdentity = resolveProxyIdentity(c, auth); return proxyIdentity - ? resolveAmbientPrincipal( - db, + ? await principalWrites(db).ambient( "proxy", proxyIdentity.identity, proxyIdentity.label @@ -374,10 +365,9 @@ export async function issueLoginSession( db: Database, lineage: LoginPrincipalLineage ): Promise { - prunePrincipals(db, Date.now()); - try { - const principal = createPrincipal(db, { + // Pruned first, then created, on the write worker (#2850). + const principal = await principalWrites(db).login({ authMethod: lineage.authMethod, label: sanitizeLoginLabel(c.req.header("user-agent")), credentialId: @@ -435,7 +425,8 @@ export async function resolveCookiePrincipal( ) { return null; } - touchLastSeen(db, principal.id, now); + // Advisory, throttled, and never waited for (#2850). + principalWrites(db).touch(principal, now); return principal; } diff --git a/packages/ui-server/tests/http-websocket-admission.test.ts b/packages/ui-server/tests/http-websocket-admission.test.ts index 86b88d39a..a04878c7c 100644 --- a/packages/ui-server/tests/http-websocket-admission.test.ts +++ b/packages/ui-server/tests/http-websocket-admission.test.ts @@ -45,10 +45,12 @@ for (const [name, trustProxy, scheme, forwarded] of [ const t = await httpContractApp({ env: { TRUST_PROXY: trustProxy ? "1" : "0" } }); const running = await listener(t); try { - await expect(hello(running.ws, { + // A plain await: admission resolves its principal on the write worker + // (#2850), whose reply `.resolves`' nested loop never delivers. + expect(await hello(running.ws, { origin: `${scheme}://${running.host}`, ...(forwarded ? { "x-forwarded-proto": forwarded } : {}), - })).resolves.toMatchObject({ type: "server_hello", protocolRev: PROTOCOL_REV }); + })).toMatchObject({ type: "server_hello", protocolRev: PROTOCOL_REV }); } finally { running.server.stop(true); await t.close(); } }); } diff --git a/packages/ui-server/tests/principal-writes-offloop.test.ts b/packages/ui-server/tests/principal-writes-offloop.test.ts new file mode 100644 index 000000000..ac5ccb864 --- /dev/null +++ b/packages/ui-server/tests/principal-writes-offloop.test.ts @@ -0,0 +1,103 @@ +/** Authentication's per-request principal writes commit on the write worker (#2850). */ +import { expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { Hono } from "hono"; +import { createUiDb } from "../src/db/client.js"; +import { openSqliteWriter } from "../src/db/write-worker.js"; +import { createPrincipal, PrincipalCredentialNotFoundError, resolvePrincipal, type Principal } from "../src/db/principals.js"; +import { principalWrites, registerPrincipalWrites } from "../src/db/principal-writes.js"; +import { authGuard, issueSessionCookie, resolveCookiePrincipal, type AuthRuntime } from "../src/middleware/auth.js"; +import { resolveServerConfig } from "../src/config/env.js"; +import type { AppEnv } from "../src/app-env.js"; +import * as probe from "./fixtures/write-worker-probe.js"; +import { expectRejection } from "./helpers/expect-rejection.js"; +import { watchWriteSites } from "./helpers/write-sites.js"; + +function fixture() { + const root = mkdtempSync(join(tmpdir(), "principal-writes-odysseus-")), path = join(root, "ui.db"); + const db = createUiDb(path), writer = openSqliteWriter(db); + return { root, db, writer, async close() { await writer.close(); db.close(); rmSync(root, { recursive: true, force: true }); } }; +} +const lastSeen = (f: ReturnType, id: string) => (f.db.query("SELECT last_seen_at AS at FROM principals WHERE id = ?").get(id) as { at: number | null }).at; + +test("request-path principal writes commit on the write worker at FULL, never on the app connection", async () => { + const f = fixture(); + try { + const athena = createPrincipal(f.db, { authMethod: "password", label: "Athena device", ttlSeconds: 3600 }); + expect(f.writer.offloop).toBe(true); + const sites = await watchWriteSites(f.db, f.writer, ["principals"]); + const writes = registerPrincipalWrites(f.db, f.writer); + expect(principalWrites(f.db), "PRINCIPAL_WRITES_REGISTERED_FOR_DB").toBe(writes); + writes.touch(athena, Date.now()); + writes.pruneIfDue(Date.now()); + // The guard creates the request's ambient principal through the same writes. + const app = new Hono().use("/api/*", authGuard("none", { mode: "none" } as never, f.db)).get("/api/who", c => c.json({ id: c.get("principal")!.id })); + const guarded = ((await (await app.request("/api/who")).json()) as { id: string }).id; + const ambient = await writes.ambient("none", "No authentication", "No authentication"); + expect(ambient.kind, "PRINCIPAL_AMBIENT_CREATED_ON_WORKER").toBe("ambient"); + expect(guarded, "PRINCIPAL_GUARD_RESOLVES_AMBIENT").toBe(ambient.id); + const login = await writes.login({ authMethod: "password", label: "Penelope's loom", ttlSeconds: 3600 }); + expect(resolvePrincipal(f.db, login.id)?.label, "PRINCIPAL_LOGIN_COMMITTED_BEFORE_SETTLING").toBe("Penelope's loom"); + await f.writer.idle(); + const log = await sites.log(); + expect(log.length, "PRINCIPAL_WRITES_LOGGED_ON_WORKER").toBeGreaterThanOrEqual(3); + expect(new Set(log), "PRINCIPAL_WRITES_ONLY_TO_PRINCIPALS").toEqual(new Set(["principals"])); + expect(lastSeen(f, athena.id), "PRINCIPAL_TOUCH_COMMITTED").not.toBeNull(); + } finally { await f.close(); } +}); + +test("a touch never waits for its commit, and a pending one is not submitted twice", async () => { + const f = fixture(); + try { + const athena: Principal = createPrincipal(f.db, { authMethod: "password", label: "Athena device", ttlSeconds: 3600 }); + const sites = await watchWriteSites(f.db, f.writer, ["principals"]); + const writes = registerPrincipalWrites(f.db, f.writer); + const held = f.writer.channel(probe, { table: "odysseus_log" }); await held.ready; + const slow = held.call("slow", "Calypso", 300); + const started = performance.now(); + const seen = writes.touch(athena, Date.now()); + // Late enough that a second submission would commit a second row. + writes.touch(athena, Date.now() + 61_000); + expect(performance.now() - started, "PRINCIPAL_TOUCH_DOES_NOT_WAIT").toBeLessThan(100); + expect(seen.lastSeenAt, "PRINCIPAL_TOUCH_ANSWERS_AT_ONCE").not.toBeNull(); + expect(lastSeen(f, athena.id), "PRINCIPAL_TOUCH_NOT_COMMITTED_ON_LOOP").toBeNull(); + await slow; + await f.writer.idle(); + expect(await sites.log(), "PRINCIPAL_PENDING_TOUCH_SUBMITTED_ONCE").toEqual(["principals"]); + expect(lastSeen(f, athena.id), "PRINCIPAL_TOUCH_COMMITTED_AFTER_HOLD").not.toBeNull(); + } finally { await f.close(); } +}); + +test("a signed-in request is answered without waiting for its touch to commit", async () => { + const f = fixture(); + try { + const config = resolveServerConfig({ AUTH_MODE: "password", BRAIN_UI_PASSWORD_HASH: "odysseus-password-hash", COOKIE_SECRET: "odysseus-cookie-secret-0123456789abcdef" }); + const auth: AuthRuntime = { ...config.auth, host: config.host }; + registerPrincipalWrites(f.db, f.writer); + const penelope = createPrincipal(f.db, { authMethod: "password", label: "Penelope's loom", ttlSeconds: 3600 }); + const issue = new Hono().get("/", async c => { await issueSessionCookie(c, auth, f.db, penelope.id); return c.text("ok"); }); + const cookie = (await issue.request("/")).headers.get("set-cookie")!.split(";")[0]!; + const held = f.writer.channel(probe, { table: "odysseus_log" }); await held.ready; + const slow = held.call("slow", "Calypso", 400); + const app = new Hono().get("/", async c => c.json({ id: (await resolveCookiePrincipal(c, auth, f.db))?.id ?? null })); + const started = performance.now(); + const resolved = await (await app.request("/", { headers: { cookie } })).json() as { id: string | null }; + expect(resolved.id, "PRINCIPAL_COOKIE_RESOLVES").toBe(penelope.id); + expect(performance.now() - started, "PRINCIPAL_COOKIE_REQUEST_DOES_NOT_WAIT_FOR_TOUCH").toBeLessThan(200); + expect(lastSeen(f, penelope.id), "PRINCIPAL_COOKIE_TOUCH_NOT_COMMITTED_ON_LOOP").toBeNull(); + await slow; await f.writer.idle(); + expect(lastSeen(f, penelope.id), "PRINCIPAL_COOKIE_TOUCH_COMMITTED").not.toBeNull(); + } finally { await f.close(); } +}); + +test("a login refusal keeps its class across the worker", async () => { + const f = fixture(); + try { + const writes = registerPrincipalWrites(f.db, f.writer); + await expectRejection(writes.login({ authMethod: "passkey", label: "Odysseus' bow", credentialId: "no-such-bow", ttlSeconds: 3600 }), + PrincipalCredentialNotFoundError, "PRINCIPAL_LOGIN_REFUSAL_CLASS_KEPT"); + } finally { await f.close(); } +}); +