Skip to content

Fix content access gaps for anonymous and public document users - #265

Closed
BigSimmo wants to merge 1 commit into
mainfrom
cursor/content-access-review-a385
Closed

Fix content access gaps for anonymous and public document users#265
BigSimmo wants to merge 1 commit into
mainfrom
cursor/content-access-review-a385

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Summary

Reviewed the repo's content access model and fixed several gaps that blocked legitimate users from reaching public content.

Access model (current)

  • Authenticated users see only their own documents (owner_id = auth.uid()), enforced at both API and RLS layers.
  • Anonymous users can search/answer against public documents (owner_id IS NULL), plus curated static snapshots for services, forms, medications, and differentials.
  • Private mutations (upload, reindex, labels, delete) remain auth-required.

Issues found and fixed

  1. Invalid/expired bearer tokens returned 401 on search/answer/registry routes instead of degrading to anonymous public scope — this caused the UI error "Search request was not authorized by the server."
  2. Registry routes parsed auth even when no auth signal was present, unlike medications/differentials — stale tokens could 401 instead of serving the public snapshot.
  3. Document preview routes required sign-in even for public documents — anonymous users could get search hits but could not open PDFs or document detail.
  4. DocumentViewer blocked all source loading unless signed in — now loads public sources when the project is ready.

Security preserved

  • Anonymous document reads are scoped to owner_id IS NULL only (no cross-tenant leakage).
  • Authenticated reads remain owner-scoped.
  • Upload and mutation routes unchanged (still require auth).

Verification

  • npm run verify:cheap (118 test files, 1060 tests passed)
  • npm run verify:ui — DocumentViewer behavior changed; recommend UI gate before merge
  • npm run check:production-readiness — touches document access and auth flows

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow introduced
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains separated from real clinical sources
  • Anonymous access limited to explicit public documents and curated snapshots

Follow-up (not in this PR)

  • Ensure production has documents with owner_id IS NULL if anonymous search should return real indexed content (currently most uploads assign an owner).
  • Optional PUBLIC_WORKSPACE_OWNER_ID for anonymous uploads (planned in docs/superpowers/plans/2026-07-04-public-anonymous-access-rate-limits.md).
  • Live two-user RLS integration test still outstanding per docs/clinical-governance.md.
Open in Web Open in Cursor 

- Degrade invalid bearer tokens to anonymous scope instead of 401
- Allow public document read routes (list, detail, signed-url, search, images)
- Align registry routes with medications/differentials auth-signal short-circuit
- Let DocumentViewer load public sources without requiring sign-in
- Add regression tests and update access-control expectations
@supabase

supabase Bot commented Jul 5, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@BigSimmo

BigSimmo commented Jul 5, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #269 (consolidated platform fixes branch).

@BigSimmo BigSimmo closed this Jul 5, 2026
BigSimmo added a commit that referenced this pull request Aug 9, 2026
…1770)

* feat(design-system): gate the ungated prohibitions and close gate 4

Closes #264 and the gate-4 half of #265. Measured against origin/main
8db1e53 rather than scoped from the handover, which mattered: four of
#264's six prohibitions were already gated while GATES.md §3 read
`planned`, and that understatement is what deferred this task twice.

Tighten two ratchets carrying stale slack to their measured values:
edgeOwnershipConflicts 28 -> 27 and legacyShadowAliases 231 -> 224. Seven
files had paid debt down without a baseline refresh, so up to seven new
violations would have passed. Every other metric and debtByPath entry was
asserted unchanged first. The remaining 224 aliases are #262's cleanup.

Add three checks to check:design-system-contract:

- statusColouredNumerals (ratcheted 2) — a text-* status hue on an element
  whose children are all figures.
- colourOnlyStatusIndicators (ratcheted 4) — gate 4's repository-wide
  enumeration. A status hue on a box with no children, no accessible name
  on it or any ancestor, no sibling text, and not a StatusMark. Also flags
  shared swatch recipes, since the analyzer is per-file and cannot follow
  an imported statusDotReady to its call sites.
- imageInversions (pinned at zero) — CSS filter/backdrop-filter and the
  Tailwind invert/hue-rotate utilities.

