Warn rather than refuse on a storage URL mismatch, and scrub the provisioning DSN before discovery
CI / full-gate (push) Successful in 2m59s

This commit is contained in:
2026-08-19 01:03:40 +02:00
parent c5c9cce2b6
commit 47541ae97b
11 changed files with 52 additions and 37 deletions
+8 -9
View File
@@ -84,8 +84,8 @@ Revisit only if the stated reason stops holding.
- **Plugin storage hands over credentials, not a client** (README → Plugin storage). The host takes - **Plugin storage hands over credentials, not a client** (README → Plugin storage). The host takes
`postgres` to run the provisioning DDL, and `storage-provisioning.ts` is the only module importing `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 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 both halves, because one value imported from the wrong module breaks it invisibly). It is never
barrel, so no driver shape enters the contract. Three properties hold the design together, so 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 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 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 database, so it ranks with the DB password itself; the provisioning DSN reaches `bootstrap` only
@@ -101,13 +101,12 @@ Revisit only if the stated reason stops holding.
after `loadConfig`, which is before discovery imports any plugin module — the ordering is the whole after `loadConfig`, which is before discovery imports any plugin module — the ordering is the whole
point, so move it earlier if anything, **never later**. **Valid while plugins are point, so move it earlier if anything, **never later**. **Valid while plugins are
operator-installed code, not third-party uploads.** operator-installed code, not third-party uploads.**
- **`ory/postgres/init/init.sql` is the only home for the Ory databases' ACL.** Re-asserting the - **`ory/postgres/init/init.sql` is the only home for the Ory databases' ACL** — don't re-assert the
`REVOKE CONNECT` from `bootstrap` each boot was tried and removed: `REVOKE` only *warns* when the `REVOKE CONNECT` from `bootstrap`. `REVOKE` only *warns* when the caller doesn't own the database,
caller doesn't own the database, so under the least-privilege provisioning account the README so under the least-privilege provisioning account the README recommends it would report success
recommends it reported success while changing nothing and it hard-failed whenever while changing nothing, and it hard-fails whenever `PLUGIN_DB_ADMIN_URL` names a server with no
`PLUGIN_DB_ADMIN_URL` named a server with no `kratos`. A volume created before that file gained the `kratos`. A volume created before that file gained the revokes keeps the default grant;
revokes keeps the default grant; `docker compose down -v` is the dev remedy. **Valid while `docker compose down -v` is the dev remedy. **Valid while pre-release, with no deployed volumes.**
pre-release, with no deployed volumes to migrate.**
- **`bootstrap.ts` stays under `src/auth/`** even though it now provisions plugin databases as well - **`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 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 `src/bootstrap.ts` would edit `compose.yml`, five e2e compose files and `src/compose.test.ts` for a
+4 -3
View File
@@ -781,11 +781,12 @@ What the host does guarantee:
boundary: plugins share the `web` process, so a plugin that goes looking can reach another's 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)). 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 - **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. Role attributes are the next `docker compose up -d` — the same rule as permission seeding. Each boot re-applies the
re-applied each time, so a privilege granted by hand out of band does not quietly persist. role's password, connection limit, and `NOCREATEDB`/`NOCREATEROLE`.
- **Your data is never dropped.** Removing a plugin folder leaves its database untouched; deleting it - **Your data is never dropped.** Removing a plugin folder leaves its database untouched; deleting it
is a deliberate act by an operator. Each boot logs any `plugin_*` database no installed plugin is a deliberate act by an operator. Each boot logs any `plugin_*` database no installed plugin
claims, so what you left behind stays findable. claims, so what you left behind stays findable — read that list before dropping anything, since a
second Plainpages stack sharing this server will have its databases named there too.
**Passwords are derived, never stored** — each is `HMAC-SHA256(PLUGIN_DB_SECRET, <plugin id>)`, so **Passwords are derived, never stored** — each is `HMAC-SHA256(PLUGIN_DB_SECRET, <plugin id>)`, so
`bootstrap` and `web` compute the same value independently and nothing has to be written down. `bootstrap` and `web` compute the same value independently and nothing has to be written down.
+6 -2
View File
@@ -1,5 +1,9 @@
# Development overrides, merged automatically by `docker compose up`. # Development overrides, merged automatically by `docker compose up`.
# Mounts the source for live editing and restarts on change via `node --watch`. # Mounts the source for live editing and restarts on change via `node --watch`.
# web connects with it and bootstrap provisions against it, so the two must agree — one home.
x-plugin-db-url: &plugin-db-url postgres://postgres:5432
services: services:
web: web:
command: node --watch src/server.ts command: node --watch src/server.ts
@@ -15,7 +19,7 @@ services:
LOG_LEVEL: "debug" # verbose by default while developing (base defaults to info) LOG_LEVEL: "debug" # verbose by default while developing (base defaults to info)
# Point plugin storage at the bundled Postgres, so a dropped-in plugin declaring `storage` # Point plugin storage at the bundled Postgres, so a dropped-in plugin declaring `storage`
# works with no further config; the secret falls back to the dev throwaway (config.ts). # works with no further config; the secret falls back to the dev throwaway (config.ts).
PLUGIN_DB_URL: postgres://postgres:5432 PLUGIN_DB_URL: *plugin-db-url
REQUIRE_SECURE_SECRETS: "false" REQUIRE_SECURE_SECRETS: "false"
SECURE_COOKIES: "false" # dev serves http — Secure cookies wouldn't be sent SECURE_COOKIES: "false" # dev serves http — Secure cookies wouldn't be sent
SCHEDULING_UPSTREAM: "http://shifts-upstream:4000" # backs the reference plugin once you copy it into plugins/ SCHEDULING_UPSTREAM: "http://shifts-upstream:4000" # backs the reference plugin once you copy it into plugins/
@@ -35,7 +39,7 @@ services:
# Provisions the plugin databases web connects to above, as the dev superuser. # Provisions the plugin databases web connects to above, as the dev superuser.
environment: environment:
PLUGIN_DB_ADMIN_URL: postgres://${POSTGRES_USER:-ory}:${POSTGRES_PASSWORD:-ory}@postgres:5432/ory PLUGIN_DB_ADMIN_URL: postgres://${POSTGRES_USER:-ory}:${POSTGRES_PASSWORD:-ory}@postgres:5432/ory
PLUGIN_DB_URL: postgres://postgres:5432 # must match web's above PLUGIN_DB_URL: *plugin-db-url
REQUIRE_SECURE_SECRETS: "false" # dev derives from the throwaway, as web does REQUIRE_SECURE_SECRETS: "false" # dev derives from the throwaway, as web does
volumes: volumes:
- .:/app - .:/app
+9 -8
View File
@@ -5,7 +5,7 @@
import { test } from "node:test"; import { test } from "node:test";
import assert from "node:assert/strict"; import assert from "node:assert/strict";
import { randomUUID } from "node:crypto"; import { randomUUID } from "node:crypto";
import { differentServer, ensureJwks, firstRunBanner, identityPayload, permissionTuple, provisionPluginStorage, seedAdmin, seedPermissions } from "./bootstrap.ts"; import { ensureJwks, firstRunBanner, identityPayload, permissionTuple, provisionPluginStorage, seedAdmin, seedPermissions, serverMismatch } from "./bootstrap.ts";
import { createLogger } from "../logger.ts"; import { createLogger } from "../logger.ts";
import type { Plugin } from "../plugin-host/plugin.ts"; import type { Plugin } from "../plugin-host/plugin.ts";
import type { ProvisionOptions, ProvisionResult } from "../plugin-host/storage-provisioning.ts"; import type { ProvisionOptions, ProvisionResult } from "../plugin-host/storage-provisioning.ts";
@@ -200,16 +200,17 @@ test("the connection limit and derived secret reach the provisioner", async () =
}); });
// bootstrap creates the role on one server; web tells the plugin to connect to another. Left // bootstrap creates the role on one server; web tells the plugin to connect to another. Left
// unchecked it surfaces inside a plugin as "password authentication failed", naming neither. // unsaid it surfaces inside a plugin as "password authentication failed", naming neither. Warned
test("provisioning refuses when the two storage URLs name different servers", async () => { // rather than refused: web reaching a pooler bootstrap cannot provision through is legitimate.
test("a storage URL mismatch is reported, and provisioning still runs", async () => {
const { calls, provision } = recordingProvisioner(); const { calls, provision } = recordingProvisioner();
const env = { PLUGIN_DB_ADMIN_URL: "postgres://ory:ory@db-a:5432/ory", PLUGIN_DB_URL: "postgres://db-b:5432" }; const env = { PLUGIN_DB_ADMIN_URL: "postgres://ory:ory@db-a:5432/ory", PLUGIN_DB_URL: "postgres://db-b:5432" };
await assert.rejects(provisionPluginStorage(env, [storagePlugin("things")], SILENT, provision), /one server.*db-a:5432 vs db-b:5432/s); await provisionPluginStorage(env, [storagePlugin("things")], SILENT, provision);
assert.deepEqual(calls, []); assert.equal(calls.length, 1);
}); });
test("the same server spelled with an implicit port still agrees", () => { test("the same server spelled with an implicit port still agrees", () => {
assert.equal(differentServer("postgres://ory:ory@db:5432/ory", "postgres://db"), null); // 5432 is the default assert.equal(serverMismatch("postgres://ory:ory@db:5432/ory", "postgres://db"), null); // 5432 is the default
assert.equal(differentServer("postgres://ory:ory@db:5432/ory", undefined), null); // web's own boot error to raise assert.equal(serverMismatch("postgres://ory:ory@db:5432/ory", undefined), null); // web's own boot error to raise
assert.equal(differentServer("postgres://ory:ory@db:5432/ory", "postgres://db:6543"), "db:5432 vs db:6543"); assert.equal(serverMismatch("postgres://ory:ory@db:5432/ory", "postgres://db:6543"), "db:5432 vs db:6543");
}); });
+11 -6
View File
@@ -146,7 +146,7 @@ export function firstRunBanner(opts: { appUrl: string; email: string; password:
// --- CLI (the bootstrap container entrypoint) ---------------------------------------- // --- CLI (the bootstrap container entrypoint) ----------------------------------------
async function main() { async function main() {
const env = process.env; const env = { ...process.env }; // snapshot: the storage credentials leave process.env before discovery
// Structured like the web app so prod logs stay uniform; honour LOG_FORMAT/SERVICE_NAME. // Structured like the web app so prod logs stay uniform; honour LOG_FORMAT/SERVICE_NAME.
const log = createLogger({ const log = createLogger({
format: env["LOG_FORMAT"] === "json" ? "json" : "text", format: env["LOG_FORMAT"] === "json" ? "json" : "text",
@@ -155,6 +155,10 @@ async function main() {
// runWithLog makes `log` ambient so seedAdmin's tracedFetch traces the Kratos/Keto seed calls. // runWithLog makes `log` ambient so seedAdmin's tracedFetch traces the Kratos/Keto seed calls.
await runWithLog(log, async () => { await runWithLog(log, async () => {
if (ensureJwks(env["JWKS_FILE"] ?? "/etc/config/kratos/tokenizer/jwks.json")) log.info("generated a JWKS signing key"); if (ensureJwks(env["JWKS_FILE"] ?? "/etc/config/kratos/tokenizer/jwks.json")) log.info("generated a JWKS signing key");
// Discovery imports every plugin module — and its dependencies — into *this* process, which holds
// the credential that may CREATE DATABASE/ROLE. Same move as server.ts, on the stronger secret.
delete process.env["PLUGIN_DB_ADMIN_URL"];
delete process.env["PLUGIN_DB_SECRET"];
const plugins = await discoverPlugins(); const plugins = await discoverPlugins();
await provisionPluginStorage(env, plugins, log); await provisionPluginStorage(env, plugins, log);
await seedAdminAndPermissions(env, plugins, log); await seedAdminAndPermissions(env, plugins, log);
@@ -172,10 +176,11 @@ export async function provisionPluginStorage(env: Env, plugins: Plugin[], log: L
// last storage plugin is exactly when an orphaned database needs naming. // last storage plugin is exactly when an orphaned database needs naming.
if (ids.length === 0 && !adminUrl) return; 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(", ")}`); if (!adminUrl) throw new Error(`bootstrap: PLUGIN_DB_ADMIN_URL must be set — these plugins declare storage: ${ids.join(", ")}`);
// Provisioned here, connected to from web: a different server means the role exists in one place // Provisioned here, connected to from web: a different server means the role is created in one
// and is looked for in another, surfacing as "password authentication failed" inside a plugin. // place and looked for in another, surfacing inside a plugin as "password authentication failed".
const serverMismatch = differentServer(adminUrl, env["PLUGIN_DB_URL"]); // Warned, not refused — web reaching a pooler that cannot run CREATE DATABASE is a legitimate split.
if (serverMismatch) throw new Error(`bootstrap: PLUGIN_DB_ADMIN_URL and PLUGIN_DB_URL must name one server (${serverMismatch})`); const mismatch = serverMismatch(adminUrl, env["PLUGIN_DB_URL"]);
if (mismatch) log.warn("PLUGIN_DB_ADMIN_URL and PLUGIN_DB_URL name different servers", { servers: mismatch });
const result = await provision({ const result = await provision({
adminUrl, adminUrl,
connectionLimit: resolvePluginDbConnectionLimit(env), connectionLimit: resolvePluginDbConnectionLimit(env),
@@ -191,7 +196,7 @@ export async function provisionPluginStorage(env: Env, plugins: Plugin[], log: L
// Describes the disagreement, or null when they agree (or when web's URL is unset — that is web's // Describes the disagreement, or null when they agree (or when web's URL is unset — that is web's
// own boot error to raise, naming the plugin that wanted storage). // own boot error to raise, naming the plugin that wanted storage).
export function differentServer(adminUrl: string, webUrl: string | undefined): string | null { export function serverMismatch(adminUrl: string, webUrl: string | undefined): string | null {
if (!webUrl) return null; if (!webUrl) return null;
const [admin, web] = [safeHostPort(adminUrl), safeHostPort(webUrl)]; const [admin, web] = [safeHostPort(adminUrl), safeHostPort(webUrl)];
if (admin === null || web === null || admin === web) return null; // a malformed URL fails in config.ts if (admin === null || web === null || admin === web) return null; // a malformed URL fails in config.ts
+3
View File
@@ -86,6 +86,9 @@ test("the provisioning superuser DSN reaches bootstrap only, never web", () => {
const boot = compose.slice(compose.indexOf("\n bootstrap:")); const boot = compose.slice(compose.indexOf("\n bootstrap:"));
const overrideWeb = override.slice(override.indexOf("\n web:"), override.indexOf("\n bootstrap:")); const overrideWeb = override.slice(override.indexOf("\n web:"), override.indexOf("\n bootstrap:"));
assert.match(boot, /PLUGIN_DB_ADMIN_URL:/, "bootstrap is given the superuser DSN"); assert.match(boot, /PLUGIN_DB_ADMIN_URL:/, "bootstrap is given the superuser DSN");
// Reordering the override's services would empty this slice, and every doesNotMatch below would
// then pass against "".
assert.ok(overrideWeb.includes("PLUGIN_DB_URL"), "sliced the dev override's web block");
for (const [name, block] of [["base", webBlock], ["dev override", overrideWeb]] as const) for (const [name, block] of [["base", webBlock], ["dev override", overrideWeb]] as const)
assert.doesNotMatch(block, /PLUGIN_DB_ADMIN_URL/, `${name} web never sees it`); assert.doesNotMatch(block, /PLUGIN_DB_ADMIN_URL/, `${name} web never sees it`);
assert.match(webBlock, /PLUGIN_DB_URL:\s*\$\{PLUGIN_DB_URL/, "base wires web's base URL from env"); assert.match(webBlock, /PLUGIN_DB_URL:\s*\$\{PLUGIN_DB_URL/, "base wires web's base URL from env");
+3 -1
View File
@@ -17,7 +17,9 @@ export interface ProvisionResult {
} }
export async function provisionStorage(options: ProvisionOptions): Promise<ProvisionResult> { export async function provisionStorage(options: ProvisionOptions): Promise<ProvisionResult> {
const sql = postgres(options.adminUrl, { connect_timeout: 10, max: 1, onnotice: () => {} }); // Notices are left to surface: a REVOKE the account cannot perform only *warns*, and silencing
// that would mean reporting a locked-down database that is still open to PUBLIC.
const sql = postgres(options.adminUrl, { connect_timeout: 10, max: 1 });
try { try {
const provisioned: string[] = []; const provisioned: string[] = [];
for (const pluginId of options.pluginIds) { for (const pluginId of options.pluginIds) {
-2
View File
@@ -63,8 +63,6 @@ test("quoting doubles an embedded quote", () => {
assert.equal(quoteLiteral("we'ird"), "'we''ird'"); assert.equal(quoteLiteral("we'ird"), "'we''ird'");
}); });
// No NOSUPERUSER: naming it in an ALTER is superuser-only, so re-asserting it would break every
// boot after the first under the least-privilege account the README recommends.
const ATTRIBUTES = "LOGIN NOCREATEDB NOCREATEROLE CONNECTION LIMIT 10"; const ATTRIBUTES = "LOGIN NOCREATEDB NOCREATEROLE CONNECTION LIMIT 10";
test("only the plugins that asked for storage are provisioned", () => { test("only the plugins that asked for storage are provisioned", () => {
+1 -3
View File
@@ -76,9 +76,7 @@ export function provisionSql(plan: ProvisionPlan): string[] {
throw new Error(`storage: connectionLimit must be a positive integer, got ${plan.connectionLimit}`); throw new Error(`storage: connectionLimit must be a positive integer, got ${plan.connectionLimit}`);
} }
const identifier = quoteIdentifier(plan.name); const identifier = quoteIdentifier(plan.name);
// No NOSUPERUSER: only a superuser may name SUPERUSER in an ALTER, so re-asserting it would fail // No NOSUPERUSER: naming SUPERUSER in an ALTER is superuser-only, and CREATE defaults to it anyway.
// every boot after the first under the CREATEDB+CREATEROLE account the README recommends. CREATE
// defaults to NOSUPERUSER and a non-superuser cannot grant it, so nothing is given up.
const attributes = `LOGIN NOCREATEDB NOCREATEROLE CONNECTION LIMIT ${plan.connectionLimit} PASSWORD ${quoteLiteral(plan.password)}`; const attributes = `LOGIN NOCREATEDB NOCREATEROLE CONNECTION LIMIT ${plan.connectionLimit} PASSWORD ${quoteLiteral(plan.password)}`;
return [ return [
plan.roleExists ? `ALTER ROLE ${identifier} WITH ${attributes}` : `CREATE ROLE ${identifier} ${attributes}`, plan.roleExists ? `ALTER ROLE ${identifier} WITH ${attributes}` : `CREATE ROLE ${identifier} ${attributes}`,
+3 -3
View File
@@ -31,9 +31,9 @@ test("init SQL gives each Ory service its own database, and leaves plugin databa
} }
}); });
// AGENTS.md records that the driver runs the provisioning DDL in bootstrap "and nothing else". // AGENTS.md records that the driver runs the provisioning DDL in bootstrap and nothing else. A
// discovery.ts once imported a validator from the module that held it, quietly putting the driver in // single value imported from the wrong module puts it in web's graph without changing behaviour,
// web's graph — the claim outlived the fact, which is what this catches. // so nothing but this would notice.
test("the Postgres driver reaches bootstrap only, never web's import graph", () => { test("the Postgres driver reaches bootstrap only, never web's import graph", () => {
const files = sourceFiles(); const files = sourceFiles();
assert.ok(files.length > 40, "walks the source tree"); assert.ok(files.length > 40, "walks the source tree");
+4
View File
@@ -61,6 +61,10 @@ const storageCredentials = new Map<string, StorageCredentials>();
if (pluginDbUrl !== undefined) { if (pluginDbUrl !== undefined) {
for (const id of declaresStorage) storageCredentials.set(id, buildCredentials(pluginDbUrl, id, config.pluginDbSecret)); for (const id of declaresStorage) storageCredentials.set(id, buildCredentials(pluginDbUrl, id, config.pluginDbSecret));
} }
// onBoot is the only way credentials are handed over, so without one the database is provisioned
// and unreachable. A warning, not a refusal — the plugin still works, it just cannot store anything.
const unreachable = plugins.filter((plugin) => plugin.storage && !plugin.hooks?.onBoot).map((plugin) => plugin.id);
if (unreachable.length > 0) log.warn("plugins declare storage but have no onBoot to receive it", { plugins: unreachable.join(", ") });
// plugin onBoot — after discovery, before listen; a throw aborts boot. // plugin onBoot — after discovery, before listen; a throw aborts boot.
await runBootHooks(plugins, (plugin) => { await runBootHooks(plugins, (plugin) => {