From d2211cf75a4ca054783f3f2f8bfa86daf6bba6dd Mon Sep 17 00:00:00 2001 From: lilleman Date: Tue, 18 Aug 2026 23:24:34 +0200 Subject: [PATCH] Refuse a throwaway plugin storage secret in bootstrap, before any role is created --- README.md | 6 ++++-- compose.override.yml | 1 + compose.yml | 1 + src/config.test.ts | 11 +++++++++++ src/config.ts | 14 ++++++++------ todo.md | 3 --- 6 files changed, 25 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 5dc4708..a24ae77 100644 --- a/README.md +++ b/README.md @@ -777,7 +777,9 @@ What the host does guarantee: **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. +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. **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 @@ -1020,7 +1022,7 @@ The app is **environment-agnostic**: no `NODE_ENV`, every behaviour its own expl | `CSRF_SECRET` | dev throwaway | signs our double-submit CSRF token; enforced by `REQUIRE_SECURE_SECRETS` | | `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; enforced by `REQUIRE_SECURE_SECRETS` once `PLUGIN_DB_URL` is set | +| `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 | ### Canonical host (one public URL) diff --git a/compose.override.yml b/compose.override.yml index e70a741..9cbccfc 100644 --- a/compose.override.yml +++ b/compose.override.yml @@ -35,6 +35,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 + REQUIRE_SECURE_SECRETS: "false" # dev derives from the throwaway, as web does volumes: - .:/app diff --git a/compose.yml b/compose.yml index ce48b74..6700bda 100644 --- a/compose.yml +++ b/compose.yml @@ -153,6 +153,7 @@ services: # `storage` fails the seed loudly. The secret must match web's; both derive the same passwords. PLUGIN_DB_ADMIN_URL: ${PLUGIN_DB_ADMIN_URL:-} PLUGIN_DB_SECRET: ${PLUGIN_DB_SECRET:-} + REQUIRE_SECURE_SECRETS: "true" # refuse the throwaway secret here too, before any role is created volumes: - ./ory/kratos/tokenizer:/etc/config/kratos/tokenizer command: node src/auth/bootstrap.ts diff --git a/src/config.test.ts b/src/config.test.ts index 982e450..74cf0df 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -19,6 +19,17 @@ test("web and bootstrap resolve the same plugin storage secret", () => { assert.match(resolvePluginDbSecret({ PLUGIN_DB_SECRET: "" }), /dev-insecure/); // empty is unset, not a secret }); +// bootstrap writes these passwords into Postgres, so it must refuse the publicly-known throwaway +// before creating a role with one — not leave web to notice afterwards. +test("bootstrap refuses a missing, empty or throwaway plugin storage secret when hardened", () => { + const hardened = { REQUIRE_SECURE_SECRETS: "true" }; + for (const secret of [undefined, "", "dev-insecure-plugin-db-secret"]) { + const env = secret === undefined ? hardened : { ...hardened, PLUGIN_DB_SECRET: secret }; + assert.throws(() => resolvePluginDbSecret(env), /PLUGIN_DB_SECRET/, `for ${JSON.stringify(secret)}`); + } + assert.equal(resolvePluginDbSecret({ ...hardened, PLUGIN_DB_SECRET: "a-real-secret" }), "a-real-secret"); +}); + test("loads dev defaults when the environment is empty", () => { const c = loadConfig({}); assert.equal(c.port, 3000); diff --git a/src/config.ts b/src/config.ts index f971e1c..2570a27 100644 --- a/src/config.ts +++ b/src/config.ts @@ -8,11 +8,13 @@ export type LogLevel = (typeof LOG_LEVELS)[number]; const DEV_PLUGIN_DB_SECRET = "dev-insecure-plugin-db-secret"; -// bootstrap resolves the plugin-storage secret through this, web through loadConfig — and the two -// must agree exactly or web connects with a password the role was never given. Compose passes an -// unset variable through as "", so empty means throwaway here as it does in readSecret. -export function resolvePluginDbSecret(env: Env): string { - return env["PLUGIN_DB_SECRET"] || DEV_PLUGIN_DB_SECRET; +// The one resolution both processes use — they must agree exactly, or web connects with a password +// the role was never given. Compose passes an unset variable through as "", so empty means unset. +// `enforce` says whether storage is actually in play: web once PLUGIN_DB_URL is configured, +// bootstrap once a plugin declares storage. Enforced, the throwaway is refused — bootstrap is what +// writes these passwords into Postgres, so it must refuse *before* creating a role with one. +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)); } export interface Config { @@ -165,7 +167,7 @@ export function loadConfig(env: Env = process.env): Config { // credentials: the superuser DSN that provisions stays in bootstrap, so a plugin cannot read it // 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: readSecret(env, "PLUGIN_DB_SECRET", DEV_PLUGIN_DB_SECRET, requireSecure && Boolean(env["PLUGIN_DB_URL"])), + pluginDbSecret: resolvePluginDbSecret(env, requireSecure && Boolean(env["PLUGIN_DB_URL"])), pluginDbUrl: readOptionalUrl(env, "PLUGIN_DB_URL"), port: readPort(env), // Optional instant-revoke, off by default. When on, an admin deactivate/delete or permission diff --git a/todo.md b/todo.md index 085a04a..2723c0a 100644 --- a/todo.md +++ b/todo.md @@ -3,9 +3,6 @@ ## Unfinnished work - [ ] Add a way to configure plugins directly when installing. **Decided: the manifest declares it, not an `.env`** — a declared schema is validatable at boot, so a missing or mistyped setting fails loud and named the way a stray `package.json` now does, and the picker/docs can be generated from the declaration. Open: where the operator *supplies* the values (env var per key, a `config/` file, or both), and whether a secret may be declared at all. -- [ ] A provisioned plugin database is invisible and unremovable. Nothing lists what exists, so an operator cannot see that `plugin_` is there, nor that its plugin is gone — and uninstalling never drops it (deliberately, so data survives), leaving orphans with no supported way to clean them. Same shape as the uninstalled-plugin grant gap above. Sketch: a read-only "provisioned, but no installed plugin claims it" list plus a documented drop procedure. -- [ ] Decide who owns the connection ceiling for plugin storage. Each storage plugin opens its own pool against one Postgres, whose default `max_connections` is 100, and nothing warns as plugins are added. Either state a per-plugin pool ceiling in README → Plugin storage or record that the plugin owns it. -- [ ] E2E that a plugin's data actually survives a restart. `storage.test.ts` proves provisioning against a real Postgres (opt-in via `PLUGIN_DB_ADMIN_URL`), but no Playwright flow writes through a plugin page and reads it back after `docker compose restart web`. - [ ] Rename the plugin "admin" to something less generic, like "auth-admin" or "users-groups-admin". - [ ] Guard the group paths against self-lockout, or accept them explicitly. The self-revoke guard covers only your own *direct* grants on the Users screen; unticking a permission on a group you belong to, removing yourself from that group, or deleting it can all still strip your own effective access with no warning. Recorded in AGENTS.md as a known gap — the robust fix is a "last effective holder" check, which needs a reverse Keto query. - [ ] The permission picker has no concurrency baseline, so two operators editing the same user/group silently discard each other's change. Sketch: post the rendered set as a hidden baseline; if it no longer matches Keto, re-render with "this changed while you had the page open" rather than applying.