From 160b5ad65a3f34951626fed3e5b414f2e390f822 Mon Sep 17 00:00:00 2001 From: jochens Date: Fri, 2 Oct 2026 00:09:09 +0200 Subject: [PATCH] postgres: the store's query runs as a read-only login, never as the admin (hq #193) The verb wrapped the caller's text in BEGIN READ ONLY ... ROLLBACK as the superuser, so 'COMMIT; ...' left the transaction and, proven on a throwaway server, COPY TO PROGRAM ran a shell command on the database host. The statement now runs as mesh_store_reader: pg_read_all_data, no other grant, read-only transactions by role and session, its password an own-secret the mesh mints. Without that password the call is refused. -q drops the command tags that came back as rows keyed by BEGIN. --- modules/postgres/client.ts | 85 ++++++++++++++++++-- modules/postgres/module.json | 9 ++- modules/postgres/package.json | 6 +- modules/postgres/test/reader.test.ts | 112 +++++++++++++++++++++++++++ modules/postgres/tools/index.ts | 2 +- 5 files changed, 204 insertions(+), 10 deletions(-) create mode 100644 modules/postgres/test/reader.test.ts diff --git a/modules/postgres/client.ts b/modules/postgres/client.ts index ee4d6a9..e38284d 100644 --- a/modules/postgres/client.ts +++ b/modules/postgres/client.ts @@ -25,12 +25,30 @@ export interface PgConn { readonly port: number; readonly user: string; readonly password: string; + /** + * The read-only login's password, which the mesh mints for this module (`own-secrets.reader`). + * Absent when the mesh has not delivered it: then a caller's statement is refused, never run as + * the admin (novox/hq issue 193). + */ + readonly readerPassword?: string; } +/** + * The login a caller's statement runs as (novox/hq issue 193). It may read every table and change + * nothing: `pg_read_all_data` and no other grant, and every transaction it opens is read-only by + * the server's own setting. A statement cannot climb out of a login the way it can out of a + * transaction wrapped around it as text: `COMMIT; DROP …` ended the old wrapper and ran the rest as + * the superuser, and even one read-only statement as a superuser can run a program on the server. + */ +export const READER = "mesh_store_reader"; + export class PostgresClient { constructor(private readonly conn: PgConn) {} + /** The reader is made once per process: idempotent, and repeating it re-sets a rotated password. */ + private readerReady?: Promise; + /** * Build from the module's resolved environment. Reads MESH_POSTGRES_* first (the documented * names), falling back to the MESH_PROVISION_* keys the manifest already sets on the provisioner @@ -54,7 +72,9 @@ export class PostgresClient { if (!host || !password) { throw new Error("postgres host or admin password is not set — postgres's own code cannot reach the server"); } - return new PostgresClient({ host, port, user, password }); + const readerPassword = env.MESH_POSTGRES_READER_PASSWORD ?? + readSecretFile(env.MESH_POSTGRES_READER_PASSWORD_FILE); + return new PostgresClient({ host, port, user, password, readerPassword }); } get host(): string { @@ -143,11 +163,66 @@ export class PostgresClient { return res.rows.map((r) => ({ name: String(r.datname), sizeBytes: Number(r.size) })); } - /** Run a read-only SQL statement against a named database, for the postgres_query tool. */ - async readOnlyQuery(database: string, sql: string): Promise { - // The read-only guarantee is a wrapping transaction the server honours. - return this.query(`BEGIN TRANSACTION READ ONLY; ${sql}; ROLLBACK;`, database); + /** + * Make the read-only login, idempotently, with the password the mesh minted for it. Run as the + * admin, because only the admin can make a role. + */ + async ensureReader(): Promise { + const password = this.conn.readerPassword; + if (!password) throw readerMissing(); + const roles = await this.query("SELECT 1 FROM pg_roles WHERE rolname = " + literal(READER)); + const verb = roles.rows.length === 0 ? "CREATE" : "ALTER"; + // Every attribute stated, so an existing role someone widened is narrowed again on every start. + await this.query( + `${verb} ROLE ${ident(READER)} WITH LOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE NOREPLICATION ` + + `NOBYPASSRLS INHERIT PASSWORD ${literal(password)} VALID UNTIL 'infinity'`, + ); + await this.query(`GRANT pg_read_all_data TO ${ident(READER)}`); + await this.query(`ALTER ROLE ${ident(READER)} SET default_transaction_read_only = on`); + await this.query(`ALTER ROLE ${ident(READER)} SET statement_timeout = '60s'`); } + + /** + * Run a caller's statement against a named database as the read-only login, for the + * postgres_query tool and the store seat's `query` verb (novox/hq ADR 0159, issue 193). + * + * **Read-only by the login, not by text around the statement.** The statement is sent as it was + * given, as the reader, whose role can write nothing and whose transactions the server makes + * read-only. Never as the admin: without the reader's password the call is refused. + */ + async readOnlyQuery(database: string, sql: string): Promise { + this.readerReady ??= this.ensureReader().catch((err) => { + this.readerReady = undefined; // asked again next call, not failed for the process's life + throw err; + }); + await this.readerReady; + const { stdout } = await run( + "psql", + // -q: no command tags, so the output is the header and the rows and nothing else — the tags + // were what came back as rows keyed by BEGIN. + ["-h", this.conn.host, "-p", String(this.conn.port), "-U", READER, "-d", database, + "-v", "ON_ERROR_STOP=1", "--no-psqlrc", "-q", "--csv", "-c", sql], + { + env: { + ...process.env, + PGPASSWORD: this.conn.readerPassword, + // Read-only from the first statement, before the role's own setting is read. + PGOPTIONS: "-c default_transaction_read_only=on -c statement_timeout=60s", + }, + maxBuffer: 16 << 20, + }, + ); + const command = /^\s*([A-Za-z]+)/.exec(sql)?.[1]?.toUpperCase() ?? ""; + return { command, rows: parseCsvRows(stdout) }; + } +} + +function readerMissing(): Error { + return new Error( + "the read-only login's password was not delivered (own-secrets.reader, " + + "MESH_POSTGRES_READER_PASSWORD_FILE), so the statement is refused rather than run as the " + + "admin (novox/hq issue 193)", + ); } /** Generate a URL-safe password. */ diff --git a/modules/postgres/module.json b/modules/postgres/module.json index d958817..98da700 100644 --- a/modules/postgres/module.json +++ b/modules/postgres/module.json @@ -53,7 +53,8 @@ }, "own-secrets": { "superuser": "${dir:state}/superuser.secret", - "broker": "${dir:mesh-state}/broker" + "broker": "${dir:mesh-state}/broker", + "reader": "${dir:state}/reader.secret" }, "resources": [ { @@ -105,14 +106,16 @@ "volumes": [ "${dir:mesh-state}/broker:/run/secrets/broker:ro", "${dir:grants}:${dir:grants}:ro", - "${dir:state}/superuser.secret:/run/secrets/superuser:ro" + "${dir:state}/superuser.secret:/run/secrets/superuser:ro", + "${dir:state}/reader.secret:/run/secrets/reader:ro" ], "env": { "MESH_PROVISION_POSTGRES": "postgres://postgres@127.0.0.1:${port:5432}/postgres?sslmode=disable", "MESH_PROVISION_POSTGRES_PORT": "${seat:mesh-store:5432}", "MESH_PROVISION_PASSWORD_FILE": "/run/secrets/superuser", "MESH_BROKER_FILE": "/run/secrets/broker", - "MESH_RECEIVES": "${dir:grants}/mesh.json" + "MESH_RECEIVES": "${dir:grants}/mesh.json", + "MESH_POSTGRES_READER_PASSWORD_FILE": "/run/secrets/reader" }, "artifact": "runtime" } diff --git a/modules/postgres/package.json b/modules/postgres/package.json index 1256cb6..f6e8c55 100644 --- a/modules/postgres/package.json +++ b/modules/postgres/package.json @@ -1,9 +1,13 @@ { "name": "@novox/module-postgres", "version": "0.1.0", - "description": "postgres — provides the mesh postgres-database interface. Its client, provisioner, tools and events live here (novox/hq ADR 0039).", + "description": "postgres \u2014 provides the mesh postgres-database interface. Its client, provisioner, tools and events live here (novox/hq ADR 0039).", "type": "module", "private": true, + "scripts": { + "build": "tsc client.ts index.ts provisioner/index.ts tools/index.ts --module NodeNext --moduleResolution NodeNext --target ES2022 --outDir dist", + "test": "npm run build && node --test --experimental-strip-types 'test/*.test.ts'" + }, "dependencies": { "@novox/mesh-sdk": "^0.1.1" }, diff --git a/modules/postgres/test/reader.test.ts b/modules/postgres/test/reader.test.ts new file mode 100644 index 0000000..daeb61e --- /dev/null +++ b/modules/postgres/test/reader.test.ts @@ -0,0 +1,112 @@ +// What holds the store's read-only query to being read-only (novox/hq issue 193): a caller's +// statement runs as the reader login and never as the admin, is sent as given with no transaction +// wrapped around it as text, and comes back as rows keyed by their columns. The reader is made once, +// as the admin, with every attribute stated; without its password the statement is refused. +// +// psql is a fake on PATH that records each call's user, options and statement, and answers in CSV +// the way the real one does with -q. That the reader cannot write is the server's to enforce and is +// proven against a real server, not here; this holds the module to asking for it. +// Run against the compiled module (npm test builds first), the way the runtime loads it. + +import { test, before, after } from "node:test"; +import assert from "node:assert/strict"; +import { chmod, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { PostgresClient, READER } from "../dist/client.js"; + +let dir: string; +let log: string; +const originalPath = process.env.PATH; + +before(async () => { + dir = await mkdtemp(join(tmpdir(), "postgres-reader-")); + log = join(dir, "calls.jsonl"); + // Records argv, the user it connected as and the options it was given; answers a role lookup + // with no rows and anything else with a two-column result. + await writeFile(join(dir, "psql"), `#!/usr/bin/env node +const fs = require("node:fs"); +const args = process.argv.slice(2); +const at = (flag) => args[args.indexOf(flag) + 1]; +fs.appendFileSync(${JSON.stringify(log)}, JSON.stringify({ + user: at("-U"), database: at("-d"), sql: at("-c"), quiet: args.includes("-q"), + password: process.env.PGPASSWORD, options: process.env.PGOPTIONS ?? "", +}) + "\\n"); +const sql = at("-c"); +if (/FROM pg_roles/.test(sql)) process.stdout.write("?column?\\n"); +else if (/^(CREATE|ALTER|GRANT)/.test(sql)) process.stdout.write(""); +else process.stdout.write("name,n\\nalpha,1\\n\\"b,eta\\",2\\n"); +`); + await chmod(join(dir, "psql"), 0o755); + process.env.PATH = `${dir}:${originalPath}`; +}); + +after(async () => { + process.env.PATH = originalPath; + await rm(dir, { recursive: true, force: true }); +}); + +async function calls(): Promise[]> { + const text = await readFile(log, "utf8").catch(() => ""); + await writeFile(log, ""); + return text.split("\n").filter(Boolean).map((line) => JSON.parse(line)); +} + +const conn = { host: "127.0.0.1", port: 5432, user: "postgres", password: "admin-secret" }; + +test("a caller's statement runs as the reader, as given, read-only, and comes back keyed by its columns", async () => { + const client = new PostgresClient({ ...conn, readerPassword: "reader-secret" }); + const statement = "COMMIT; DROP TABLE everything"; + const result = await client.readOnlyQuery("inventory", statement); + + const made = await calls(); + const asked = made.at(-1)!; + assert.equal(asked.user, READER, "the statement never runs as the admin"); + assert.equal(asked.password, "reader-secret"); + assert.equal(asked.sql, statement, "sent as given: no transaction wrapped around it as text"); + assert.equal(asked.database, "inventory"); + assert.equal(asked.quiet, true, "no command tags, which came back as rows keyed by BEGIN"); + assert.match(String(asked.options), /default_transaction_read_only=on/); + + assert.deepEqual(result.rows, [{ name: "alpha", n: "1" }, { name: "b,eta", n: "2" }]); + assert.equal(result.command, "COMMIT"); +}); + +test("the reader is made as the admin, with every attribute stated, once per process", async () => { + const client = new PostgresClient({ ...conn, readerPassword: "reader-secret" }); + await client.readOnlyQuery("inventory", "SELECT 1"); + await client.readOnlyQuery("inventory", "SELECT 2"); + + const made = await calls(); + const asAdmin = made.filter((c) => c.user === "postgres"); + assert.ok(asAdmin.every((c) => c.password === "admin-secret")); + const ddl = asAdmin.map((c) => String(c.sql)); + const role = ddl.find((s) => s.startsWith(`CREATE ROLE "${READER}"`)); + assert.ok(role, "made when it does not exist"); + for (const attribute of ["LOGIN", "NOSUPERUSER", "NOCREATEDB", "NOCREATEROLE", "NOREPLICATION", "NOBYPASSRLS"]) { + assert.match(role!, new RegExp(`\\b${attribute}\\b`)); + } + assert.ok(ddl.includes(`GRANT pg_read_all_data TO "${READER}"`), "reads everything and is granted nothing else"); + assert.ok(ddl.some((s) => /default_transaction_read_only = on/.test(s))); + assert.equal(ddl.filter((s) => s.startsWith("CREATE ROLE")).length, 1, "made once, not per call"); + assert.equal(made.filter((c) => c.user === READER).length, 2); +}); + +test("without the reader's password the statement is refused, and nothing runs as the admin", async () => { + const client = new PostgresClient(conn); + await assert.rejects(client.readOnlyQuery("inventory", "SELECT 1"), /refused rather than run as the admin/); + assert.deepEqual(await calls(), []); +}); + +test("the reader's password is read from the file the mesh delivers", async () => { + const file = join(dir, "reader.secret"); + await writeFile(file, "from-the-file\n"); + const client = PostgresClient.fromEnv({ + MESH_POSTGRES_HOST: "127.0.0.1", MESH_POSTGRES_PASSWORD: "admin-secret", + MESH_POSTGRES_READER_PASSWORD_FILE: file, + }); + await client.readOnlyQuery("inventory", "SELECT 1"); + const asked = (await calls()).at(-1)!; + assert.equal(asked.password, "from-the-file"); +}); diff --git a/modules/postgres/tools/index.ts b/modules/postgres/tools/index.ts index 90c048d..a205658 100644 --- a/modules/postgres/tools/index.ts +++ b/modules/postgres/tools/index.ts @@ -16,7 +16,7 @@ export function getPostgresTools(postgres: PostgresClient): ToolDefinition[] { }, { name: "postgres_query", - description: "Run a read-only SQL query against a named database (wrapped in a read-only transaction).", + description: "Run a read-only SQL query against a named database, as a login that can read every table and change nothing.", input: { database: { type: "string", description: "the database to query" }, sql: { type: "string", description: "the SELECT (or other read-only) statement" },