From aa9f0ba9e0ea4c01f41ffec6f3551b0bb58feb1d Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Wed, 8 Jul 2026 14:53:16 +0800 Subject: [PATCH 1/3] test: fix WebKit hide-on-scroll flake + guard retrieval owner_filter callsites Two follow-ups from the 48h PR review. - tests/ui-tools.spec.ts (#5): the "phone bottom search dock hides while scrolling" test failed on WebKit only (release-browser-matrix) because WebKit doesn't reliably emit a native scroll event for a programmatic scrollTo, so useHideOnScroll never ran and data-scroll-hidden stayed unset. Dispatch an explicit scroll event after each scrollTo; harmless on Chromium/Firefox (the rAF guard dedupes). - tests/retrieval-owner-filter-guard.test.ts (#3): defense-in-depth tripwire. The SQL retrieval_owner_matches fails OPEN on a null owner_filter and RLS is service-role-only, so the app filter is the sole tenant boundary. Until the matcher is made fail-closed (gated on the Supabase Preview migration-drift fix so the SQL change is verifiable), this asserts every src .rpc() owner_filter argument comes from a sanctioned scope helper / the public sentinel, never a literal null. Co-Authored-By: Claude Opus 4.8 --- tests/retrieval-owner-filter-guard.test.ts | 71 ++++++++++++++++++++++ tests/ui-tools.spec.ts | 13 +++- 2 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 tests/retrieval-owner-filter-guard.test.ts diff --git a/tests/retrieval-owner-filter-guard.test.ts b/tests/retrieval-owner-filter-guard.test.ts new file mode 100644 index 000000000..787b37038 --- /dev/null +++ b/tests/retrieval-owner-filter-guard.test.ts @@ -0,0 +1,71 @@ +import { readFileSync, readdirSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +// Guard for the retrieval owner-scope boundary (48h-review finding #3). +// +// The SQL `retrieval_owner_matches(owner_filter, row_owner_id)` returns TRUE for EVERY row when +// `owner_filter IS NULL` (fails open by design; RLS is service-role-only, so the app filter is the +// sole tenant boundary). Until a migration makes null fail closed — gated on the Supabase Preview +// drift fix so the SQL change can be verified — this test is the tripwire: no `.rpc(...)` call in +// `src/` may pass a *literal* null/undefined `owner_filter`, and every owner_filter value must come +// from the sanctioned scope helpers (which fail closed in production) or the public sentinel — never +// a raw/unaudited value that could reach the RPC as null and silently return another tenant's rows. + +const SRC_DIR = join(process.cwd(), "src"); + +// Sanctioned right-hand sides for an `owner_filter` / `p_owner_filter` RPC argument. +const SANCTIONED_SOURCES = [ + /^retrievalOwnerFilter\(/, + /^requireOwnerScope\(/, + /^ownerScopeForDocumentFilteredRetrieval\(/, + /^PUBLIC_OWNER_FILTER_SENTINEL\b/, + // corpus-grounding threads through the exact scope it was handed; documented safe because rag.ts + // derives `args.ownerFilter` from ownerScopeForDocumentFilteredRetrieval (never a raw null in prod). + /^args\.ownerFilter\b/, +]; + +const OWNER_FILTER_ARG = /\b(?:p_)?owner_filter\s*:\s*(.+?)\s*,?\s*$/; + +function sourceFiles(dir: string): string[] { + return readdirSync(dir, { withFileTypes: true, recursive: true }) + .filter((entry) => entry.isFile() && /\.tsx?$/.test(entry.name) && !entry.name.endsWith(".d.ts")) + .map((entry) => join((entry as { parentPath?: string; path?: string }).parentPath ?? entry.path ?? dir, entry.name)) + .filter((path) => !path.includes("database.types")); +} + +describe("retrieval owner_filter callsite guard (finding #3)", () => { + it("routes every owner_filter RPC argument through a sanctioned scope source (never literal null)", () => { + const offenders: string[] = []; + let checked = 0; + + for (const file of sourceFiles(SRC_DIR)) { + const lines = readFileSync(file, "utf8").split(/\r?\n/); + lines.forEach((line, index) => { + const trimmed = line.trim(); + if (trimmed.startsWith("//") || trimmed.startsWith("*") || trimmed.startsWith("/*")) return; + const match = trimmed.match(OWNER_FILTER_ARG); + if (!match) return; + + checked += 1; + const rhs = match[1].trim(); + const location = `${file.replace(process.cwd(), "").replace(/\\/g, "/")}:${index + 1}`; + if (/^(null|undefined)\b/.test(rhs)) { + offenders.push(`${location} — literal ${rhs}`); + } else if (!SANCTIONED_SOURCES.some((pattern) => pattern.test(rhs))) { + offenders.push(`${location} — unsanctioned owner_filter source: ${rhs}`); + } + }); + } + + // Fail loudly if the scan matched nothing (e.g. the RPC param was renamed) rather than passing vacuously. + expect(checked, "found no owner_filter RPC callsites to guard — has the param name changed?").toBeGreaterThanOrEqual(5); + expect( + offenders, + `owner_filter must come from retrievalOwnerFilter / requireOwnerScope / ` + + `ownerScopeForDocumentFilteredRetrieval / PUBLIC_OWNER_FILTER_SENTINEL — never a raw or null value:\n` + + offenders.join("\n"), + ).toEqual([]); + }); +}); diff --git a/tests/ui-tools.spec.ts b/tests/ui-tools.spec.ts index 1bb8b8b54..1a99dc510 100644 --- a/tests/ui-tools.spec.ts +++ b/tests/ui-tools.spec.ts @@ -762,13 +762,22 @@ test.describe("Clinical KB tools launcher", () => { await expect(dock).toBeVisible(); await expect(dock).not.toHaveAttribute("data-scroll-hidden", "true"); - await page.evaluate(() => window.scrollTo({ top: 120, behavior: "auto" })); + await page.evaluate(() => { + window.scrollTo({ top: 120, behavior: "auto" }); + // WebKit doesn't reliably emit a native scroll event for a programmatic scrollTo, so the + // hide-on-scroll listener never ran and data-scroll-hidden stayed unset (release-browser-matrix + // WebKit flake). Dispatch one explicitly; harmless on Chromium/Firefox (the rAF guard dedupes). + window.dispatchEvent(new Event("scroll")); + }); await expect(dock).toHaveAttribute("data-scroll-hidden", "true"); await expect .poll(async () => dock.evaluate((node) => window.getComputedStyle(node).transform !== "none")) .toBe(true); - await page.evaluate(() => window.scrollTo({ top: 60, behavior: "auto" })); + await page.evaluate(() => { + window.scrollTo({ top: 60, behavior: "auto" }); + window.dispatchEvent(new Event("scroll")); + }); await expect(dock).not.toHaveAttribute("data-scroll-hidden", "true"); await expect .poll(async () => dock.evaluate((node) => window.getComputedStyle(node).transform === "none")) From 4fec80f327a01722ed3169436be528b1ddc4d3a3 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Wed, 8 Jul 2026 14:57:36 +0800 Subject: [PATCH 2/3] style: prettier-format owner_filter guard test Co-Authored-By: Claude Opus 4.8 --- tests/retrieval-owner-filter-guard.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/retrieval-owner-filter-guard.test.ts b/tests/retrieval-owner-filter-guard.test.ts index 787b37038..41e6a1dd7 100644 --- a/tests/retrieval-owner-filter-guard.test.ts +++ b/tests/retrieval-owner-filter-guard.test.ts @@ -60,7 +60,10 @@ describe("retrieval owner_filter callsite guard (finding #3)", () => { } // Fail loudly if the scan matched nothing (e.g. the RPC param was renamed) rather than passing vacuously. - expect(checked, "found no owner_filter RPC callsites to guard — has the param name changed?").toBeGreaterThanOrEqual(5); + expect( + checked, + "found no owner_filter RPC callsites to guard — has the param name changed?", + ).toBeGreaterThanOrEqual(5); expect( offenders, `owner_filter must come from retrievalOwnerFilter / requireOwnerScope / ` + From 2b7fed23280a7910a5623a48deefbbacfcc72b49 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Wed, 8 Jul 2026 15:11:54 +0800 Subject: [PATCH 3/3] fix(test): avoid Dirent.path typing in owner_filter guard (CI @types/node drift) Use the string-returning readdirSync recursive overload instead of withFileTypes so the scan doesn't depend on Dirent.parentPath/path, which differ across @types/node versions and failed the CI typecheck (TS2339 Property 'path' does not exist on Dirent). Co-Authored-By: Claude Opus 4.8 --- tests/retrieval-owner-filter-guard.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/retrieval-owner-filter-guard.test.ts b/tests/retrieval-owner-filter-guard.test.ts index 41e6a1dd7..4f65fb2b4 100644 --- a/tests/retrieval-owner-filter-guard.test.ts +++ b/tests/retrieval-owner-filter-guard.test.ts @@ -29,10 +29,12 @@ const SANCTIONED_SOURCES = [ const OWNER_FILTER_ARG = /\b(?:p_)?owner_filter\s*:\s*(.+?)\s*,?\s*$/; function sourceFiles(dir: string): string[] { - return readdirSync(dir, { withFileTypes: true, recursive: true }) - .filter((entry) => entry.isFile() && /\.tsx?$/.test(entry.name) && !entry.name.endsWith(".d.ts")) - .map((entry) => join((entry as { parentPath?: string; path?: string }).parentPath ?? entry.path ?? dir, entry.name)) - .filter((path) => !path.includes("database.types")); + // `recursive: true` without `withFileTypes` returns relative paths as strings — avoids the + // Dirent.parentPath/path typing churn across @types/node versions. + return readdirSync(dir, { recursive: true }) + .map((entry) => String(entry)) + .filter((name) => /\.tsx?$/.test(name) && !name.endsWith(".d.ts") && !name.includes("database.types")) + .map((name) => join(dir, name)); } describe("retrieval owner_filter callsite guard (finding #3)", () => {