Skip to content

Move read enforcement into Postgres row-level security #808

Description

@bencap

Summary

MaveDB's permission system guards the entity a request names, via assert_permission at the router boundary. It does not guard the entities a response contains. This epic moves read enforcement into Postgres row-level security, so a query cannot return a row the caller is not entitled to regardless of query shape, call site, or process.

Motivated by #807 and #822. In #822, 24 permission checks in routers/collections.py were silently non-functional, with a green test suite throughout and four tests actively asserting the leaked behaviour. Fixed in #821.

Division of responsibility

RLS is the floor; Python is the ceiling. RLS guarantees you can never see or touch a row you are ineligible for. Python narrows per domain action, because Postgres has four command classes while MaveDB has six actions — UPDATE, PUBLISH, and SET_SCORES are all UPDATE ON scoresets and their rules differ.

Concern Owner
READ filtering RLS
Row-level write eligibility RLS
Which domain action has_permission
Business preconditions Python
401 / 403 on visible rows has_permission
404 falls out of RLS absence

Decisions

These are ratified here; the linked issue specifies each one.

1. Role topology (#826, built by #827). Two properties carry the guarantee: mavedb_api is a member of nothing, so failures degrade toward less permission; and mavedb_owner is a member of nothing, because a SECURITY DEFINER function runs as its owner and the applied policy set includes the TO mavedb_system grant. mavedb_owner owns every object by default, so system membership there would elevate every function anyone ever adds. FORCE ROW LEVEL SECURITY on every guarded table, so ownership is not an invisible bypass.

2. System access is a policy, not a membership (#809). Membership alone confers nothing, and a PERMISSIVE policy with no TO clause applies to PUBLIC. Each guarded table carries FOR ALL TO mavedb_system USING (true) WITH CHECK (true) alongside its business rule.

3. Policies never raise (#809, #823). Policy bodies call current_setting('app.user_id', true) and wrap every cast value in nullif. Without this, every non-request context errors on any policied table, and that surface is unenumerable.

4. Foreign-key delegation (#809, enumerated by #832). Privacy is inherited through foreign keys, but no mechanism follows one.

Invariant. Any row a query can reach must either carry on itself everything needed to decide visibility, or be guaranteed to join to the row that does.

Rule. Delegate to the nearest ancestor carrying a visibility rule of its own, provided its visible set is small and bounded. Carry its key locally only when every path to it passes through a large table.

Delegation is written inline as a semi-join, never wrapped in a function. A SECURITY DEFINER function in a policy body costs ~2× by disqualifying the plan from parallelism, and a system-owned one is itself a bypass.

Carve-outs are listed in #809. They are a disclosure, not a neutral omission: mavedb_api can read the contributor list of any private score set and the membership of any private collection. Accepted and recorded.

5. Named-path lookups (#816). RLS absence is exactly private and not user_may_view_private, which deny_action_for_entity already maps to 404. 401 and 403 are action denials presupposing a visible entity. So no unscoped load and no elevated probe: get-or-404, then assert the action.

6. Boot assertion (#824). Behavioural probes first — negative on the api, positive on the worker — plus three catalog checks for bypasses no probe can observe. The two membership assertions need different predicates: MEMBER for the api, USAGE for the worker. No policy-text comparison; pg_get_expr normalisation changes across versions.

What RLS does not replace

All non-READ authorization; the 401/403 distinction; contributor-ship in Python; and the differential test, since two implementations of the READ rule coexist. explain_visibility is dropped — it would be a third implementation and the one nobody maintains.

Scope not yet costed

  • Test fixtures (Test and local fixture parity with the production role topology #834), the largest single item. Three fixtures each independently prevent the suite from exercising RLS: create_all builds no policies, the connection is a superuser, and get_current_user is overridden.
  • Non-request entry points. The arq job context, 8 manual migrations, and src/mavedb/scripts/ each need SessionScope.SYSTEM at their session construction site.
  • Absence ambiguity on the write path. RLS makes "invisible" indistinguishable from "nonexistent", so branches that read absence as nonexistence change meaning: URN generation, duplicate detection, the publish-time chain walk (Delete remaining reached-path filters; resolve the publish-time chain walk #813), counts, and every .one() that now raises. This is the write-side dual of the read leak and carries no work yet.
  • Error mapping. Nothing maps 42501/RLS-violation errors to an HTTP status or scrubs the message; PostgreSQL names the table in the error text.

The acceptance criteria below are aspirational until those are traced.

Acceptance criteria

  • Every guarded table has business and system policies for SELECT, INSERT, UPDATE, DELETE. The scope of the write half is open — one visibility predicate applied FOR ALL USING (vis) WITH CHECK (vis) may be the better trade than four hand-written command policies per table.
  • The guarded set is enumerated by the §4 invariant (Step 0: enumerate the guarded set by FK reachability and measure delegation cost #832), with carve-outs listed and justified.
  • No has_permission call is used as a read filter. Action-authorization and carve-out sites remain, deliberately.
  • Boot fails on each of the six conditions, and the canary probe is proven by dropping a policy.
  • A differential test asserts has_permission(..., READ).permitted agrees with row visibility over a persona × entity-state matrix.
  • No permitted action results in a silent 0-row write.
  • The canary sweeps every GET route as an unentitled persona and asserts on payload, not just parent entities (Canary test and route completeness tripwire for permission leaks #810).
  • Local and CI run under the production role topology with policies present (Test and local fixture parity with the production role topology #834).
  • No policy body contains a function call, and the SECURITY DEFINER allowlist matches the three elevated objects exactly.
  • Every plain view carries security_invoker, set as a default in db/view.py rather than caught by a lint.

Sequencing

  1. Role topology and parityRole topology and idempotent bootstrap script #827, Test and local fixture parity with the production role topology #834. Gates everything.
  2. Scope discoveryStep 0: enumerate the guarded set by FK reachability and measure delegation cost #832. Go/no-go gate: if the guarded set is materially larger than expected, the shape of this epic changes.
  3. GUC plumbing and boot assertion in warn-only modeBind the request principal to the session and re-emit it per transaction #823, Startup enforcement assertion for RLS liveness #824.
  4. Canary built against current behaviourCanary test and route completeness tripwire for permission leaks #810.
  5. Pilot on score_calibrations in stagingPilot: enable RLS on score_calibrations in staging #825. This is not the motivating case; the ClinGen allele search is not protected until step 6.
  6. Policies for the remaining entities and the subtreeDefine RLS policies for the six permission-aware entities #809, Carry scoreset_id on mapped_variants under a composite foreign key #833.
  7. Observational phaseObservational phase: instrument the read filters before deleting them #835. Instrument every filter site and soak until the policy-gap counter reads zero. One gate over all three deletion issues.
  8. DeletionDelete has_permission filter sites in the variants router #811, Delete association filter and narrowing sites in the collections router #812, Delete remaining reached-path filters; resolve the publish-time chain walk #813, Named-path lookups: get-or-404 plus action assert #816, Prove the collection-narrowing idiom is gone and keep it gone #818.

Metadata

Metadata

Assignees

No one assigned

    Labels

    app: backendTask implementation touches the backend

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions