feat(safegres): L11/L12 — materialized-view snapshots and non-barrier filtering views - #1614
Merged
Merged
Conversation
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Gaps 5 and 7 from the constructive-db view review, as one PR: the two remaining ways a readable view is not what its definition says. Both are reach edges in the existing
RoleReachmodel withproof: 'ast', both shipinfo/ score-neutral, and neither recommends a revoke.L11 — a materialized view is a snapshot, not a query. Its rows were computed once by whoever ran
REFRESHand are then served verbatim: the base relations are never consulted at read time, so their ACLs do not apply and their RLS policies do not run. A matview can carry neither policies (RLS attaches to tables) norsecurity_invoker(a view-only reloption) — pinned by a new assertion inview-introspect.test.ts— so there is no option on the object that reinstates the filter. Fires on both shapes:The second is the one catalog-only analysis is worst at: the ACL says the role may read the table, RLS says it may read three rows of it, and the matview hands it all of them.
L12 — a view's
WHEREis a filter, not a boundary. Withoutsecurity_barrierthe planner may push the caller's qual below the view's own, so a leaky operator or aCOST 0.0001function is evaluated against the rows the view was written to hide. Verified on PG 18 with aNOTICE-raising function in the caller's predicate: the non-barrier view emitted a notice for the hidden row, thesecurity_barrier = trueview did not.L12 is deliberately narrow — it needs all four of:
A caller that can read the base table directly loses nothing to a pushed-down qual, and row-limiting through a join or a
LIMITis not how a boundary gets written, so only an explicitWHERE/HAVINGcounts. The overlap with L8 is intentional: L8 says the table is reachable, L12 says even the rows the view hides are — and the remedies differ (security_invokervs.security_barrier).Conservatism
bodyFiltersRowsreturnsboolean | null, andnull(unparseable) suppresses — a body we cannot read is "unknown", never "does not filter". Same for an opaque body on either rule. Neither hint mentions revoking; both say so explicitly, and the corpus asserts it.Shape of the change
ViewSnapshot.securityBarrier, read the same waysecurity_invokeris (pg_options_to_table(...)::boolean, soon/1/yesare handled).RoleReachEdgegains{ kind: 'matview' }, emitted for the outermost hop whenViewReachInput.materialized— the rows are stored, so the edge holds whatever the reader's privileges are.definer-view.tsis extracted verbatim into exportedreadBodies/resolveViewBases; L8's behavior is unchanged and its tests are untouched apart from the new snapshot field. A matview is deliberately kept out of the relation index (it is terminal for anything reading through it) while its own body is still read.src/commands/audit.ts/src/index.ts: additive only — a dispatch block, aneedsViewsdisjunct, and three exports.Severity
infois the new-rule posture, not the verdict. L11 on an RLS-protected base ishighon its own merits — a SELECT grant on the matview is an unconditional grant on every row a refresh captured. L12 is the weakest of the L-series and is honestlymedium: it needs a leaky operator or a cheap function in the caller's qual, so it is a capability rather than an unconditional read.Verification
pnpm build/pnpm lint(2 pre-existing warnings) /pnpm test→ 34 suites, 417 tests. Corpus cases 31–34, two of them negatives (32a matview that changes nothing about reach,34case 33 after its fix). A full constructive-db audit with this build produces zero L11/L12 findings and an unchanged score (100 A+ / perf 91.2 A) — no matviews today, and every generated view issecurity_invoker.Link to Devin session: https://app.devin.ai/sessions/f340c08768814b278a179aea7994f924
Requested by: @pyramation