feat(safegres): source-level convention linter (C1–C4) in the constructive preset

packages/safegres/README.md

@@ -104,8 +104,8 @@ trust boundaries on the way.
 
 ## What it checks
 
-30 rules across two dimensions. The prefix letter is a family, **not** the dimension: `P1`/`P1b`
-are performance, `P5` is security.
+34 rules across two dimensions, plus a source-level convention linter. The prefix letter is a
+family, **not** the dimension: `P1`/`P1b` are performance, `P5` is security.
 
 ### Security (19 rules)
 
@@ -144,6 +144,42 @@ reaches more than intended. `fail-closed` findings are *denied by Postgres at ru
 availability and hygiene concern, not a leak. They contribute **zero** to the score by default
 (`scoring.failClosedWeight`). safegres does not cry wolf about a grant the database already refuses.
 
+### Convention (source-level lint, 4 rules — `safegres:constructive`)
+
+House-style rules that read function **definitions** (`pg_get_functiondef`), not the catalog. They
+are pure `source → findings` — no `pg` dependency in the lint module — and are **off outside the
+`safegres:constructive` preset**, which enables them. A function on a non-exposed schema costs
+nothing, like every other rule.
+
+| Code | Severity | Direction | Check |
+| --- | --- | --- | --- |
+| C1 | high | fail-open | Function **sets `search_path`** (house rule: never set it — fully-qualify instead) |
+| C2 | medium | neutral | Function uses a **`#variable_conflict`** directive |
+| C3 | low | neutral | Function has an **unqualified relation reference** (relies on `search_path`) |
+| C4 | high | fail-open | Function uses **dynamic SQL** (`EXECUTE` / `EXECUTE … USING` / `FOR … IN EXECUTE`) |
+
+C3 ships at `low` (adoption severity — ratchet to error once the tree is clean). C4 cannot be
+statically proven read-only, so every dynamic-SQL site is flagged and must be **waived inline with a
+categorized reason** (`lookup-only`, `codegen`); a reasonless waiver does not suppress it.
+
+**Inline suppressions** (ESLint/Prettier style), written as SQL comments inside the body:
+
+```sql
+-- safegres-disable-next-line no-dynamic-sql -- lookup-only: building an IN-list of integers
+EXECUTE format('SELECT ... WHERE id = ANY(%L)', ids);
+
+EXECUTE 'REFRESH MATERIALIZED VIEW app.mv';  -- safegres-disable-line no-dynamic-sql -- codegen: fixed DDL
+
+-- safegres-disable no-dynamic-sql -- lookup-only: this whole block probes the catalog
+...
+-- safegres-enable no-dynamic-sql
+
+-- safegres-disable-file no-set-search-path -- vendored extension shim
+```
+
+A directive with no rule id applies to every convention rule. Waived findings are **not dropped** —
+they surface as `acknowledged` (accepted-risk) findings carrying their reason, off the score.
+
 ### Performance (11 rules, `--perf`)
 
 | Code | Severity | Check |

packages/safegres/__tests__/lint-audit.test.ts

