Skip to content

Commit cba84d6

Browse files
authored
Merge pull request #1623 from constructive-io/feat/opaque-taint-and-object-acls
feat(safegres): L15–L17 — opaque-tainted reach, and the objects that are not tables
2 parents 1f0d52c + 9237f89 commit cba84d6

27 files changed

Lines changed: 1220 additions & 24 deletions

File tree

.agents/skills/safegres/SKILL.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ The score only improves by being **explicit** (declaring exposure and intent) or
8585
| L12 | info | fail-open | **Non-barrier filtering view** — the row filter is not a boundary, so a leaky caller predicate reads below it |
8686
| L13 | info | fail-open | **Column-level grant** — reach through `pg_attribute.attacl`, which no relation ACL shows |
8787
| L14 | info | neutral | **Unaudited base relation** — a definer view reads a schema the audit never introspected |
88+
| L15 | info | neutral | **Unreadable view body** — an untrusted role reads a definer view whose body could not be followed |
89+
| L16 | info | fail-open | **Sequence privilege**`nextval`/`setval`/`last_value` reach, which no policy filters |
90+
| L17 | info | fail-open | **Foreign-table grant** — a relation Postgres will not let you protect with RLS at all |
8891
| W1 | medium | meta | No exposure surface configured — DB assumed reachable, score capped |
8992

9093
**The L-series is the reachability lattice: what an untrusted role can make Postgres *do*, not what the ACL rows say.** Four things feed it, and they compose:
@@ -94,21 +97,23 @@ The score only improves by being **explicit** (declaring exposure and intent) or
9497
- **Exposure** — which relations an API can actually name (L6), and which roles a request can arrive as (`exposure.anonRoles` drives every untrusted-role option).
9598
- **`SET ROLE`** — on PG16+ a membership can confer `set_option` without `INHERIT`, so a role that passively holds nothing still executes as its target (L7).
9699

97-
Reach is modelled as cells in `checks/role-reach.ts`, each carrying the role the access **executes as** (`effectiveRole`), the **path** of edges it arrived by (`grant` / `setrole` / `view` / `matview` / `rule`), and a **proof** bit: `catalog` (an ACL row or `pg_auth_members`) or `ast` (read out of a SQL body). `opaque-tainted` exists for a chain that could not be followed; nothing produces it yet, because opacity is currently whole-body.
100+
Reach is modelled as cells in `checks/role-reach.ts`, each carrying the role the access **executes as** (`effectiveRole`), the **path** of edges it arrived by (`grant` / `setrole` / `view` / `matview` / `rule`), and a **proof** bit: `catalog` (an ACL row or `pg_auth_members`) or `ast` (read out of a SQL body). `opaque-tainted` marks a proven read whose downstream effects are unknown, and L15 is its producer: a definer view whose body executes SQL the analysis cannot see through yields a cell naming the *view itself*, since what lies past it cannot be named.
98101

99102
**L8–L12 are the AST half: a view is not what its definition looks like.** A view without `security_invoker` runs as its owner, so a caller's SELECT on the view reads base relations under the *owner's* privileges (L8), and if the view is auto-updatable, writes land the same way (L9). Rewrite rules are worse: their actions are **not** governed by `security_invoker`, so L10 fires on invoker views where L9 does not. A materialized view stores rows computed at REFRESH time, so the bases are never consulted and their policies never run (L11). And a filtering view that is not `security_barrier` lets a leaky caller predicate be pushed below the filter (L12).
100103

101104
**Which columns escape is a catalog fact, not a parsing problem.** `ViewSnapshot.columnDeps` reads the `pg_depend` rows the rewriter wrote for the view's `_RETURN` rule: `SELECT *` arrives expanded, a column used only in a `WHERE` counts as read, and a nested view depends on the *inner view's* columns. L8 puts that set in the message and in `context.columns`, and suppresses itself when every escaping column is one the role already holds by column grant **and** the base has no RLS — with RLS on, the owner reads rows the caller's policies hide, so the projection is beside the point. An absent column set is unknown, never narrow.
102105

