Isolate the storage CI stack, prove least-privilege provisioning, drop the secret before discovery

AGENTS.md

@@ -97,9 +97,14 @@ Revisit only if the stated reason stops holding.
   *accidents*, not hostile plugins: `PLUGIN_DB_SECRET` is in `web`'s environment during `onBoot`, and
   a plugin already holds `ctx.system`'s Ory admin clients — so cross-plugin DB isolation is
   containment, and README says so rather than implying a sandbox. Consistent with priority #7
-  (crash-isolation is a non-goal). `server.ts` still derives every credential *before* the boot hooks
+  (crash-isolation is a non-goal). `server.ts` still deletes the secret from `process.env` right
-  and then deletes the secret from `process.env` — defence in depth, and the ordering is the whole
+  after `loadConfig`, which is before discovery imports any plugin module — the ordering is the whole
-  point, so don't move it. **Valid while plugins are operator-installed code, not third-party uploads.**
+  point, so move it earlier if anything, **never later**. **Valid while plugins are
+  operator-installed code, not third-party uploads.**
+- **`bootstrap.ts` stays under `src/auth/`** even though it now provisions plugin databases as well
+  as seeding Ory. It is the one-shot service's entrypoint, not an auth module; moving it to
+  `src/bootstrap.ts` would edit `compose.yml`, five e2e compose files and `src/compose.test.ts` for a
+  rename. Reconsider when a third seeding concern lands.
 - **`BootContext.storage` keeps all six credential fields, and there is no `onShutdown` hook.** While
   `HOST_API_VERSION` is frozen both are free to revisit; after the freeze, adding is compatible and
   removing is not, so the shape errs small elsewhere. Pools handed to a plugin are reaped on process

ci.sh

