close the cross-member database leak with a login event trigger

The residue the last commit documented is gone. A database one member creates is now
refused to every other member at connection time, so the catalogue metadata never
becomes readable in the first place.

Both obvious routes are dead ends, measured rather than assumed: datacl is not
inherited from the template, and CREATE DATABASE fires no event trigger because it is
a global object. What works is a `login` event trigger (PG17+) installed in template1
— event triggers live in a per-database catalogue and CREATE DATABASE copies the
template's catalogues, so every member-created database carries it automatically. No
naming convention, no sweep, no window.

The first version put the function in `public` and a member defeated it in one
statement:

  DROP FUNCTION public.officer_owner_only() CASCADE;   -- takes the trigger with it

They could not drop or disable the trigger itself, but in PG15+ `public` is owned by
pg_database_owner — which resolves to THEM in their own database — and a schema owner
may drop objects in it they do not own. Both objects were owned by postgres and it
made no difference. Caught because I tried it rather than reasoned about it.

Moved into a platform-owned schema with PUBLIC revoked. Every route then refused:
DROP EVENT TRIGGER, ALTER .. DISABLE, DROP FUNCTION, DROP SCHEMA, ALTER SCHEMA ..
OWNER TO, CREATE OR REPLACE over the top, and PGOPTIONS=-c event_triggers=off (that
GUC is superuser-only). A superuser can still set it, which is the recovery path.

ensureTemplateIsolation opens its own short-lived connection because a connection
cannot change database and CREATE DATABASE requires no other session on the template
— a pooled connection to template1 would make every member's `createdb` fail.

Verified live, end to end: alice in her own, bob refused, alice refused from bob's,
postgres in, owner reads and writes normally, and both members refused CONNECT on
`officer`. Probe roles and databases dropped; template1 keeps the trigger, which is
the intended state.

Still not typechecked — node_modules is empty in this tree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/servers/os-user-postgres.ts

@@ -2,6 +2,7 @@ import { mkdtemp, rm, writeFile } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { sql } from 'drizzle-orm';
+import postgres from 'postgres';
 import { db } from 'officerdb/db';
 import { osUserHome, runAs } from './os-user';
 
@@ -33,16 +34,32 @@ import { osUserHome, runAs } from './os-user';
 //   - Revoking from the ROLE does not help. Postgres privileges are additive and there is no DENY, so a
 //     PUBLIC grant is not overridden by a role-level revoke. Revoking from PUBLIC is the only lock.
 //
-// ── The residue, stated rather than implied ──
+// ── Keeping members out of EACH OTHER'S databases ──
 //
-// A database one member creates is readable-as-metadata by another: `datacl` is NOT inherited from the
-// template (measured — closing `template1` and creating from it still produced a NULL `datacl`), and
-// `CREATE DATABASE` fires no event trigger, so nothing can close a member's database at the moment they
-// create it. A second member can connect and read table and column NAMES from the catalogue. They cannot
-// read a row, and they cannot create anything (PG15+ removed PUBLIC's CREATE on `public`).
+// Harder than it sounds, and the two obvious routes are both dead ends. `datacl` is NOT inherited from the
+// template — measured: closing `template1` and creating a database from it still produced a NULL `datacl`,
+// so PUBLIC gets CONNECT on every database a member makes. And `CREATE DATABASE` fires no event trigger,
+// because it is a global object, so nothing can react at the moment of creation.
 //
-// Closing that needs either a sweep or a `pg_hba.conf` rule per member, and both are decisions rather
-// than details. Not built. Do not let this comment be read as "handled".
+// What does work is a `login` event trigger (PG17+) installed in `template1`. Event triggers live in a
+// per-database catalogue, and `CREATE DATABASE` copies the template's catalogues — so every database a
+// member creates carries it, automatically, with no naming convention, no sweep and no window.
+//
+// ── Why the function lives in its own schema ──
+//
+// The first version put it in `public` and a member DEFEATED IT IN ONE STATEMENT:
+//
+//   DROP FUNCTION public.officer_owner_only() CASCADE;   -- takes the event trigger with it
+//
+// They could not drop or disable the trigger — `must be owner of event trigger` — but in PG15+ the
+// `public` schema is owned by `pg_database_owner`, which resolves to THEM inside their own database, and a
+// schema owner may drop objects in it that they do not own. Both objects were owned by `postgres` and it
+// made no difference.
+//
+// So the function goes in a schema owned by the platform with PUBLIC revoked. Measured after the move —
+// every route refused: DROP EVENT TRIGGER, ALTER … DISABLE, DROP FUNCTION, DROP SCHEMA, ALTER SCHEMA …
+// OWNER TO, CREATE OR REPLACE over the top, and `PGOPTIONS=-c event_triggers=off` (that GUC is superuser
+// only). A superuser can still use it, which is the recovery path if this ever locks something out.
 
 /** Legal Linux/Postgres account name. Identical to `validateUsername`, restated because this one reaches SQL. */
 const ROLE_NAME_RE = /^[a-zA-Z0-9._-]{2,32}$/;