103-
**Conservatism is the rule, not a nicety.** A body safegres cannot see through — dynamic SQL, an unparseable definition, a chain deeper than the hop limit — **suppresses** the finding; it never becomes a weaker guess. L14 is the one place that reports the *absence* of knowledge: a qualified reference into a schema the audit never introspected is real reach with an ungraded far end, distinct from a name it simply could not resolve (a CTE, an alias), which is still dropped.
106+
**Conservatism is the rule, not a nicety.** A body safegres cannot see through — dynamic SQL, an unparseable definition, a chain deeper than the hop limit — **suppresses** the finding; it never becomes a weaker guess. Two rules report the *absence* of knowledge rather than letting it read as a pass: L14 for a qualified reference into a schema the audit never introspected (real reach, ungraded far end — distinct from a name it simply could not resolve, like a CTE or an alias, which is still dropped), and L15 for a body it could not follow. Opacity is graded in two degrees: `opaque` (unparseable — nothing is graded) and `tainted` (parses, but calls something carrying its query in a string, like `query_to_xml` or `dblink` — everything it *did* name is still proven and still graded). An invoker view is exempt from L15: its body runs as the caller, so an unreadable one confers nothing.
107+
108+
**Not every relation is a table.** L16/L17 grade the two `relkind`s nothing else reads (`src/pg/objects.ts`). A sequence takes `USAGE`/`UPDATE` (`nextval`/`setval` — burn or reset the counter) and `SELECT` (`last_value`, a live row count for the owning table), and RLS does not apply to one. A foreign table cannot carry RLS *at all* — Postgres rejects `ENABLE ROW LEVEL SECURITY` on one — so unlike the A2 shape it resembles, "add a policy" is not an available remedy. L16 carries the sequence's `OWNED BY` column so the advice is `AS IDENTITY`, never a `REVOKE` that breaks a `serial` insert.
104109

105110
**No rule may recommend revoking a grant it cannot prove unused.** The L-series remedies are *fix the view* (`security_invoker = true`, change the owner, add `security_barrier`), *bring the schema into scope*, or *narrow the role model* — never "revoke this grant", because the grant is usually what the API serves. L2/L3/L4/L6 are the exceptions that do recommend removal, and each carries an explicit veto (a policy reference, a live column grant, a reachable view) that suppresses the advice when anything is load-bearing.
106111

107112
New L-rules ship `info` and **score-neutral** on purpose: the honest severity of a definer view handing an anonymous role a table is not informational, but a new rule earns its weight after it has been run against real schemas. Promoting one is a deliberate scoring change.
108113

109114
Perf-dimension rules (only collected with `--perf`, scored on their own axis; `S*` additionally need `--stats`): **X1** FK with no covering index (medium), **X2** policy filters on a column that leads no index (medium), **X3** policy casts/wraps its own column with no matching expression index (medium), **X4** policy calls a non-LEAKPROOF function (low), **X5** redundant/duplicate index (low), **X6** no primary key and no usable replica identity (low), **X7** search column with no index the search can use — `tsvector` w/o GIN/GiST, `vector` w/o HNSW/IVFFlat (medium), **X8** sort-shaped `timestamptz`/`date` column leading no index (info, heuristic), **X9** policy calls a STABLE function per row because it is not wrapped in a scalar sub-select (medium), plus P1/P1b and the runtime-statistics rules **S1**-**S4**.
110115

111-
**Direction is the key idea:** `fail-open` = real exposure (untrusted side reaches more than intended). `fail-closed` = denied at runtime (hygiene/availability, not a leak) — contributes **0** to the score by default. R1/R2 and the untrusted-role L-rules are no-ops until you configure a role list: `"L8": ["info", { "roles": ["anonymous"] }]`, or `{ "rolesFrom": "anon" }` to take them from `exposure.anonRoles`, which is what `safegres:recommended` does for L5 and L7–L14. `safegres:constructive` sets R1/R2 for `anonymous`.
116+
**Direction is the key idea:** `fail-open` = real exposure (untrusted side reaches more than intended). `fail-closed` = denied at runtime (hygiene/availability, not a leak) — contributes **0** to the score by default. R1/R2 and the untrusted-role L-rules are no-ops until you configure a role list: `"L8": ["info", { "roles": ["anonymous"] }]`, or `{ "rolesFrom": "anon" }` to take them from `exposure.anonRoles`, which is what `safegres:recommended` does for L5 and L7–L17. `safegres:constructive` sets R1/R2 for `anonymous`.
112117

