From a11ea870ab0236673f3b8de1a61afb8356212dde Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 18 Jul 2026 03:27:04 +0800 Subject: [PATCH 1/3] docs: record retrieval RPC review --- docs/branch-review-ledger.md | 1 + ..._bound_versioned_retrieval_match_count.sql | 86 +++++++++++++++++++ tests/retrieval-access-scope.test.ts | 20 +++++ 3 files changed, 107 insertions(+) create mode 100644 supabase/migrations/20260717170000_bound_versioned_retrieval_match_count.sql diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 43cd46874..213c9d6da 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -578,3 +578,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-17 | PR #716 / codex/pr-process-hardening-20260717 | 220de891f10f82df11eb2e8137367514c139a206 + reviewed implementation/follow-up diff | PR metadata, CI triage, review routing, Actions permissions, secret scanning, and branch-hygiene hardening | No remaining high-confidence P0-P2 defect in the reviewed process diff. Added a trusted base-SHA PR metadata/risk policy with draft and merge-queue handling; corrected CI triage to compare only the same workflow's latest completed `main` run; wired self-tests into local/hosted gates; documented the policy; created the missing durable Codex routing labels; removed four unused per-head routing labels; and applied read-only default Actions tokens, immutable Action pinning, no Actions-authored approvals, push protection, and automatic merged-branch deletion. Four policy defects were fixed before merge: API-only paths no longer trigger UI evidence, slash-bearing outcome titles are accepted, all seven governance items are required exactly, and placeholder risk/rollback text is rejected. | Offline `check:pr-policy`, `check:ci-triage`, `check:ci-scope`, action-pin check, scoped Prettier, and `git diff --check` passed. Initial hosted CI passed static, safety, coverage, build, images, Semgrep, Gitleaks, GitGuardian, and the aggregate gate; exact-head CI is required after the review fix. Broader local gates were deferred because other registered worktrees repeatedly held the heavyweight lock. GitHub provider inspection/settings changes were explicitly authorized. No Supabase/OpenAI/product-provider command ran. | | 2026-07-17 | codex/mobile-search-phone-refresh-20260717 (supersedes PR #700) | 42a3e3ce65dc5a0e1dce386e0b91fccd23d13d6c + reviewed follow-up diff | phone universal-search command-panel recovery and merge-readiness review | Recovered the still-useful behavior from PR #700 onto current `main`, including its hydration fix and wide-touch regression coverage. Hosted Production UI then exposed one desktop focus race: capability state intentionally initializes false for hydration safety, but an input could receive focus before the post-hydration effect synchronized the real browser state. The follow-up recomputes the same guarded predicate synchronously on focus; it requires the placement breakpoint plus either a fine pointer or a zero-touch desktop fallback, so wide touch devices remain suppressed while desktop keeps the first command-panel interaction. No remaining high-confidence P0-P2 defect was found in the scoped diff. | Focused Vitest 7/7; `npm run ensure` verified the project at `http://localhost:3751`; hosted static, safety, coverage, build, advisory UI, Semgrep, Gitleaks, and GitGuardian passed; the first hosted Production UI run isolated the nine desktop regressions. The focused browser proof reproduced the desktop race while the wide-touch regression passed, and exact-head hosted Production UI remains required after the focus fix. `format:changed -- --check` and `git diff --check` passed before the final follow-up. No Supabase/OpenAI/product-provider command ran. | | 2026-07-17 | final historical branch/worktree cleanup against `origin/main` | 5d195d7ca8752b2ae4006725c6b145c5662bb687 | branch-cleanup | Merged PRs #716 and #717, closed superseded PR #700, and removed the three clean task worktrees. Deleted exact remote refs for `codex/mobile-search-phone-fix-20260717` (`590f32b73`), `claude/audit-findings-review-phgz92` (`ea3b8f95b8`), `codex/chat-forms-import-6914` (`b05da82f82`), `codex/chat-supabase-migration-preflight-b463` (`1ef0faee95`), and `codex/dsm-diagnosis-mode` (`f6cda83ca6`); the merged #716/#717 branches were deleted automatically. Deleted 14 unregistered local refs only after direct-main ancestry, exact ledger deletion-pending proof, or exact merged-PR commit provenance. Final inventory found zero remote branches without an open PR or registered worktree, zero locally merged or exact deletion-pending orphan refs, and no retired target refs or paths. Thirteen non-ancestor local refs and 25 registered worktrees remain preserved because they are backups, patch-unique/unresolved, open-PR-owned, or ownership could not be safely disproved. | Fresh fetch/prune; exact GitHub PR/head/merge associations; cherry-pick-aware logs; DSM PR #661 exact commit/file provenance; exact leased remote deletes; exact-old-value local `update-ref` deletes; clean-worktree and path/process checks; worktree prune; final zero-orphan inventory. The Codex task registry lookup timed out, so ambiguous registered worktrees were conservatively retained. No Supabase, OpenAI, production-data, or live clinical workflow ran. | +| 2026-07-17 | codex/supabase-owner-rpc-hardening | 5aa571504ac3f20069e2d61a9b96bf99aa5bcd2e | database/retrieval migration review | Fixed a P2 bounded-resource defect: explicitly null or negative `match_count` reaches PostgreSQL `LIMIT` unbounded semantics in the two versioned retrieval RPC wrappers. The forward migration clamps returned rows to 1–96 while preserving owner-plus-public filtering, signatures, defaults, and service-role-only execute grants. No further high-confidence P0–P2 issue was found in the changed scope. | Static migration-contract audit and `git diff --check` passed; `check:function-grants` passed. Full Vitest and aggregate verification could not start because this worktree lacks `tsx`/Vitest dependencies and runs Node 20 while the repository requires Node 24. No Docker replay, live Supabase, hosted CI, or provider-backed checks ran. | diff --git a/supabase/migrations/20260717170000_bound_versioned_retrieval_match_count.sql b/supabase/migrations/20260717170000_bound_versioned_retrieval_match_count.sql new file mode 100644 index 000000000..6eba59a4d --- /dev/null +++ b/supabase/migrations/20260717170000_bound_versioned_retrieval_match_count.sql @@ -0,0 +1,86 @@ +-- Bound the result cardinality of the two versioned owner-plus-public retrieval +-- entry points. PostgREST permits explicitly-null RPC arguments; PostgreSQL +-- interprets LIMIT NULL (and a negative LIMIT) as no limit. Before this guard, +-- a malformed or compromised service-role caller could turn the final LIMIT +-- into an unbounded result within each helper's candidate window, multiplying +-- JSON hydration and response size on a hot retrieval path. +-- +-- Keep the public signatures and defaults intact for generated types and +-- callers. The inner helpers already cap candidate work; this outer clamp +-- makes the exposed result contract deterministic: 1..96 rows. +-- +-- Rollback / forward repair: re-create these wrappers from +-- 20260717160000_optimize_owner_public_retrieval.sql to restore the former +-- pass-through count behaviour. No data is changed. + +create or replace function public.match_document_chunks_text_v2( + query_text text, + match_count integer default 12, + document_filters uuid[] default null, + owner_filter uuid default '00000000-0000-0000-0000-000000000000'::uuid, + include_public boolean default true +) +returns table ( + id uuid, document_id uuid, title text, file_name text, page_number integer, + chunk_index integer, section_heading text, content text, retrieval_synopsis text, + image_ids uuid[], source_metadata jsonb, document_labels jsonb, document_summary text, + similarity double precision, text_rank double precision, hybrid_score double precision, + lexical_score double precision, images jsonb +) +language sql +stable +set search_path = public, extensions, pg_temp +as $$ + select * + from public.match_document_chunks_text_scoped( + $1, + least(greatest(coalesce($2, 12), 1), 96), + $3, + $4, + $5 + ); +$$; + +create or replace function public.match_document_index_units_hybrid_v2( + query_embedding extensions.vector(1536), + query_text text, + match_count integer default 24, + min_similarity double precision default 0.1, + document_filters uuid[] default null, + owner_filter uuid default '00000000-0000-0000-0000-000000000000'::uuid, + include_public boolean default true +) +returns table ( + id uuid, document_id uuid, source_chunk_id uuid, source_image_id uuid, unit_type text, title text, + content text, page_start integer, page_end integer, heading_path text[], normalized_terms text[], + source_span jsonb, quality_score real, extraction_mode text, similarity double precision, + text_rank double precision, hybrid_score double precision, metadata jsonb +) +language sql +stable +set search_path = public, extensions, pg_temp +as $$ + select * + from public.match_document_index_units_hybrid_scoped( + $1, + $2, + least(greatest(coalesce($3, 24), 1), 96), + $4, + $5, + $6, + $7 + ); +$$; + +-- CREATE OR REPLACE preserves privileges in PostgreSQL, but re-state the +-- service-role-only contract so future function changes cannot reopen it. +revoke all on function public.match_document_chunks_text_v2(text, integer, uuid[], uuid, boolean) + from public, anon, authenticated; +grant execute on function public.match_document_chunks_text_v2(text, integer, uuid[], uuid, boolean) + to service_role; +revoke all on function public.match_document_index_units_hybrid_v2( + extensions.vector, text, integer, double precision, uuid[], uuid, boolean +) from public, anon, authenticated; +grant execute on function public.match_document_index_units_hybrid_v2( + extensions.vector, text, integer, double precision, uuid[], uuid, boolean +) to service_role; diff --git a/tests/retrieval-access-scope.test.ts b/tests/retrieval-access-scope.test.ts index 0e224333b..485c3a81c 100644 --- a/tests/retrieval-access-scope.test.ts +++ b/tests/retrieval-access-scope.test.ts @@ -8,6 +8,10 @@ const optimizedRetrievalMigration = readFileSync( join(root, "supabase/migrations/20260717160000_optimize_owner_public_retrieval.sql"), "utf8", ); +const boundedRetrievalMigration = readFileSync( + join(root, "supabase/migrations/20260717170000_bound_versioned_retrieval_match_count.sql"), + "utf8", +); const canonicalSchema = readFileSync(join(root, "supabase/schema.sql"), "utf8"); function between(source: string, start: string, end: string): string { @@ -111,6 +115,22 @@ describe("owner-plus-public retrieval contract", () => { } }); + it("bounds nullable and negative result counts at the exposed versioned RPCs", () => { + // LIMIT NULL and LIMIT -1 mean "no limit" in PostgreSQL. The wrappers are + // the RPC boundary, so clamp here rather than trusting every service-role + // caller to validate an optional PostgREST argument. + expect(boundedRetrievalMigration).toContain("coalesce($2, 12)"); + expect(boundedRetrievalMigration).toContain("coalesce($3, 24)"); + expect(boundedRetrievalMigration).toContain("least(greatest(coalesce($2, 12), 1), 96)"); + expect(boundedRetrievalMigration).toContain("least(greatest(coalesce($3, 24), 1), 96)"); + expect(boundedRetrievalMigration).toContain( + "revoke all on function public.match_document_chunks_text_v2(text, integer, uuid[], uuid, boolean)", + ); + expect(boundedRetrievalMigration).toContain( + "revoke all on function public.match_document_index_units_hybrid_v2(", + ); + }); + it("merges exact-owner and public rows when a versioned RPC is not deployed yet", async () => { const { callVersionedRetrievalRpc } = await import("../src/lib/rag"); const rpc = vi.fn(async (name: string, args: Record) => { From ed1f71b470bd5569e3114f113bc51adfad3cb172 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 17 Jul 2026 19:50:14 +0000 Subject: [PATCH 2/3] fix: rename migration to 20260717174000 to avoid timestamp collision and fix format Migration 20260717170000_bound_versioned_retrieval_match_count.sql collided with main's 20260717170000_registry_projection_cleanup.sql. Renamed to 20260717174000 (next available slot after main's 20260717173000 tip). Also update the migration filename reference in tests/retrieval-access-scope.test.ts and apply prettier format fix to pass the format:check gate. --- ...0260717174000_bound_versioned_retrieval_match_count.sql} | 0 tests/retrieval-access-scope.test.ts | 6 ++---- 2 files changed, 2 insertions(+), 4 deletions(-) rename supabase/migrations/{20260717170000_bound_versioned_retrieval_match_count.sql => 20260717174000_bound_versioned_retrieval_match_count.sql} (100%) diff --git a/supabase/migrations/20260717170000_bound_versioned_retrieval_match_count.sql b/supabase/migrations/20260717174000_bound_versioned_retrieval_match_count.sql similarity index 100% rename from supabase/migrations/20260717170000_bound_versioned_retrieval_match_count.sql rename to supabase/migrations/20260717174000_bound_versioned_retrieval_match_count.sql diff --git a/tests/retrieval-access-scope.test.ts b/tests/retrieval-access-scope.test.ts index 485c3a81c..f48f8f6b9 100644 --- a/tests/retrieval-access-scope.test.ts +++ b/tests/retrieval-access-scope.test.ts @@ -9,7 +9,7 @@ const optimizedRetrievalMigration = readFileSync( "utf8", ); const boundedRetrievalMigration = readFileSync( - join(root, "supabase/migrations/20260717170000_bound_versioned_retrieval_match_count.sql"), + join(root, "supabase/migrations/20260717174000_bound_versioned_retrieval_match_count.sql"), "utf8", ); const canonicalSchema = readFileSync(join(root, "supabase/schema.sql"), "utf8"); @@ -126,9 +126,7 @@ describe("owner-plus-public retrieval contract", () => { expect(boundedRetrievalMigration).toContain( "revoke all on function public.match_document_chunks_text_v2(text, integer, uuid[], uuid, boolean)", ); - expect(boundedRetrievalMigration).toContain( - "revoke all on function public.match_document_index_units_hybrid_v2(", - ); + expect(boundedRetrievalMigration).toContain("revoke all on function public.match_document_index_units_hybrid_v2("); }); it("merges exact-owner and public rows when a versioned RPC is not deployed yet", async () => { From 21c07203739dc2f99abdf70f54320481087730da Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 17 Jul 2026 20:03:36 +0000 Subject: [PATCH 3/3] fix: move migration to 20260717162000 to preserve reassert-as-last constraint The supabase-schema.test.ts guard requires the final alphabetical migration to be 20260717173000_reassert_supabase_admin_default_privileges.sql. Moving the bound-match-count migration from 20260717174000 to 20260717162000 (a free slot between the 161000 assert and the 170000 main cluster) satisfies this ordering constraint while avoiding all existing timestamp collisions. Update tests/retrieval-access-scope.test.ts to reference the new filename. --- ...=> 20260717162000_bound_versioned_retrieval_match_count.sql} | 0 tests/retrieval-access-scope.test.ts | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename supabase/migrations/{20260717174000_bound_versioned_retrieval_match_count.sql => 20260717162000_bound_versioned_retrieval_match_count.sql} (100%) diff --git a/supabase/migrations/20260717174000_bound_versioned_retrieval_match_count.sql b/supabase/migrations/20260717162000_bound_versioned_retrieval_match_count.sql similarity index 100% rename from supabase/migrations/20260717174000_bound_versioned_retrieval_match_count.sql rename to supabase/migrations/20260717162000_bound_versioned_retrieval_match_count.sql diff --git a/tests/retrieval-access-scope.test.ts b/tests/retrieval-access-scope.test.ts index f48f8f6b9..f49acf10c 100644 --- a/tests/retrieval-access-scope.test.ts +++ b/tests/retrieval-access-scope.test.ts @@ -9,7 +9,7 @@ const optimizedRetrievalMigration = readFileSync( "utf8", ); const boundedRetrievalMigration = readFileSync( - join(root, "supabase/migrations/20260717174000_bound_versioned_retrieval_match_count.sql"), + join(root, "supabase/migrations/20260717162000_bound_versioned_retrieval_match_count.sql"), "utf8", ); const canonicalSchema = readFileSync(join(root, "supabase/schema.sql"), "utf8");