feat(safegres): perf rule X9 (RLS quals re-evaluated per row)

.agents/skills/safegres/SKILL.md

@@ -72,7 +72,7 @@ The score only improves by being **explicit** (declaring exposure and intent) or
 | R3 | medium | fail-open | RLS table has grants **TO PUBLIC** |
 | W1 | medium | meta | No exposure surface configured — DB assumed reachable, score capped |
 
-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), plus P1/P1b and the runtime-statistics rules **S1**-**S4**.
+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**.
 
 **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 are no-ops until you configure a role list; `safegres:constructive` sets them for `anonymous`.
 
@@ -103,11 +103,13 @@ Typed variant (`defineConfig` from `confstash`) works too — see the package RE
 
 ## Performance dimension (`--perf`, off by default)
 
-`safegres perf` / `safegres audit --perf` adds `report.perf` — its own findings, summary, and 0-100 score over index hygiene (X1/X5/X6/X7/X8), policy-aware index rules (X2/X3/X4) and policy cost (P1/P1b). Security and perf scores are never mixed: `report.score` sees only security findings, `report.perf.score` only perf ones. All checks are catalog + policy-AST analysis, so they're deterministic against an empty CI database.
+`safegres perf` / `safegres audit --perf` adds `report.perf` — its own findings, summary, and 0-100 score over index hygiene (X1/X5/X6/X7/X8), policy-aware index rules (X2/X3/X4/X9) and policy cost (P1/P1b). Security and perf scores are never mixed: `report.score` sees only security findings, `report.perf.score` only perf ones. All checks are catalog + policy-AST analysis, so they're deterministic against an empty CI database.
 
 X7 is Constructive-specific: `graphile-search` exposes a full-text filter for every `tsvector` column and similarity search for every `vector` column *from the codec alone*, so an unindexed one is a live API field served by a seq scan. BM25/pg_trgm are intentionally not checked — those adapters are discovered from their indexes, so no index means the feature isn't exposed. X8 is the one heuristic (any column is orderable; timestamps are what feeds are actually sorted by), so it is `info`, scores 0, and is meant to be read rather than gated on.
 