@@ -64,13 +64,20 @@ count=$(echo "$units" | grep -oE 'tests [0-9]+' | grep -oE '[0-9]+' | head -1 ||
 # test skips there — and it is the only thing proving the DDL actually grants what it claims, rather
 # than that the SQL text is the text we wrote. `node --test` counts a skip, so the floor won't catch it.
 step "Plugin storage (real Postgres)"
+# Own project name, like every E2E suite below: the default project is the DEV stack, so a bare
+# `down -v` here would delete the operator's pgdata — Ory identities and every plugin database.
+# --wait, because initdb on a cold volume outlasts the suite's connect timeout.
 storage_rc=0
-docker compose up -d postgres >/dev/null
+storage_proj=plainpages-storage
-docker compose run --rm --no-deps \
+docker compose -p "$storage_proj" up -d --wait postgres >/dev/null
+storage_out=$(docker compose -p "$storage_proj" run --rm --no-deps \
 	-e PLUGIN_DB_ADMIN_URL=postgres://${POSTGRES_USER:-ory}:${POSTGRES_PASSWORD:-ory}@postgres:5432/ory \
-	web node --test src/plugin-host/storage.test.ts || storage_rc=$?
+	web node --test src/plugin-host/storage.test.ts 2>&1) || storage_rc=$?
-docker compose down -v >/dev/null 2>&1 || true
+docker compose -p "$storage_proj" down -v >/dev/null 2>&1 || true
-[ "$storage_rc" -eq 0 ] || { echo "plugin storage integration tests failed (exit $storage_rc)"; exit "$storage_rc"; }
+echo "$storage_out" | grep -E '^. (tests|pass|fail|skipped) ' || true
+[ "$storage_rc" -eq 0 ] || { echo "$storage_out"; echo "plugin storage integration tests failed (exit $storage_rc)"; exit "$storage_rc"; }
+# A skip here exits 0 and proves nothing — the same trap the unit floor above guards against.
+echo "$storage_out" | grep -qE '^. skipped 0$' || { echo "storage integration test skipped — PLUGIN_DB_ADMIN_URL not wired through"; exit 1; }
 
 # Run one E2E suite against its OWN named stack, then always tear it down (even on failure). The
 # per-suite project name keeps a flaky teardown from leaking containers/volumes into the next suite.

src/auth/bootstrap.ts

@@ -18,6 +18,10 @@ import { createLogger, runWithLog, tracedFetch, type Log } from "../logger.ts";
 
 type Env = Record<string, string | undefined>;
 
+// Kept closed to PUBLIC on every boot, not just on a fresh volume — a plugin role would otherwise
+// reach the auth plane's catalogs and connection slots (ory/postgres/init/init.sql seeds the same).
+const ORY_DATABASES = ["hydra", "keto", "kratos"];
+
 // --- Pure payload builders (the Kratos/Keto request contracts) -----------------------
 
 export function identityPayload(email: string, password: string) {
@@ -175,6 +179,7 @@ async function provisionPluginStorage(env: Env, plugins: Plugin[], log: Log): Pr
   const result = await provisionStorage({
     adminUrl,
     connectionLimit: resolvePluginDbConnectionLimit(env),
+    lockdownDatabases: ORY_DATABASES,
     pluginIds: ids,
     secret: resolvePluginDbSecret(env),
   });

src/config.test.ts

@@ -1,6 +1,6 @@
 import assert from "node:assert/strict";
 import { test } from "node:test";
-import { loadConfig, resolvePluginDbSecret } from "./config.ts";
+import { loadConfig, resolvePluginDbConnectionLimit, resolvePluginDbSecret } from "./config.ts";
 
 // Explicit secure-secret enforcement (no environment sniffing): secrets are the only
 // thing a hardened deploy must supply.
@@ -30,6 +30,22 @@ test("bootstrap refuses a missing, empty or throwaway plugin storage secret when
   assert.equal(resolvePluginDbSecret({ ...hardened, PLUGIN_DB_SECRET: "a-real-secret" }), "a-real-secret");
 });
 
+// buildCredentials overwrites the userinfo, so a pasted admin DSN would *work* — and leave a
+// privileged password in the process that runs plugin code. Refusing it is the whole guard.
+test("PLUGIN_DB_URL carrying credentials is refused, not silently overwritten", () => {
+  assert.throws(() => loadConfig({ PLUGIN_DB_URL: "postgres://root:hunter2@db:5432/ory" }), /no username or password/);
+  assert.throws(() => loadConfig({ PLUGIN_DB_URL: "postgres://root@db:5432" }), /no username or password/);
+  assert.equal(loadConfig({ PLUGIN_DB_URL: "postgres://db:5432" }).pluginDbUrl, "postgres://db:5432");
+  assert.equal(loadConfig({}).pluginDbUrl, undefined); // unset ⇒ storage off
+});
+
+test("the per-role connection ceiling defaults to 10 and rejects nonsense", () => {
+  assert.equal(resolvePluginDbConnectionLimit({}), 10);
+  assert.equal(resolvePluginDbConnectionLimit({ PLUGIN_DB_CONNECTION_LIMIT: "25" }), 25);
+  assert.throws(() => resolvePluginDbConnectionLimit({ PLUGIN_DB_CONNECTION_LIMIT: "0" }), /positive integer/);
+  assert.throws(() => resolvePluginDbConnectionLimit({ PLUGIN_DB_CONNECTION_LIMIT: "ten" }), /positive integer/);
+});
+
 test("loads dev defaults when the environment is empty", () => {
   const c = loadConfig({});
   assert.equal(c.port, 3000);

src/plugin-host/storage-provisioning.ts

@@ -2,11 +2,14 @@
 // alone — the only process holding superuser credentials, which is why the driver stops here.
 
 import postgres from "postgres";
-import { derivePassword, NAME_PREFIX, provisionSql, storageName } from "./storage.ts";
+import { derivePassword, NAME_PREFIX, orphanNames, provisionSql, quoteIdentifier, storageName } from "./storage.ts";
 
 export interface ProvisionOptions {
   adminUrl: string; // needs CREATEDB + CREATEROLE, not superuser
   connectionLimit: number;
+  // Databases to keep closed to PUBLIC on every run. init.sql seeds this for the Ory databases, but
+  // it runs once on an empty data dir — an existing volume would keep the default grant forever.
+  lockdownDatabases?: string[];
   pluginIds: string[];
   secret: string;
 }
@@ -36,9 +39,11 @@ export async function provisionStorage(options: ProvisionOptions): Promise<Provi
       for (const statement of plan) await sql.unsafe(statement); // provisionSql quotes what it interpolates
       provisioned.push(name);
     }
+    for (const database of options.lockdownDatabases ?? []) {
+      await sql.unsafe(`REVOKE CONNECT ON DATABASE ${quoteIdentifier(database)} FROM PUBLIC`);
+    }
     const existing = await sql<{ datname: string }[]>`SELECT datname FROM pg_database WHERE starts_with(datname, ${NAME_PREFIX})`;
-    const orphans = existing.map((row) => row.datname).filter((name) => !provisioned.includes(name)).sort();
+    return { orphans: orphanNames(existing.map((row) => row.datname), provisioned), provisioned };
-    return { orphans, provisioned };
   } finally {
     await sql.end();
   }

src/plugin-host/storage.test.ts

@@ -10,6 +10,7 @@ import {
   derivePassword,
   isValidStoragePluginId,
   MAX_STORAGE_PLUGIN_ID,
+  orphanNames,
   provisionSql,
   quoteIdentifier,
   quoteLiteral,
@@ -71,10 +72,17 @@ test("only the plugins that asked for storage are provisioned", () => {
   );
 });
 
+test("an orphan is a plugin_ database no installed plugin claims", () => {
+  const existing = ["plugin_gone", "plugin_here", "kratos", "ory"];
+  assert.deepEqual(orphanNames(existing, ["plugin_here"]), ["plugin_gone"]); // Ory's are not ours to report
+  assert.deepEqual(orphanNames(existing, ["plugin_here", "plugin_gone"]), []);
+});
+
 test("provisioning creates the role and the database when neither exists", () => {
   const plan = { connectionLimit: 10, databaseExists: false, name: "plugin_things", password: "pw", roleExists: false };
   assert.deepEqual(provisionSql(plan), [
     `CREATE ROLE "plugin_things" ${ATTRIBUTES} PASSWORD 'pw'`,
+    `GRANT "plugin_things" TO CURRENT_USER`, // else a CREATEROLE (non-superuser) account cannot own it
     `CREATE DATABASE "plugin_things" OWNER "plugin_things"`,
     `REVOKE ALL ON DATABASE "plugin_things" FROM PUBLIC`,
     `GRANT ALL PRIVILEGES ON DATABASE "plugin_things" TO "plugin_things"`,
@@ -163,3 +171,30 @@ test("provisions a database its plugin can use and a peer plugin cannot reach",
     await admin.end();
   }
 });
+
+// README tells an operator CREATEDB + CREATEROLE is enough and superuser is more than it needs.
+// That is a promise about their production credentials, so prove it rather than assert it.
+test("provisions through a CREATEDB + CREATEROLE account, without superuser", integration, async () => {
+  const pluginId = "storage-itest-lowpriv";
+  const provisioner = "storage-itest-provisioner";
+  const admin = postgres(ADMIN_URL, { connect_timeout: 10, max: 1, onnotice: () => {} });
+  try {
+    await admin.unsafe(`DROP ROLE IF EXISTS ${quoteIdentifier(provisioner)}`);
+    await admin.unsafe(`CREATE ROLE ${quoteIdentifier(provisioner)} LOGIN CREATEDB CREATEROLE PASSWORD 'itest-provisioner'`);
+    const asProvisioner = new URL(ADMIN_URL);
+    asProvisioner.username = provisioner;
+    asProvisioner.password = "itest-provisioner";
+    await provisionStorage({ adminUrl: asProvisioner.href, connectionLimit: 10, pluginIds: [pluginId], secret: SECRET });
+
+    const owner = buildCredentials(baseUrlOf(ADMIN_URL), pluginId, SECRET);
+    await queryAs(owner.url, "CREATE TABLE IF NOT EXISTS notes (body text)");
+    const rows = (await queryAs(owner.url, "SELECT 1 AS ok")) as { ok: number }[];
+    assert.deepEqual(rows.map((row) => row.ok), [1]); // the plugin owns and can use what it was given
+  } finally {
+    const name = quoteIdentifier(storageName(pluginId));
+    await admin.unsafe(`DROP DATABASE IF EXISTS ${name} WITH (FORCE)`);
+    await admin.unsafe(`DROP ROLE IF EXISTS ${name}`);
+    await admin.unsafe(`DROP ROLE IF EXISTS ${quoteIdentifier(provisioner)}`);
+    await admin.end();
+  }
+});

src/plugin-host/storage.ts

@@ -58,6 +58,12 @@ export function quoteLiteral(value: string): string {
   return `'${value.replaceAll("'", "''")}'`;
 }
 
+// A database this host provisioned once and no installed plugin claims any more. Nothing drops it,
+// so naming it is the only way an operator finds it again.
+export function orphanNames(existing: string[], provisioned: string[]): string[] {
+  return existing.filter((name) => name.startsWith(NAME_PREFIX) && !provisioned.includes(name)).sort();
+}
+
 export interface ProvisionPlan {
   connectionLimit: number;
   databaseExists: boolean;
@@ -78,7 +84,10 @@ export function provisionSql(plan: ProvisionPlan): string[] {
   const attributes = `LOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE CONNECTION LIMIT ${plan.connectionLimit} PASSWORD ${quoteLiteral(plan.password)}`;
   return [
     plan.roleExists ? `ALTER ROLE ${identifier} WITH ${attributes}` : `CREATE ROLE ${identifier} ${attributes}`,
-    ...(plan.databaseExists ? [] : [`CREATE DATABASE ${identifier} OWNER ${identifier}`]),
+    // CREATE DATABASE ... OWNER needs SET ROLE on the owner, and PG16+ gives a CREATEROLE account
+    // ADMIN but *not* SET on the roles it creates — so it grants itself membership first. A
+    // superuser could skip this; issuing it anyway is what keeps a least-privilege account working.
+    ...(plan.databaseExists ? [] : [`GRANT ${identifier} TO CURRENT_USER`, `CREATE DATABASE ${identifier} OWNER ${identifier}`]),
     `REVOKE ALL ON DATABASE ${identifier} FROM PUBLIC`,
     `GRANT ALL PRIVILEGES ON DATABASE ${identifier} TO ${identifier}`,
   ];

src/server.ts

@@ -16,6 +16,10 @@ import { loadMenuConfig } from "./ui/menu-config.ts";
 import { buildCredentials, storagePluginIds, type StorageCredentials } from "./plugin-host/storage.ts";
 
 const config = loadConfig(); // validates the env (incl. enforced secrets) — fails loud at boot
+// The storage secret is in `config` now, so drop it from the environment before ANY plugin code
+// runs: a plugin module's top level evaluates during discovery, long before onBoot. Defence in
+// depth, not a boundary (AGENTS.md) — and only ever move this line earlier, never later.
+delete process.env["PLUGIN_DB_SECRET"];
 // App-level logger: structured, OTLP-capable when OTLP_ENDPOINT is set. The hot path clones it
 // per request for access logging + a trace span (src/http/app.ts); console-only otherwise.
 const log = createLogger({ format: config.logFormat, level: config.logLevel, otlpEndpoint: config.otlpEndpoint, otlpProtocol: config.otlpProtocol, serviceName: config.serviceName });
@@ -53,14 +57,10 @@ if (declaresStorage.length > 0 && pluginDbUrl === undefined) {
   throw new Error(`config: PLUGIN_DB_URL must be set — these plugins declare storage: ${declaresStorage.join(", ")}`);
 }
 
-// Derive every plugin's credentials first, then drop the secret: onBoot is the window in which
-// plugin code could read it out of the environment and derive a *peer's* password. Order matters —
-// deleting after the hooks would protect nothing. Defence in depth, not a boundary (AGENTS.md).
 const storageCredentials = new Map<string, StorageCredentials>();
 if (pluginDbUrl !== undefined) {
   for (const id of declaresStorage) storageCredentials.set(id, buildCredentials(pluginDbUrl, id, config.pluginDbSecret));
 }
-delete process.env["PLUGIN_DB_SECRET"];
 
 // plugin onBoot — after discovery, before listen; a throw aborts boot.
 await runBootHooks(plugins, (plugin) => {