From ae8f105360b47096b9a503c0e332c1ce4e44849d Mon Sep 17 00:00:00 2001 From: lilleman Date: Wed, 19 Aug 2026 00:08:35 +0200 Subject: [PATCH] Confine the Postgres driver to bootstrap, bound plugin connections, and gate the storage DDL --- AGENTS.md | 33 ++++++--- README.md | 71 ++++++++++++------- ci.sh | 12 ++++ ory/postgres/init/init.sql | 7 ++ src/auth/bootstrap.ts | 92 +++++++++++++++---------- src/config.ts | 20 +++++- src/plugin-host/storage-provisioning.ts | 45 ++++++++++++ src/plugin-host/storage.test.ts | 44 +++++++++--- src/plugin-host/storage.ts | 67 +++++++----------- src/postgres.test.ts | 31 ++++++++- src/server.ts | 20 ++++-- 11 files changed, 313 insertions(+), 129 deletions(-) create mode 100644 src/plugin-host/storage-provisioning.ts diff --git a/AGENTS.md b/AGENTS.md index c3a6b14..a244c2c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,7 +39,7 @@ branch, create a PR and merge it when the CI/CD turns green. 2. **Few dependencies** — runtime deps stay minimal (today `ejs`, `lucide-static`, `@larvit/log`, `postgres`). Prefer the Node standard library; justify any new dependency; do not add frameworks. The **host is stateless — it owns no schema and stores nothing of its own**; a plugin may own a - Postgres database, which the host provisions and never reads or writes. Auth/identity/OAuth are + Postgres database, which the host provisions but never reads or writes inside. Auth/identity/OAuth are **Ory sidecar services** reached over their REST APIs with built-in `fetch` — no SDK. New capabilities ship as **plugin folders** under `plugins/` that get their data from an upstream service or their own database, not as core code. @@ -82,15 +82,28 @@ Revisit only if the stated reason stops holding. `node_modules`: two instances of the barrel break `instanceof` across the boundary, which `plugin-api.test.ts` guards by asserting both paths reach one module. - **Plugin storage hands over credentials, not a client** (README → Plugin storage). The host takes - `postgres` to run the provisioning DDL in `bootstrap` and nothing else: it is never re-exported - through the barrel, so no driver shape enters the contract and a plugin depends on whichever client - it likes. Three properties hold the design together, so don't trade one away in isolation: - passwords are `HMAC-SHA256(PLUGIN_DB_SECRET, id)` rather than stored, which is what keeps the host - stateless; the superuser DSN reaches `bootstrap` only, because plugin code runs inside `web` and - can read that process' environment (`src/compose.test.ts` guards the split); and provisioning - never drops anything, so uninstalling a plugin cannot destroy data. Because the host's copy sits in - the ambient `/node_modules`, a plugin can `import "postgres"` without declaring it — that is - incidental, not a packaging promise, and a plugin must still depend on its own driver. + `postgres` to run the provisioning DDL, and `storage-provisioning.ts` is the only module importing + it — `storage.ts` beside it stays pure so `web` never loads a driver (`src/postgres.test.ts` guards + both halves; the claim silently went false once already). It is never re-exported through the + barrel, so no driver shape enters the contract. Three properties hold the design together, so + don't trade one away in isolation: passwords are `HMAC-SHA256(PLUGIN_DB_SECRET, id)` rather than + stored, which is what keeps the host stateless — whoever holds that secret holds every plugin + database, so it ranks with the DB password itself; the provisioning DSN reaches `bootstrap` only + (`src/compose.test.ts` guards the split); and provisioning never drops anything, so uninstalling a + plugin cannot destroy data — boot logs the orphans instead. Because the host's copy sits in the + ambient `/node_modules`, a plugin can `import "postgres"` without declaring it — incidental, not a + packaging promise, and a plugin must still depend on its own driver. +- **The trust boundary is the `web` process, not the plugin.** Per-plugin databases and roles bound + *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 + and then deletes the secret from `process.env` — defence in depth, and the ordering is the whole + point, so don't move it. **Valid while plugins are operator-installed code, not third-party uploads.** +- **`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 + exit — revisit if a plugin ever needs an orderly drain. **Valid while the freeze holds.** - **`config/` is still a plain dir — no `package.json` of its own**, or `#menu-config` resolves against that instead and boot fails loud. An operator's menu override has no use for dependencies; if that changes, it needs the same package treatment. diff --git a/README.md b/README.md index a24ae77..2ae80e8 100644 --- a/README.md +++ b/README.md @@ -737,7 +737,7 @@ A plugin that needs to keep data sets `storage: true`. The host then provisions ```ts import postgres from "postgres"; // your dependency, not the host's -import { definePlugin, type StorageCredentials } from "@plainpages/plugin-api"; +import { definePlugin } from "@plainpages/plugin-api"; let sql: ReturnType; @@ -745,47 +745,63 @@ export default definePlugin({ apiVersion: "1.0.0", storage: true, hooks: { - onBoot: async (host) => { - if (!host.storage) throw new Error("things: storage was not provisioned"); - sql = postgres(host.storage.url); - await sql`CREATE TABLE IF NOT EXISTS things (id uuid PRIMARY KEY, name text NOT NULL)`; + onBoot: async (boot) => { + if (!boot.storage) throw new Error("things: storage was not provisioned"); + sql = postgres(boot.storage.url); + // Every web instance runs onBoot, and concurrent CREATE TABLE IF NOT EXISTS is an error in + // Postgres — the lock is released when the transaction ends. + await sql.begin(async (tx) => { + await tx`SELECT pg_advisory_xact_lock(hashtext('things:schema'))`; + await tx`CREATE TABLE IF NOT EXISTS things (id uuid PRIMARY KEY, name text NOT NULL)`; + }); }, }, }); ``` -`host.storage` is a `StorageCredentials` — `database`, `host`, `password`, `port`, `user`, and `url`, -the same values pre-assembled as a DSN, which most clients take directly. +`boot.storage` is a `StorageCredentials` — `database`, `host`, `password`, `port`, `user`, and `url`, +the same values pre-assembled as a DSN, which most clients take directly. It is typed optional, so +the guard above is expected of every storage plugin rather than a sign something is wrong. **Credentials, not a client.** The host has no opinion on how you reach Postgres: depend on `postgres`, `pg`, a query builder or an ORM ([Plugin dependencies](#plugin-dependencies)). No driver -is part of the contract, so upgrading yours is yours alone to time. +is part of the contract, so upgrading yours is yours alone to time. The flip side is that pool sizing +is yours too — keep yours under `PLUGIN_DB_CONNECTION_LIMIT` (default 10), the per-role ceiling the +host sets so one plugin cannot exhaust the Postgres this stack shares with Ory. -**The schema is yours.** The host creates the database empty and never reads or writes inside it. -Create your tables in `onBoot`: it runs before the server listens, so a failure aborts boot instead -of surfacing later as a broken page. +**The schema is yours, migrations included.** The host creates the database empty, never reads or +writes inside it, and ships no migration machinery — evolving your tables compatibly (expand, then +contract, so a rolled-back version still runs) is yours to own. Create your tables in `onBoot`: it +runs before the server listens, so a failure aborts boot instead of surfacing later as a broken page. What the host does guarantee: -- **One database and one role per plugin.** `CONNECT` is revoked from `PUBLIC`, so another installed - plugin's role cannot reach your database. +- **One database and one role per plugin**, with `CONNECT` revoked from `PUBLIC`. This bounds + *accidents* — a wrong database name, a mistyped DSN, a stray query — and it is not a security + boundary: plugins share the `web` process, so a plugin that goes looking can reach another's + credentials. Install plugins you trust ([Security model](#security-model)). - **Provisioning is idempotent and runs every boot**, so a plugin dropped in later is picked up by - the next `docker compose up -d` — the same rule as permission seeding. + the next `docker compose up -d` — the same rule as permission seeding. Role attributes are + re-applied each time, so a privilege granted by hand out of band does not quietly persist. - **Your data is never dropped.** Removing a plugin folder leaves its database untouched; deleting it - is a deliberate act by an operator. + is a deliberate act by an operator. Each boot logs any `plugin_*` database no installed plugin + claims, so what you left behind stays findable. **Passwords are derived, never stored** — each is `HMAC-SHA256(PLUGIN_DB_SECRET, )`, so `bootstrap` and `web` compute the same value independently and nothing has to be written down. Rotate every plugin's password by changing `PLUGIN_DB_SECRET` and running `docker compose up -d`, -which re-applies each role's password and leaves the data alone. Under `REQUIRE_SECURE_SECRETS` a -missing, empty or throwaway secret is refused — in `bootstrap` before it creates any role, so no -database is ever given a password derivable from a constant in this repo. +which re-applies each role's password and leaves the data alone — restart every `web` instance as +part of it, since one still holding the old secret can open no new connections. Treat the secret as +you would a database password: whoever holds it holds every plugin database. Under +`REQUIRE_SECURE_SECRETS` a missing, empty or throwaway secret is refused — in `bootstrap` before it +creates any role, so no database is ever given a password derivable from a constant in this repo. -**Only `bootstrap` holds superuser credentials.** It alone gets `PLUGIN_DB_ADMIN_URL`, the DSN that -may `CREATE DATABASE`/`CREATE ROLE`. `web` gets `PLUGIN_DB_URL`, which names the server and carries -no credentials — so plugin code, which runs inside `web`, cannot read a superuser password out of its -own environment. Set both, plus `PLUGIN_DB_SECRET` ([Configuration](#configuration)); the dev stack -sets them for you. +**Only `bootstrap` holds provisioning credentials.** It alone gets `PLUGIN_DB_ADMIN_URL`, an account +with `CREATEDB` and `CREATEROLE` (superuser works but is more than it needs; the dev stack simply +reuses Ory's). `web` gets `PLUGIN_DB_URL`, which names the server and must carry no credentials — +supply one with a username or password and boot fails, rather than leaving a privileged password in +the process that runs plugin code. Set both, plus `PLUGIN_DB_SECRET` +([Configuration](#configuration)); the dev stack sets them for you. Storage stays off until `PLUGIN_DB_URL` is set, and a plugin declaring it while that is unset **aborts boot** naming itself — rather than serving pages without its data. One naming limit: a @@ -1023,6 +1039,7 @@ The app is **environment-agnostic**: no `NODE_ENV`, every behaviour its own expl | `PLUGIN_DB_URL` | _unset_ (dev: `postgres://postgres:5432`) | credential-free Postgres base URL for [plugin storage](#plugin-storage); unset ⇒ storage off, and a plugin declaring it aborts boot | | `PLUGIN_DB_ADMIN_URL` | _unset_ (dev: the bundled superuser) | the DSN that provisions each plugin's database and role — read by the one-shot `bootstrap` service **only**, never by `web` | | `PLUGIN_DB_SECRET` | dev throwaway | derives each plugin's database password; `REQUIRE_SECURE_SECRETS` enforces it in `web` once `PLUGIN_DB_URL` is set, and in `bootstrap` whenever a plugin declares storage | +| `PLUGIN_DB_CONNECTION_LIMIT` | `10` | per-role Postgres connection ceiling, so one plugin's pools cannot exhaust the server Ory shares; read by `bootstrap` when provisioning | ### Canonical host (one public URL) @@ -1445,6 +1462,11 @@ the one-shot bootstrap) — and mounts no source. Secrets come from the environm running insecure. Before going live, supply the production secrets and any SSO credentials — the **only** manual prep ([What you must supply](#what-you-must-supply-the-only-manual-prep)). +**Back up the `pgdata` volume.** Once a plugin declares [storage](#plugin-storage), Postgres holds +business data that exists nowhere else, alongside Ory's identities — the stack stops being +reproducible from the image and config alone. Snapshot the volume, or `pg_dump` each database on a +schedule, and rehearse the restore. + Every response carries security headers (`src/http/security-headers.ts`): a strict `Content-Security-Policy` (the core is zero-JS — `script-src 'self'`, no inline scripts), `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY` + `frame-ancestors 'none'`, @@ -1595,7 +1617,8 @@ src/ The app — strict tsc, no build step. *.test.ts sit beside i18n/ catalog (parity rules) · locale (resolution) · translate · load · runtime · english · view-locals · locales/ (the core en-US + sv-SE catalogs) plugin-host/ plugin.ts (the contract) · plugin-api.ts (the `@plainpages/plugin-api` barrel) · system.ts - (ctx.system) · discovery · router · hooks · view-resolver + (ctx.system) · discovery · router · hooks · view-resolver · storage (the rules) · + storage-provisioning (the DDL; bootstrap-only, holds the driver) ui/ chrome (the one global menu) · shell-context · dashboard · nav (composeNav) · menu-config (`#menu-config`) · icons (lucide sprite builder) · list-query · paginate diff --git a/ci.sh b/ci.sh index e9782d1..41ab585 100755 --- a/ci.sh +++ b/ci.sh @@ -60,6 +60,18 @@ echo "$units" | grep -E '^. (tests|pass|fail) ' || true count=$(echo "$units" | grep -oE 'tests [0-9]+' | grep -oE '[0-9]+' | head -1 || true) [ "${count:-0}" -ge 50 ] || { echo "only ${count:-0} unit tests ran — test glob broken?"; exit 1; } +# Plugin storage against a real Postgres. The step above runs --no-deps, so this suite's integration +# 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)" +storage_rc=0 +docker compose up -d postgres >/dev/null +docker compose 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=$? +docker compose down -v >/dev/null 2>&1 || true +[ "$storage_rc" -eq 0 ] || { echo "plugin storage integration tests failed (exit $storage_rc)"; exit "$storage_rc"; } + # 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. # --user: the runner writes screenshots + the report into the checkout, so they must belong to diff --git a/ory/postgres/init/init.sql b/ory/postgres/init/init.sql index 83cd64b..cbf7114 100644 --- a/ory/postgres/init/init.sql +++ b/ory/postgres/init/init.sql @@ -5,3 +5,10 @@ CREATE DATABASE kratos; CREATE DATABASE keto; CREATE DATABASE hydra; + +-- Postgres grants CONNECT to PUBLIC by default, so every plugin role would otherwise reach the auth +-- plane: table data stays protected, but pg_catalog and the connection slots do not. Ory connects as +-- the POSTGRES_USER, which owns these and keeps its access. +REVOKE CONNECT ON DATABASE kratos FROM PUBLIC; +REVOKE CONNECT ON DATABASE keto FROM PUBLIC; +REVOKE CONNECT ON DATABASE hydra FROM PUBLIC; diff --git a/src/auth/bootstrap.ts b/src/auth/bootstrap.ts index b65389d..0a96481 100644 --- a/src/auth/bootstrap.ts +++ b/src/auth/bootstrap.ts @@ -8,12 +8,15 @@ // Then prints a first-run banner; fails loud on any unexpected upstream error. import { existsSync, writeFileSync } from "node:fs"; import { fileURLToPath } from "node:url"; -import { resolvePluginDbSecret } from "../config.ts"; +import { resolvePluginDbConnectionLimit, resolvePluginDbSecret } from "../config.ts"; import { discoverPlugins } from "../plugin-host/discovery.ts"; -import { declaredPermissions, isValidPermissionName } from "../plugin-host/plugin.ts"; -import { provisionStorage } from "../plugin-host/storage.ts"; +import { declaredPermissions, isValidPermissionName, type Plugin } from "../plugin-host/plugin.ts"; +import { provisionStorage } from "../plugin-host/storage-provisioning.ts"; +import { storagePluginIds } from "../plugin-host/storage.ts"; import { generateJwks, type JwkSet } from "./gen-jwks.ts"; -import { createLogger, runWithLog, tracedFetch } from "../logger.ts"; +import { createLogger, runWithLog, tracedFetch, type Log } from "../logger.ts"; + +type Env = Record; // --- Pure payload builders (the Kratos/Keto request contracts) ----------------------- @@ -152,42 +155,57 @@ async function main() { // runWithLog makes `log` ambient so seedAdmin's tracedFetch traces the Kratos/Keto seed calls. await runWithLog(log, async () => { if (ensureJwks(env["JWKS_FILE"] ?? "/etc/config/kratos/tokenizer/jwks.json")) log.info("generated a JWKS signing key"); - const plugins = await discoverPlugins(); - - // A database and login role for each plugin that asked for one. It happens here because - // bootstrap holds the stack's only superuser credentials — web derives the same password from - // PLUGIN_DB_SECRET and connects as the plugin's own role, never as a superuser. - const storagePlugins = plugins.filter((plugin) => plugin.storage).map((plugin) => plugin.id); - if (storagePlugins.length > 0) { - const adminUrl = env["PLUGIN_DB_ADMIN_URL"]; - if (!adminUrl) throw new Error(`bootstrap: PLUGIN_DB_ADMIN_URL must be set — these plugins declare storage: ${storagePlugins.join(", ")}`); - const provisioned = await provisionStorage({ adminUrl, pluginIds: storagePlugins, secret: resolvePluginDbSecret(env) }); - log.info("plugin storage provisioned", { databases: provisioned.join(", ") }); - } - - // Seed every discovered plugin's declared permission names (plus any ADMIN_PERMISSIONS), so the - // shipped example — and any dropped-in plugin — works for the demo admin without a host edit. - const declared = declaredPermissions(plugins).map((decl) => decl.name); - const { ignored, permissions } = seedPermissions(env["ADMIN_PERMISSIONS"], declared); - if (ignored.length > 0) { - log.warn("ignoring ADMIN_PERMISSIONS entries that are not :", { ignored: ignored.join(", ") }); - } - const email = env["ADMIN_EMAIL"] ?? "admin@plainpages.local"; - const password = env["ADMIN_PASSWORD"] ?? "admin"; - const result = await seedAdmin({ - email, - fetchImpl: tracedFetch, - ketoWriteUrl: env["KETO_WRITE_URL"] ?? "http://keto:4467", - kratosAdminUrl: env["KRATOS_ADMIN_URL"] ?? "http://kratos:4434", - password, - permissions, - }); - log.info("admin seeded", { created: result.created, id: result.id, permissions: result.permissions.join(", ") }); - // The banner is human-facing UX (the first-run "you're ready" block), not a log event — print raw. - console.log(firstRunBanner({ appUrl: env["APP_URL"] ?? "http://localhost:3000", email, password })); + await provisionPluginStorage(env, plugins, log); + await seedAdminAndPermissions(env, plugins, log); }); await log.end(); // flush any pending OTLP spans/logs before the one-shot exits } +// A database and login role for each plugin that asked for one. It happens here because bootstrap +// holds the stack's only provisioning credentials — web derives the same password and connects as +// the plugin's own role. +async function provisionPluginStorage(env: Env, plugins: Plugin[], log: Log): Promise { + const ids = storagePluginIds(plugins); + const adminUrl = env["PLUGIN_DB_ADMIN_URL"]; + // Still connect with nothing to provision, as long as storage is configured: uninstalling the + // last storage plugin is exactly when an orphaned database needs naming. + if (ids.length === 0 && !adminUrl) return; + if (!adminUrl) throw new Error(`bootstrap: PLUGIN_DB_ADMIN_URL must be set — these plugins declare storage: ${ids.join(", ")}`); + const result = await provisionStorage({ + adminUrl, + connectionLimit: resolvePluginDbConnectionLimit(env), + pluginIds: ids, + secret: resolvePluginDbSecret(env), + }); + if (result.provisioned.length > 0) log.info("plugin storage provisioned", { databases: result.provisioned.join(", ") }); + // Never dropped, so an uninstalled plugin's data outlives it — say so, or nobody can find it. + if (result.orphans.length > 0) { + log.warn("plugin databases no installed plugin claims", { databases: result.orphans.join(", ") }); + } +} + +// Seed every discovered plugin's declared permission names (plus any ADMIN_PERMISSIONS), so the +// shipped example — and any dropped-in plugin — works for the demo admin without a host edit. +async function seedAdminAndPermissions(env: Env, plugins: Plugin[], log: Log): Promise { + const declared = declaredPermissions(plugins).map((decl) => decl.name); + const { ignored, permissions } = seedPermissions(env["ADMIN_PERMISSIONS"], declared); + if (ignored.length > 0) { + log.warn("ignoring ADMIN_PERMISSIONS entries that are not :", { ignored: ignored.join(", ") }); + } + const email = env["ADMIN_EMAIL"] ?? "admin@plainpages.local"; + const password = env["ADMIN_PASSWORD"] ?? "admin"; + const result = await seedAdmin({ + email, + fetchImpl: tracedFetch, + ketoWriteUrl: env["KETO_WRITE_URL"] ?? "http://keto:4467", + kratosAdminUrl: env["KRATOS_ADMIN_URL"] ?? "http://kratos:4434", + password, + permissions, + }); + log.info("admin seeded", { created: result.created, id: result.id, permissions: result.permissions.join(", ") }); + // The banner is human-facing UX (the first-run "you're ready" block), not a log event — print raw. + console.log(firstRunBanner({ appUrl: env["APP_URL"] ?? "http://localhost:3000", email, password })); +} + if (process.argv[1] === fileURLToPath(import.meta.url)) await main(); diff --git a/src/config.ts b/src/config.ts index 2570a27..c46f622 100644 --- a/src/config.ts +++ b/src/config.ts @@ -17,6 +17,24 @@ export function resolvePluginDbSecret(env: Env, enforce?: boolean): string { return readSecret(env, "PLUGIN_DB_SECRET", DEV_PLUGIN_DB_SECRET, enforce ?? readBool(env, "REQUIRE_SECURE_SECRETS", false)); } +// Only bootstrap provisions, so only bootstrap reads this; env still gets read in one place. +export function resolvePluginDbConnectionLimit(env: Env): number { + return readPosInt(env, "PLUGIN_DB_CONNECTION_LIMIT", 10); +} + +// PLUGIN_DB_URL is web's, and web must never hold credentials that outrank a plugin's own role. +// Pasting the admin DSN here would otherwise work — buildCredentials overwrites the userinfo — and +// leave a superuser password in the environment plugin code can read. +function readCredentiallessUrl(env: Env, key: string): string | undefined { + const value = readOptionalUrl(env, key); + if (value === undefined) return undefined; + const url = new URL(value); + if (url.username || url.password) { + throw new Error(`config: ${key} must carry no username or password — each plugin connects as its own role`); + } + return value; +} + export interface Config { appUrl: string | undefined; // canonical public URL; set ⇒ off-host visitors are redirected here. Unset ⇒ no redirect (explicit toggle) cacheTemplates: boolean; @@ -168,7 +186,7 @@ export function loadConfig(env: Env = process.env): Config { // out of web's environment. Unset ⇒ storage is off and a plugin declaring it fails loud at boot, // which is also why the secret is only enforced once a URL is configured. pluginDbSecret: resolvePluginDbSecret(env, requireSecure && Boolean(env["PLUGIN_DB_URL"])), - pluginDbUrl: readOptionalUrl(env, "PLUGIN_DB_URL"), + pluginDbUrl: readCredentiallessUrl(env, "PLUGIN_DB_URL"), port: readPort(env), // Optional instant-revoke, off by default. When on, an admin deactivate/delete or permission // change revokes the subject's live tokens at once; the entry lives ttl seconds (≥ the 10m diff --git a/src/plugin-host/storage-provisioning.ts b/src/plugin-host/storage-provisioning.ts new file mode 100644 index 0000000..8c39f6e --- /dev/null +++ b/src/plugin-host/storage-provisioning.ts @@ -0,0 +1,45 @@ +// The connecting half of plugin storage: runs the DDL storage.ts plans. Imported by bootstrap +// 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"; + +export interface ProvisionOptions { + adminUrl: string; // needs CREATEDB + CREATEROLE, not superuser + connectionLimit: number; + pluginIds: string[]; + secret: string; +} + +export interface ProvisionResult { + orphans: string[]; // provisioned once, but no installed plugin claims them any more + provisioned: string[]; +} + +// Idempotent, and it drops nothing: an uninstalled plugin keeps its data until an operator removes +// it deliberately. Orphans are reported rather than removed, so nobody has to guess they exist. +export async function provisionStorage(options: ProvisionOptions): Promise { + const sql = postgres(options.adminUrl, { connect_timeout: 10, max: 1, onnotice: () => {} }); + try { + const provisioned: string[] = []; + for (const pluginId of options.pluginIds) { + const name = storageName(pluginId); + const [role] = await sql`SELECT 1 FROM pg_roles WHERE rolname = ${name}`; + const [database] = await sql`SELECT 1 FROM pg_database WHERE datname = ${name}`; + const plan = provisionSql({ + connectionLimit: options.connectionLimit, + databaseExists: database !== undefined, + name, + password: derivePassword(options.secret, pluginId), + roleExists: role !== undefined, + }); + for (const statement of plan) await sql.unsafe(statement); // provisionSql quotes what it interpolates + provisioned.push(name); + } + 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, provisioned }; + } finally { + await sql.end(); + } +} diff --git a/src/plugin-host/storage.test.ts b/src/plugin-host/storage.test.ts index 8ff9fa4..f0c78e5 100644 --- a/src/plugin-host/storage.test.ts +++ b/src/plugin-host/storage.test.ts @@ -4,16 +4,17 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import postgres from "postgres"; +import { provisionStorage } from "./storage-provisioning.ts"; import { buildCredentials, derivePassword, isValidStoragePluginId, MAX_STORAGE_PLUGIN_ID, provisionSql, - provisionStorage, quoteIdentifier, quoteLiteral, storageName, + storagePluginIds, } from "./storage.ts"; const SECRET = "a-test-secret"; @@ -61,23 +62,42 @@ test("quoting doubles an embedded quote", () => { assert.equal(quoteLiteral("we'ird"), "'we''ird'"); }); +const ATTRIBUTES = "LOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE CONNECTION LIMIT 10"; + +test("only the plugins that asked for storage are provisioned", () => { + assert.deepEqual( + storagePluginIds([{ apiVersion: "1.0.0", id: "a", storage: true }, { apiVersion: "1.0.0", id: "b" }, { apiVersion: "1.0.0", id: "c", storage: true }]), + ["a", "c"], + ); +}); + test("provisioning creates the role and the database when neither exists", () => { - assert.deepEqual(provisionSql("plugin_things", "pw", { databaseExists: false, roleExists: false }), [ - `CREATE ROLE "plugin_things" LOGIN PASSWORD 'pw'`, + const plan = { connectionLimit: 10, databaseExists: false, name: "plugin_things", password: "pw", roleExists: false }; + assert.deepEqual(provisionSql(plan), [ + `CREATE ROLE "plugin_things" ${ATTRIBUTES} PASSWORD 'pw'`, `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"`, ]); }); -test("re-provisioning re-sets the password and creates nothing twice", () => { - assert.deepEqual(provisionSql("plugin_things", "rotated", { databaseExists: true, roleExists: true }), [ - `ALTER ROLE "plugin_things" WITH LOGIN PASSWORD 'rotated'`, +// Re-asserting the attributes, not just the password, is what makes "idempotent" mean the role +// cannot drift — a CREATEDB granted by hand out of band is taken back on the next boot. +test("re-provisioning re-asserts every attribute and creates nothing twice", () => { + const plan = { connectionLimit: 10, databaseExists: true, name: "plugin_things", password: "rotated", roleExists: true }; + assert.deepEqual(provisionSql(plan), [ + `ALTER ROLE "plugin_things" WITH ${ATTRIBUTES} PASSWORD 'rotated'`, `REVOKE ALL ON DATABASE "plugin_things" FROM PUBLIC`, `GRANT ALL PRIVILEGES ON DATABASE "plugin_things" TO "plugin_things"`, ]); }); +// The limit is interpolated unquoted, so a non-integer would corrupt the statement text. +test("a non-integer connection limit is refused, not interpolated", () => { + const plan = { connectionLimit: 1.5, databaseExists: false, name: "plugin_things", password: "pw", roleExists: false }; + assert.throws(() => provisionSql(plan), /connectionLimit must be an integer/); +}); + // --- Integration: the statements above, against a real Postgres ----------------------- // Opt-in via PLUGIN_DB_ADMIN_URL (a superuser DSN); the unit gate runs no Postgres. What the unit // tests cannot prove lives here: the owner may create tables, and a peer role is locked out. @@ -107,7 +127,7 @@ test("provisions a database its plugin can use and a peer plugin cannot reach", const base = baseUrlOf(ADMIN_URL); const admin = postgres(ADMIN_URL, { connect_timeout: 10, max: 1, onnotice: () => {} }); try { - await provisionStorage({ adminUrl: ADMIN_URL, pluginIds: ids, secret: SECRET }); + await provisionStorage({ adminUrl: ADMIN_URL, connectionLimit: 10, pluginIds: ids, secret: SECRET }); const owner = buildCredentials(base, "storage-itest-a", SECRET); await queryAs(owner.url, "CREATE TABLE IF NOT EXISTS notes (body text)"); @@ -121,11 +141,19 @@ test("provisions a database its plugin can use and a peer plugin cannot reach", await assert.rejects(queryAs(peer.href, "SELECT 1"), /permission denied|not permitted/i); // Re-running is idempotent, and a rotated secret lands on the existing role. - await provisionStorage({ adminUrl: ADMIN_URL, pluginIds: ids, secret: "a-rotated-secret" }); + const rerun = await provisionStorage({ adminUrl: ADMIN_URL, connectionLimit: 10, pluginIds: ids, secret: "a-rotated-secret" }); + // Scoped to this test's own ids: another plugin's database on the same server is not this + // test's business, and asserting otherwise would make the suite order-dependent. + for (const id of ids) assert.ok(!rerun.orphans.includes(storageName(id)), `${id} is still installed`); const rotated = buildCredentials(base, "storage-itest-a", "a-rotated-secret"); const kept = (await queryAs(rotated.url, "SELECT body FROM notes")) as { body: string }[]; assert.deepEqual(kept.map((row) => row.body), ["persisted"]); // rotating the secret keeps the data await assert.rejects(queryAs(owner.url, "SELECT 1"), /password authentication failed/i); + + // Uninstalling drops nothing, so what is left behind must be named — including when the LAST + // storage plugin goes and there is nothing left to provision. + const uninstalled = await provisionStorage({ adminUrl: ADMIN_URL, connectionLimit: 10, pluginIds: [], secret: "a-rotated-secret" }); + for (const id of ids) assert.ok(uninstalled.orphans.includes(storageName(id)), `${id}'s database is reported`); } finally { for (const id of ids) { const name = quoteIdentifier(storageName(id)); diff --git a/src/plugin-host/storage.ts b/src/plugin-host/storage.ts index 6847e0d..485d0fe 100644 --- a/src/plugin-host/storage.ts +++ b/src/plugin-host/storage.ts @@ -1,12 +1,12 @@ -// Per-plugin Postgres storage (README → Plugin storage). Only bootstrap provisions, because only it -// is given superuser credentials; web derives the same passwords and never sees them. +// Per-plugin Postgres storage: the naming, credential and DDL rules (README → Plugin storage). +// Pure — the connecting half lives in storage-provisioning.ts, so `web` never loads a driver. import { createHmac } from "node:crypto"; -import postgres from "postgres"; +import type { Plugin } from "./plugin.ts"; // Database and role share one name, so reconnecting needs nothing looked up. The prefix also keeps // a plugin id from ever naming an Ory database. -const NAME_PREFIX = "plugin_"; +export const NAME_PREFIX = "plugin_"; // Postgres truncates an identifier at 63 bytes, which would silently collide two long ids. export const MAX_STORAGE_PLUGIN_ID = 63 - NAME_PREFIX.length; @@ -29,6 +29,10 @@ export function isValidStoragePluginId(pluginId: string): boolean { return pluginId.length <= MAX_STORAGE_PLUGIN_ID; } +export function storagePluginIds(plugins: Plugin[]): string[] { + return plugins.filter((plugin) => plugin.storage).map((plugin) => plugin.id); +} + // Derived, never stored — which is what keeps the host free of state it would have to persist. export function derivePassword(secret: string, pluginId: string): string { return createHmac("sha256", secret).update(pluginId).digest("base64url"); @@ -54,51 +58,28 @@ export function quoteLiteral(value: string): string { return `'${value.replaceAll("'", "''")}'`; } -export interface ProvisionState { +export interface ProvisionPlan { + connectionLimit: number; databaseExists: boolean; + name: string; + password: string; roleExists: boolean; } -// One plugin's full plan, in order. The password is re-set on every run so rotating the secret needs -// no separate step, and PUBLIC loses CONNECT so no other plugin's role can reach this database. -export function provisionSql(name: string, password: string, state: ProvisionState): string[] { - const identifier = quoteIdentifier(name); - const secret = quoteLiteral(password); +// One plugin's full plan, in order. Both role branches state the same attributes, so every boot +// re-asserts them: a privilege granted by hand out of band does not survive silently. +export function provisionSql(plan: ProvisionPlan): string[] { + if (!Number.isSafeInteger(plan.connectionLimit)) { + throw new Error(`storage: connectionLimit must be an integer, got ${plan.connectionLimit}`); // interpolated unquoted + } + const identifier = quoteIdentifier(plan.name); + // NOSUPERUSER/NOCREATEDB/NOCREATEROLE: a plugin owns its own database and nothing beyond it. + // The limit bounds one plugin's pools so they cannot starve Ory, which shares this server. + const attributes = `LOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE CONNECTION LIMIT ${plan.connectionLimit} PASSWORD ${quoteLiteral(plan.password)}`; return [ - state.roleExists - ? `ALTER ROLE ${identifier} WITH LOGIN PASSWORD ${secret}` - : `CREATE ROLE ${identifier} LOGIN PASSWORD ${secret}`, - ...(state.databaseExists ? [] : [`CREATE DATABASE ${identifier} OWNER ${identifier}`]), + plan.roleExists ? `ALTER ROLE ${identifier} WITH ${attributes}` : `CREATE ROLE ${identifier} ${attributes}`, + ...(plan.databaseExists ? [] : [`CREATE DATABASE ${identifier} OWNER ${identifier}`]), `REVOKE ALL ON DATABASE ${identifier} FROM PUBLIC`, `GRANT ALL PRIVILEGES ON DATABASE ${identifier} TO ${identifier}`, ]; } - -export interface ProvisionOptions { - adminUrl: string; // superuser DSN — the rights to create a database and a role - pluginIds: string[]; - secret: string; -} - -// Idempotent, and it drops nothing: an uninstalled plugin keeps its data until an operator removes -// it deliberately. -export async function provisionStorage(options: ProvisionOptions): Promise { - const sql = postgres(options.adminUrl, { connect_timeout: 10, max: 1, onnotice: () => {} }); - try { - const names: string[] = []; - for (const pluginId of options.pluginIds) { - const name = storageName(pluginId); - const [role] = await sql`SELECT 1 FROM pg_roles WHERE rolname = ${name}`; - const [database] = await sql`SELECT 1 FROM pg_database WHERE datname = ${name}`; - const plan = provisionSql(name, derivePassword(options.secret, pluginId), { - databaseExists: database !== undefined, - roleExists: role !== undefined, - }); - for (const statement of plan) await sql.unsafe(statement); // provisionSql quotes what it interpolates - names.push(name); - } - return names; - } finally { - await sql.end(); - } -} diff --git a/src/postgres.test.ts b/src/postgres.test.ts index 0cbc543..ffa2c23 100644 --- a/src/postgres.test.ts +++ b/src/postgres.test.ts @@ -3,11 +3,20 @@ // verified by booting postgres in CI/e2e; this catches edits. import { test } from "node:test"; import assert from "node:assert/strict"; -import { readFileSync } from "node:fs"; +import { readdirSync, readFileSync } from "node:fs"; const read = (p: string) => readFileSync(new URL(`../${p}`, import.meta.url), "utf8"); const ORY_DATABASES = ["hydra", "keto", "kratos"]; // one DB per Ory service +function sourceFiles(dir = "src"): string[] { + const out: string[] = []; + for (const entry of readdirSync(new URL(`../${dir}/`, import.meta.url), { withFileTypes: true })) { + if (entry.isDirectory()) out.push(...sourceFiles(`${dir}/${entry.name}`)); + else if (entry.name.endsWith(".ts")) out.push(`${dir}/${entry.name}`); + } + return out.sort(); +} + test("init SQL gives each Ory service its own database, and leaves plugin databases to bootstrap", () => { const sql = read("ory/postgres/init/init.sql"); for (const db of ORY_DATABASES) { @@ -16,4 +25,24 @@ test("init SQL gives each Ory service its own database, and leaves plugin databa // This file runs once, on an empty data dir — a plugin database added here would never appear for // a plugin dropped in later. bootstrap provisions them on every boot instead. assert.doesNotMatch(sql, /plugin_/i, "no plugin database is seeded here"); + // PUBLIC keeps CONNECT unless it is revoked, which would put every plugin role on the auth plane. + for (const db of ORY_DATABASES) { + assert.match(sql, new RegExp(`REVOKE CONNECT ON DATABASE ${db} FROM PUBLIC`, "i"), `${db} is closed to PUBLIC`); + } +}); + +// AGENTS.md records that the driver runs the provisioning DDL in bootstrap "and nothing else". +// discovery.ts once imported a validator from the module that held it, quietly putting the driver in +// web's graph — the claim outlived the fact, which is what this catches. +test("the Postgres driver reaches bootstrap only, never web's import graph", () => { + const files = sourceFiles(); + assert.ok(files.length > 40, "walks the source tree"); + assert.deepEqual( + files.filter((f) => /^import .*"postgres"/m.test(read(f))), // an import line, not a mention of one + ["src/plugin-host/storage-provisioning.ts", "src/plugin-host/storage.test.ts"], + ); + assert.deepEqual( + files.filter((f) => !f.endsWith(".test.ts") && /from "[^"]*storage-provisioning\.ts"/.test(read(f))), + ["src/auth/bootstrap.ts"], + ); }); diff --git a/src/server.ts b/src/server.ts index 960284d..ea73d98 100644 --- a/src/server.ts +++ b/src/server.ts @@ -13,7 +13,7 @@ import { createKratosAdmin } from "./auth/kratos-admin.ts"; import { createKratosPublic } from "./auth/kratos-public.ts"; import { createLogger, tracedFetch } from "./logger.ts"; import { loadMenuConfig } from "./ui/menu-config.ts"; -import { buildCredentials } from "./plugin-host/storage.ts"; +import { buildCredentials, storagePluginIds, type StorageCredentials } from "./plugin-host/storage.ts"; const config = loadConfig(); // validates the env (incl. enforced secrets) — fails loud at boot // App-level logger: structured, OTLP-capable when OTLP_ENDPOINT is set. The hot path clones it @@ -48,15 +48,25 @@ log.info("locales loaded", { locales: i18n.available.join(", ") }); // A plugin's database credentials are derived, never stored — so the only thing that can be missing // is the server itself. Refuse at boot rather than at that plugin's first query, hours later. const pluginDbUrl = config.pluginDbUrl; -const declaresStorage = plugins.filter((plugin) => plugin.storage).map((plugin) => plugin.id); +const declaresStorage = storagePluginIds(plugins); 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(); +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) => - plugin.storage && pluginDbUrl !== undefined ? { storage: buildCredentials(pluginDbUrl, plugin.id, config.pluginDbSecret) } : {}, -); +await runBootHooks(plugins, (plugin) => { + const storage = storageCredentials.get(plugin.id); + return storage ? { storage } : {}; +}); const server = createApp({ // Canonical-host redirect target (off-host GET/HEAD visitors are sent here). Opt-in: omitted unless