@@ -0,0 +1,139 @@
+import { getConnections, PgTestClient } from 'pgsql-test';
+
+import { audit } from '../src/commands/audit';
+import { constructive, recommended } from '../src/config/presets';
+import type { Finding } from '../src/types';
+
+jest.setTimeout(120000);
+
+let pg: PgTestClient;
+let teardown: () => Promise<void>;
+
+beforeAll(async () => {
+  ({ pg, teardown } = await getConnections());
+  await pg.any('CREATE SCHEMA fx_lint');
+  await pg.any('CREATE TABLE fx_lint.widgets (id int primary key)');
+
+  // C1: pins search_path — a house-rule violation. Body kept trivial so only
+  // C1 fires on it.
+  await pg.any(`
+    CREATE FUNCTION fx_lint.pinned() RETURNS int
+    LANGUAGE sql
+    SET search_path = public
+    AS $$ SELECT 1 $$;
+  `);
+
+  // C3: an unqualified relation reference (relies on search_path).
+  await pg.any(`
+    CREATE FUNCTION fx_lint.unqualified() RETURNS bigint
+    LANGUAGE plpgsql
+    AS $$
+    DECLARE n bigint;
+    BEGIN
+      SELECT count(*) INTO n FROM widgets;
+      RETURN n;
+    END;
+    $$;
+  `);
+
+  // C4 active: dynamic SQL with no waiver.
+  await pg.any(`
+    CREATE FUNCTION fx_lint.dyn_unwaived() RETURNS void
+    LANGUAGE plpgsql
+    AS $$
+    BEGIN
+      EXECUTE 'SELECT 1';
+    END;
+    $$;
+  `);
+
+  // C4 waived: dynamic SQL with a reasoned inline waiver — preserved as an
+  // acknowledged (accepted-risk) finding rather than dropped.
+  await pg.any(`
+    CREATE FUNCTION fx_lint.dyn_waived() RETURNS void
+    LANGUAGE plpgsql
+    AS $$
+    BEGIN
+      -- safegres-disable-next-line no-dynamic-sql -- lookup-only: static probe
+      EXECUTE 'SELECT 1';
+    END;
+    $$;
+  `);
+
+  // Clean: fully-qualified, no search_path, no dynamic SQL.
+  await pg.any(`
+    CREATE FUNCTION fx_lint.clean() RETURNS bigint
+    LANGUAGE plpgsql
+    AS $$
+    DECLARE n bigint;
+    BEGIN
+      SELECT count(*) INTO n FROM fx_lint.widgets;
+      RETURN n;
+    END;
+    $$;
+  `);
+});
+
+afterAll(async () => {
+  if (teardown) await teardown();
+});
+
+function forFn(findings: Finding[], code: string, fn: string): Finding | undefined {
+  return findings.find(
+    (f) => f.code === code && (f.context as { function?: string }).function?.startsWith(`fx_lint.${fn}(`)
+  );
+}
+
+async function lintAudit(): Promise<Finding[]> {
+  const report = await audit(pg.client as never, {
+    schemas: ['fx_lint'],
+    config: { rules: { C1: 'high', C2: 'medium', C3: 'low', C4: 'high' } }
+  });
+  return report.findings.filter((f) => f.schema === 'fx_lint');
+}
+
+describe('audit: convention linter wiring (C*)', () => {
+  it('flags a function that sets search_path (C1)', async () => {
+    const c1 = forFn(await lintAudit(), 'C1', 'pinned');
+    expect(c1).toBeDefined();
+    expect(c1?.category).toBe('convention');
+    expect(c1?.severity).toBe('high');
+    expect(c1?.acknowledged).toBeFalsy();
+  });
+
+  it('flags an unqualified relation reference (C3)', async () => {
+    const c3 = forFn(await lintAudit(), 'C3', 'unqualified');
+    expect(c3).toBeDefined();
+    expect(c3?.severity).toBe('low');
+  });
+
+  it('flags dynamic SQL as active when it is not waived (C4)', async () => {
+    const c4 = forFn(await lintAudit(), 'C4', 'dyn_unwaived');
+    expect(c4).toBeDefined();
+    expect(c4?.acknowledged).toBeFalsy();
+  });
+
+  it('keeps a reasoned dynamic-SQL waiver as an acknowledged finding (C4)', async () => {
+    const c4 = forFn(await lintAudit(), 'C4', 'dyn_waived');
+    expect(c4).toBeDefined();
+    expect(c4?.acknowledged).toBe(true);
+    const ctx = c4?.context as { suppressed?: boolean; reason?: string };
+    expect(ctx.suppressed).toBe(true);
+    expect(ctx.reason).toContain('lookup-only');
+  });
+
+  it('does not flag a clean, fully-qualified function', async () => {
+    const findings = await lintAudit();
+    const onClean = findings.filter(
+      (f) => (f.context as { function?: string }).function?.startsWith('fx_lint.clean(')
+    );
+    expect(onClean).toEqual([]);
+  });
+
+  it('runs the linter under the constructive preset but not under recommended', () => {
+    // recommended carries `C*` off; constructive turns them on.
+    expect(recommended.rules!['C*']).toBe('off');
+    expect(constructive.rules!.C1).toBe('high');
+    expect(constructive.rules!.C4).toBe('high');
+  });
+});

packages/safegres/__tests__/lint.test.ts