-X2/X3/X4 read the policy predicate itself: RLS quals run before user quals on every candidate row, so an unindexed policy column (X2), a cast/function wrapping it (X3), or a non-LEAKPROOF call that blocks qual pushdown (X4) is a tax on every query against the table, not just one slow report.
+X2/X3/X4/X9 read the policy predicate itself: RLS quals run before user quals on every candidate row, so an unindexed policy column (X2), a cast/function wrapping it (X3), or a non-LEAKPROOF call that blocks qual pushdown (X4) is a tax on every query against the table, not just one slow report.
+
+X9 is the InitPlan rule. `STABLE` does not mean "evaluated once": measured on 200k rows, `USING (other_id = current_principal_id())` executed the function 200,000 times (424 ms) while `USING (other_id = (SELECT current_principal_id()))` executed it once (22 ms). The penalty is plan-dependent — an unwrapped call the planner turns into an index condition is evaluated once per scan, and the same policy costs 200k calls the moment the qual lands in a Filter (unindexed column, join, OR branch). Wrapping removes the dependence: the sub-select references no column, so it is hoisted into an InitPlan whatever plan is chosen, and the result is a constant the index can probe with. Detection is structural (non-IMMUTABLE + no column-referencing argument + not already inside an uncorrelated scalar sub-select), so it needs no list of known identity functions and catches a bare `current_setting()` too. VOLATILE calls are excluded on purpose (per-row is their contract; P1 covers them), and an `EXISTS` sub-select does not count as hoisted — it is correlated, so it runs per row.
 
 ```jsonc
 {

packages/safegres/README.md

@@ -147,14 +147,24 @@ A slow database is a different problem from an unsafe one, so safegres scores th
 | X6 | low | index | **No primary key** and no usable replica identity — rows cannot be addressed by updates, deletes, or logical replication |
 | X7 | medium | index | **Search column with no index the search can use** — a `tsvector` without GIN/GiST, a `vector` without HNSW/IVFFlat |
 | X8 | info | index | **Sort-shaped column leads no index** (`timestamptz`/`date`) — ordering or cursor-paginating a connection by it sorts the whole table |
+| X9 | medium | index | **Policy calls a STABLE function per row** — the call isn't wrapped in a scalar sub-select, so the planner can't hoist it into an InitPlan |
 | P1 | high | anti-pattern | Policy body calls a **VOLATILE function** — re-evaluated per row |
 | P1b | medium | anti-pattern | Policy body calls a **STABLE function** in a per-row position |
 
 Every check is pure catalog + AST analysis: deterministic, workload-free, and safe to run against an empty CI database. An index covers a foreign key only when its *leading* columns are the FK's columns and it covers every row — partial and expression indexes don't count, because the planner can't use them for the referential-integrity lookup. Constraint-backed, unique, partial, and expression indexes are never reported as redundant.
 
 X7 exists because the column type *is* the API declaration: `graphile-search` exposes a full-text filter for every `tsvector` column and a similarity search for every `vector` column, purely from the codec — so an unindexed one is a first-class API field backed by a sequential scan plus a per-row match or distance computation. BM25 and pg_trgm are deliberately not checked: those adapters are discovered *from* their indexes, so a missing index means the feature was never exposed. X8 is the one heuristic in the set — any column is orderable over a connection, but timestamps are what feeds are actually sorted and keyset-paginated by — so it defaults to `info`, contributes 0 to the score, and is meant to be read, not gated on (`perf.rules: { "X8": "off" }` to silence it). Trailing-position and partial indexes don't count for either rule: neither can serve the sort or the search on its own.
 
-X2–X4 are the checks a generic index linter can't make, because they read the policy predicate. RLS quals are evaluated *before* user quals, on every candidate row, for every caller — so an unindexed or cast-wrapped policy column is a whole-table tax rather than a slow query. X2 requires the policy column to be the *leading* column of some index (a trailing position can't serve the qual alone); X3 looks for an expression index matching the exact wrapped shape; X4 skips built-ins, whose leakproofness is a property of the server rather than a schema choice.
+X2–X4 and X9 are the checks a generic index linter can't make, because they read the policy predicate. RLS quals are evaluated *before* user quals, on every candidate row, for every caller — so an unindexed or cast-wrapped policy column is a whole-table tax rather than a slow query. X2 requires the policy column to be the *leading* column of some index (a trailing position can't serve the qual alone); X3 looks for an expression index matching the exact wrapped shape; X4 skips built-ins, whose leakproofness is a property of the server rather than a schema choice.
+
+X9 is the one that costs the most and looks the most innocent. `STABLE` promises a function's result won't change within the statement; it does **not** make the planner evaluate it once. Measured on 200k rows with a policy function that counts its own invocations:
+
+| Policy qual | Calls | Time |
+| --- | --- | --- |
+| `other_id = current_principal_id()` (Filter) | 200,000 | 424 ms |
+| `other_id = (SELECT current_principal_id())` (InitPlan) | 1 | 22 ms |
+
+The honest caveat: the penalty is **plan-dependent**. When the planner can turn the qual into an index condition it evaluates the function once per scan even unwrapped — so the same policy costs one call on an indexed column and 200,000 on a Filter (unindexed column, a join, an OR branch, a plan change after `ANALYZE`). Wrapping removes the dependence: `(SELECT f())` references no column, so it is hoisted into an **InitPlan** and evaluated once per query whatever plan is chosen, and the result is a constant the index can be probed with. X9 is structural, not a name list — it fires on any non-IMMUTABLE call whose arguments reference no column of the row and that isn't already inside an uncorrelated scalar sub-select, so a GUC-reading helper added next year is caught without configuration. `current_setting()` itself is STABLE and is flagged too: removing the wrapper function doesn't avoid the per-row call. VOLATILE calls are deliberately excluded — per-row evaluation is their defined behaviour, and hoisting one would change semantics (that's P1's job). Being inside an `EXISTS` sub-select is not a defence: that subquery is correlated with the outer row, so it runs per row and takes the call with it.
 
 ```bash
 safegres perf --database mydb

