diff --git a/src/store/index.ts b/src/store/index.ts index 90ffd79..d4d2ab5 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -8,24 +8,24 @@ export { InMemoryStore } from "./in-memory.js"; export { PostgresStore, createPostgresStore } from "./postgres.js"; export async function createStore(config: HeimdallConfig): Promise { - const store = - config.storage.backend === "postgres" - ? createPostgresStore(config.storage.databaseUrl ?? "postgres://127.0.0.1/heimdall") - : new InMemoryStore(); + if (config.storage.backend !== "postgres") { + return new InMemoryStore(); + } - if (config.storage.applySchemaOnStartup) { - try { - await store.ensureSchema(); - } catch (error) { - // pg's and Node's errors name the host, the database or the URL itself, - // and any of those can hold part of a secret that was bound or escaped - // wrongly. Startup prints this error, so it names only the input and a - // code, and carries no cause. - const code = (error as { code?: unknown }).code; - throw new Error( - `Postgres storage could not be prepared (${typeof code === "string" ? code : "error"}); check GC_ACCESS_DATABASE_URL_FILE.` - ); - } + const store = createPostgresStore(config.storage.databaseUrl ?? "postgres://127.0.0.1/heimdall"); + try { + // Connect on every start, whether or not it applies the schema, so a + // misbound URL fails here rather than in the first request. + await (config.storage.applySchemaOnStartup ? store.ensureSchema() : store.checkConnection()); + } catch (error) { + // pg's and Node's errors name the host, the database or the URL itself, + // and any of those can hold part of a secret that was bound or escaped + // wrongly. Startup prints this error, so it names only the input and a + // code, and carries no cause. + const code = (error as { code?: unknown }).code; + throw new Error( + `Postgres storage could not be prepared (${typeof code === "string" ? code : "error"}); check GC_ACCESS_DATABASE_URL_FILE.` + ); } return store; diff --git a/src/store/postgres.ts b/src/store/postgres.ts index a9c376f..3ea5f88 100644 --- a/src/store/postgres.ts +++ b/src/store/postgres.ts @@ -291,6 +291,11 @@ export class PostgresStore implements HeimdallStore { await this.pool.query(CREATE_SCHEMA_SQL); } + /** Opens a connection and runs a trivial query, so a start that does not apply the schema still proves the URL. */ + async checkConnection(): Promise { + await this.pool.query("SELECT 1"); + } + async close(): Promise { await this.pool.end(); } @@ -791,7 +796,7 @@ export class PostgresStore implements HeimdallStore { } } -export function createPostgresStore(databaseUrl: string): HeimdallStore { +export function createPostgresStore(databaseUrl: string): PostgresStore { const pool = new Pool({ connectionString: databaseUrl, }); diff --git a/tests/entrypoint-secrets.test.ts b/tests/entrypoint-secrets.test.ts index c52c2ef..941c8c5 100644 --- a/tests/entrypoint-secrets.test.ts +++ b/tests/entrypoint-secrets.test.ts @@ -1,5 +1,5 @@ import { spawn } from "node:child_process"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdirSync, mkdtempSync, rmSync, statSync, writeFileSync } from "node:fs"; import { createRequire } from "node:module"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -35,11 +35,31 @@ interface Run { output: string; } +interface Identity { + uid: number; + gid: number; +} + +/** + * Who can be denied a read. Root reads a mode-000 file, so a read failure is + * only reachable as another user: the process itself when it is not root, or + * else the checkout's owner, who can still load the entrypoint. A string is + * the reason no such user exists here. + */ +function nonRootReader(): Identity | "self" | string { + if (process.platform === "win32") return "file modes do not deny reads on Windows"; + if (process.getuid?.() !== 0) return "self"; + const owner = statSync(repoRoot); + if (owner.uid === 0) return "running as root in a root-owned checkout, so no other user can load the entrypoint"; + return { uid: owner.uid, gid: owner.gid }; +} + /** Runs `tsx src/index.ts` with only the given environment, merging stdout and stderr. */ -function runEntrypoint(name: string, env: Record): Promise { +function runEntrypoint(name: string, env: Record, identity?: Identity): Promise { return new Promise((resolve, reject) => { const child = spawn(process.execPath, [tsxCli, "src/index.ts"], { cwd: repoRoot, + ...identity, env: { PATH: process.env.PATH ?? "", HOME: scratch, @@ -73,6 +93,8 @@ interface Case { env: () => Record; /** Something the failure must say, so a case cannot pass by never reaching its path. */ expect: RegExp; + /** Run the entrypoint as a user a file mode can refuse. */ + nonRoot?: true; } const cases: Case[] = [ @@ -136,6 +158,16 @@ const cases: Case[] = [ env: () => ({ GC_ACCESS_DATABASE_URL_FILE: file("db-host", `postgres://heimdall:pw@${CANARY}host.invalid/heimdall\n`) }), expect: /Postgres storage could not be prepared \(E[A-Z_]+\)/, }, + { + // A start that does not apply the schema still connects, so a misbound URL + // fails at startup rather than in a request's error answer. + name: "a database host that does not resolve, on a start that does not apply the schema", + env: () => ({ + GC_ACCESS_APPLY_SCHEMA_ON_STARTUP: "0", + GC_ACCESS_DATABASE_URL_FILE: file("db-host-noapply", `postgres://heimdall:pw@${CANARY}host.invalid/heimdall\n`), + }), + expect: /Postgres storage could not be prepared \(E[A-Z_]+\)/, + }, { name: "private key text bound to the signing key path", env: () => ({ @@ -153,6 +185,19 @@ const cases: Case[] = [ env: () => ({ GC_ACCESS_SIGNING_PRIVATE_KEY_PATH: file("not-a-key", `${CANARY}notakey\n`) }), expect: /GC_ACCESS_SIGNING_PRIVATE_KEY_PATH does not hold a usable private key/, }, + { + name: "a signing key file its reader may not read", + env: () => { + const key = file(`${CANARY}locked-key`, `${CANARY}pem\n`); + chmodSync(key, 0o000); + const dataRoot = join(scratch, "locked-key-data"); + mkdirSync(dataRoot, { recursive: true }); + chmodSync(dataRoot, 0o777); + return { GC_ACCESS_SIGNING_PRIVATE_KEY_PATH: key, GC_ACCESS_DATA_ROOT: dataRoot }; + }, + expect: /GC_ACCESS_SIGNING_PRIVATE_KEY_PATH could not be read \(EACCES\)/, + nonRoot: true, + }, { name: "a signing key path that is a directory", env: () => { @@ -187,10 +232,17 @@ const cases: Case[] = [ describe("the entrypoint never prints a secret", () => { writeFileSync(join(scratch, "blank"), " \n"); + // mkdtemp makes the scratch directory private to its creator; a non-root + // reader must still reach the files a case names inside it. + chmodSync(scratch, 0o755); + const reader = nonRootReader(); for (const testCase of cases) { - it(`refuses ${testCase.name} without printing the canary`, async () => { - const run = await runEntrypoint(testCase.name.replace(/\W+/g, "-"), testCase.env()); + const skipReason = testCase.nonRoot && typeof reader === "string" && reader !== "self" ? reader : undefined; + const title = `refuses ${testCase.name} without printing the canary${skipReason ? ` (skipped: ${skipReason})` : ""}`; + it.skipIf(skipReason !== undefined)(title, async () => { + const identity = testCase.nonRoot && typeof reader === "object" ? reader : undefined; + const run = await runEntrypoint(testCase.name.replace(/\W+/g, "-"), testCase.env(), identity); expect(run.signal, "the process must stop by itself").toBeNull(); expect(run.code, "startup must fail").not.toBe(0);