Record-scoped allowRead: unified row-level read access control (#1422 gap 2)

integrationTests/security/fixtures/reject-thenable-allowread/config.yaml

@@ -0,0 +1,7 @@
+# Regression fixture for #1786 — a sync-declared allowRead that returns a rejected thenable
+# during per-record evaluation must not leak an unhandledRejection.
+graphqlSchema:
+  files: '*.graphql'
+jsResource:
+  files: resources.js
+rest: true

integrationTests/security/fixtures/reject-thenable-allowread/resources.js

@@ -0,0 +1,43 @@
+// Regression fixture for #1786 — a sync-declared (no `async` keyword) allowRead that returns a
+// REJECTED thenable during per-record evaluation. Before the fix, neither the query-traversal
+// guard nor the subscription delivery filter attached a handler to that rejection before
+// discarding it, so each denied candidate/event reached Harper's global unhandledRejection
+// logger. The fix attaches a no-op handler while keeping the fail-closed decision and the
+// existing one-time diagnostic warning.
+
+function isSuper(user) {
+	return !!user?.role?.permission?.super_user;
+}
+
+// Owner marking a record whose per-record allowRead evaluation always rejects. Kept distinct
+// from real owners so a normal, allowed record can also be present — an SSE subscription whose
+// EVERY record is denied never writes a byte, so headers wouldn't flush and the test client
+// would hang waiting for a response that never comes. This isn't a Harper bug to work around in
+// the test; it just means the fixture needs at least one deliverable record.
+const REJECTING_OWNER = 'REJECT_SENTINEL';
+
+export class Rejecting extends tables.Rejecting {
+	allowRead(user, target, context) {
+		if (isSuper(user)) return true;
+		if (!super.allowRead(user, target, context)) return false;
+		const owner = this?.owner;
+		// Collection scope (subscribe entry check) / no loaded record: allow the connection to open.
+		if (owner === undefined || owner === null) return true;
+		if (owner === REJECTING_OWNER) {
+			// Per-record (query traversal, subscription event delivery): sync-declared but returns a
+			// rejected thenable — must fail closed WITHOUT leaking an unhandledRejection.
+			return Promise.reject(new Error('allowRead intentionally rejects during per-record evaluation (#1786)'));
+		}
+		return owner === user?.username;
+	}
+
+	allowUpdate(user) {
+		return isSuper(user);
+	}
+	allowCreate(user) {
+		return isSuper(user);
+	}
+	allowDelete(user) {
+		return isSuper(user);
+	}
+}

integrationTests/security/fixtures/reject-thenable-allowread/schema.graphql

@@ -0,0 +1,8 @@
+# Regression fixture for #1786 — a sync-declared allowRead that returns a rejected thenable
+# during per-record evaluation must not leak an unhandledRejection (query traversal guard and
+# subscription delivery filter share this same override).
+type Rejecting @table @export {
+	id: ID @primaryKey
+	owner: String
+	secret: String
+}

integrationTests/security/fixtures/subscription-row-allowread/config.yaml

@@ -0,0 +1,6 @@
+# Regression fixture for #1419 — per-row allowRead must filter subscription delivery.
+graphqlSchema:
+  files: '*.graphql'
+jsResource:
+  files: resources.js
+rest: true

integrationTests/security/fixtures/subscription-row-allowread/resources.js

@@ -0,0 +1,39 @@
+// Regression fixture for #1419 — per-row allowRead must filter subscription delivery.
+//
+// The recommended composition pattern for a record-scoped override:
+//   - Gate on the base table/RBAC grant via `super.allowRead(...)` first, so a user who loses the
+//     role's table-read is denied — at request entry, and (for a live subscription) when the #1414
+//     re-auth recheck re-runs this same override against the fresh user.
+//   - At collection scope (no record loaded — a whole-table subscribe, or the re-auth recheck),
+//     return the base grant so the connection opens; per-row filtering happens during delivery.
+//   - Per record (a loaded row), additionally require the requesting user to own the row.
+//
+// Super users always pass so that seed writes and setup ops succeed.
+
+function isSuper(user) {
+	return !!user?.role?.permission?.super_user;
+}
+
+export class Vault extends tables.Vault {
+	allowRead(user, target, context) {
+		if (isSuper(user)) return true;
+		// Base table/RBAC grant — composes with the override so revoking the role's read terminates
+		// (at entry and via #1414 re-auth), rather than the override standing in for RBAC.
+		if (!super.allowRead(user, target, context)) return false;
+		const owner = this?.owner;
+		// Collection subscribe / no loaded record: RBAC passed, allow the connection to open.
+		if (owner === undefined || owner === null) return true;
+		// Per-row: only the owner can read this specific record.
+		return owner === user?.username;
+	}
+
+	allowUpdate(user, _record, _context) {
+		return isSuper(user);
+	}
+	allowCreate(user, _record, _context) {
+		return isSuper(user);
+	}
+	allowDelete(user, _target, _context) {
+		return isSuper(user);
+	}
+}

integrationTests/security/fixtures/subscription-row-allowread/schema.graphql

@@ -0,0 +1,11 @@
+# Regression fixture for #1419 — per-row allowRead must filter subscription delivery.
+#
+# Vault rows are owned by a specific user (`owner` field). The allowRead override in
+# resources.js permits a read only when the record's owner matches the requesting user,
+# while still allowing the collection-level subscription to open. The test verifies that a
+# whole-table SSE subscription no longer leaks other owners' row events.
+type Vault @table @export {
+	id: ID @primaryKey
+	owner: String
+	secret: String
+}

integrationTests/security/query-row-allowread-checkpermission.test.ts

@@ -0,0 +1,132 @@
+/**
+ * #1786 review (heskew) — QUERY's body must not be able to disable the row-level `allowRead`
+ * guard.
+ *
+ * `Resource.query` threads `checkPermission` from the framework-controlled URL `query` onto the
+ * client-controlled request `data` (the QUERY body) so `Table.search` sees the flag and enforces
+ * the record-scoped `allowRead` override (#1422 gap 2). Before the fix, that thread-through only
+ * filled `data.checkPermission` when it was nullish — a client sending `checkPermission: false`
+ * in the QUERY body survived untouched and disabled the row-level guard entirely, returning the
+ * full unfiltered set.
+ *
+ * Fixture: reuses the `Vault` table from `subscription-row-allowread` (owner-scoped allowRead,
+ * composes the base RBAC grant via `super.allowRead`).
+ *
+ * Reproduction:
+ *   npm run test:integration -- "integrationTests/security/query-row-allowread-checkpermission.test.ts"
+ */
+import { suite, test, before, after } from 'node:test';
+import { deepStrictEqual, ok } 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/subscription-row-allowread');
+const skipSuite = process.env.HARPER_RUNTIME === 'bun' || process.platform === 'win32';
+
+const ALICE = { username: 'query_allowread_alice', password: 'Alice-pw-1786!' };
+const BOB = { username: 'query_allowread_bob', password: 'Bobby-pw-1786!' };
+const ROLE = 'query_allowread_role';
+
+const ALICE_ROWS = ['qrow-a1', 'qrow-a2'];
+const BOB_ROWS = ['qrow-b1', 'qrow-b2'];
+
+/**
+ * Issue an HTTP QUERY request via fetch — supertest/superagent has no API for non-standard
+ * verbs, while undici's fetch passes custom method tokens through.
+ */
+async function queryVault(restURL: string, headers: Record<string, string>, body: any): Promise<any[]> {
+	const resp = await fetch(`${restURL}/Vault/`, {
+		method: 'QUERY',
+		headers: { ...headers, 'Content-Type': 'application/json' },
+		body: JSON.stringify(body),
+	});
+	const text = await resp.text();
+	ok(resp.status === 200, `QUERY /Vault/ returned ${resp.status}: ${text}`);
+	const data = JSON.parse(text);
+	return Array.isArray(data) ? data : [];
+}
+
+suite(
+	'#1786 QUERY body checkPermission cannot bypass row-level allowRead',
+	{ skip: skipSuite },
+	(ctx: ContextWithHarper) => {
+		let client: ReturnType<typeof createApiClient>;
+		let restURL = '';
+		let aliceHeaders: Record<string, string>;
+
+		before(async () => {
+			await setupHarperWithFixture(ctx, FIXTURE_PATH, { config: {}, env: {} });
+			client = createApiClient(ctx.harper);
+			restURL = ctx.harper.httpURL;
+
+			const deadline = Date.now() + 30_000;
+			while (Date.now() < deadline) {
+				try {
+					const probe = await client.reqRest('/Vault/').timeout(3000);
+					if (probe.status !== 404) break;
+				} catch {
+					/* not ready */
+				}
+				await new Promise((r) => setTimeout(r, 250));
+			}
+
+			await client
+				.req()
+				.send({
+					operation: 'add_role',
+					role: ROLE,
+					permission: {
+						super_user: false,
+						data: {
+							tables: {
+								Vault: { read: true, insert: true, update: true, delete: true, attribute_permissions: [] },
+							},
+						},
+					},
+				})
+				.expect(200);
+
+			for (const u of [ALICE, BOB]) {
+				await client
+					.req()
+					.send({ operation: 'add_user', role: ROLE, username: u.username, password: u.password, active: true })
+					.expect(200);
+			}
+
+			const records = [
+				...ALICE_ROWS.map((id) => ({ id, owner: ALICE.username, secret: `alice-secret-${id}` })),
+				...BOB_ROWS.map((id) => ({ id, owner: BOB.username, secret: `bob-secret-${id}` })),
+			];
+			await client.req().send({ operation: 'insert', schema: 'data', table: 'Vault', records }).expect(200);
+
+			aliceHeaders = createHeaders(ALICE.username, ALICE.password);
+		});
+
+		after(async () => {
+			await teardownHarper(ctx);
+		});
+
+		test('CONTROL: a plain QUERY only returns the requesting user’s own rows', async () => {
+			const results = await queryVault(restURL, aliceHeaders, {});
+			const ids = results.map((r: any) => r.id).sort();
+			deepStrictEqual(ids, [...ALICE_ROWS].sort(), 'row-level allowRead should scope a QUERY to the caller’s own rows');
+		});
+
+		test('BYPASS ATTEMPT: checkPermission: false in the QUERY body must not disable the guard', async () => {
+			const results = await queryVault(restURL, aliceHeaders, { checkPermission: false });
+			const ids = results.map((r: any) => r.id).sort();
+			ok(
+				!BOB_ROWS.some((id) => ids.includes(id)),
+				`QUERY-VERB BYPASS (#1786): client-supplied checkPermission:false leaked Bob's rows: ${JSON.stringify(ids)}`
+			);
+			deepStrictEqual(
+				ids,
+				[...ALICE_ROWS].sort(),
+				'a client-supplied checkPermission:false must not widen the QUERY result beyond the caller’s own rows'
+			);
+		});
+	}
+);