Merge pull request 'postgres: the store's query runs as a read-only login, never as the admin (hq #193)' (#209) from fix/193-the-store-reads-as-a-reader into main

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<void>;
+
   /**
    * 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<QueryResult> {
-    // 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<void> {
+    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<QueryResult> {
+    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. */

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"
     }

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"
   },

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<Record<string, unknown>[]> {
+  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");
+});

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" },