All three are mutation-verified, as is the tightening: reintroducing one
var(--shadow-tight) now fails with `legacyShadowAliases increased from 224
to 225` plus the per-path assertion, where the same edit passed at 231.

The two status metrics were re-measured three times before the baseline was
written. The first draft found 19 colour-only indicators and 3 numerals; 15
were false positives — a name test loose enough that `size` read as a
numeral, an ancestor walk checking for JsxOpeningElement when ancestors are
JsxElement, and no sibling-text rule, which condemned the ordinary legend
pattern. All six recorded entries were read in source and confirmed real.

Gate 2 is NOT closed. A rendered-interactive tap enumeration was written,
shown to find genuine defects, and reverted: six runs against one
production build returned 6, 5, 4, 3, 3 and 9 distinct sub-floor shapes
because the audit races the async render, and networkidle plus shape
deduplication did not settle it. ui-style-contract.spec.ts runs in the
required Production UI job, so an intermittent version would have blocked
every merge. Recorded with the defect it found as #289.

Correct GATES.md throughout, including the gate 2 row's claim that
test:e2e:style-contract needs wiring into verify:cheap — the spec already
runs in required CI via productionSpecPattern and playwright-pr-shards, and
adding it to verify:cheap:internal would trip check:gate-manifest, which
requires every gate in that chain to also run in the browser-less static-pr
job. Close #277, verified already satisfied on main.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(ledger): record the M2 design-system gates review

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(ledger): supersede the M2 row after renumbering #289 to #291

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix: expand NUMERAL_TEXT regex to accept clinical unit suffixes

Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com>

* fix(design-system): close seven gaps found in review of the new gates

All seven review findings reproduced against a probe before any change, and
every one was real. Two mattered:

- Neither status pattern accepted Tailwind's `/NN` opacity modifier, and 83
  status-token utilities in `src` carry one. The anchored patterns rejected
  every one before the semantic checks ran, so a colour-only indicator written
  `bg-[color:var(--danger)]/90` walked past a ratchet described as
  repository-wide.
- `NUMERAL_TEXT` rejected every letter while its own comment claimed units were
  covered, so a dose painted in a status colour — the case the rule exists for
  — passed silently. Replaced with `isNumeralTextFragment`, which accepts an
  explicit unit list and still rejects prose. It carries no digit requirement,
  because `{dose} mg` splits into an expression and a unit-only text child.

Also:

- `isNumeralExpression` now requires an arithmetic operator, rejects any
  string/template operand anywhere in the tree, and follows only `+`/`-` prefix
  unaries, so `{count + " errors"}` is no longer a numeral. The previous comment
  claiming a concatenation "would have a string literal rather than a numeric
  one" was wrong: one numeric side was enough.
- `INVERSION_FUNCTION` matches the `invert(`/`hue-rotate(` call itself, covering
  `filter-[invert(1)]`, `[filter:invert(1)]` and both `backdrop-` forms.
- `IMAGE_INVERSION_UTILITY` excludes `-0`; `invert-0` disables inversion and a
  hard-zero gate must not reject the reset.
- `rendersVisibleText` no longer counts `{null}`, `{false}`, `{undefined}` or a
  bare self-closing element as a text channel, and judges markup-building
  expressions by that markup.
- `hasNonEmptyAccessibleName` replaces the attribute-presence test, so
  `aria-label=""` no longer exempts an indicator.
- The Gate 9 row said a layout-property lint was "planned" while
  `layoutTransitionExceptions` already ships and ratchets per path — the exact
  contradiction this branch exists to remove.

Fixing these surfaced two false positives of my own, both caught by
re-verifying every baseline entry in source rather than trusting the count: the
sibling walk stopped at the first non-JSX parent, so a conditional badge with a
label was reported colour-only; correcting that then let a sibling
`{started ? <div/> : null}` — another coloured div — count as text.

Baseline unchanged at 4 colour-only / 2 numerals / 0 inversions, and unchanged
by the unit widening, so nothing in the repo paints a dose in a status colour.
`legacyShadowAliases` 224 -> 220 from the main merge. Every new form is
mutation-verified.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(ledger): supersede the M2 row after the review round and main merge

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
BigSimmo added a commit that referenced this pull request Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant