Keep a secret setting's raw value out of the boot error that rejects it #201

Merged
lilleman merged 5 commits from item-46 into main 2026-10-31 13:47:02 +01:00
6 changed files with 61 additions and 16 deletions
+5
View File
@@ -6,6 +6,11 @@ at boot. Entries start at 0.3.0.
## Unreleased ## Unreleased
### A secret plugin setting's bad value stays out of the boot error
When a `secret` setting's value fails validation, the boot error no longer prints it, so a malformed
DSN doesn't put its password in the log.
### The admin's recovery-code notice links to the recovery flow the code belongs to ### The admin's recovery-code notice links to the recovery flow the code belongs to
After an admin generates a recovery code for a user, the notice asks them to send the user both the After an admin generates a recovery code for a user, the notice asks them to send the user both the
+1 -1
View File
@@ -868,7 +868,7 @@ export default definePlugin({
``` ```
`type` is one of `string`, `number`, `boolean`, `enum` (with `values`) or `url`. A declared type is `type` is one of `string`, `number`, `boolean`, `enum` (with `values`) or `url`. A declared type is
coerced and checked at boot, so a mistyped value names the plugin, the key and the variable instead coerced and checked at boot, so a mistyped value names the plugin and the variable instead
of surfacing later as a broken page. of surfacing later as a broken page.
**`required` and `default` are mutually exclusive** — a default means the setting can never fail, so **`required` and `default` are mutually exclusive** — a default means the setting can never fail, so
+17
View File
@@ -274,3 +274,20 @@ Read most (opens and searches):
- `src/i18n/locales/en-US.ts` 3 - `src/i18n/locales/en-US.ts` 3
- `e2e-tests/compose.full.yml` 2 - `e2e-tests/compose.full.yml` 2
- `src/auth/kratos-admin.ts` 2 - `src/auth/kratos-admin.ts` 2
## Reading cost, chunk d8b41620d631, finished 2026-10-31T12:37:38Z
70583 new input tokens before the first commit.
Read most (opens and searches):
- `todo.md` 5
- `AGENTS.md` 2
- `README.md` 2
- `examples/plugins/admin/README.md` 2
- `src/auth/kratos-admin.ts` 2
- `src/http/app-admin.test.ts` 2
- `CHANGELOG.md` 1
- `e2e-tests/full-flow.spec.ts` 1
- `examples/plugins/admin/admin-users.test.ts` 1
- `examples/plugins/admin/admin-users.ts` 1
+19 -4
View File
@@ -109,12 +109,27 @@ test("each type coerces from the environment, and a bad value fails loud", () =>
PLUGIN_SETTING_P_MODE: "loose", PLUGIN_SETTING_P_MODE: "loose",
}); });
assert.equal(bad.errors.length, 4); assert.equal(bad.errors.length, 4);
assert.match(bad.errors.join("\n"), /PLUGIN_SETTING_P_COUNT.*number/); assert.match(bad.errors.join("\n"), /PLUGIN_SETTING_P_COUNT.*number, got "twelve"/);
assert.match(bad.errors.join("\n"), /PLUGIN_SETTING_P_FLAG.*"true".*"false"/); assert.match(bad.errors.join("\n"), /PLUGIN_SETTING_P_FLAG.*"true".*"false", got "yes"/);
assert.match(bad.errors.join("\n"), /PLUGIN_SETTING_P_MODE.*strict, lenient/); assert.match(bad.errors.join("\n"), /PLUGIN_SETTING_P_MODE.*strict, lenient, got "loose"/);
assert.match(bad.errors.join("\n"), /PLUGIN_SETTING_P_BASE.*URL/); assert.match(bad.errors.join("\n"), /PLUGIN_SETTING_P_BASE.*URL, got "not a url"/);
}); });
const SECRET_CASES: [SettingDecl, string, RegExp][] = [
[{ key: "s", secret: true, type: "boolean" }, "hunter2-bool", /PLUGIN_SETTING_P_S.*"true".*"false"/],
[{ key: "s", secret: true, type: "enum", values: ["a", "b"] }, "hunter2-enum", /PLUGIN_SETTING_P_S.*a, b/],
[{ key: "s", secret: true, type: "number" }, "hunter2-number", /PLUGIN_SETTING_P_S.*number/],
[{ key: "s", secret: true, type: "url" }, "postgres user:hunter2-url@db", /PLUGIN_SETTING_P_S.*URL/],
];
for (const [decl, raw, names] of SECRET_CASES) {
test(`a secret ${decl.type}'s bad value never reaches the boot error`, () => {
const errors = resolveSettings([pluginWith("p", [decl])], { PLUGIN_SETTING_P_S: raw }).errors;
assert.equal(errors.length, 1);
assert.match(errors[0] ?? "", names);
assert.ok(!(errors[0] ?? "").includes("hunter2"));
});
}
test("a boolean is only \"true\"/\"false\" — a typo never degrades to false", () => { test("a boolean is only \"true\"/\"false\" — a typo never degrades to false", () => {
const plugins = [pluginWith("p", [{ default: true, key: "flag", type: "boolean" }])]; const plugins = [pluginWith("p", [{ default: true, key: "flag", type: "boolean" }])];
assert.equal(resolveSettings(plugins, { PLUGIN_SETTING_P_FLAG: "false" }).values.get("p")?.["flag"], false); assert.equal(resolveSettings(plugins, { PLUGIN_SETTING_P_FLAG: "false" }).values.get("p")?.["flag"], false);
+5 -4
View File
@@ -233,24 +233,25 @@ function summarize(decl: SettingDecl, name: string, source: SettingSummary["sour
// A coerced value, or the boot error naming the variable and what it accepts. // A coerced value, or the boot error naming the variable and what it accepts.
function coerce(decl: SettingDecl, raw: string, name: string): { value: SettingValue } | string { function coerce(decl: SettingDecl, raw: string, name: string): { value: SettingValue } | string {
const got = decl.secret === true ? "" : `, got "${raw}"`;
switch (decl.type) { switch (decl.type) {
case "boolean": case "boolean":
if (raw === "true") return { value: true }; if (raw === "true") return { value: true };
if (raw === "false") return { value: false }; if (raw === "false") return { value: false };
return `${name} must be "true" or "false", got "${raw}"`; return `${name} must be "true" or "false"${got}`;
case "enum": case "enum":
return (decl.values ?? []).includes(raw) return (decl.values ?? []).includes(raw)
? { value: raw } ? { value: raw }
: `${name} must be one of ${(decl.values ?? []).join(", ")}, got "${raw}"`; : `${name} must be one of ${(decl.values ?? []).join(", ")}${got}`;
case "number": { case "number": {
const value = Number(raw); const value = Number(raw);
return Number.isFinite(value) ? { value } : `${name} must be a number, got "${raw}"`; return Number.isFinite(value) ? { value } : `${name} must be a number${got}`;
} }
case "url": case "url":
try { try {
new URL(raw); new URL(raw);
} catch { } catch {
return `${name} is not a valid URL: ${raw}`; return `${name} must be a valid URL${got}`;
} }
return { value: raw }; return { value: raw };
case "string": case "string":
+14 -7
View File
@@ -6,13 +6,13 @@
`Bar = 1` `Bar = 1`
`Next ID = 55` `Next ID = 56`
## Items ## Items
| ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score |
|---|---|---|---|---|---|---|---|---|---|---|---| |---|---|---|---|---|---|---|---|---|---|---|---|
| 46 | 0.4.21 | | security | **Keep a secret setting's raw value out of the boot error that rejects it.** | 1 | 1 | 4 | 5 | 2 | | 0.0 | | 55 | 0.4.21 | | security | **Keep a URL config value out of the boot error that rejects it.** | 1 | 1 | 4 | 5 | 2 | | 0.0 |
| 19 | 0.5.0 | | question | **Run the documented Docker commands on macOS and fix whatever misbehaves.** | 2 | 2 | 1 | 3 | 3 | 1 by 3 | 3.3 | | 19 | 0.5.0 | | question | **Run the documented Docker commands on macOS and fix whatever misbehaves.** | 2 | 2 | 1 | 3 | 3 | 1 by 3 | 3.3 |
| 16 | 0.5.0 | | question | **Decide whether the single Keto `Resource` namespace splits into per-domain namespaces.** | 3 | 2 | 3 | 6 | 6 | 7 by 2 | 0.5 | | 16 | 0.5.0 | | question | **Decide whether the single Keto `Resource` namespace splits into per-domain namespaces.** | 3 | 2 | 3 | 6 | 6 | 7 by 2 | 0.5 |
| 48 | 0.5.0 | | | **Correct the Keto tuple in the `login.ts` and `bootstrap.ts` comments to the `granted` relation the code writes.** | 1 | 1 | 1 | 1 | 1 | code health by 4 | 28.7 | | 48 | 0.5.0 | | | **Correct the Keto tuple in the `login.ts` and `bootstrap.ts` comments to the `granted` relation the code writes.** | 1 | 1 | 1 | 1 | 1 | code health by 4 | 28.7 |
@@ -34,12 +34,19 @@
## Details ## Details
### 46. Keep a secret setting's raw value out of the boot error that rejects it. ### 55. Keep a URL config value out of the boot error that rejects it.
`coerce` in `src/plugin-host/settings.ts` puts `got "${raw}"` (boolean, enum, number) and `: ${raw}` `readOptionalUrl` and `readUrl` in `src/config.ts` throw `config: ${key} is not a valid URL: ${value}`,
(url) into the error it returns, whatever `secret` says. A secret of type `url`, such as a DSN with so a malformed value reaches the boot error and the log. A DSN whose password breaks parsing, such as
a password, that fails `new URL` reaches the boot error and so the log. Name the variable and what it `postgres://admin:pa/ss@db:5432/plugins`, leaks that password. The keys that can carry credentials:
accepts, never the value; test one secret of each type. `PLUGIN_DB_URL` (`readCredentiallessUrl` refuses userinfo only after `readOptionalUrl` has parsed it,
so a pasted admin DSN that fails to parse is echoed), and `OTLP_ENDPOINT`, whose collector URL may
embed basic-auth userinfo. The other URL keys, `APP_URL`, `HYDRA_ADMIN_URL`, `JWKS_URL`,
`KETO_READ_URL`, `KETO_WRITE_URL`, `KRATOS_ADMIN_URL` and `KRATOS_PUBLIC_URL`, pass through the same
two readers, so the fix covers them too. Name the variable and what it accepts, never the value, and
add a test per credential-bearing key. The other readers in `config.ts` echo only non-credential
values. `PLUGIN_DB_ADMIN_URL` isn't read in `config.ts`: `bootstrap.ts` hands it straight to
`postgres()`. Check whether the driver echoes a malformed one.
### 19. Run the documented Docker commands on macOS and fix whatever misbehaves. ### 19. Run the documented Docker commands on macOS and fix whatever misbehaves.