packages/safegres/__tests__/fixtures/x9-initplan.sql

@@ -0,0 +1,78 @@
+-- X9: a STABLE, row-independent call in a policy qual that the planner will
+-- re-evaluate per row because it is not wrapped in a scalar sub-select.
+DROP SCHEMA IF EXISTS fx_x9 CASCADE;
+CREATE SCHEMA fx_x9;
+
+CREATE FUNCTION fx_x9.current_tenant() RETURNS uuid AS $$
+  SELECT nullif(current_setting('jwt.claims.tenant_id', true), '')::uuid;
+$$ LANGUAGE sql STABLE;
+
+-- Row-dependent: the argument references a column, so there is nothing to hoist.
+CREATE FUNCTION fx_x9.owns(row_tenant uuid) RETURNS boolean AS $$
+  SELECT row_tenant IS NOT NULL;
+$$ LANGUAGE sql STABLE;
+
+-- Volatile: per-row evaluation is its defined behaviour (P1's business, not X9's).
+CREATE FUNCTION fx_x9.roll() RETURNS uuid AS $$
+  SELECT gen_random_uuid();
+$$ LANGUAGE sql VOLATILE;
+
+-- Immutable: folded at plan time, never flagged.
+CREATE FUNCTION fx_x9.zero() RETURNS uuid AS $$
+  SELECT '00000000-0000-0000-0000-000000000000'::uuid;
+$$ LANGUAGE sql IMMUTABLE;
+
+CREATE TABLE fx_x9.tenants (
+  id uuid PRIMARY KEY,
+  tenant_id uuid NOT NULL
+);
+CREATE INDEX tenants_tenant_idx ON fx_x9.tenants (tenant_id);
+
+-- X9: bare call, evaluated per row.
+CREATE TABLE fx_x9.bare (LIKE fx_x9.tenants INCLUDING ALL);
+ALTER TABLE fx_x9.bare ENABLE ROW LEVEL SECURITY;
+CREATE POLICY bare_tenant ON fx_x9.bare
+  USING (tenant_id = fx_x9.current_tenant());
+
+-- No X9: already wrapped in an uncorrelated scalar sub-select (InitPlan).
+CREATE TABLE fx_x9.hoisted (LIKE fx_x9.tenants INCLUDING ALL);
+ALTER TABLE fx_x9.hoisted ENABLE ROW LEVEL SECURITY;
+CREATE POLICY hoisted_tenant ON fx_x9.hoisted
+  USING (tenant_id = (SELECT fx_x9.current_tenant()));
+
+-- X9: inside an EXISTS sub-select. That subquery is correlated with the outer
+-- row, so it runs per row and takes the unhoisted call with it.
+CREATE TABLE fx_x9.correlated (LIKE fx_x9.tenants INCLUDING ALL);
+ALTER TABLE fx_x9.correlated ENABLE ROW LEVEL SECURITY;
+CREATE POLICY correlated_tenant ON fx_x9.correlated
+  USING (EXISTS (
+    SELECT 1 FROM fx_x9.tenants t
+    WHERE t.id = correlated.id AND t.tenant_id = fx_x9.current_tenant()
+  ));
+
+-- No X9: the argument references a column, so the call cannot be hoisted.
+CREATE TABLE fx_x9.row_dependent (LIKE fx_x9.tenants INCLUDING ALL);
+ALTER TABLE fx_x9.row_dependent ENABLE ROW LEVEL SECURITY;
+CREATE POLICY row_dependent_tenant ON fx_x9.row_dependent
+  USING (fx_x9.owns(tenant_id));
+
+-- No X9: VOLATILE and IMMUTABLE calls are both out of scope.
+CREATE TABLE fx_x9.other_volatility (LIKE fx_x9.tenants INCLUDING ALL);
+ALTER TABLE fx_x9.other_volatility ENABLE ROW LEVEL SECURITY;
+CREATE POLICY other_volatility_tenant ON fx_x9.other_volatility
+  USING (tenant_id = fx_x9.roll() OR tenant_id = fx_x9.zero());
+
+-- X9: current_setting() is STABLE too — dropping the wrapper does not fix it.
+CREATE TABLE fx_x9.raw_guc (
+  id uuid PRIMARY KEY,
+  tenant_name text NOT NULL
+);
+ALTER TABLE fx_x9.raw_guc ENABLE ROW LEVEL SECURITY;
+CREATE POLICY raw_guc_tenant ON fx_x9.raw_guc
+  USING (tenant_name = current_setting('jwt.claims.tenant', true));
+
+-- X9 on WITH CHECK as well as USING.
+CREATE TABLE fx_x9.writes (LIKE fx_x9.tenants INCLUDING ALL);
+ALTER TABLE fx_x9.writes ENABLE ROW LEVEL SECURITY;
+CREATE POLICY writes_tenant ON fx_x9.writes FOR INSERT
+  WITH CHECK (tenant_id = fx_x9.current_tenant());

