fix(security): authorize schema-unqualified SQL against the table the engine resolves

AGENTS.md

@@ -190,6 +190,7 @@ source tree — which permanently shadows git's config file. Every subsequent ag
 - `Resource` static methods must stay wrapped with `transactional()` — removing this breaks transaction isolation.
 - Worker threads (`server/threads/`) receive `workerData.noServerStart = true` to prevent recursive server startup; never start the server inside a worker.
 - `contextStorage` (AsyncLocalStorage) carries per-request context (user, transaction) across async boundaries — this is how authorization and transactions work without explicit parameter threading.
+- SQL authorization (`verifyPermsAST` → `hasPermissions`) only checks the tables recorded in the statement bucket's affected-attribute map — it iterates that map, so a table missing from it is never checked, and an empty map authorizes by vacuous truth. Two rules follow. Resolve a table reference exactly once, through `sqlEngine/binder/defaultDatabase.ts`, so the authorization layer and the engine's binder cannot disagree about which `database.table` a bare name means. And when adding a new SQL construct, either record its table references in that map or make `getUnauthorizedTableRefs()` report them — an unrecorded reference is a permission bypass, not a missing feature (GHSA-5c29-q62v-jrwf).
 - Tests under `unitTests/apiTests/` require the server to be stopped first (`node ./dist/bin/harper.js stop`) — `test:unit:apitests` does this automatically.
 - `@export` annotation on a schema class auto-generates a REST API for that table — this is the primary developer-facing API.
 - Test style: write new unit tests with `assert` (the bare `node:assert` module) against real modules — **do not add new uses of `sinon` or `rewire`**. Use plain `assert`, **not** `node:assert/strict` — strict mode's deep-equality and coercion rules cause more friction and surprising failures than they prevent; plain `assert` is the house style. When a specific check genuinely needs strict/deep-strict semantics, call `assert.strictEqual`/`assert.deepStrictEqual` explicitly (both exist on plain `assert`) rather than importing `/strict`. This is lint-enforced: oxlint's `no-restricted-imports` rule rejects `node:assert/strict` and `assert/strict` imports. Older tests in `unitTests/security/` and `unitTests/utility/` still depend on them but they are not the target shape; match newer tests in `unitTests/config/*`, `unitTests/resources/*`, `unitTests/components/*`. If you can't write a test without stubbing, comment on the issue describing what's missing and stop — don't reach for sinon/rewire as a shortcut.

integrationTests/security/fixtures/sql-unqualified-authz/config.yaml

@@ -0,0 +1,4 @@
+# Regression fixture for GHSA-5c29-q62v-jrwf — schema-unqualified SQL must be authorized.
+graphqlSchema:
+  files: '*.graphql'
+rest: true

integrationTests/security/fixtures/sql-unqualified-authz/schema.graphql

@@ -0,0 +1,16 @@
+# Regression fixture for GHSA-5c29-q62v-jrwf — schema-unqualified SQL must be authorized.
+#
+# `UnqualSecret` is the forbidden table: the test role holds no grant on it at all.
+# `UnqualPublic` is the permitted table: the role can read it, so the test can prove the
+# fix still resolves a legitimate unqualified reference instead of rejecting everything.
+# Both names are unique across databases so the engine's pickDefaultDatabase resolves them.
+type UnqualSecret @table @export {
+	id: ID @primaryKey
+	owner: String
+	ssn: String
+}
+
+type UnqualPublic @table @export {
+	id: ID @primaryKey
+	label: String
+}

integrationTests/security/sql-unqualified-table-authz.test.ts

