Every start connects to Postgres; pin the signing key's read refusal
Soul pass 3 (R1, R4). With GC_ACCESS_APPLY_SCHEMA_ON_STARTUP=0, startup never opened a connection, so a misbound database URL surfaced only in a request's error answer. createStore now connects on every Postgres start: it applies the schema when asked and otherwise runs `SELECT 1`, and either failure is the same fixed message naming GC_ACCESS_DATABASE_URL_FILE and a code. The entrypoint harness gains that case with the canary in the host. The harness also pins the signing key's read-failure branch with a key file of mode 000. Root reads such a file, so the case runs the entrypoint as a user a mode can refuse: the test process itself when it is not root, or else the checkout's owner. It is skipped, with the reason in its title, on Windows or as root in a root-owned checkout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
33afcbf364
commit
0f5ffd9e69
3 changed files with 79 additions and 22 deletions
|
|
@ -8,24 +8,24 @@ export { InMemoryStore } from "./in-memory.js";
|
|||
export { PostgresStore, createPostgresStore } from "./postgres.js";
|
||||
|
||||
export async function createStore(config: HeimdallConfig): Promise<HeimdallStore> {
|
||||
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;
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
await this.pool.query("SELECT 1");
|
||||
}
|
||||
|
||||
async close(): Promise<void> {
|
||||
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,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<string, string>): Promise<Run> {
|
||||
function runEntrypoint(name: string, env: Record<string, string>, identity?: Identity): Promise<Run> {
|
||||
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<string, string>;
|
||||
/** 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);
|
||||
|
|
|
|||
Loading…
Reference in a new issue