packages/safegres/__tests__/perf.test.ts

@@ -151,6 +151,47 @@ describe('policy-aware perf rules', () => {
   });
 });
 
+describe('X9 — per-row policy calls that the planner could hoist', () => {
+  it('flags unwrapped STABLE calls and spares wrapped ones', async () => {
+    await applyFixture('x9-initplan.sql');
+    const report = await audit(pg.client as never, { schemas: ['fx_x9'], perf: true });
+    const x9 = report.perf!.findings.filter((f) => f.code === 'X9');
+
+    expect(tablesFor(x9, 'X9')).toEqual([
+      // Correlated subquery: runs per outer row, so the call inside it does too.
+      'fx_x9.bare',
+      'fx_x9.correlated',
+      // current_setting() is STABLE — dropping the wrapper doesn't fix anything.
+      'fx_x9.raw_guc',
+      'fx_x9.writes'
+    ]);
+
+    const bare = x9.find((f) => f.table === 'bare');
+    expect(bare?.severity).toBe('medium');
+    expect(bare?.dimension).toBe('perf');
+    expect(bare?.context).toMatchObject({ function: 'fx_x9.current_tenant', clause: 'USING' });
+    expect(bare?.hint).toContain('(SELECT fx_x9.current_tenant())');
+
+    expect(x9.find((f) => f.table === 'writes')?.context).toMatchObject({ clause: 'WITH CHECK' });
+  });
+
+  it('ignores row-dependent, volatile and immutable calls', async () => {
+    const report = await audit(pg.client as never, { schemas: ['fx_x9'], perf: true });
+    const tables = tablesFor(report.perf!.findings, 'X9');
+    // Argument references a column — nothing to hoist.
+    expect(tables).not.toContain('fx_x9.row_dependent');
+    // VOLATILE is per-row by definition; IMMUTABLE is folded at plan time.
+    expect(tables).not.toContain('fx_x9.other_volatility');
+    // Already an InitPlan.
+    expect(tables).not.toContain('fx_x9.hoisted');
+  });
+
+  it('stays off when perf is disabled', async () => {
+    const report = await audit(pg.client as never, { schemas: ['fx_x9'] });
+    expect(report.findings.filter((f) => f.code === 'X9')).toHaveLength(0);
+  });
+});
+
 describe('P1/P1b re-homed to the perf dimension', () => {
   it('keeps P1 out of the security score', async () => {
     await applyFixture('p1-volatile-func.sql');