@@ -0,0 +1,354 @@
+/**
+ * GHSA-5c29-q62v-jrwf — a schema-unqualified SQL statement must be authorized against the
+ * same database.table the engine will actually touch.
+ *
+ * The SQL authorization layer derives the affected schema/table set from the AST's
+ * `databaseid`. When the statement omits the schema qualifier that field is empty, so
+ * `getSelectAttributes` / `addSchemaTableToMap` returned without recording anything and
+ * `verifyPermsAST` ran `hasPermissions` against an empty map — which authorizes. The v2
+ * engine then resolved the same bare name to a concrete database via `pickDefaultDatabase`
+ * and read (or wrote) it, so a role with no grant on the table reached its rows.
+ *
+ * The tests below drive the ops `sql` operation as a non-super_user whose role holds no
+ * permission at all on `UnqualSecret`:
+ *
+ *   CONTROL   — the schema-qualified form is refused, proving the role really lacks the grant.
+ *   READ      — bare `SELECT ... FROM UnqualSecret` must be refused, not served, both as an
+ *               indexed lookup (which stays in the v2 engine) and as a full scan (which falls
+ *               back to legacy) — the denial must come from authorization, not from an engine
+ *               capability accident.
+ *   SYSTEM    — bare `SELECT ... FROM hdb_user` must not surrender credential material.
+ *   WRITE     — bare DELETE / UPDATE / INSERT must be refused AND must not mutate the table.
+ *   JOIN      — a bare JOIN target is bound like a bare FROM target, so it must be authorized too.
+ *   WILDCARD  — bare `SELECT *` must be refused; wildcard expansion keys off the resolved
+ *               database, so it exercises the resolution write-back on both paths.
+ *   POSITIVE  — a bare reference to a table the role *can* read still resolves and returns
+ *               rows, so the fix authorizes the resolved name rather than banning bare names.
+ *
+ * Reproduction:
+ *   npm run build && npm run test:integration -- "integrationTests/security/sql-unqualified-table-authz.test.ts"
+ */
+import { suite, test, before, after } from 'node:test';
+import { ok, strictEqual } from 'node:assert';
+import { resolve } from 'node:path';
+
+import { setupHarperWithFixture, teardownHarper, type ContextWithHarper } from '@harperfast/integration-testing';
+// @ts-expect-error utils/client.mjs has no type declarations; runtime resolves fine
+import { createApiClient, createHeaders } from '../apiTests/utils/client.mjs';
+
+const FIXTURE_PATH = resolve(import.meta.dirname, 'fixtures/sql-unqualified-authz');
+const skipSuite = process.env.HARPER_RUNTIME === 'bun' || process.platform === 'win32';
+
+const MALLORY = { username: 'unqual_mallory', password: 'Mallory-pw-5c29!' };
+const ROLE = 'unqual_reader_role';
+const DB = 'data';
+const SECRET_TABLE = 'UnqualSecret';
+const PUBLIC_TABLE = 'UnqualPublic';
+
+const SECRET_ROW = 'secret-row-1';
+const SECRET_SSN = '111-22-3333';
+const PUBLIC_ROW = 'public-row-1';
+
+/** Any of these mean the request was refused rather than served. */
+function isDenied(status: number): boolean {
+	return status === 401 || status === 403;
+}
+
+suite(
+	'GHSA-5c29-q62v-jrwf — unqualified SQL table references are authorized',
+	{ skip: skipSuite },
+	(ctx: ContextWithHarper) => {
+		let client: ReturnType<typeof createApiClient>;
+		let malloryHeaders: Record<string, string>;
+
+		/** Read the secret row back as admin, to check a denied write really did not land. */
+		async function readSecretRowAsAdmin(): Promise<Record<string, unknown> | undefined> {
+			const r = await client
+				.req()
+				.send({ operation: 'sql', sql: `SELECT * FROM ${DB}.${SECRET_TABLE} WHERE id = '${SECRET_ROW}'` })
+				.expect(200);
+			return Array.isArray(r.body) ? (r.body[0] as Record<string, unknown> | undefined) : undefined;
+		}
+
+		before(async () => {
+			await setupHarperWithFixture(ctx, FIXTURE_PATH, { config: {}, env: {} });
+			client = createApiClient(ctx.harper);
+			malloryHeaders = createHeaders(MALLORY.username, MALLORY.password);
+
+			// Mallory's role can read UnqualPublic and has NO entry for UnqualSecret — no read,
+			// no write, not even describe.
+			await client
+				.req()
+				.send({
+					operation: 'add_role',
+					role: ROLE,
+					permission: {
+						super_user: false,
+						[DB]: {
+							tables: {
+								[PUBLIC_TABLE]: {
+									read: true,
+									insert: false,
+									update: false,
+									delete: false,
+									attribute_permissions: [],
+								},
+							},
+						},
+					},
+				})
+				.expect(200);
+
+			await client
+				.req()
+				.send({
+					operation: 'add_user',
+					role: ROLE,
+					username: MALLORY.username,
+					password: MALLORY.password,
+					active: true,
+				})
+				.expect(200);
+
+			await client
+				.req()
+				.send({
+					operation: 'insert',
+					schema: DB,
+					table: SECRET_TABLE,
+					records: [{ id: SECRET_ROW, owner: 'victim', ssn: SECRET_SSN }],
+				})
+				.expect(200);
+
+			await client
+				.req()
+				.send({
+					operation: 'insert',
+					schema: DB,
+					table: PUBLIC_TABLE,
+					records: [{ id: PUBLIC_ROW, label: 'public-label' }],
+				})
+				.expect(200);
+		});
+
+		after(async () => {
+			await teardownHarper(ctx);
+		});
+
+		test('CONTROL: schema-qualified SELECT on the forbidden table is denied', async () => {
+			const r = await client
+				.reqAs(malloryHeaders)
+				.send({ operation: 'sql', sql: `SELECT id, ssn FROM ${DB}.${SECRET_TABLE}` });
+
+			ok(
+				isDenied(r.status),
+				`CONTROL BROKEN: the role was expected to have no grant on ${DB}.${SECRET_TABLE}, ` +
+					`but the qualified SELECT returned ${r.status}: ${JSON.stringify(r.body).slice(0, 300)}`
+			);
+		});
+
+		// An indexed WHERE keeps the statement inside the v2 engine; a bare full scan hits
+		// EngineUnsupportedError and falls back to legacy, which refuses unqualified names on
+		// its own. Both forms must be refused by authorization, not by an engine accident.
+		test('unqualified SELECT with an indexed predicate on the forbidden table is denied', async () => {
+			const r = await client
+				.reqAs(malloryHeaders)
+				.send({ operation: 'sql', sql: `SELECT id, ssn FROM ${SECRET_TABLE} WHERE id = '${SECRET_ROW}'` });
+
+			ok(
+				!JSON.stringify(r.body ?? '').includes(SECRET_SSN),
+				`AUTHZ BYPASS: the unqualified SELECT leaked the protected ssn value (status=${r.status}): ` +
+					`${JSON.stringify(r.body).slice(0, 300)}`
+			);
+			ok(
+				isDenied(r.status),
+				`AUTHZ BYPASS: dropping the schema qualifier let a role with no grant on ${DB}.${SECRET_TABLE} ` +
+					`run the SELECT (status=${r.status}): ${JSON.stringify(r.body).slice(0, 300)}`
+			);
+		});
+
+		test('unqualified full-scan SELECT on the forbidden table is denied', async () => {
+			const r = await client
+				.reqAs(malloryHeaders)
+				.send({ operation: 'sql', sql: `SELECT id, ssn FROM ${SECRET_TABLE}` });
+
+			ok(
+				!JSON.stringify(r.body ?? '').includes(SECRET_SSN),
+				`AUTHZ BYPASS: the unqualified SELECT leaked the protected ssn value`
+			);
+			ok(
+				isDenied(r.status),
+				`AUTHZ BYPASS: dropping the schema qualifier let a role with no grant on ${DB}.${SECRET_TABLE} ` +
+					`run the SELECT (status=${r.status}): ${JSON.stringify(r.body).slice(0, 300)}`
+			);
+		});
+
+		test('unqualified SELECT on a system table is denied', async () => {
+			const r = await client.reqAs(malloryHeaders).send({ operation: 'sql', sql: 'SELECT * FROM hdb_user' });
+
+			ok(
+				isDenied(r.status),
+				`AUTHZ BYPASS: a non-super_user read the system user table via an unqualified reference ` +
+					`(status=${r.status}): ${JSON.stringify(r.body).slice(0, 300)}`
+			);
+		});
+
+		// Constructs whose reach the authorization layer cannot determine. None of them execute
+		// today (Harper rejects derived tables and ignores SELECT ... INTO), so these lock in the
+		// fail-closed denial rather than leaving the outcome to a downstream engine error.
+		test('a statement mixing a resolvable table with an unresolvable one is denied', async () => {
+			const r = await client
+				.reqAs(malloryHeaders)
+				.send({ operation: 'sql', sql: `SELECT * FROM ${DB}.${PUBLIC_TABLE}, ${SECRET_TABLE}` });
+
+			ok(
+				!JSON.stringify(r.body ?? '').includes(SECRET_SSN),
+				`AUTHZ BYPASS: a multi-table FROM leaked the protected ssn value (status=${r.status})`
+			);
+			ok(
+				isDenied(r.status),
+				`AUTHZ BYPASS: a multi-table FROM was authorized on the strength of the table that resolved ` +
+					`(status=${r.status}): ${JSON.stringify(r.body).slice(0, 300)}`
+			);
+		});
+
+		test('a derived table wrapping the forbidden table is denied', async () => {
+			const r = await client
+				.reqAs(malloryHeaders)
+				.send({ operation: 'sql', sql: `SELECT * FROM (SELECT * FROM ${SECRET_TABLE}) AS sub` });
+
+			ok(
+				!JSON.stringify(r.body ?? '').includes(SECRET_SSN),
+				`AUTHZ BYPASS: a derived table leaked the protected ssn value (status=${r.status})`
+			);
+			ok(
+				isDenied(r.status),
+				`a derived table must be refused by authorization, not left to a downstream error ` +
+					`(status=${r.status}): ${JSON.stringify(r.body).slice(0, 300)}`
+			);
+		});
+
+		test('SELECT ... INTO the forbidden table is denied', async () => {
+			const r = await client
+				.reqAs(malloryHeaders)
+				.send({ operation: 'sql', sql: `SELECT 1 AS id INTO ${SECRET_TABLE}` });
+
+			ok(
+				isDenied(r.status),
+				`a SELECT INTO target is invisible to the affected-attribute map and must be refused ` +
+					`(status=${r.status}): ${JSON.stringify(r.body).slice(0, 300)}`
+			);
+		});
+
+		test('unqualified DELETE on the forbidden table is denied and does not remove the row', async () => {
+			const r = await client
+				.reqAs(malloryHeaders)
+				.send({ operation: 'sql', sql: `DELETE FROM ${SECRET_TABLE} WHERE id = '${SECRET_ROW}'` });
+
+			const row = await readSecretRowAsAdmin();
+			ok(row, `AUTHZ BYPASS: an unqualified DELETE removed a row the role has no delete grant on (status=${r.status})`);
+			ok(isDenied(r.status), `AUTHZ BYPASS: unqualified DELETE was accepted (status=${r.status})`);
+		});
+
+		test('unqualified UPDATE on the forbidden table is denied and does not change the row', async () => {
+			const r = await client
+				.reqAs(malloryHeaders)
+				.send({ operation: 'sql', sql: `UPDATE ${SECRET_TABLE} SET ssn = 'tampered' WHERE id = '${SECRET_ROW}'` });
+
+			const row = await readSecretRowAsAdmin();
+			strictEqual(
+				row?.ssn,
+				SECRET_SSN,
+				`AUTHZ BYPASS: an unqualified UPDATE mutated a row the role has no update grant on (status=${r.status})`
+			);
+			ok(isDenied(r.status), `AUTHZ BYPASS: unqualified UPDATE was accepted (status=${r.status})`);
+		});
+
+		test('unqualified INSERT on the forbidden table is denied and does not add a row', async () => {
+			const injectedId = 'injected-row-1';
+			const r = await client
+				.reqAs(malloryHeaders)
+				.send({ operation: 'sql', sql: `INSERT INTO ${SECRET_TABLE} (id, owner) VALUES ('${injectedId}', 'mallory')` });
+
+			const check = await client
+				.req()
+				.send({ operation: 'sql', sql: `SELECT id FROM ${DB}.${SECRET_TABLE} WHERE id = '${injectedId}'` })
+				.expect(200);
+			strictEqual(
+				Array.isArray(check.body) ? check.body.length : -1,
+				0,
+				`AUTHZ BYPASS: an unqualified INSERT wrote to a table the role has no insert grant on (status=${r.status})`
+			);
+			ok(isDenied(r.status), `AUTHZ BYPASS: unqualified INSERT was accepted (status=${r.status})`);
+		});
+
+		// A bare JOIN target is resolved by the binder just like a bare FROM target, so it has to be
+		// authorized too — otherwise the forbidden table is reachable as the right-hand side.
+		test('unqualified JOIN onto the forbidden table is denied', async () => {
+			const r = await client.reqAs(malloryHeaders).send({
+				operation: 'sql',
+				sql:
+					`SELECT p.id, s.ssn FROM ${PUBLIC_TABLE} AS p ` +
+					`INNER JOIN ${SECRET_TABLE} AS s ON p.id = s.id WHERE p.id = '${PUBLIC_ROW}'`,
+			});
+
+			ok(
+				!JSON.stringify(r.body ?? '').includes(SECRET_SSN),
+				`AUTHZ BYPASS: an unqualified JOIN leaked the protected ssn value (status=${r.status}): ` +
+					`${JSON.stringify(r.body).slice(0, 300)}`
+			);
+			ok(
+				isDenied(r.status),
+				`AUTHZ BYPASS: an unqualified JOIN reached a table the role has no grant on (status=${r.status}): ` +
+					`${JSON.stringify(r.body).slice(0, 300)}`
+			);
+		});
+
+		// SELECT * expansion is driven by the resolved database (updateAttributeWildcardsForRolePerms
+		// reads from[0].databaseid), so it exercises the resolution write-back on the allow path.
+		test('unqualified SELECT * on the forbidden table is denied', async () => {
+			const r = await client.reqAs(malloryHeaders).send({ operation: 'sql', sql: `SELECT * FROM ${SECRET_TABLE}` });
+
+			ok(
+				!JSON.stringify(r.body ?? '').includes(SECRET_SSN),
+				`AUTHZ BYPASS: an unqualified SELECT * leaked the protected ssn value (status=${r.status})`
+			);
+			ok(isDenied(r.status), `AUTHZ BYPASS: an unqualified SELECT * was accepted (status=${r.status})`);
+		});
+
+		test('unqualified SELECT * on a permitted table still resolves and returns rows', async () => {
+			const r = await client.reqAs(malloryHeaders).send({ operation: 'sql', sql: `SELECT * FROM ${PUBLIC_TABLE}` });
+
+			strictEqual(
+				r.status,
+				200,
+				`REGRESSION: an unqualified SELECT * on a table the role CAN read was refused (status=${r.status}): ` +
+					`${JSON.stringify(r.body).slice(0, 300)}`
+			);
+			const rows = Array.isArray(r.body) ? r.body : [];
+			ok(
+				rows.some((row: any) => row?.id === PUBLIC_ROW && row?.label === 'public-label'),
+				`REGRESSION: unqualified SELECT * on the permitted table returned no usable rows: ` +
+					`${JSON.stringify(r.body).slice(0, 300)}`
+			);
+		});
+
+		test('unqualified SELECT on a permitted table still resolves and returns rows', async () => {
+			const r = await client
+				.reqAs(malloryHeaders)
+				.send({ operation: 'sql', sql: `SELECT id, label FROM ${PUBLIC_TABLE} WHERE id = '${PUBLIC_ROW}'` });
+
+			strictEqual(
+				r.status,
+				200,
+				`REGRESSION: an unqualified SELECT on a table the role CAN read was refused (status=${r.status}): ` +
+					`${JSON.stringify(r.body).slice(0, 300)}`
+			);
+			const rows = Array.isArray(r.body) ? r.body : [];
+			ok(
+				rows.some((row: any) => row?.id === PUBLIC_ROW),
+				`REGRESSION: unqualified SELECT on the permitted table returned no rows: ${JSON.stringify(r.body).slice(0, 300)}`
+			);
+		});
+	}
+);