diff --git a/packages/safegres/README.md b/packages/safegres/README.md index 96f42be87..842d65214 100644 --- a/packages/safegres/README.md +++ b/packages/safegres/README.md @@ -132,6 +132,8 @@ family, **not** the dimension: `P1`/`P1b` are performance, `P5` is security. | L8 | info | fail-open | **DEFINER view bypass** — an untrusted role reads a base relation as the view's owner † | | L9 | info | fail-open | **DEFINER view write** — an auto-updatable definer view writes a base relation as its owner † | | L10 | info | fail-open | **Rewrite-rule bypass** — a rule on a view writes a relation as the view's owner, `security_invoker` notwithstanding † | +| L11 | info | fail-open | **Materialized-view snapshot** — stored rows serve an untrusted role what the base relation's grants and policies would not † | +| L12 | info | fail-open | **Non-barrier filtering view** — a view is an untrusted role's only path to a relation, but its row filter is not a boundary † | | W1 | medium | — | **No exposure surface configured** — whole database assumed reachable, score capped | † R1/R2/L5 are no-ops until you name the untrusted roles: diff --git a/packages/safegres/__tests__/definer-view.test.ts b/packages/safegres/__tests__/definer-view.test.ts index 7abe7559a..bcf2fb0f9 100644 --- a/packages/safegres/__tests__/definer-view.test.ts +++ b/packages/safegres/__tests__/definer-view.test.ts @@ -27,6 +27,7 @@ function view(partial: Partial = {}): ViewSnapshot { owner: 'app_owner', materialized: false, securityInvoker: false, + securityBarrier: false, ownerBypassesRls: false, grants: [grant('anon', 'SELECT')], definition: 'SELECT id, total FROM app.orders', diff --git a/packages/safegres/__tests__/view-exposure.test.ts b/packages/safegres/__tests__/view-exposure.test.ts new file mode 100644 index 000000000..787957c8e --- /dev/null +++ b/packages/safegres/__tests__/view-exposure.test.ts @@ -0,0 +1,258 @@ +import { type RoleGraph } from '../src/checks/lattice'; +import { + analyzeViewExposure, + checkLeakyFilterView, + checkMatviewSnapshot +} from '../src/checks/view-exposure'; +import type { RoleAttributes } from '../src/pg/acl'; +import type { ViewSnapshot } from '../src/pg/indexes'; +import type { GrantInfo, TableSnapshot } from '../src/pg/introspect'; + +function table(partial: Partial = {}): TableSnapshot { + return { + schema: 'app', + name: 'secrets', + oid: 1, + rlsEnabled: true, + rlsForced: false, + isPartitioned: false, + owner: 'app_owner', + grants: [], + policies: [], + ...partial + }; +} + +function view(partial: Partial = {}): ViewSnapshot { + return { + schema: 'app', + name: 'secrets_mv', + owner: 'app_owner', + materialized: true, + securityInvoker: false, + securityBarrier: false, + ownerBypassesRls: false, + grants: [grant('anon', 'SELECT')], + definition: 'SELECT id, body FROM app.secrets', + writable: [], + insteadOfTriggers: false, + rules: [], + ...partial + }; +} + +function grant(role: string, privilege: GrantInfo['privilege']): GrantInfo { + return { role, privilege, grantable: false, bypassRls: false }; +} + +function role(name: string, partial: Partial = {}): [string, RoleAttributes] { + return [name, { name, bypassRls: false, isSuper: false, inheritsFrom: [], canSetRole: [], ...partial }]; +} + +function graph(...entries: Array<[string, RoleAttributes]>): RoleGraph { + return new Map(entries); +} + +const GRAPH = graph(role('anon'), role('app_owner'), role('member')); + +/** A plain view over one table, filtered, definer, no barrier — the L12 shape. */ +function filterView(partial: Partial = {}): ViewSnapshot { + return view({ + name: 'my_secrets', + materialized: false, + definition: 'SELECT id, body FROM app.secrets WHERE owner_name = CURRENT_USER', + ...partial + }); +} + +describe('analyzeViewExposure — materialized views', () => { + it('resolves the relations a matview refresh copied from', async () => { + const { matviews } = await analyzeViewExposure([view()], [table()]); + expect(matviews).toHaveLength(1); + expect(matviews[0].materialized).toBe(true); + expect(matviews[0].baseRelations).toEqual([ + { schema: 'app', table: 'secrets', hops: [{ view: 'app.secrets_mv', owner: 'app_owner' }] } + ]); + }); + + it('reads a matview body regardless of security_invoker, which matviews cannot carry', async () => { + const { matviews } = await analyzeViewExposure([view({ securityInvoker: true })], [table()]); + expect(matviews).toHaveLength(1); + }); + + it('suppresses a matview whose body cannot be parsed', async () => { + const { matviews, suppressed } = await analyzeViewExposure( + [view({ definition: 'SELECT FROM WHERE ((' })], + [table()] + ); + expect(matviews).toEqual([]); + expect(suppressed).toHaveLength(1); + }); + + it('does not follow a matview as a nested relation of another view', async () => { + // `app.secrets_mv` stores rows; a view over it reads the snapshot, not the + // table the snapshot came from. + const outer = view({ + name: 'wrapper', + materialized: false, + definition: 'SELECT id FROM app.secrets_mv' + }); + const { leaky, matviews } = await analyzeViewExposure([view(), outer], [table()]); + expect(matviews).toHaveLength(1); + expect(leaky).toEqual([]); + }); +}); + +describe('analyzeViewExposure — non-barrier filtering views', () => { + it('collects a filtering definer view with no barrier', async () => { + const { leaky } = await analyzeViewExposure([filterView()], [table()]); + expect(leaky).toHaveLength(1); + expect(leaky[0].name).toBe('my_secrets'); + }); + + it('ignores the same view once it is a barrier', async () => { + const { leaky } = await analyzeViewExposure([filterView({ securityBarrier: true })], [table()]); + expect(leaky).toEqual([]); + }); + + it('ignores an invoker view: the caller needs its own grant, so nothing is hidden by the view', async () => { + const { leaky } = await analyzeViewExposure([filterView({ securityInvoker: true })], [table()]); + expect(leaky).toEqual([]); + }); + + it('ignores a view with no row filter — there are no excluded rows to reach', async () => { + const { leaky } = await analyzeViewExposure( + [filterView({ definition: 'SELECT id, body FROM app.secrets' })], + [table()] + ); + expect(leaky).toEqual([]); + }); + + it('counts a HAVING clause as a row filter', async () => { + const { leaky } = await analyzeViewExposure( + [filterView({ + definition: 'SELECT owner_name, count(*) FROM app.secrets GROUP BY owner_name HAVING count(*) > 1' + })], + [table()] + ); + expect(leaky).toHaveLength(1); + }); + + it('suppresses a filtering view whose body cannot be parsed', async () => { + const { leaky, suppressed } = await analyzeViewExposure( + [filterView({ definition: 'SELECT ) WHERE (' })], + [table()] + ); + expect(leaky).toEqual([]); + expect(suppressed).toHaveLength(1); + }); +}); + +describe('checkMatviewSnapshot (L11)', () => { + async function check(views: ViewSnapshot[], tables: TableSnapshot[], roles: string[]) { + const { matviews } = await analyzeViewExposure(views, tables); + return checkMatviewSnapshot(matviews, tables, GRAPH, { roles }); + } + + it('reports a matview handing an untrusted role a table it holds nothing on', async () => { + const findings = await check([view()], [table()], ['anon']); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + code: 'L11', + severity: 'info', + schema: 'app', + table: 'secrets', + role: 'anon', + privilege: 'SELECT' + }); + expect(findings[0].context).toMatchObject({ + matview: 'app.secrets_mv', + effectiveRole: 'app_owner', + holdsBaseSelect: false, + proof: 'ast' + }); + }); + + it('still reports when the role can read the table, because RLS filtered rows the snapshot did not', async () => { + const base = table({ grants: [grant('anon', 'SELECT')] }); + const findings = await check([view()], [base], ['anon']); + expect(findings).toHaveLength(1); + expect(findings[0].context).toMatchObject({ holdsBaseSelect: true, rlsBypassed: true }); + expect(findings[0].message).toContain('without the row filter its policies apply'); + }); + + it('stays silent when the role reads the table directly and no policy filters it', async () => { + const base = table({ rlsEnabled: false, grants: [grant('anon', 'SELECT')] }); + expect(await check([view()], [base], ['anon'])).toEqual([]); + }); + + it('stays silent for a role that bypasses RLS and holds the grant', async () => { + const base = table({ grants: [grant('member', 'SELECT')] }); + const findings = checkMatviewSnapshot( + (await analyzeViewExposure([view({ grants: [grant('member', 'SELECT')] })], [base])).matviews, + [base], + graph(role('member', { bypassRls: true })), + { roles: ['member'] } + ); + expect(findings).toEqual([]); + }); + + it('needs a grant on the matview itself', async () => { + const findings = await check([view({ grants: [] })], [table()], ['anon']); + expect(findings).toEqual([]); + }); + + it('follows a PUBLIC grant on the matview', async () => { + const findings = await check([view({ grants: [grant('PUBLIC', 'SELECT')] })], [table()], ['anon']); + expect(findings).toHaveLength(1); + }); + + it('reports nothing without configured roles', async () => { + expect(await check([view()], [table()], [])).toEqual([]); + }); + + it('never recommends revoking the grant', async () => { + const findings = await check([view()], [table()], ['anon']); + expect(findings[0].hint).toContain('Do not revoke'); + expect(findings[0].hint).not.toMatch(/\bREVOKE\b/); + }); +}); + +describe('checkLeakyFilterView (L12)', () => { + async function check(views: ViewSnapshot[], tables: TableSnapshot[], roles: string[]) { + const { leaky } = await analyzeViewExposure(views, tables); + return checkLeakyFilterView(leaky, tables, GRAPH, { roles }); + } + + it('reports a filtering view that is an untrusted role\'s only path to the relation', async () => { + const findings = await check([filterView()], [table()], ['anon']); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + code: 'L12', + severity: 'info', + schema: 'app', + table: 'secrets', + role: 'anon' + }); + expect(findings[0].context).toMatchObject({ view: 'app.my_secrets', proof: 'ast' }); + }); + + it('stays silent once the view is a barrier', async () => { + expect(await check([filterView({ securityBarrier: true })], [table()], ['anon'])).toEqual([]); + }); + + it('stays silent when the role can read the base relation directly', async () => { + const base = table({ grants: [grant('anon', 'SELECT')] }); + expect(await check([filterView()], [base], ['anon'])).toEqual([]); + }); + + it('stays silent when the caller cannot read the view at all', async () => { + expect(await check([filterView({ grants: [] })], [table()], ['anon'])).toEqual([]); + }); + + it('recommends the barrier, and never a revoke', async () => { + const findings = await check([filterView()], [table()], ['anon']); + expect(findings[0].hint).toContain('security_barrier = true'); + expect(findings[0].hint).toContain('Do not revoke'); + }); +}); diff --git a/packages/safegres/__tests__/view-introspect.test.ts b/packages/safegres/__tests__/view-introspect.test.ts index 6b157cdd8..256bcf2bd 100644 --- a/packages/safegres/__tests__/view-introspect.test.ts +++ b/packages/safegres/__tests__/view-introspect.test.ts @@ -35,6 +35,17 @@ beforeAll(async () => { CREATE RULE v_ruled_ins AS ON INSERT TO fx_viewwrite.v_ruled DO INSTEAD INSERT INTO fx_viewwrite.audit (note) VALUES ('x'); CREATE RULE v_ruled_del AS ON DELETE TO fx_viewwrite.v_ruled DO INSTEAD NOTHING; + + CREATE SCHEMA fx_viewbarrier; + CREATE TABLE fx_viewbarrier.t (id int, owner_name text); + CREATE VIEW fx_viewbarrier.v_plain AS + SELECT id FROM fx_viewbarrier.t WHERE owner_name = CURRENT_USER; + CREATE VIEW fx_viewbarrier.v_barrier WITH (security_barrier = true) AS + SELECT id FROM fx_viewbarrier.t WHERE owner_name = CURRENT_USER; + -- Both reloptions at once: reading one must not disturb the other. + CREATE VIEW fx_viewbarrier.v_both WITH (security_barrier = on, security_invoker = 1) AS + SELECT id FROM fx_viewbarrier.t; + CREATE MATERIALIZED VIEW fx_viewbarrier.mv AS SELECT id FROM fx_viewbarrier.t; `); }); @@ -80,3 +91,22 @@ describe('introspectViews — write paths', () => { expect(byName.v_ruled.rules[0].definition).toContain('CREATE RULE'); }); }); + +describe('introspectViews — security_barrier and materialization', () => { + it('reads the barrier flag independently of security_invoker', async () => { + const views = await introspectViews(pg.client as never, { schemas: ['fx_viewbarrier'] }); + const byName = Object.fromEntries(views.map((v) => [v.name, v])); + + expect(byName.v_plain.securityBarrier).toBe(false); + expect(byName.v_barrier.securityBarrier).toBe(true); + expect(byName.v_both).toMatchObject({ securityBarrier: true, securityInvoker: true }); + + // A matview carries neither option — both are view-only reloptions — and + // that is precisely why it cannot be made to execute as its reader. + expect(byName.mv).toMatchObject({ + materialized: true, + securityBarrier: false, + securityInvoker: false + }); + }); +}); diff --git a/packages/safegres/__tests__/view-writes.test.ts b/packages/safegres/__tests__/view-writes.test.ts index 70aa8ea92..80e76d4ff 100644 --- a/packages/safegres/__tests__/view-writes.test.ts +++ b/packages/safegres/__tests__/view-writes.test.ts @@ -31,6 +31,7 @@ function view(partial: Partial = {}): ViewSnapshot { owner: 'app_owner', materialized: false, securityInvoker: false, + securityBarrier: false, ownerBypassesRls: false, grants: [grant('anon', 'INSERT')], definition: 'SELECT id, body FROM app.submissions', diff --git a/packages/safegres/corpus/cases/31-matview-snapshot/case.json b/packages/safegres/corpus/cases/31-matview-snapshot/case.json new file mode 100644 index 000000000..117896c79 --- /dev/null +++ b/packages/safegres/corpus/cases/31-matview-snapshot/case.json @@ -0,0 +1,32 @@ +{ + "title": "Anonymous role reads an RLS-protected table's rows out of a materialized view", + "dimension": "security", + "exposure": { + "schemas": [ + "c_matview_snapshot" + ], + "roles": [ + "corpus_anon", + "corpus_user" + ], + "anonRoles": [ + "corpus_anon" + ] + }, + "expect": [ + { + "code": "L11", + "relation": "c_matview_snapshot.readings", + "note": "the matview stores rows computed as c_matview_owner, so corpus_anon reads every tenant's readings without holding a grant on the table and without its policies ever running" + } + ], + "forbid": [ + "L8", + "L9", + "L10", + "L12" + ], + "worstSeverity": "info", + "fix": "Serve the rollup from a `security_invoker` view, or materialize only the columns and rows that are safe to hand out unconditionally and keep the full snapshot in a schema the API does not expose. A materialized view can carry neither RLS policies nor `security_invoker`, so there is no option on the matview itself that reinstates the filter — and revoking corpus_anon's SELECT on it is not the fix, that grant is what the API serves.", + "id": "31-matview-snapshot" +} diff --git a/packages/safegres/corpus/cases/31-matview-snapshot/schema.sql b/packages/safegres/corpus/cases/31-matview-snapshot/schema.sql new file mode 100644 index 000000000..1f1edc37a --- /dev/null +++ b/packages/safegres/corpus/cases/31-matview-snapshot/schema.sql @@ -0,0 +1,36 @@ +DROP SCHEMA IF EXISTS c_matview_snapshot CASCADE; +CREATE SCHEMA c_matview_snapshot; + +-- The role that owns the table and runs REFRESH. +DO $$ BEGIN + CREATE ROLE c_matview_owner NOLOGIN; +EXCEPTION WHEN duplicate_object THEN NULL; +END $$; + +GRANT USAGE ON SCHEMA c_matview_snapshot TO corpus_anon, corpus_user, c_matview_owner; + +CREATE TABLE c_matview_snapshot.readings ( + id bigserial PRIMARY KEY, + tenant text NOT NULL, + value numeric NOT NULL +); +ALTER TABLE c_matview_snapshot.readings OWNER TO c_matview_owner; +ALTER TABLE c_matview_snapshot.readings ENABLE ROW LEVEL SECURITY; +ALTER TABLE c_matview_snapshot.readings FORCE ROW LEVEL SECURITY; + +-- Signed-in users see only their own tenant's rows. +CREATE POLICY readings_own_tenant ON c_matview_snapshot.readings + FOR SELECT TO corpus_user + USING (tenant = (SELECT current_setting('app.tenant', true))); +CREATE INDEX readings_tenant_idx ON c_matview_snapshot.readings (tenant); +GRANT SELECT ON c_matview_snapshot.readings TO corpus_user; + +-- The flaw: a materialized view is a stored copy. Its rows were computed once, +-- as c_matview_owner, and reading it never consults `readings` — so neither +-- the table's ACL nor its policies apply, and the matview can carry neither +-- policies nor `security_invoker` of its own. corpus_anon holds nothing on +-- `readings` and reads every tenant's rows out of the snapshot. +CREATE MATERIALIZED VIEW c_matview_snapshot.readings_rollup AS + SELECT id, tenant, value FROM c_matview_snapshot.readings; +ALTER MATERIALIZED VIEW c_matview_snapshot.readings_rollup OWNER TO c_matview_owner; +GRANT SELECT ON c_matview_snapshot.readings_rollup TO corpus_anon, corpus_user; diff --git a/packages/safegres/corpus/cases/32-matview-no-rls-no-bypass/case.json b/packages/safegres/corpus/cases/32-matview-no-rls-no-bypass/case.json new file mode 100644 index 000000000..479b61dfb --- /dev/null +++ b/packages/safegres/corpus/cases/32-matview-no-rls-no-bypass/case.json @@ -0,0 +1,30 @@ +{ + "title": "A materialized view over a table the role can already read is a cache, not a bypass", + "dimension": "security", + "exposure": { + "schemas": [ + "c_matview_no_bypass" + ], + "roles": [ + "corpus_anon", + "corpus_user" + ], + "anonRoles": [ + "corpus_anon" + ] + }, + "expect": [ + { + "code": "A2", + "relation": "c_matview_no_bypass.regions", + "note": "the only real finding left: the table is granted without RLS, which is the intent here — it is public reference data" + } + ], + "forbid": [ + "L11", + "L12" + ], + "worstSeverity": "high", + "fix": "Nothing on the matview. The snapshot shows corpus_anon exactly what `regions` would, and no policy was skipped because there is none — this case exists to pin that L11 stays silent when materialization changes nothing about reach.", + "id": "32-matview-no-rls-no-bypass" +} diff --git a/packages/safegres/corpus/cases/32-matview-no-rls-no-bypass/schema.sql b/packages/safegres/corpus/cases/32-matview-no-rls-no-bypass/schema.sql new file mode 100644 index 000000000..3acdd47c5 --- /dev/null +++ b/packages/safegres/corpus/cases/32-matview-no-rls-no-bypass/schema.sql @@ -0,0 +1,18 @@ +DROP SCHEMA IF EXISTS c_matview_no_bypass CASCADE; +CREATE SCHEMA c_matview_no_bypass; + +GRANT USAGE ON SCHEMA c_matview_no_bypass TO corpus_anon, corpus_user; + +-- Reference data: public by intent, no RLS, readable directly. +CREATE TABLE c_matview_no_bypass.regions ( + code text PRIMARY KEY, + label text NOT NULL +); +GRANT SELECT ON c_matview_no_bypass.regions TO corpus_anon, corpus_user; + +-- A matview over it is a cache, not a bypass: the snapshot shows corpus_anon +-- nothing it could not select from `regions` itself, and there are no policies +-- for the stored rows to have skipped. L11 must stay silent. +CREATE MATERIALIZED VIEW c_matview_no_bypass.regions_cached AS + SELECT code, label FROM c_matview_no_bypass.regions; +GRANT SELECT ON c_matview_no_bypass.regions_cached TO corpus_anon, corpus_user; diff --git a/packages/safegres/corpus/cases/33-leaky-filter-view/case.json b/packages/safegres/corpus/cases/33-leaky-filter-view/case.json new file mode 100644 index 000000000..967417c76 --- /dev/null +++ b/packages/safegres/corpus/cases/33-leaky-filter-view/case.json @@ -0,0 +1,36 @@ +{ + "title": "A view's WHERE is the only boundary an anonymous role faces, and it is not a security barrier", + "dimension": "security", + "exposure": { + "schemas": [ + "c_leaky_filter_view" + ], + "roles": [ + "corpus_anon", + "corpus_user" + ], + "anonRoles": [ + "corpus_anon" + ] + }, + "expect": [ + { + "code": "L12", + "relation": "c_leaky_filter_view.documents", + "note": "corpus_anon reaches `documents` only through the view, so the view's `visibility = 'public'` is a security boundary — and without `security_barrier` a caller-supplied leaky qual is evaluated against the private rows too" + }, + { + "code": "L8", + "relation": "c_leaky_filter_view.documents", + "note": "the same view also executes as its owner, which is what puts corpus_anon in reach of the table at all" + } + ], + "forbid": [ + "L9", + "L10", + "L11" + ], + "worstSeverity": "info", + "fix": "Recreate c_leaky_filter_view.public_documents WITH (security_barrier = true) so the planner cannot evaluate a caller's qual below the view's own. Where the filter is per-caller rather than a constant, an RLS policy on `documents` is stronger still: policy quals already get barrier treatment. Do not revoke corpus_anon's SELECT on the view.", + "id": "33-leaky-filter-view" +} diff --git a/packages/safegres/corpus/cases/33-leaky-filter-view/schema.sql b/packages/safegres/corpus/cases/33-leaky-filter-view/schema.sql new file mode 100644 index 000000000..2261d51a1 --- /dev/null +++ b/packages/safegres/corpus/cases/33-leaky-filter-view/schema.sql @@ -0,0 +1,29 @@ +DROP SCHEMA IF EXISTS c_leaky_filter_view CASCADE; +CREATE SCHEMA c_leaky_filter_view; + +DO $$ BEGIN + CREATE ROLE c_leaky_view_owner NOLOGIN; +EXCEPTION WHEN duplicate_object THEN NULL; +END $$; + +GRANT USAGE ON SCHEMA c_leaky_filter_view TO corpus_anon, corpus_user, c_leaky_view_owner; + +CREATE TABLE c_leaky_filter_view.documents ( + id bigserial PRIMARY KEY, + visibility text NOT NULL, + title text NOT NULL, + body text NOT NULL +); +ALTER TABLE c_leaky_filter_view.documents OWNER TO c_leaky_view_owner; + +-- The flaw: the view's WHERE is the only thing separating corpus_anon from the +-- private documents, and without `security_barrier` the planner may evaluate a +-- caller-supplied qual before it. A cheap leaky function +-- (`... WHERE leak(body)`) then runs against every row, private ones included, +-- and reports what it sees through a notice, an error or a timing difference. +CREATE VIEW c_leaky_filter_view.public_documents AS + SELECT id, title, body + FROM c_leaky_filter_view.documents + WHERE visibility = 'public'; +ALTER VIEW c_leaky_filter_view.public_documents OWNER TO c_leaky_view_owner; +GRANT SELECT ON c_leaky_filter_view.public_documents TO corpus_anon, corpus_user; diff --git a/packages/safegres/corpus/cases/34-barrier-view-no-leak/case.json b/packages/safegres/corpus/cases/34-barrier-view-no-leak/case.json new file mode 100644 index 000000000..b278ff59d --- /dev/null +++ b/packages/safegres/corpus/cases/34-barrier-view-no-leak/case.json @@ -0,0 +1,32 @@ +{ + "title": "The same filtering view with security_barrier is a boundary, and L12 stays silent", + "dimension": "security", + "exposure": { + "schemas": [ + "c_barrier_view" + ], + "roles": [ + "corpus_anon", + "corpus_user" + ], + "anonRoles": [ + "corpus_anon" + ] + }, + "expect": [ + { + "code": "L8", + "relation": "c_barrier_view.documents", + "note": "the view still executes as its owner — that is what it is for — so the definer-view reach edge is still reported; only the question of *which* rows is settled by the barrier" + } + ], + "forbid": [ + "L9", + "L10", + "L11", + "L12" + ], + "worstSeverity": "info", + "fix": "Nothing on the barrier. This case is case 33 after its fix, and exists to pin that L12 stops firing once the row filter is a boundary rather than a convenience.", + "id": "34-barrier-view-no-leak" +} diff --git a/packages/safegres/corpus/cases/34-barrier-view-no-leak/schema.sql b/packages/safegres/corpus/cases/34-barrier-view-no-leak/schema.sql new file mode 100644 index 000000000..6c3451603 --- /dev/null +++ b/packages/safegres/corpus/cases/34-barrier-view-no-leak/schema.sql @@ -0,0 +1,29 @@ +DROP SCHEMA IF EXISTS c_barrier_view CASCADE; +CREATE SCHEMA c_barrier_view; + +DO $$ BEGIN + CREATE ROLE c_barrier_view_owner NOLOGIN; +EXCEPTION WHEN duplicate_object THEN NULL; +END $$; + +GRANT USAGE ON SCHEMA c_barrier_view TO corpus_anon, corpus_user, c_barrier_view_owner; + +CREATE TABLE c_barrier_view.documents ( + id bigserial PRIMARY KEY, + visibility text NOT NULL, + title text NOT NULL, + body text NOT NULL +); +ALTER TABLE c_barrier_view.documents OWNER TO c_barrier_view_owner; + +-- Case 33 with the one option that makes the filter a boundary: the planner +-- may not push a caller's qual below the view's own, so the excluded rows are +-- never evaluated. L12 must stay silent. L8 still fires — the view does hand +-- corpus_anon rows of a table it holds no grant on, which is what the view is +-- for; the barrier only settles *which* rows. +CREATE VIEW c_barrier_view.public_documents WITH (security_barrier = true) AS + SELECT id, title, body + FROM c_barrier_view.documents + WHERE visibility = 'public'; +ALTER VIEW c_barrier_view.public_documents OWNER TO c_barrier_view_owner; +GRANT SELECT ON c_barrier_view.public_documents TO corpus_anon, corpus_user; diff --git a/packages/safegres/docs/rules.md b/packages/safegres/docs/rules.md index 2f26f95c1..e8a559475 100644 --- a/packages/safegres/docs/rules.md +++ b/packages/safegres/docs/rules.md @@ -75,6 +75,26 @@ an unreadable rule action is unknown — all three suppress. `DO INSTEAD NOTHING in the wild, reaches no relation and so reports nothing, which is the correct answer for the read-only views it is used to build. +L11 and L12 are the last two things a *readable* view does that neither its owner nor its body +explains on its own. **L11** is materialization: a matview's rows were computed once, by whoever ran +`REFRESH`, and are then served verbatim — the base relations are never consulted at read time, so +their ACLs do not apply and their policies do not run. A matview can carry neither policies (RLS +attaches to tables) nor `security_invoker` (a view-only reloption), so there is no option on the +object that reinstates the filter; the finding fires both when the role holds no grant on the base +relation and when it holds one but is subject to policies the stored rows never passed. **L12** is +`security_barrier`: without it the planner may push the *caller's* qual below the view's own, so a +leaky operator or a `COST 0.0001` function is evaluated against the rows the view was written to +hide. They are not returned, but they are seen — enough to read them out through a notice, an error +or a timing difference. L12 is deliberately narrow: it needs the view to actually filter (an +explicit `WHERE`/`HAVING` — row-limiting through a join or a `LIMIT` is not how a boundary gets +written), to be a definer view, and to be the role's *only* path to the relation. A caller that can +read the base table directly loses nothing to a pushed-down qual. + +Both inherit the same refusals, with one addition: a body safegres cannot parse is *unknown*, never +"does not filter", so it suppresses rather than clearing the view. And neither recommends a revoke — +the remedies are `security_barrier = true`, an RLS policy on the base relation (policy quals already +get barrier treatment), or not materializing rows that are not safe to hand out unconditionally. + Restrictive-only policies never count as coverage. `BYPASSRLS` and superuser roles are exempt from policy checks — they are not subject to RLS, so a "missing policy" finding for them would be noise. Policies are matched with `pg_has_role` semantics: a policy `TO authenticated` covers a diff --git a/packages/safegres/src/callgraph/extract.ts b/packages/safegres/src/callgraph/extract.ts index e89bd0fce..5227068fe 100644 --- a/packages/safegres/src/callgraph/extract.ts +++ b/packages/safegres/src/callgraph/extract.ts @@ -239,6 +239,29 @@ export async function extractAccess(sql: string): Promise { return { accesses, opaque, ...(opaqueReason ? { opaqueReason } : {}) }; } +/** + * Does this body restrict which rows come back? + * + * `null` when the SQL could not be parsed — "unknown", never "no". A view that + * filters is a view someone may be relying on as a row-level boundary, which + * is the precondition for the `security_barrier` question: without a `WHERE` + * there are no hidden rows for a leaky qual to reach. + * + * Only an explicit `WHERE`/`HAVING` counts. Row-limiting through a join, + * `DISTINCT` or `LIMIT` is real but is not the pattern that gets written as a + * security boundary, and treating it as one would fire on ordinary reporting + * views. + */ +export async function bodyFiltersRows(sql: string): Promise { + let ast: unknown; + try { + ast = await parse(sql); + } catch { + return null; + } + return findAll(ast, 'SelectStmt').some((s) => !!s.whereClause || !!s.havingClause); +} + function firstStringArg(call: Record): string | null { const args = call.args; if (!Array.isArray(args) || args.length === 0) return null; diff --git a/packages/safegres/src/checks/definer-view.ts b/packages/safegres/src/checks/definer-view.ts index 876d85f3b..46439fd06 100644 --- a/packages/safegres/src/checks/definer-view.ts +++ b/packages/safegres/src/checks/definer-view.ts @@ -24,7 +24,7 @@ * `security_invoker = true` or a different owner, and nothing else. */ -import { extractQuery } from '../callgraph/extract'; +import { type ExtractedBody, extractQuery } from '../callgraph/extract'; import type { ViewSnapshot } from '../pg/indexes'; import type { TableSnapshot } from '../pg/introspect'; import type { Finding } from '../types'; @@ -64,10 +64,7 @@ export async function analyzeViewBodies( const queryable = views.filter((v) => !v.materialized); const index = buildRelationIndex(queryable, tables); - const bodies = new Map>>(); - for (const v of queryable) { - bodies.set(`${v.schema}.${v.name}`, await extractQuery(v.definition)); - } + const bodies = await readBodies(queryable); const out: ViewReachInput[] = []; const suppressed: SuppressedView[] = []; @@ -75,47 +72,7 @@ export async function analyzeViewBodies( for (const view of queryable) { if (view.securityInvoker) continue; - const bases: ViewBaseRelation[] = []; - const seen = new Set(); - let opaque: string | undefined; - - const walk = (current: ViewSnapshot, hops: Array<{ view: string; owner: string }>): void => { - const key = `${current.schema}.${current.name}`; - if (hops.length > MAX_VIEW_DEPTH) { - opaque ??= `view chain deeper than ${MAX_VIEW_DEPTH} hops`; - return; - } - const body = bodies.get(key); - if (!body) return; - if (body.opaque) { - opaque ??= body.opaqueReason ?? 'body could not be read'; - return; - } - - for (const ref of body.tables) { - const relation = resolveRelation(ref, current.schema, index); - if (!relation) continue; // a CTE, an alias, or a name we cannot pin down - - if (relation.kind === 'view') { - const nested = relation.view; - if (`${nested.schema}.${nested.name}` === key) continue; - // An inner definer view re-owns the read; an inner invoker view runs - // under whichever owner is already in force. - const owner = nested.securityInvoker ? hops[hops.length - 1].owner : nested.owner; - walk(nested, [...hops, { view: `${nested.schema}.${nested.name}`, owner }]); - continue; - } - - const relKey = `${relation.schema}.${relation.name}`; - const dedupe = `${relKey}::${hops[hops.length - 1].owner}`; - if (seen.has(dedupe)) continue; - seen.add(dedupe); - bases.push({ schema: relation.schema, table: relation.name, hops: [...hops] }); - } - }; - - walk(view, [{ view: `${view.schema}.${view.name}`, owner: view.owner }]); - + const { bases, opaque } = resolveViewBases(view, index, bodies); if (opaque) { suppressed.push({ view: `${view.schema}.${view.name}`, reason: opaque }); continue; @@ -134,6 +91,76 @@ export async function analyzeViewBodies( return { views: out, suppressed }; } +/** Parsed bodies, keyed `schema.name`, as {@link resolveViewBases} expects. */ +export type ViewBodies = Map; + +export async function readBodies(views: ViewSnapshot[]): Promise { + const bodies: ViewBodies = new Map(); + for (const v of views) bodies.set(`${v.schema}.${v.name}`, await extractQuery(v.definition)); + return bodies; +} + +/** + * The base relations `root` reads, following nested views, with the owner in + * force at each hop. + * + * `opaque` is set to the first reason the walk had to stop, and when it is set + * the relation list is a fragment: the caller must discard it, because a body + * we could only partly read under-reports what the view reaches. + * + * `root` itself need not be in `index` — a materialized view is not a + * queryable relation for the purposes of resolving *other* bodies, but its own + * body reads the same way. + */ +export function resolveViewBases( + root: ViewSnapshot, + index: RelationIndex, + bodies: ViewBodies +): { bases: ViewBaseRelation[]; opaque?: string } { + const bases: ViewBaseRelation[] = []; + const seen = new Set(); + let opaque: string | undefined; + + const walk = (current: ViewSnapshot, hops: Array<{ view: string; owner: string }>): void => { + const key = `${current.schema}.${current.name}`; + if (hops.length > MAX_VIEW_DEPTH) { + opaque ??= `view chain deeper than ${MAX_VIEW_DEPTH} hops`; + return; + } + const body = bodies.get(key); + if (!body) return; + if (body.opaque) { + opaque ??= body.opaqueReason ?? 'body could not be read'; + return; + } + + for (const ref of body.tables) { + const relation = resolveRelation(ref, current.schema, index); + if (!relation) continue; // a CTE, an alias, or a name we cannot pin down + + if (relation.kind === 'view') { + const nested = relation.view; + if (`${nested.schema}.${nested.name}` === key) continue; + // An inner definer view re-owns the read; an inner invoker view runs + // under whichever owner is already in force. + const owner = nested.securityInvoker ? hops[hops.length - 1].owner : nested.owner; + walk(nested, [...hops, { view: `${nested.schema}.${nested.name}`, owner }]); + continue; + } + + const relKey = `${relation.schema}.${relation.name}`; + const dedupe = `${relKey}::${hops[hops.length - 1].owner}`; + if (seen.has(dedupe)) continue; + seen.add(dedupe); + bases.push({ schema: relation.schema, table: relation.name, hops: [...hops] }); + } + }; + + walk(root, [{ view: `${root.schema}.${root.name}`, owner: root.owner }]); + + return { bases, ...(opaque ? { opaque } : {}) }; +} + export type Resolved = | { kind: 'table'; schema: string; name: string } | { kind: 'view'; view: ViewSnapshot }; diff --git a/packages/safegres/src/checks/role-reach.ts b/packages/safegres/src/checks/role-reach.ts index 008411762..48be22e2a 100644 --- a/packages/safegres/src/checks/role-reach.ts +++ b/packages/safegres/src/checks/role-reach.ts @@ -35,6 +35,13 @@ export type RoleReachEdge = * the body names is read under the owner's privileges, not the caller's. */ | { kind: 'view'; view: string; owner: string } + /** + * The caller read a materialized view whose rows were computed as `owner` + * at REFRESH time. Unlike the view edge this holds whatever the reader's + * privileges are: the rows are stored, so the bases are never consulted and + * their policies never run. + */ + | { kind: 'matview'; view: string; owner: string } /** * The caller's command fired a rewrite rule on `view`. The rule's actions * are permission-checked against the rule's table owner, and unlike the @@ -153,6 +160,12 @@ export interface ViewReachInput { /** ACL rows on the view itself — who can SELECT the view at all. */ grants: GrantInfo[]; baseRelations: ViewBaseRelation[]; + /** + * The outermost relation is a materialized view, so its first hop is a + * `matview` edge: the rows were computed at REFRESH time rather than read + * through on demand. + */ + materialized?: boolean; } /** @@ -188,7 +201,11 @@ export function computeViewReach( effectiveRole: base.hops[base.hops.length - 1].owner, path: [ { kind: 'grant', via: select.via, privilege: 'SELECT' }, - ...base.hops.map((h) => ({ kind: 'view' as const, view: h.view, owner: h.owner })) + ...base.hops.map((h, i) => ({ + kind: view.materialized && i === 0 ? ('matview' as const) : ('view' as const), + view: h.view, + owner: h.owner + })) ], proof: 'ast' }); diff --git a/packages/safegres/src/checks/view-exposure.ts b/packages/safegres/src/checks/view-exposure.ts new file mode 100644 index 000000000..ac0511a57 --- /dev/null +++ b/packages/safegres/src/checks/view-exposure.ts @@ -0,0 +1,269 @@ +/** + * L11 and L12: the two things a *readable* view does that its owner and its + * body, taken separately, do not explain. + * + * **L11 — a materialized view is a snapshot, not a query.** Its rows were + * computed once, by whoever ran `REFRESH`, and are then handed to every reader + * of the matview verbatim. The base relations are not consulted at read time, + * so their ACLs never apply and — the part that matters — their RLS policies + * never run. A matview cannot carry policies of its own either: RLS attaches + * to tables, and `security_invoker` is a view-only reloption. So a SELECT + * grant on a matview over an RLS-protected table is an unconditional grant on + * the rows that were visible at refresh time. + * + * **L12 — a view's `WHERE` is a filter, not a boundary.** Without + * `security_barrier` the planner may push the *caller's* qual below the view's + * own, so a leaky operator or a `COST 0.0001` function evaluates against rows + * the view was written to hide. The rows are not returned, but they are seen — + * and a function that raises them, writes them, or times differently on them + * exfiltrates them. This only matters where the view is the only path to the + * relation; if the caller can read the base table directly there is nothing + * for the view to hide. + * + * Both ship the same way as the rest of the L-series: `info`, score-neutral, + * and never recommending a revoke. The remedies are properties of the view. + */ + +import { bodyFiltersRows } from '../callgraph/extract'; +import type { ViewSnapshot } from '../pg/indexes'; +import type { TableSnapshot } from '../pg/introspect'; +import type { Finding } from '../types'; +import { + buildRelationIndex, + readBodies, + resolveViewBases, + type SuppressedView +} from './definer-view'; +import { effectiveGrants, type LatticeRoleOptions, type RoleGraph } from './lattice'; +import { computeViewReach, type ViewReachInput } from './role-reach'; + +export interface ViewExposureAnalysis { + /** Materialized views whose bodies resolved — L11 inputs. */ + matviews: ViewReachInput[]; + /** Filtering definer views without `security_barrier` — L12 inputs. */ + leaky: ViewReachInput[]; + /** Views left out, with why. An unread body is not a clean bill. */ + suppressed: SuppressedView[]; +} + +/** + * Resolve both populations in one pass over the view bodies. + * + * A materialized view is deliberately absent from the relation index: it is a + * terminal relation for anything reading *through* it (its rows are stored), + * so it must never be followed as a nested view. Its own body is still read, + * to learn which relations the refresh copied from. + */ +export async function analyzeViewExposure( + views: ViewSnapshot[], + tables: TableSnapshot[] +): Promise { + const queryable = views.filter((v) => !v.materialized); + const index = buildRelationIndex(queryable, tables); + const bodies = await readBodies(views); + + const matviews: ViewReachInput[] = []; + const leaky: ViewReachInput[] = []; + const suppressed: SuppressedView[] = []; + + for (const view of views) { + const label = `${view.schema}.${view.name}`; + + if (view.materialized) { + const { bases, opaque } = resolveViewBases(view, index, bodies); + if (opaque) { + suppressed.push({ view: label, reason: opaque }); + continue; + } + if (bases.length === 0) continue; + matviews.push({ + schema: view.schema, + name: view.name, + owner: view.owner, + grants: view.grants, + baseRelations: bases, + materialized: true + }); + continue; + } + + // An invoker view confers nothing, so its `WHERE` is never the only thing + // standing between a caller and a relation: the caller needs its own + // grant on the base to read the view at all. + if (view.securityInvoker || view.securityBarrier) continue; + + const filters = await bodyFiltersRows(view.definition); + if (filters === null) { + suppressed.push({ view: label, reason: 'body could not be parsed to look for a row filter' }); + continue; + } + if (!filters) continue; + + const { bases, opaque } = resolveViewBases(view, index, bodies); + if (opaque) { + suppressed.push({ view: label, reason: opaque }); + continue; + } + if (bases.length === 0) continue; + leaky.push({ + schema: view.schema, + name: view.name, + owner: view.owner, + grants: view.grants, + baseRelations: bases + }); + } + + return { matviews, leaky, suppressed }; +} + +/** Is `role` actually filtered by `table`'s policies? */ +function subjectToRls(table: TableSnapshot, role: string, graph: RoleGraph): boolean { + if (!table.rlsEnabled) return false; + if (graph.get(role)?.bypassRls) return false; + // The owner is exempt unless the table FORCEs policies on itself. + return !(table.owner === role && !table.rlsForced); +} + +/** + * L11: an untrusted role reads a table's rows out of a materialized view. + * + * Fires where the role can SELECT the matview and either holds no SELECT on + * the relation the refresh read, or holds one but is subject to policies the + * stored rows never passed through. The second case 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. + */ +export function checkMatviewSnapshot( + matviews: ViewReachInput[], + tables: TableSnapshot[], + graph: RoleGraph, + options: LatticeRoleOptions = {} +): Finding[] { + const untrusted = options.roles ?? []; + if (untrusted.length === 0 || matviews.length === 0) return []; + + const byKey = new Map(tables.map((t) => [`${t.schema}.${t.name}`, t])); + const out: Finding[] = []; + + for (const { role, cells } of computeViewReach(matviews, graph, untrusted)) { + for (const cell of cells) { + const base = byKey.get(`${cell.schema}.${cell.table}`); + if (!base) continue; + + const hasSelect = effectiveGrants(base, role, graph).some((g) => g.privilege === 'SELECT'); + const filtered = subjectToRls(base, role, graph); + // Reachable in its own right and not row-filtered: the snapshot shows + // the role nothing it could not select from the table itself. + if (hasSelect && !filtered) continue; + + const matview = cell.path.find((e) => e.kind === 'matview'); + if (!matview) continue; + const refresher = cell.effectiveRole; + + out.push({ + code: 'L11', + severity: 'info', + category: 'anti-pattern', + schema: base.schema, + table: base.name, + role, + privilege: 'SELECT', + message: hasSelect + ? `Untrusted role ${role} reads ${base.schema}.${base.name} through materialized view ` + + `${matview.view} without the row filter its policies apply — the rows were stored by ` + + `${refresher} at REFRESH time` + : `Untrusted role ${role} reads ${base.schema}.${base.name} through materialized view ` + + `${matview.view}, which stores rows computed as ${refresher} — ${role} holds no SELECT ` + + `on the base relation`, + hint: + `A materialized view is a stored copy: reading it never consults ${base.schema}.` + + `${base.name}, so neither its grants nor its RLS policies apply, and the matview cannot ` + + `carry policies or \`security_invoker\` of its own. Replace it with a plain ` + + `\`security_invoker\` view if the freshness is not what it is for, materialize only the ` + + `columns and rows that are safe to hand out unconditionally, or keep it in a schema the ` + + `API does not expose and serve a filtered view from it. Do not revoke the SELECT on the ` + + `matview — that grant is what the API serves.`, + context: { + matview: matview.view, + effectiveRole: refresher, + baseRlsEnabled: base.rlsEnabled, + rlsBypassed: filtered, + holdsBaseSelect: hasSelect, + proof: cell.proof + } + }); + } + } + + return out; +} + +/** + * L12: a definer view filters rows for an untrusted role but is not a barrier. + * + * Fires where the role reaches a relation *only* through a view whose body has + * a `WHERE`, and the view is not `security_barrier`. The finding is about the + * rows the view excludes: they are the ones the author meant to withhold, and + * without the barrier a cheap leaky qual supplied by the caller is evaluated + * against them. + */ +export function checkLeakyFilterView( + views: ViewReachInput[], + tables: TableSnapshot[], + graph: RoleGraph, + options: LatticeRoleOptions = {} +): Finding[] { + const untrusted = options.roles ?? []; + if (untrusted.length === 0 || views.length === 0) return []; + + const byKey = new Map(tables.map((t) => [`${t.schema}.${t.name}`, t])); + const out: Finding[] = []; + + for (const { role, cells } of computeViewReach(views, graph, untrusted)) { + for (const cell of cells) { + if (cell.effectiveRole === role) continue; + + const base = byKey.get(`${cell.schema}.${cell.table}`); + if (!base) continue; + // The caller can read the relation directly, so the view's WHERE is a + // convenience, not a boundary, and pushing a qual past it reveals + // nothing new. + if (effectiveGrants(base, role, graph).some((g) => g.privilege === 'SELECT')) continue; + + const hops = cell.path.filter((e) => e.kind === 'view'); + const outermost = hops[0]; + if (!outermost) continue; + + out.push({ + code: 'L12', + severity: 'info', + category: 'anti-pattern', + schema: base.schema, + table: base.name, + role, + privilege: 'SELECT', + message: + `View ${outermost.view} is the only path untrusted role ${role} has to ` + + `${base.schema}.${base.name}, and its row filter is not a security barrier — a leaky ` + + `qual from ${role} can be evaluated against the rows the view excludes`, + hint: + `Recreate the view \`WITH (security_barrier = true)\` so the planner cannot push a ` + + `caller-supplied qual below its own \`WHERE\`. A cheap function or a leaky operator is ` + + `otherwise evaluated on every row of ${base.schema}.${base.name}, including the hidden ` + + `ones, which is enough to read them out through errors, notices or timing. Where the ` + + `filter is per-caller, an RLS policy on the base relation is the stronger form — policy ` + + `quals already get barrier treatment. Do not revoke the SELECT on the view.`, + context: { + view: outermost.view, + effectiveRole: cell.effectiveRole, + viaViews: hops.map((h) => h.view), + baseRlsEnabled: base.rlsEnabled, + proof: cell.proof + } + }); + } + } + + return out; +} diff --git a/packages/safegres/src/commands/audit.ts b/packages/safegres/src/commands/audit.ts index d0e51d7ef..9b16d9ba2 100644 --- a/packages/safegres/src/commands/audit.ts +++ b/packages/safegres/src/commands/audit.ts @@ -55,6 +55,11 @@ import { } from '../checks/role-trust'; import { checkSetRoleEscalation } from '../checks/set-role'; import { checkStats, DEFAULT_STATS_THRESHOLDS, type StatsThresholds } from '../checks/stats'; +import { + analyzeViewExposure, + checkLeakyFilterView, + checkMatviewSnapshot +} from '../checks/view-exposure'; import { analyzeViewWrites, checkDefinerViewWrite, checkViewRuleBypass } from '../checks/view-writes'; import { configFingerprint } from '../config/fingerprint'; import { allAstRulesDisabled, applyRulesToFindings, matchTablePattern, resolveRules, rulesForTable } from '../config/resolve'; @@ -253,6 +258,18 @@ export async function audit( resolved.rules.get('L10')?.options as LatticeRoleOptions, exposure )?.roles ?? []; + const matviewRoles = withExposedRoles( + resolved.rules.get('L11')?.options as LatticeRoleOptions, + exposure + )?.roles ?? []; + const leakyViewRoles = withExposedRoles( + resolved.rules.get('L12')?.options as LatticeRoleOptions, + exposure + )?.roles ?? []; + const viewExposureEnabled = + !skipAst + && ((matviewRoles.length > 0 && resolved.rules.get('L11')?.enabled !== false) + || (leakyViewRoles.length > 0 && resolved.rules.get('L12')?.enabled !== false)); const viewWritesEnabled = !skipAst && ((viewWriteRoles.length > 0 && resolved.rules.get('L9')?.enabled !== false) @@ -261,7 +278,8 @@ export async function audit( (perfEnabled && config.perf?.paths?.infer !== false) || resolved.rules.get('L4')?.enabled !== false || (!skipAst && definerViewRoles.length > 0 && resolved.rules.get('L8')?.enabled !== false) - || viewWritesEnabled; + || viewWritesEnabled + || viewExposureEnabled; const viewSnapshot = needsViews ? await introspectViews(exec, { schemas: options.schemas ?? config.schemas, @@ -380,6 +398,22 @@ export async function audit( } } + // L11/L12 are the other two things a readable view does: store rows, and + // filter them without being a boundary. One body pass serves both. + if (viewExposureEnabled) { + const exposureViews = await analyzeViewExposure(viewSnapshot, snapshot); + if (matviewRoles.length > 0 && resolved.rules.get('L11')?.enabled !== false) { + findings.push( + ...checkMatviewSnapshot(exposureViews.matviews, snapshot, roleGraph, { roles: matviewRoles }) + ); + } + if (leakyViewRoles.length > 0 && resolved.rules.get('L12')?.enabled !== false) { + findings.push( + ...checkLeakyFilterView(exposureViews.leaky, snapshot, roleGraph, { roles: leakyViewRoles }) + ); + } + } + const statsSnapshot: StatsSnapshot | null = statsEnabled ? await introspectStats(exec, { schemas: options.schemas ?? config.schemas, diff --git a/packages/safegres/src/config/presets.ts b/packages/safegres/src/config/presets.ts index 49554cb83..3082f6bbb 100644 --- a/packages/safegres/src/config/presets.ts +++ b/packages/safegres/src/config/presets.ts @@ -35,7 +35,12 @@ export const recommended: SafegresConfig = { // rewrite rule whose action runs as the view owner. Same posture again — // new, body-derived, zero weight until validated. L9: ['info', { rolesFrom: 'anon' }], - L10: ['info', { rolesFrom: 'anon' }] + L10: ['info', { rolesFrom: 'anon' }], + // The other two ways a readable view is not what its definition says: a + // materialized view serves rows RLS never filtered, and a filtering view + // without `security_barrier` is not a boundary. Same posture again. + L11: ['info', { rolesFrom: 'anon' }], + L12: ['info', { rolesFrom: 'anon' }] } }; diff --git a/packages/safegres/src/index.ts b/packages/safegres/src/index.ts index 31bc709f5..37113051b 100644 --- a/packages/safegres/src/index.ts +++ b/packages/safegres/src/index.ts @@ -75,6 +75,12 @@ export { checkUnusedIndexes, DEFAULT_STATS_THRESHOLDS } from './checks/stats'; +export type { ViewExposureAnalysis } from './checks/view-exposure'; +export { + analyzeViewExposure, + checkLeakyFilterView, + checkMatviewSnapshot +} from './checks/view-exposure'; export type { ViewWriteAnalysis } from './checks/view-writes'; export { analyzeViewWrites, diff --git a/packages/safegres/src/pg/indexes.ts b/packages/safegres/src/pg/indexes.ts index ab859f245..7fc028e37 100644 --- a/packages/safegres/src/pg/indexes.ts +++ b/packages/safegres/src/pg/indexes.ts @@ -227,6 +227,12 @@ export interface ViewSnapshot { * and confers nothing; when false (the default) it executes as `owner`. */ securityInvoker: boolean; + /** + * `reloptions.security_barrier`. Without it the planner may push a caller's + * own qual below the view's `WHERE`, so a leaky operator or function sees + * the rows the view filters out — the view is a filter, not a boundary. + */ + securityBarrier: boolean; /** The owner is a superuser or has BYPASSRLS: the bases' policies never run. */ ownerBypassesRls: boolean; /** ACL rows on the view itself, in the same shape as a table's. */ @@ -299,6 +305,7 @@ export async function introspectViews( owner: string; materialized: boolean; security_invoker: boolean; + security_barrier: boolean; owner_bypasses_rls: boolean; grants: Array<{ role: string; privilege: string; grantable: boolean; bypassRls: boolean }>; definition: string; @@ -328,6 +335,13 @@ export async function introspectViews( WHERE option_name = 'security_invoker'), 'false' )::boolean AS security_invoker, + -- Same spelling latitude, and the same reason to read it: a view + -- used as a row filter is only a boundary when it is a barrier. + COALESCE( + (SELECT option_value FROM pg_options_to_table(c.reloptions) + WHERE option_name = 'security_barrier'), + 'false' + )::boolean AS security_barrier, c.relacl AS relacl, pg_get_viewdef(c.oid) AS definition, -- Bitmask over 1 << CMD_*: UPDATE 4, INSERT 8, DELETE 16. The second @@ -378,6 +392,7 @@ export async function introspectViews( v.owner, v.materialized, v.security_invoker, + v.security_barrier, v.owner_bypasses_rls, v.definition, COALESCE( @@ -411,6 +426,7 @@ export async function introspectViews( owner: r.owner, materialized: r.materialized, securityInvoker: r.security_invoker, + securityBarrier: r.security_barrier, ownerBypassesRls: r.owner_bypasses_rls, grants: r.grants.map((g) => ({ role: g.role, diff --git a/packages/safegres/src/rules/registry.ts b/packages/safegres/src/rules/registry.ts index 240f1c46f..22f03bee7 100644 --- a/packages/safegres/src/rules/registry.ts +++ b/packages/safegres/src/rules/registry.ts @@ -261,6 +261,32 @@ export const RULES: RuleMeta[] = [ title: 'Rewrite-rule bypass — a rule on a view writes a relation as the view owner (options: { roles: [...] })', scope: 'table' }, + { + code: 'L11', + category: 'anti-pattern', + // Ships `info` on the same new-rule posture, and understates: a matview is + // a stored copy, so a SELECT grant on it is an unconditional grant on the + // rows a REFRESH captured — RLS on the base relation never runs, and the + // matview can carry neither policies nor `security_invoker`. On its own + // merits that is `high` when the base is RLS-protected. + defaultSeverity: 'info', + direction: 'fail-open', + title: 'Materialized-view snapshot — an untrusted role reads stored rows the base relation would not serve it (options: { roles: [...] })', + scope: 'table' + }, + { + code: 'L12', + category: 'anti-pattern', + // Ships `info`, and is the weakest of the L-series on purpose: the leak + // needs a leaky operator or a cheap function in the caller's own qual, so + // it is a capability rather than an unconditional read. `medium` once + // proven — the rows it exposes are precisely the ones the view was written + // to withhold. + defaultSeverity: 'info', + direction: 'fail-open', + title: 'Non-barrier filtering view — a view is an untrusted role\'s only path to a relation but its row filter is not a boundary (options: { roles: [...] })', + scope: 'table' + }, { code: 'W1', category: 'meta',