Skip to content

Step 0: enumerate the guarded set by FK reachability and measure delegation cost #832

Description

@bencap

To decide how to implement RLS, we must know what it needs to protect. This issue enumerates the guarded set.

Scope

1. Enumerate by the invariant, not by columns

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.

For each table FK-reachable from scoresets, experiments, or collections, answer: can this row be returned by any query that does not join to its gating ancestor? If yes, it needs its own policy.

Do this against actual query shapes in routers/ and lib/, not against the schema alone. A table only ever reached by a join from a guarded table is safe today, but one new route away from not being safe.

Do not use WHERE column_name IN ('private','created_by_id','modified_by_id') as the instrument. It returns the tables already known and produces a false all-clear.

2. Enumerate the relkinds RLS does not reach

Two other relkinds carry the same data and are invisible to a pg_policy audit:

  • Plain views. db/view.py emits bare CREATE VIEW, and PG 15 defaults security_invoker to false, so RLS on base tables is evaluated as the view owner. The concrete instance is v_variant_annotations (models/variant_annotation_view.py): a plain view joining Variant/ScoreSet/MappedVariant with no privacy predicate at all, exposing every variant of every score set including private ones, with HGVS and mapping payload. List every plain view and its owner. The fix is WITH (security_invoker = true) as a default in db/view.py's CreateView compiler rather than a lint, plus catalog test C8 in Startup enforcement assertion for RLS liveness #824.
  • Materialized views. RLS never applies to reads. published_variants_materialized_view is safe only because its definition filters published_date IS NOT NULL, which assumes published implies not-private — true for score sets by convention rather than constraint, and false for calibrations, whose rule is orthogonal to publication. The rule to enforce is not "test the filter" but: no materialized view may carry a column governed by a rule other than publication. List every MV and every column against that rule.

3. Measure cardinalities

The delegation rule is "delegate to the nearest ancestor whose visible set is small and bounded." That needs numbers:

SELECT relname, n_live_tup FROM pg_stat_user_tables ORDER BY n_live_tup DESC;

Anything large that sits between a subtree table and scoresets puts that table on the must-carry-scoreset_id list. The expectation is that only mapped_variants qualifies, because target_genes and score_calibrations are a few rows per score set. Confirm it by measurement.

4. Confirm the plan shape on production cardinalities

The matrix below was measured at 2k score sets / 400k variants / 400k mapped variants. Re-run the winning form against production row counts and confirm the plan still contains a hashed SubPlan rather than a per-row subquery. The invariant being tested is that policy cost stays bounded by the visible row count of scoresets.

Acceptance criteria

  • A written list of every guarded table, each with the query shape that justifies it, recorded in this issue.
  • A written list of every plain view with its owner and security_invoker setting, and every materialized view checked against the publication rule.
  • Cardinalities captured from production, and the must-carry-scoreset_id list derived from them rather than assumed.
  • The winning policy form re-measured at production cardinalities, with the plan output attached.
  • Any table where the answer is "safe only because no route selects it independently" is listed explicitly as a standing risk rather than silently omitted.
  • Reconcile schema drift between prod, migrations, and models #829's triage query is corrected or removed, since it cannot answer this question.

Baseline measurements to re-run against production

Synthetic bed on PG 15.17: 2,000 score sets (10% private) / 400k variants / 400k mapped variants. Every form returned exactly 360,000 of 400,000 — correctness never differentiated between them, only cost did.

Policy form inner set 400k scan 40-row lookup
none (baseline) 12 ms ~0.5 ms
scoreset_id IN (SELECT id FROM scoresets) 1,800 15 ms 1.2 ms
materialised private/owner_id on the row 11 ms ~0.5 ms
app_can_read_scoreset(scoreset_id) — function, 1 hop 614 ms 0.4 ms
variant_id IN (SELECT id FROM variants) 360,000 738 ms 688 ms
app_can_read_variant(variant_id) — function, 2 hops 5,364 ms 0.9 ms

The table above omits the contributor check. With it live on the same bed: as a SECURITY DEFINER function the 400k scan costs 31.7 ms against a 17.7 ms baseline, because SECURITY DEFINER defaults to PARALLEL UNSAFE and drops the plan from parallel to serial. As an inline semi-join over the carved-out contributor table it is 18.9 ms. #809 specifies the inline form.

The plan shape to reproduce — visible score-set ids hashed once, then probed per row:

Parallel Seq Scan on mv
  Filter: (hashed SubPlan 1)
    Seq Scan on ss (actual rows=1800)
      Filter: (NOT private) OR (owner_id = current_setting('app.user_id')::integer)
Execution Time: 14.7 ms

The shape to confirm is absent — the two-hop form, where the index scan works perfectly and is then wasted:

Bitmap Index Scan on mv_ca_idx (actual rows=16)
Seq Scan on v (actual rows=360000)      ← hash built regardless
Execution Time: 647 ms

Why the guarded set is larger than six

The original enumeration derived from "tables with a private column". The scores live in variants.data, and the ClinGen route reads:

select(Variant, MappedVariant.clingen_allele_id)
  .join(MappedVariant)
  .options(joinedload(Variant.score_set).joinedload(ScoreSet.experiment))

scoresets enters only through joinedload — a LEFT OUTER JOIN, and innerjoin appears nowhere in the models — so a policy on scoresets nulls the relationship and returns the variant with its payload. with_loader_criteria behaves identically, placing its predicate in the ON clause to preserve outer-join semantics:

variant 10  data='public-scores'    score_set=<ScoreSet>
variant 11  data='PRIVATE-SCORES'   score_set=None

Expected must-carry-a-key list

To be confirmed by the cardinality measurement:

Table Nearest rule-carrying ancestor Expectation
mapped_variants scoresets, via variants (large) carry scoreset_id#833
score_calibration_functional_classification score_calibrations (own stricter rule) delegate to the calibration
score_calibration_publication_identifier score_calibrations delegate to the calibration
target_gene_mapping, uniprot/refseq/ensembl offsets scoresets via target_genes (small) delegate to target_genes
score_calibration_functional_classification_variant_association two paths — classification and variant delegate via the classification; needs the composite FK to make that sound

The multi-parent case is only correct if both paths lead to the same score set, which nothing currently enforces.

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