Answer stability round 1: 0 🔴, 4 🟡 (0 misread); the driver decides what parses so a multi-host DSN still works, and the nine-key test shrinks to four
This commit is contained in:
+2
-2
@@ -6,9 +6,9 @@ at boot. Entries start at 0.3.0.
|
|||||||
|
|
||||||
## Unreleased
|
## Unreleased
|
||||||
|
|
||||||
### A malformed URL setting's boot error no longer prints the value
|
### A malformed URL variable's boot error no longer prints the value
|
||||||
|
|
||||||
When a URL setting such as `PLUGIN_DB_URL`, `PLUGIN_DB_ADMIN_URL` or `OTLP_ENDPOINT` can't be parsed,
|
When a URL variable such as `PLUGIN_DB_URL`, `PLUGIN_DB_ADMIN_URL` or `OTLP_ENDPOINT` can't be parsed,
|
||||||
the boot error names the variable and not its value, so a DSN whose password breaks parsing doesn't
|
the boot error names the variable and not its value, so a DSN whose password breaks parsing doesn't
|
||||||
put the password in the log.
|
put the password in the log.
|
||||||
|
|
||||||
|
|||||||
@@ -181,16 +181,6 @@ test("provisioning still runs with nothing to provision, so orphans are reported
|
|||||||
assert.deepEqual(calls[0]?.pluginIds, []);
|
assert.deepEqual(calls[0]?.pluginIds, []);
|
||||||
});
|
});
|
||||||
|
|
||||||
// The postgres driver rethrows a malformed DSN with the whole string on the error (`input`), so
|
|
||||||
// the URL is checked here, before the driver sees it.
|
|
||||||
test("a malformed PLUGIN_DB_ADMIN_URL is refused by name, never echoing the password", async () => {
|
|
||||||
const env = { PLUGIN_DB_ADMIN_URL: "postgres://admin:pa/ss@db:5432/plugins" };
|
|
||||||
await assert.rejects(
|
|
||||||
provisionPluginStorage(env, [storagePlugin("things")], SILENT, async () => ({ orphans: [], provisioned: [] })),
|
|
||||||
(err: Error) => err.message.includes("PLUGIN_DB_ADMIN_URL") && !err.message.includes("pa/ss"),
|
|
||||||
);
|
|
||||||
});
|
|
||||||
|
|
||||||
test("a plugin declaring storage without a provisioning DSN fails loud, naming the plugin", async () => {
|
test("a plugin declaring storage without a provisioning DSN fails loud, naming the plugin", async () => {
|
||||||
const { calls, provision } = recordingProvisioner();
|
const { calls, provision } = recordingProvisioner();
|
||||||
await assert.rejects(
|
await assert.rejects(
|
||||||
|
|||||||
@@ -8,7 +8,7 @@
|
|||||||
// Then prints a first-run banner; fails loud on any unexpected upstream error.
|
// Then prints a first-run banner; fails loud on any unexpected upstream error.
|
||||||
import { existsSync, writeFileSync } from "node:fs";
|
import { existsSync, writeFileSync } from "node:fs";
|
||||||
import { fileURLToPath } from "node:url";
|
import { fileURLToPath } from "node:url";
|
||||||
import { assertUrl, resolvePluginDbConnectionLimit, resolvePluginDbSecret } from "../config.ts";
|
import { resolvePluginDbConnectionLimit, resolvePluginDbSecret } from "../config.ts";
|
||||||
import { discoverPlugins } from "../plugin-host/discovery.ts";
|
import { discoverPlugins } from "../plugin-host/discovery.ts";
|
||||||
import { declaredPermissions, isValidPermissionName, type Plugin } from "../plugin-host/plugin.ts";
|
import { declaredPermissions, isValidPermissionName, type Plugin } from "../plugin-host/plugin.ts";
|
||||||
import { provisionStorage } from "../plugin-host/storage-provisioning.ts";
|
import { provisionStorage } from "../plugin-host/storage-provisioning.ts";
|
||||||
@@ -179,7 +179,6 @@ export async function provisionPluginStorage(env: Env, plugins: Plugin[], log: L
|
|||||||
// Provisioned here, connected to from web: a different server means the role is created in one
|
// 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".
|
// 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.
|
// Warned, not refused — web reaching a pooler that cannot run CREATE DATABASE is a legitimate split.
|
||||||
assertUrl("PLUGIN_DB_ADMIN_URL", adminUrl);
|
|
||||||
const mismatch = serverMismatch(adminUrl, env["PLUGIN_DB_URL"]);
|
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 });
|
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({
|
||||||
@@ -200,7 +199,7 @@ export async function provisionPluginStorage(env: Env, plugins: Plugin[], log: L
|
|||||||
export function serverMismatch(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 assertUrl
|
if (admin === null || web === null || admin === web) return null; // a malformed URL fails in config.ts
|
||||||
return `${admin} vs ${web}`;
|
return `${admin} vs ${web}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+1
-7
@@ -39,16 +39,10 @@ test("PLUGIN_DB_URL carrying credentials is refused, not silently overwritten",
|
|||||||
assert.equal(loadConfig({}).pluginDbUrl, undefined); // unset ⇒ storage off
|
assert.equal(loadConfig({}).pluginDbUrl, undefined); // unset ⇒ storage off
|
||||||
});
|
});
|
||||||
|
|
||||||
// A DSN whose password breaks URL parsing (a bare "/") must not reach the boot error or the log.
|
|
||||||
// The credential-bearing keys get their own value; the rest share the two readers, so one loop covers them.
|
|
||||||
test("a malformed URL value is refused by naming the variable, never echoing the value", () => {
|
test("a malformed URL value is refused by naming the variable, never echoing the value", () => {
|
||||||
|
// PLUGIN_DB_URL and OTLP_ENDPOINT can carry credentials; APP_URL and KRATOS_PUBLIC_URL stand for the other keys, one per reader.
|
||||||
const bad: Record<string, string> = {
|
const bad: Record<string, string> = {
|
||||||
APP_URL: "https://acme:pa/ss@[host",
|
APP_URL: "https://acme:pa/ss@[host",
|
||||||
HYDRA_ADMIN_URL: "http://u:pa/ss@[hydra:4445",
|
|
||||||
JWKS_URL: "http://u:pa/ss@[jwks",
|
|
||||||
KETO_READ_URL: "http://u:pa/ss@[keto:4466",
|
|
||||||
KETO_WRITE_URL: "http://u:pa/ss@[keto:4467",
|
|
||||||
KRATOS_ADMIN_URL: "http://u:pa/ss@[kratos:4434",
|
|
||||||
KRATOS_PUBLIC_URL: "http://u:pa/ss@[kratos:4433",
|
KRATOS_PUBLIC_URL: "http://u:pa/ss@[kratos:4433",
|
||||||
OTLP_ENDPOINT: "http://otel:pa/ss@[collector:4318",
|
OTLP_ENDPOINT: "http://otel:pa/ss@[collector:4318",
|
||||||
PLUGIN_DB_URL: "postgres://admin:pa/ss@db:5432/plugins",
|
PLUGIN_DB_URL: "postgres://admin:pa/ss@db:5432/plugins",
|
||||||
|
|||||||
+2
-3
@@ -99,9 +99,8 @@ function readEnum<T extends string>(env: Env, key: string, allowed: readonly T[]
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Never echoes the value: a DSN whose password breaks URL parsing (a bare "/") would put the
|
// Never echoes the value: a DSN whose password breaks URL parsing (a bare "/") would put the
|
||||||
// password in the boot error and the log. bootstrap uses it too, before the postgres driver, which
|
// password in the boot error and the log.
|
||||||
// rethrows a malformed DSN with the whole string attached.
|
function assertUrl(key: string, value: string): void {
|
||||||
export function assertUrl(key: string, value: string): void {
|
|
||||||
try {
|
try {
|
||||||
new URL(value);
|
new URL(value);
|
||||||
} catch {
|
} catch {
|
||||||
|
|||||||
@@ -16,10 +16,20 @@ export interface ProvisionResult {
|
|||||||
provisioned: string[];
|
provisioned: string[];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The driver's error for a DSN it can't parse carries the whole string (`input`) — password included —
|
||||||
|
// so it is swallowed here. The driver, not `new URL`, decides what parses: it takes a multi-host DSN.
|
||||||
|
function connect(adminUrl: string): ReturnType<typeof postgres> {
|
||||||
|
try {
|
||||||
|
return postgres(adminUrl, { connect_timeout: 10, max: 1 });
|
||||||
|
} catch {
|
||||||
|
throw new Error("PLUGIN_DB_ADMIN_URL must be a Postgres URL (its value is not shown; percent-encode special characters in a password)");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export async function provisionStorage(options: ProvisionOptions): Promise<ProvisionResult> {
|
export async function provisionStorage(options: ProvisionOptions): Promise<ProvisionResult> {
|
||||||
// Notices are left to surface: a REVOKE the account cannot perform only *warns*, and silencing
|
// 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.
|
// that would mean reporting a locked-down database that is still open to PUBLIC.
|
||||||
const sql = postgres(options.adminUrl, { connect_timeout: 10, max: 1 });
|
const sql = connect(options.adminUrl);
|
||||||
try {
|
try {
|
||||||
const provisioned: string[] = [];
|
const provisioned: string[] = [];
|
||||||
for (const pluginId of options.pluginIds) {
|
for (const pluginId of options.pluginIds) {
|
||||||
|
|||||||
@@ -108,6 +108,13 @@ test("a connection limit that is not a positive integer is refused, not interpol
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("a malformed admin DSN is refused by name, without echoing its password", async () => {
|
||||||
|
await assert.rejects(
|
||||||
|
provisionStorage({ adminUrl: "postgres://admin:pa/ss@db:5432/plugins", connectionLimit: 10, pluginIds: [], secret: SECRET }),
|
||||||
|
(err: Error) => err.message.includes("PLUGIN_DB_ADMIN_URL") && !err.message.includes("pa/ss"),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
// --- Integration: the statements above, against a real Postgres -----------------------
|
// --- 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
|
// 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.
|
// tests cannot prove lives here: the owner may create tables, and a peer role is locked out.
|
||||||
|
|||||||
Reference in New Issue
Block a user