@@ -0,0 +1,184 @@
+import { LINT_RULES,lintDefinition } from '../src/lint';
+
+describe('lint: rule registry', () => {
+  it('exposes the four convention rules with stable codes', () => {
+    expect(LINT_RULES.map((r) => r.code).sort()).toEqual(['C1', 'C2', 'C3', 'C4']);
+    expect(LINT_RULES.map((r) => r.id).sort()).toEqual([
+      'no-dynamic-sql',
+      'no-set-search-path',
+      'no-variable-conflict',
+      'require-qualified-refs'
+    ]);
+  });
+
+  it('only no-dynamic-sql requires a suppression reason', () => {
+    const reasonRequired = LINT_RULES.filter((r) => r.reasonRequired).map((r) => r.id);
+    expect(reasonRequired).toEqual(['no-dynamic-sql']);
+  });
+});
+
+describe('lint: no-set-search-path (C1)', () => {
+  it('flags a SET search_path clause', async () => {
+    const def = `CREATE FUNCTION app.f() RETURNS void LANGUAGE plpgsql
+SET search_path = pg_catalog, public
+AS $$
+BEGIN
+  PERFORM app.g();
+END;
+$$;`;
+    const { problems } = await lintDefinition(def, 'plpgsql', 'app.f()');
+    const c1 = problems.filter((p) => p.ruleId === 'no-set-search-path');
+    expect(c1).toHaveLength(1);
+    expect(c1[0].line).toBe(2);
+  });
+
+  it('flags set_config(\'search_path\', ...) in the body', async () => {
+    const def = `CREATE FUNCTION app.f() RETURNS void LANGUAGE plpgsql
+AS $$
+BEGIN
+  PERFORM set_config('search_path', 'public', true);
+END;
+$$;`;
+    const { problems } = await lintDefinition(def, 'plpgsql');
+    expect(problems.some((p) => p.ruleId === 'no-set-search-path')).toBe(true);
+  });
+
+  it('does not flag a function that never touches search_path', async () => {
+    const def = `CREATE FUNCTION app.f() RETURNS void LANGUAGE plpgsql
+AS $$
+BEGIN
+  PERFORM app.g();
+END;
+$$;`;
+    const { problems } = await lintDefinition(def, 'plpgsql');
+    expect(problems.some((p) => p.ruleId === 'no-set-search-path')).toBe(false);
+  });
+});
+
+describe('lint: no-variable-conflict (C2)', () => {
+  it('flags a #variable_conflict directive', async () => {
+    const def = `CREATE FUNCTION app.f() RETURNS void LANGUAGE plpgsql
+AS $$
+#variable_conflict use_column
+DECLARE x int;
+BEGIN
+  x := 1;
+END;
+$$;`;
+    const { problems } = await lintDefinition(def, 'plpgsql');
+    const c2 = problems.filter((p) => p.ruleId === 'no-variable-conflict');
+    expect(c2).toHaveLength(1);
+    expect(c2[0].line).toBe(3);
+    expect(c2[0].message).toContain('use_column');
+  });
+});
+
+describe('lint: no-dynamic-sql (C4)', () => {
+  it('flags EXECUTE and requires a reason to waive', async () => {
+    const def = `CREATE FUNCTION app.f(tbl text) RETURNS void LANGUAGE plpgsql
+AS $$
+BEGIN
+  EXECUTE format('SELECT 1 FROM %I', tbl);
+END;
+$$;`;
+    const { problems, suppressed } = await lintDefinition(def, 'plpgsql');
+    expect(problems.some((p) => p.ruleId === 'no-dynamic-sql')).toBe(true);
+    expect(suppressed).toHaveLength(0);
+  });
+
+  it('a reasonless waiver does not suppress (reason required)', async () => {
+    const def = `CREATE FUNCTION app.f(tbl text) RETURNS void LANGUAGE plpgsql
+AS $$
+BEGIN
+  -- safegres-disable-next-line no-dynamic-sql
+  EXECUTE format('SELECT 1 FROM %I', tbl);
+END;
+$$;`;
+    const { problems, suppressed } = await lintDefinition(def, 'plpgsql');
+    const c4 = problems.filter((p) => p.ruleId === 'no-dynamic-sql');
+    expect(c4).toHaveLength(1);
+    expect(c4[0].message).toContain('suppression ignored');
+    expect(suppressed).toHaveLength(0);
+  });
+
+  it('a reasoned waiver suppresses and is retained as accepted risk', async () => {
+    const def = `CREATE FUNCTION app.f(tbl text) RETURNS void LANGUAGE plpgsql
+AS $$
+BEGIN
+  -- safegres-disable-next-line no-dynamic-sql -- lookup-only: building IN-list from integers
+  EXECUTE format('SELECT 1 FROM %I', tbl);
+END;
+$$;`;
+    const { problems, suppressed } = await lintDefinition(def, 'plpgsql');
+    expect(problems.some((p) => p.ruleId === 'no-dynamic-sql')).toBe(false);
+    expect(suppressed).toHaveLength(1);
+    expect(suppressed[0].reason).toBe('lookup-only: building IN-list from integers');
+    expect(suppressed[0].scope).toBe('next-line');
+  });
+});
+
+describe('lint: require-qualified-refs (C3)', () => {
+  it('flags an unqualified relation reference', async () => {
+    const def = `CREATE FUNCTION app.f() RETURNS int LANGUAGE sql
+AS $$
+  SELECT count(*) FROM users
+$$;`;
+    const { problems } = await lintDefinition(def, 'sql');
+    const c3 = problems.filter((p) => p.ruleId === 'require-qualified-refs');
+    expect(c3).toHaveLength(1);
+    expect(c3[0].message).toContain('users');
+  });
+
+  it('does not flag a schema-qualified reference', async () => {
+    const def = `CREATE FUNCTION app.f() RETURNS int LANGUAGE sql
+AS $$
+  SELECT count(*) FROM app_public.users
+$$;`;
+    const { problems } = await lintDefinition(def, 'sql');
+    expect(problems.some((p) => p.ruleId === 'require-qualified-refs')).toBe(false);
+  });
+
+  it('does not flag a CTE name used as a relation', async () => {
+    const def = `CREATE FUNCTION app.f() RETURNS int LANGUAGE sql
+AS $$
+  WITH recent AS (SELECT id FROM app_public.users)
+  SELECT count(*) FROM recent
+$$;`;
+    const { problems } = await lintDefinition(def, 'sql');
+    expect(problems.some((p) => p.ruleId === 'require-qualified-refs')).toBe(false);
+  });
+});
+
+describe('lint: suppression scopes', () => {
+  it('disable-file silences a rule for the whole definition', async () => {
+    const def = `CREATE FUNCTION app.f() RETURNS int LANGUAGE sql
+AS $$
+  -- safegres-disable-file require-qualified-refs -- legacy, tracked in #123
+  SELECT count(*) FROM users
+$$;`;
+    const { problems, suppressed } = await lintDefinition(def, 'sql');
+    expect(problems.some((p) => p.ruleId === 'require-qualified-refs')).toBe(false);
+    expect(suppressed.some((p) => p.ruleId === 'require-qualified-refs')).toBe(true);
+  });
+
+  it('disable/enable bracket a range', async () => {
+    const def = `CREATE FUNCTION app.f() RETURNS void LANGUAGE plpgsql
+AS $$
+BEGIN
+  -- safegres-disable no-dynamic-sql -- codegen block
+  EXECUTE 'SELECT 1';
+  EXECUTE 'SELECT 2';
+  -- safegres-enable no-dynamic-sql
+END;
+$$;`;
+    const { problems, suppressed } = await lintDefinition(def, 'plpgsql');
+    expect(problems.some((p) => p.ruleId === 'no-dynamic-sql')).toBe(false);
+    expect(suppressed.filter((p) => p.ruleId === 'no-dynamic-sql')).toHaveLength(2);
+  });
+
+  it('an unparseable definition yields no findings', async () => {
+    const { problems, suppressed } = await lintDefinition('this is not sql', 'plpgsql');
+    expect(problems).toHaveLength(0);
+    expect(suppressed).toHaveLength(0);
+  });
+});

packages/safegres/__tests__/presets.test.ts

@@ -56,6 +56,11 @@ describe('built-in presets', () => {
     for (const [name, config] of Object.entries(PRESETS)) {
       if (name === 'safegres:minimal') continue;
       for (const [rule, setting] of Object.entries(settings(config))) {
+        // The `C*` convention linter is Constructive house style, not a
+        // universal fact, so the big-tent presets carry it off and only
+        // `constructive` turns it on — a deliberate opt-in, not a hidden
+        // finding.
+        if (rule.startsWith('C')) continue;
         const severity = Array.isArray(setting) ? setting[0] : setting;
         expect(`${name}/${rule}=${severity as string}`).not.toContain('=off');
       }