Warn rather than refuse on a storage URL mismatch, and scrub the provisioning DSN before discovery

This commit is contained in:
2026-08-19 01:03:40 +02:00
parent 5589472e25
commit e0046e5068
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
`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
both halves, because one value imported from the wrong module breaks it invisibly). 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
@@ -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
point, so move it earlier if anything, **never later**. **Valid while plugins are
operator-installed code, not third-party uploads.**
- **`ory/postgres/init/init.sql` is the only home for the Ory databases' ACL.** Re-asserting the
`REVOKE CONNECT` from `bootstrap` each boot was tried and removed: `REVOKE` only *warns* when the
caller doesn't own the database, so under the least-privilege provisioning account the README
recommends it reported success while changing nothing and it hard-failed whenever
`PLUGIN_DB_ADMIN_URL` named a server with no `kratos`. A volume created before that file gained the
revokes keeps the default grant; `docker compose down -v` is the dev remedy. **Valid while
pre-release, with no deployed volumes to migrate.**
- **`ory/postgres/init/init.sql` is the only home for the Ory databases' ACL** — don't re-assert the
`REVOKE CONNECT` from `bootstrap`. `REVOKE` only *warns* when the caller doesn't own the database,
so under the least-privilege provisioning account the README recommends it would report success
while changing nothing, and it hard-fails whenever `PLUGIN_DB_ADMIN_URL` names a server with no
`kratos`. A volume created before that file gained the revokes keeps the default grant;
`docker compose down -v` is the dev remedy. **Valid while pre-release, with no deployed volumes.**
- **`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
+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
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. Role attributes are
re-applied each time, so a privilege granted by hand out of band does not quietly persist.
the next `docker compose up -d` — the same rule as permission seeding. Each boot re-applies the
role's password, connection limit, and `NOCREATEDB`/`NOCREATEROLE`.
- **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
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
`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`.
# 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:
web:
command: node --watch src/server.ts
@@ -15,7 +19,7 @@ services:
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`
# 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"
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/
@@ -35,7 +39,7 @@ services:
# Provisions the plugin databases web connects to above, as the dev superuser.
environment:
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
volumes:
- .:/app
+9 -8
View File
@@ -5,7 +5,7 @@
import { test } from "node:test";
import assert from "node:assert/strict";
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 type { Plugin } from "../plugin-host/plugin.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
// unchecked it surfaces inside a plugin as "password authentication failed", naming neither.
test("provisioning refuses when the two storage URLs name different servers", async () => {
// unsaid it surfaces inside a plugin as "password authentication failed", naming neither. Warned
// 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 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);
assert.deepEqual(calls, []);
await provisionPluginStorage(env, [storagePlugin("things")], SILENT, provision);
assert.equal(calls.length, 1);
});
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(differentServer("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"), null); // 5432 is the default
assert.equal(serverMismatch("postgres://ory:ory@db:5432/ory", undefined), null); // web's own boot error to raise
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) ----------------------------------------
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.
const log = createLogger({
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.
await runWithLog(log, async () => {
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();
await provisionPluginStorage(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.
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(", ")}`);
// Provisioned here, connected to from web: a different server means the role exists in one place
// and is looked for in another, surfacing as "password authentication failed" inside a plugin.
const serverMismatch = differentServer(adminUrl, env["PLUGIN_DB_URL"]);
if (serverMismatch) throw new Error(`bootstrap: PLUGIN_DB_ADMIN_URL and PLUGIN_DB_URL must name one server (${serverMismatch})`);
// Provisioned here, connected to from web: a different server means the role is created in one
// place and looked for in another, surfacing inside a plugin as "password authentication failed".
// Warned, not refused — web reaching a pooler that cannot run CREATE DATABASE is a legitimate split.
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({
adminUrl,
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
// 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;
const [admin, web] = [safeHostPort(adminUrl), safeHostPort(webUrl)];
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 overrideWeb = override.slice(override.indexOf("\n web:"), override.indexOf("\n bootstrap:"));
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)
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");
+3 -1
View File
@@ -17,7 +17,9 @@ export interface 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 {
const provisioned: string[] = [];
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'");
});
// 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";
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}`);
}
const identifier = quoteIdentifier(plan.name);
// No NOSUPERUSER: only a superuser may name SUPERUSER in an ALTER, so re-asserting it would fail
// 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.
// No NOSUPERUSER: naming SUPERUSER in an ALTER is superuser-only, and CREATE defaults to it anyway.
const attributes = `LOGIN NOCREATEDB NOCREATEROLE CONNECTION LIMIT ${plan.connectionLimit} PASSWORD ${quoteLiteral(plan.password)}`;
return [
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".
// 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.
// AGENTS.md records that the driver runs the provisioning DDL in bootstrap and nothing else. A
// single value imported from the wrong module puts it in web's graph without changing behaviour,
// so nothing but this would notice.
test("the Postgres driver reaches bootstrap only, never web's import graph", () => {
const files = sourceFiles();
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) {
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.
await runBootHooks(plugins, (plugin) => {