@@ -150,6 +167,65 @@ export async function ensureAppDatabaseClosed(): Promise<AppDatabaseClosedResult
 
 const asMessage = (ex: unknown): string => (ex instanceof Error ? ex.message : String(ex));
 
+/** The isolation schema, function and login trigger. Ordered; each is idempotent on its own. */
+const TEMPLATE_ISOLATION_SQL = [
+  `CREATE SCHEMA IF NOT EXISTS officer AUTHORIZATION CURRENT_USER`,
+  `REVOKE ALL ON SCHEMA officer FROM PUBLIC`,
+  // SECURITY INVOKER (the default) with a pinned search_path. DEFINER is not needed — every catalogue this
+  // reads is world-readable — and not using it removes the search_path-shadowing class of problem outright.
+  `CREATE OR REPLACE FUNCTION officer.owner_only() RETURNS event_trigger
+     LANGUAGE plpgsql SET search_path = pg_catalog AS $fn$
+   BEGIN
+     IF (SELECT rolsuper FROM pg_roles WHERE rolname = current_user) THEN RETURN; END IF;
+     IF pg_has_role(current_user, (SELECT datdba FROM pg_database WHERE datname = current_database()), 'USAGE')
+       THEN RETURN; END IF;
+     RAISE EXCEPTION 'permission denied for database "%"', current_database()
+       USING HINT = 'Officer: a database is private to the account that created it.';
+   END $fn$`,
+  // No IF NOT EXISTS for event triggers, so drop and recreate. The gap is inside template1 and only matters
+  // to a CREATE DATABASE landing in the same instant, which cannot happen — see the connection note below.
+  `DROP EVENT TRIGGER IF EXISTS officer_owner_only_login`,
+  `CREATE EVENT TRIGGER officer_owner_only_login ON login EXECUTE FUNCTION officer.owner_only()`,
+];
+
+/**
+ * Install the isolation trigger into `template1`, so every database created afterwards inherits it.
+ *
+ * ── Why this opens its own connection, and closes it immediately ──
+ *
+ * The platform's pool is connected to `officer`; a connection cannot change database. And `CREATE DATABASE`
+ * requires that NO other session is connected to the template it copies — so a pooled or long-lived
+ * connection to `template1` would make every member's `createdb` fail with "source database is being
+ * accessed by other users". Hence one connection, `max: 1`, ended in a `finally`.
+ *
+ * Only ever runs at provisioning time, which is rare enough that the race with a member running `createdb`
+ * in the same second is worth naming and not worth engineering around: it surfaces as a retryable error on
+ * their side.
+ */
+export async function ensureTemplateIsolation(): Promise<{ ok: true } | { ok: false; error: string }> {
+  const url = process.env.POSTGRES_URL;
+  if (!url) return { ok: false, error: 'POSTGRES_URL is not set' };
+
+  let target: string;
+  try {
+    const parsed = new URL(url);
+    parsed.pathname = '/template1';
+    target = parsed.toString();
+  } catch {
+    return { ok: false, error: 'POSTGRES_URL could not be parsed' };
+  }
+
+  const client = postgres(target, { max: 1, idle_timeout: 5, connection: { application_name: 'officer-provision' } });
+  try {
+    for (const statement of TEMPLATE_ISOLATION_SQL) await client.unsafe(statement);
+    return { ok: true };
+  } catch (ex) {
+    return { ok: false, error: `could not install database isolation into template1: ${asMessage(ex)}` };
+  } finally {
+    await client.end({ timeout: 5 });
+  }
+}
+
 export type PostgresRoleResult =
   | { ok: true; role: string; created: boolean; passwordSet: boolean }
   | { ok: false; error: string };
@@ -172,10 +248,13 @@ export async function provisionPostgresRole(params: {
   uid: number;
   gid: number;
 }): Promise<PostgresRoleResult> {
-  // Before anything else, and before the role can exist to take advantage of it.
+  // Both before anything else, and before the role can exist to take advantage of either being absent.
   const closed = await ensureAppDatabaseClosed();
   if (!closed.ok) return { ok: false, error: closed.error };
 
+  const isolated = await ensureTemplateIsolation();
+  if (!isolated.ok) return { ok: false, error: isolated.error };
+
   // Re-asserted here even though `validateUsername` already ran at the route. This string reaches SQL as an
   // identifier, and the distance between that check and this one is a whole call chain — the kind of gap
   // where a future caller arrives without having passed the first one.