113118
## Configuration (confstash)
114119

packages/safegres/README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,9 @@ family, **not** the dimension: `P1`/`P1b` are performance, `P5` is security.
150150
| 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 † |
151151
| L13 | info | fail-open | **Column-level grant** — an untrusted role reaches a relation through `pg_attribute.attacl`, which no relation ACL shows † |
152152
| L14 | info | neutral | **Unaudited base relation** — a definer view reads a relation in a schema the audit never introspected † |
153+
| L15 | info | neutral | **Unreadable view body** — an untrusted role reads a definer view whose definition the analysis could not follow † |
154+
| L16 | info | fail-open | **Sequence privilege** — an untrusted role can advance or read a sequence, which no policy filters † |
155+
| L17 | info | fail-open | **Foreign-table grant** — an untrusted role reaches a relation that cannot carry RLS at all † |
153156
| W1 | medium || **No exposure surface configured** — whole database assumed reachable, score capped |
154157

155158
† R1/R2/L5 are no-ops until you name the untrusted roles:

packages/safegres/__tests__/definer-view.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,16 @@ describe('analyzeViewBodies', () => {
117117
]);
118118
});
119119

120-
it('suppresses a view whose body it cannot read, rather than reporting an empty one', async () => {
120+
it('grades nothing from a body it cannot read, and says so rather than reporting an empty one', async () => {
121121
const { views, suppressed } = await analyzeViewBodies(
122122
[view({ definition: 'SELECT ((( FROM nowhere' })],
123123
[table()]
124124
);
125-
expect(views).toEqual([]);
125+
// No relation is graded — a fragment of an unread body under-reports what
126+
// the view reaches — but the view itself stays in the model, carrying why,
127+
// so the gap is reportable (L15) instead of a silent clean bill.
128+
expect(views[0].baseRelations).toEqual([]);
129+
expect(views[0].unreadable).toBe('SQL fragment failed to parse');
126130
expect(suppressed).toEqual([
127131
{ view: 'app.order_totals', reason: 'SQL fragment failed to parse' }
128132
]);
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
import { type RoleGraph } from '../src/checks/lattice';
2+
import {
3+
checkUntrustedForeignTableGrants,
4+
checkUntrustedSequenceGrants
5+
} from '../src/checks/object-acls';
6+
import type { RoleAttributes } from '../src/pg/acl';
7+
import type { GrantInfo, PgPrivilege } from '../src/pg/introspect';
8+
import type { ObjectAclSnapshot } from '../src/pg/objects';
9+
10+
function grant(role: string, privilege: PgPrivilege): GrantInfo {
11+
return { role, privilege, grantable: false, bypassRls: false };
12+
}
13+
14+
function sequence(partial: Partial<ObjectAclSnapshot> = {}): ObjectAclSnapshot {
15+
return {
16+
schema: 'app',
17+
name: 'orders_id_seq',
18+
kind: 'sequence',
19+
owner: 'app_owner',
20+
grants: [grant('anon', 'USAGE')],
21+
...partial
22+
};
23+
}
24+
25+
function foreignTable(partial: Partial<ObjectAclSnapshot> = {}): ObjectAclSnapshot {
26+
return {
27+
schema: 'app',
28+
name: 'remote_orders',
29+
kind: 'foreign table',
30+
owner: 'app_owner',
31+
grants: [grant('anon', 'SELECT')],
32+
server: 'analytics',
33+
...partial
34+
};
35+
}
36+
37+
function role(name: string, partial: Partial<RoleAttributes> = {}): [string, RoleAttributes] {
38+
return [
39+
name,
40+
{ name, bypassRls: false, isSuper: false, inheritsFrom: [], canSetRole: [], ...partial }
41+
];
42+
}
43+
44+
const GRAPH: RoleGraph = new Map([role('anon'), role('app_owner'), role('reader')]);
45+
const ANON = { roles: ['anon'] };
46+
47+
describe('L16 — sequence privileges', () => {
48+
it('reports what USAGE actually confers, not just that a grant exists', () => {
49+
const [f] = checkUntrustedSequenceGrants([sequence()], GRAPH, ANON);
50+
expect(f.code).toBe('L16');
51+
expect(f.table).toBe('orders_id_seq');
52+
expect(f.message).toMatch(/nextval/);
53+
expect(f.context).toMatchObject({ objectKind: 'sequence', privileges: ['USAGE'] });
54+
});
55+
56+
it('separates reading the counter from advancing it', () => {
57+
const [read] = checkUntrustedSequenceGrants(
58+
[sequence({ grants: [grant('anon', 'SELECT')] })],
59+
GRAPH,
60+
ANON
61+
);
62+
expect(read.message).toMatch(/last_value/);
63+
expect(read.message).not.toMatch(/nextval/);
64+
});
65+
66+
it('follows PUBLIC, so a grant nobody named still reports', () => {
67+
const [f] = checkUntrustedSequenceGrants(
68+
[sequence({ grants: [grant('PUBLIC', 'USAGE')] })],
69+
GRAPH,
70+
ANON
71+
);
72+
expect(f?.code).toBe('L16');
73+
expect(f.context).toMatchObject({ via: 'PUBLIC' });
74+
});
75+
76+
it('follows role inheritance the same way the rest of the lattice does', () => {
77+
const graph: RoleGraph = new Map([role('anon', { inheritsFrom: ['reader'] }), role('reader')]);
78+
const [f] = checkUntrustedSequenceGrants(
79+
[sequence({ grants: [grant('reader', 'USAGE')] })],
80+
graph,
81+
ANON
82+
);
83+
expect(f.context).toMatchObject({ via: 'member of reader' });
84+
});
85+
86+
it('leads with identity columns, not a revoke, when the sequence feeds a column', () => {
87+
const [f] = checkUntrustedSequenceGrants(
88+
[sequence({ ownedBy: 'app.orders.id' })],
89+
GRAPH,
90+
ANON
91+
);
92+
expect(f.message).toMatch(/feeds app\.orders\.id/);
93+
expect(f.hint).toMatch(/AS IDENTITY/);
94+
expect(f.hint).toMatch(/needs USAGE, and revoking it/);
95+
expect(f.context).toMatchObject({ ownedBy: 'app.orders.id' });
96+
});
97+
98+
it('stays silent on a sequence the role cannot touch', () => {
99+
expect(
100+
checkUntrustedSequenceGrants([sequence({ grants: [] })], GRAPH, ANON)
101+
).toEqual([]);
102+
});
103+
104+
it('does nothing without a configured role list', () => {
105+
expect(checkUntrustedSequenceGrants([sequence()], GRAPH)).toEqual([]);
106+
});
107+
});
108+
109+
describe('L17 — foreign-table grants', () => {
110+
it('reports the grant and names the server behind it', () => {
111+
const [f] = checkUntrustedForeignTableGrants([foreignTable()], GRAPH, ANON);
112+
expect(f.code).toBe('L17');
113+
expect(f.message).toMatch(/server analytics/);
114+
expect(f.context).toMatchObject({ objectKind: 'foreign table', privileges: ['SELECT'] });
115+
});
116+
117+
it('says the A2 remedy is unavailable here, because Postgres refuses RLS on one', () => {
118+
const [f] = checkUntrustedForeignTableGrants([foreignTable()], GRAPH, ANON);
119+
expect(f.message).toMatch(/cannot carry RLS/);
120+
expect(f.hint).toMatch(/rejects `ENABLE ROW LEVEL SECURITY`/);
121+
});
122+
123+
it('does not grade a sequence, and the sequence rule does not grade it', () => {
124+
expect(checkUntrustedForeignTableGrants([sequence()], GRAPH, ANON)).toEqual([]);
125+
expect(checkUntrustedSequenceGrants([foreignTable()], GRAPH, ANON)).toEqual([]);
126+
});
127+
128+
it('stays silent when the untrusted role holds nothing on it', () => {
129+
expect(
130+
checkUntrustedForeignTableGrants([foreignTable({ grants: [] })], GRAPH, ANON)
131+
).toEqual([]);
132+
});
133+
});
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import { getConnections, PgTestClient } from 'pgsql-test';
2+
3+
import { introspectObjectAcls, type ObjectAclSnapshot } from '../src/pg/objects';
4+
5+
jest.setTimeout(120000);
6+
7+
let pg: PgTestClient;
8+
let teardown: () => Promise<void>;
9+
10+
beforeAll(async () => {
11+
({ pg, teardown } = await getConnections());
12+
// Sequences and foreign tables are `relkind` 'S' and 'f': neither is read by
13+
// the table snapshot, and both carry ACLs.
14+
await pg.any(`
15+
CREATE SCHEMA fx_objacl;
16+
CREATE ROLE fx_obj_anon;
17+
18+
CREATE SEQUENCE fx_objacl.free_standing;
19+
GRANT USAGE, SELECT ON SEQUENCE fx_objacl.free_standing TO fx_obj_anon;
20+
21+
CREATE TABLE fx_objacl.orders (id serial PRIMARY KEY, total numeric);
22+
GRANT USAGE ON SEQUENCE fx_objacl.orders_id_seq TO fx_obj_anon;
23+
24+
CREATE SEQUENCE fx_objacl.ungranted;
25+
26+
CREATE EXTENSION IF NOT EXISTS postgres_fdw;
27+
CREATE SERVER fx_obj_srv FOREIGN DATA WRAPPER postgres_fdw
28+
OPTIONS (host 'localhost', dbname 'postgres');
29+
CREATE FOREIGN TABLE fx_objacl.remote (id int)
30+
SERVER fx_obj_srv OPTIONS (schema_name 'public', table_name 'nothing');
31+
GRANT SELECT ON fx_objacl.remote TO fx_obj_anon;
32+
`);
33+
});
34+
35+
afterAll(async () => {
36+
await pg.any(`
37+
DROP SCHEMA fx_objacl CASCADE;
38+
DROP SERVER fx_obj_srv CASCADE;
39+
DROP ROLE fx_obj_anon;
40+
`);
41+
await teardown();
42+
});
43+
44+
const find = (objects: ObjectAclSnapshot[], name: string) =>
45+
objects.find((o) => o.name === name);
46+
47+
const granted = (o: ObjectAclSnapshot | undefined) =>
48+
(o?.grants ?? []).filter((g) => g.role === 'fx_obj_anon').map((g) => g.privilege).sort();
49+
50+
describe('introspectObjectAcls', () => {
51+
it('reads sequences and foreign tables, and nothing else', async () => {
52+
const objects = await introspectObjectAcls(pg.client, { schemas: ['fx_objacl'] });
53+
expect(objects.map((o) => `${o.kind}:${o.name}`).sort()).toEqual([
54+
'foreign table:remote',
55+
'sequence:free_standing',
56+
'sequence:orders_id_seq',
57+
'sequence:ungranted'
58+
]);
59+
});
60+
61+
it('reports sequence privileges the way the ACL stores them', async () => {
62+
const objects = await introspectObjectAcls(pg.client, { schemas: ['fx_objacl'] });
63+
expect(granted(find(objects, 'free_standing'))).toEqual(['SELECT', 'USAGE']);
64+
expect(granted(find(objects, 'ungranted'))).toEqual([]);
65+
});
66+
67+
it('links a serial sequence to the column it feeds, and leaves a free-standing one unlinked', async () => {
68+
const objects = await introspectObjectAcls(pg.client, { schemas: ['fx_objacl'] });
69+
// The difference between a load-bearing USAGE grant and a gratuitous one.
70+
expect(find(objects, 'orders_id_seq')?.ownedBy).toBe('fx_objacl.orders.id');
71+
expect(find(objects, 'free_standing')?.ownedBy).toBeUndefined();
72+
});
73+
74+
it('names the server behind a foreign table', async () => {
75+
const objects = await introspectObjectAcls(pg.client, { schemas: ['fx_objacl'] });
76+
const remote = find(objects, 'remote');
77+
expect(remote?.server).toBe('fx_obj_srv');
78+
expect(granted(remote)).toEqual(['SELECT']);
79+
});
80+
81+
it('cannot be handed RLS by Postgres at all, which is why L17 exists', async () => {
82+
await expect(
83+
pg.any('ALTER FOREIGN TABLE fx_objacl.remote ENABLE ROW LEVEL SECURITY')
84+
).rejects.toThrow(/not supported for foreign tables/);
85+
});
86+
});

0 commit comments

Comments
 (0)