From 1d5406feccd2098650ce32ccbfc96e25dc40aa5f Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Sun, 2 Aug 2026 19:48:36 +0000 Subject: [PATCH] feat(safegres): consume @pgsql/lint instead of the bundled linter copy Depend on @pgsql/lint@^18.1.0 and turn src/lint into a thin re-export; delete the duplicated engine/rules/suppressions/parse-unit now that the standalone package is published. safegres keeps its catalog introspection, audit orchestration, config/preset, scoring, and finding mapping. --- packages/safegres/package.json | 1 + packages/safegres/src/lint/engine.ts | 60 ------ packages/safegres/src/lint/index.ts | 26 +-- packages/safegres/src/lint/parse-unit.ts | 183 ------------------ packages/safegres/src/lint/rules/index.ts | 18 -- .../safegres/src/lint/rules/no-dynamic-sql.ts | 29 --- .../src/lint/rules/no-set-search-path.ts | 67 ------- .../src/lint/rules/no-variable-conflict.ts | 38 ---- .../src/lint/rules/require-qualified-refs.ts | 67 ------- packages/safegres/src/lint/suppressions.ts | 162 ---------------- packages/safegres/src/lint/types.ts | 94 --------- packages/safegres/src/lint/util.ts | 7 - pnpm-lock.yaml | 20 ++ 13 files changed, 36 insertions(+), 736 deletions(-) delete mode 100644 packages/safegres/src/lint/engine.ts delete mode 100644 packages/safegres/src/lint/parse-unit.ts delete mode 100644 packages/safegres/src/lint/rules/index.ts delete mode 100644 packages/safegres/src/lint/rules/no-dynamic-sql.ts delete mode 100644 packages/safegres/src/lint/rules/no-set-search-path.ts delete mode 100644 packages/safegres/src/lint/rules/no-variable-conflict.ts delete mode 100644 packages/safegres/src/lint/rules/require-qualified-refs.ts delete mode 100644 packages/safegres/src/lint/suppressions.ts delete mode 100644 packages/safegres/src/lint/types.ts delete mode 100644 packages/safegres/src/lint/util.ts diff --git a/packages/safegres/package.json b/packages/safegres/package.json index d53015cb9..3a153e0c1 100644 --- a/packages/safegres/package.json +++ b/packages/safegres/package.json @@ -59,6 +59,7 @@ "dependencies": { "@inquirerer/utils": "^3.3.9", "@pgpmjs/logger": "workspace:^", + "@pgsql/lint": "^18.1.0", "@pgsql/traverse": "^18.7.1", "@pgsql/types": "^18.0.0", "confstash": "^0.1.0", diff --git a/packages/safegres/src/lint/engine.ts b/packages/safegres/src/lint/engine.ts deleted file mode 100644 index 35cf54948..000000000 --- a/packages/safegres/src/lint/engine.ts +++ /dev/null @@ -1,60 +0,0 @@ -/** - * The lint engine: parse a definition, run the rules, then apply - * suppressions. Pure `source → result`, with no `pg` dependency, so it can be - * unit-tested on string literals and lifted into a standalone package later. - */ - -import { parseUnit } from './parse-unit'; -import { LINT_RULES, LINT_RULES_BY_ID } from './rules'; -import { Suppressions } from './suppressions'; -import type { LintProblem, LintResult, LintRule, SuppressedProblem } from './types'; - -export interface LintOptions { - /** Restrict to these rule ids; omit to run all. */ - rules?: string[]; -} - -/** Lint a single function definition. */ -export async function lintDefinition( - text: string, - language: string, - name?: string, - options: LintOptions = {} -): Promise { - const active: LintProblem[] = []; - const suppressed: SuppressedProblem[] = []; - - const selected: LintRule[] = options.rules - ? options.rules.map((id) => LINT_RULES_BY_ID.get(id)).filter((r): r is LintRule => Boolean(r)) - : LINT_RULES; - if (selected.length === 0) return { problems: active, suppressed }; - - const unit = await parseUnit(text, language, name); - // An unparseable definition produces no lint findings — dynamic/opaque bodies - // are the call-graph's concern (CG5), not the linter's. - if (unit.parseError) return { problems: active, suppressed }; - - const suppressions = new Suppressions(unit.lines); - - for (const rule of selected) { - for (const problem of rule.run(unit)) { - const res = suppressions.resolve(problem.ruleId, problem.line, rule.reasonRequired); - if (res.suppressed) { - suppressed.push({ ...problem, reason: res.reason ?? null, scope: res.scope }); - continue; - } - if (res.invalidMissingReason) { - active.push({ - ...problem, - message: `${problem.message} (suppression ignored: a reason is required)`, - context: { ...problem.context, invalidSuppression: 'missing-reason' } - }); - continue; - } - active.push(problem); - } - } - - active.sort((a, b) => a.line - b.line || a.ruleId.localeCompare(b.ruleId)); - return { problems: active, suppressed }; -} diff --git a/packages/safegres/src/lint/index.ts b/packages/safegres/src/lint/index.ts index a14d7a0af..3cfb67983 100644 --- a/packages/safegres/src/lint/index.ts +++ b/packages/safegres/src/lint/index.ts @@ -1,20 +1,16 @@ /** * Source-level SQL/PL/pgSQL convention linter. * - * Distinct from safegres's catalog checks: it reasons about the *text* of a - * function definition (fully-qualified references, dynamic SQL, forbidden - * directives) rather than live-database facts, and carries no `pg` dependency. - * safegres is its first consumer; the seam is drawn so it can become a - * standalone `@pgsql/lint` package unchanged. + * This module used to carry safegres's own copy of the linter. It now lives in + * the standalone `@pgsql/lint` package (source text in → findings out, no `pg` + * dependency); safegres consumes it as "the catalog adapter", feeding it the + * `pg_get_functiondef` text it already reads. This file is a thin re-export so + * the rest of safegres keeps importing from `../lint` unchanged. */ -export type { LintOptions } from './engine'; -export { lintDefinition } from './engine'; -export { parseUnit } from './parse-unit'; -export { LINT_RULES, LINT_RULES_BY_CODE, LINT_RULES_BY_ID } from './rules'; -export { Suppressions } from './suppressions'; export type { DynamicSqlSite, + LintOptions, LintProblem, LintResult, LintRule, @@ -23,4 +19,12 @@ export type { SqlFragment, SuppressedProblem, SuppressionScope -} from './types'; +} from '@pgsql/lint'; +export { + LINT_RULES, + LINT_RULES_BY_CODE, + LINT_RULES_BY_ID, + lintDefinition, + parseUnit, + Suppressions +} from '@pgsql/lint'; diff --git a/packages/safegres/src/lint/parse-unit.ts b/packages/safegres/src/lint/parse-unit.ts deleted file mode 100644 index a9f80186c..000000000 --- a/packages/safegres/src/lint/parse-unit.ts +++ /dev/null @@ -1,183 +0,0 @@ -/** - * Turn a `CREATE FUNCTION …` definition into a {@link LintUnit}: the parsed - * SQL statement, the embedded body fragments, and the machinery to map any - * AST location back to an absolute line in the original text. - * - * Line mapping is the whole trick. PL/pgSQL statement line numbers are - * relative to the *body* (`prosrc`), and embedded SQL expressions are parsed - * in isolation, so both have to be re-anchored to the definition text before a - * finding — or a suppression comment — can be matched to them. - */ - -import { parsePlPgSQL } from 'libpg-query'; -import { parse } from 'pgsql-parser'; - -import { findAll } from '../ast/walk'; -import type { DynamicSqlSite, LintUnit, SqlFragment } from './types'; - -/** Count newlines in `s[0..offset)` — i.e. how many lines precede `offset`. */ -function newlinesBefore(s: string, offset: number): number { - let n = 0; - const end = Math.min(offset, s.length); - for (let i = 0; i < end; i++) if (s.charCodeAt(i) === 10) n++; - return n; -} - -/** Absolute 1-based line of a char offset within `text`. */ -function lineOf(text: string, offset: number): number { - return newlinesBefore(text, offset) + 1; -} - -/** The body string of a `CreateFunctionStmt` (`AS $$ … $$`), or null. */ -function functionBody(createFnStmt: Record): string | null { - const options = createFnStmt.options; - if (!Array.isArray(options)) return null; - for (const opt of options) { - const de = (opt as Record).DefElem as Record | undefined; - if (!de || de.defname !== 'as') continue; - const arg = de.arg as Record | undefined; - const list = arg?.List as Record | undefined; - const items = list?.items; - if (!Array.isArray(items) || items.length === 0) return null; - // A two-item AS (`obj_file`, `link_symbol`) is a C function — no SQL body. - if (items.length > 1) return null; - const str = (items[0] as Record).String as Record | undefined; - const sval = str?.sval; - return typeof sval === 'string' ? sval : null; - } - return null; -} - -/** - * Walk the PL/pgSQL JSON tree, collecting (a) every embedded SQL expression - * with the line number of its enclosing statement and (b) every dynamic-SQL - * site. Line numbers here are body-relative; the caller re-anchors them. - */ -function collectPlpgsql( - node: unknown, - currentLine: number, - exprs: Array<{ query: string; parseMode: number; line: number }>, - dynamic: Array<{ line: number; form: string }> -): void { - if (Array.isArray(node)) { - for (const item of node) collectPlpgsql(item, currentLine, exprs, dynamic); - return; - } - if (!node || typeof node !== 'object') return; - const rec = node as Record; - - // A statement node carries its own line; descendants inherit it until the - // next statement re-sets it. - let line = currentLine; - for (const [key, value] of Object.entries(rec)) { - if (key.startsWith('PLpgSQL_stmt_')) { - const stmt = value as Record; - if (typeof stmt.lineno === 'number') line = stmt.lineno; - if (key === 'PLpgSQL_stmt_dynexecute') { - dynamic.push({ line, form: 'EXECUTE' }); - } else if (key === 'PLpgSQL_stmt_dynfors') { - dynamic.push({ line, form: 'FOR … IN EXECUTE' }); - } - } - } - - const expr = rec.PLpgSQL_expr as Record | undefined; - if (expr && typeof expr.query === 'string') { - exprs.push({ - query: expr.query, - parseMode: typeof expr.parseMode === 'number' ? expr.parseMode : 2, - line - }); - } - - for (const value of Object.values(rec)) collectPlpgsql(value, line, exprs, dynamic); -} - -/** Reconstruct a parseable SQL string from a PL/pgSQL embedded expression. */ -function fragmentSql(query: string, parseMode: number): string { - // parseMode 0 = full statement; 3 = assignment (strip the anchored target so - // the RHS parses); anything else is a bare expression. - if (parseMode === 0) return query; - let q = query; - if (parseMode === 3) { - q = q.replace(/^\s*[a-zA-Z_"][\w$".]*(\[[^\]]*\])*\s*:?=\s*/, ''); - } - return `SELECT ${q}`; -} - -/** - * Parse a function definition into a {@link LintUnit}. Never throws: an - * unparseable definition comes back with `parseError` set and no fragments, - * so rules that need the AST simply find nothing. - */ -export async function parseUnit( - text: string, - language: string, - name?: string -): Promise { - const lines = text.split('\n'); - const base: LintUnit = { text, lines, language, name, fragments: [], dynamicSql: [] }; - - let sqlAst: unknown; - try { - sqlAst = await parse(text); - } catch (err) { - return { ...base, parseError: `definition failed to parse: ${(err as Error).message}` }; - } - - const createFnStmt = findAll(sqlAst, 'CreateFunctionStmt')[0]; - if (!createFnStmt) return { ...base, parseError: 'not a CREATE FUNCTION statement' }; - - const body = functionBody(createFnStmt); - const bodyOffset = body !== null ? text.indexOf(body) : -1; - const bodyStartLine = bodyOffset >= 0 ? lineOf(text, bodyOffset) : undefined; - - const fragments: SqlFragment[] = []; - const dynamicSql: DynamicSqlSite[] = []; - - const lang = language.toLowerCase(); - - if (lang === 'sql' && body !== null && bodyStartLine !== undefined) { - // A SQL-language body is itself SQL: parse it whole. Locations are - // relative to the body, so re-anchor them onto the definition. - try { - const ast = await parse(body); - fragments.push({ - ast, - lineForOffset: (offset) => bodyStartLine + newlinesBefore(body, offset) - }); - } catch { - // Body may contain positional parameters etc. that don't parse alone — - // leave it as no fragment rather than erroring the whole unit. - } - } else if (lang === 'plpgsql') { - let plpgsql: unknown; - try { - plpgsql = await parsePlPgSQL(text); - } catch (err) { - return { ...base, createFnStmt, bodyStartLine, parseError: `PL/pgSQL body failed to parse: ${(err as Error).message}` }; - } - const exprs: Array<{ query: string; parseMode: number; line: number }> = []; - const dyn: Array<{ line: number; form: string }> = []; - collectPlpgsql(plpgsql, 0, exprs, dyn); - - const anchor = (bodyLine: number): number => - bodyStartLine !== undefined && bodyLine > 0 ? bodyStartLine + (bodyLine - 1) : (bodyStartLine ?? 1); - - for (const d of dyn) dynamicSql.push({ line: anchor(d.line), form: d.form }); - - for (const e of exprs) { - const sql = fragmentSql(e.query, e.parseMode); - let ast: unknown; - try { - ast = await parse(sql); - } catch { - continue; // opaque fragment — the call-graph's concern, not the linter's - } - const absLine = anchor(e.line); - fragments.push({ ast, lineForOffset: () => absLine }); - } - } - - return { ...base, createFnStmt, bodyStartLine, fragments, dynamicSql }; -} diff --git a/packages/safegres/src/lint/rules/index.ts b/packages/safegres/src/lint/rules/index.ts deleted file mode 100644 index 08f3bb9a2..000000000 --- a/packages/safegres/src/lint/rules/index.ts +++ /dev/null @@ -1,18 +0,0 @@ -import type { LintRule } from '../types'; -import { noDynamicSql } from './no-dynamic-sql'; -import { noSetSearchPath } from './no-set-search-path'; -import { noVariableConflict } from './no-variable-conflict'; -import { requireQualifiedRefs } from './require-qualified-refs'; - -/** The lint rules, in report order. */ -export const LINT_RULES: LintRule[] = [ - noSetSearchPath, - requireQualifiedRefs, - noVariableConflict, - noDynamicSql -]; - -export const LINT_RULES_BY_ID = new Map(LINT_RULES.map((r) => [r.id, r])); -export const LINT_RULES_BY_CODE = new Map(LINT_RULES.map((r) => [r.code, r])); - -export { noDynamicSql, noSetSearchPath, noVariableConflict, requireQualifiedRefs }; diff --git a/packages/safegres/src/lint/rules/no-dynamic-sql.ts b/packages/safegres/src/lint/rules/no-dynamic-sql.ts deleted file mode 100644 index 11356f772..000000000 --- a/packages/safegres/src/lint/rules/no-dynamic-sql.ts +++ /dev/null @@ -1,29 +0,0 @@ -/** - * `no-dynamic-sql` (C4): a function must not use dynamic SQL. - * - * Dynamic SQL (`EXECUTE`, `EXECUTE … USING`, `FOR … IN EXECUTE`) is permitted - * only for lookup-only or code-generation work, and never for writes — but the - * string handed to `EXECUTE` is opaque to the parser, so we cannot statically - * tell read from write. The enforceable form is therefore: flag every site, - * and require a categorized waiver. This is the one rule whose suppression - * must carry a reason (see `reasonRequired`), so an approved use always names - * *why* (`lookup-only` / `codegen`). - */ - -import type { LintRule } from '../types'; - -export const noDynamicSql: LintRule = { - id: 'no-dynamic-sql', - code: 'C4', - title: 'Function must not use dynamic SQL', - reasonRequired: true, - run(unit) { - return unit.dynamicSql.map((site) => ({ - ruleId: 'no-dynamic-sql', - line: site.line, - message: `Function uses dynamic SQL (${site.form})`, - hint: 'Avoid dynamic SQL. If it is genuinely lookup-only or code-generation (never a write), waive it with a reason: `-- safegres-disable-next-line no-dynamic-sql -- lookup-only: `.', - context: { form: site.form } - })); - } -}; diff --git a/packages/safegres/src/lint/rules/no-set-search-path.ts b/packages/safegres/src/lint/rules/no-set-search-path.ts deleted file mode 100644 index 1006b55b7..000000000 --- a/packages/safegres/src/lint/rules/no-set-search-path.ts +++ /dev/null @@ -1,67 +0,0 @@ -/** - * `no-set-search-path` (C1): a function must never set `search_path`. - * - * House rule: rather than pin `search_path` (the usual CWE-426 mitigation for - * SECURITY DEFINER), we fully-qualify every reference and never touch the - * setting at all. This flags both forms: - * - the declarative `CREATE FUNCTION … SET search_path = …` clause (this is - * exactly what `pg_proc.proconfig` / `searchPathPinned` records), and - * - a runtime `set_config('search_path', …)` in the body. - */ - -import type { LintProblem, LintRule, LintUnit } from '../types'; -import { lineOfOffset } from '../util'; - -function optionSites(unit: LintUnit): LintProblem[] { - const out: LintProblem[] = []; - const options = unit.createFnStmt?.options; - if (!Array.isArray(options)) return out; - for (const opt of options) { - const de = (opt as Record).DefElem as Record | undefined; - if (!de || de.defname !== 'set') continue; - const vss = (de.arg as Record | undefined)?.VariableSetStmt as - | Record - | undefined; - if (!vss || vss.name !== 'search_path') continue; - const loc = typeof de.location === 'number' ? de.location : 0; - out.push({ - ruleId: 'no-set-search-path', - line: lineOfOffset(unit.text, loc), - message: 'Function sets search_path', - hint: 'Never set search_path. Fully-qualify every relation, function and type reference instead.', - context: { form: 'SET clause' } - }); - } - return out; -} - -function setConfigSites(unit: LintUnit): LintProblem[] { - const out: LintProblem[] = []; - const re = /\bset_config\s*\(\s*'search_path'/i; - unit.lines.forEach((text, i) => { - if (re.test(text)) { - out.push({ - ruleId: 'no-set-search-path', - line: i + 1, - message: 'Function sets search_path via set_config()', - hint: 'Never set search_path. Fully-qualify references instead of relying on it.', - context: { form: 'set_config()' } - }); - } - }); - return out; -} - -export const noSetSearchPath: LintRule = { - id: 'no-set-search-path', - code: 'C1', - title: 'Function must not set search_path', - reasonRequired: false, - run(unit) { - const byLine = new Map(); - for (const p of [...optionSites(unit), ...setConfigSites(unit)]) { - if (!byLine.has(p.line)) byLine.set(p.line, p); - } - return [...byLine.values()].sort((a, b) => a.line - b.line); - } -}; diff --git a/packages/safegres/src/lint/rules/no-variable-conflict.ts b/packages/safegres/src/lint/rules/no-variable-conflict.ts deleted file mode 100644 index 543ff0034..000000000 --- a/packages/safegres/src/lint/rules/no-variable-conflict.ts +++ /dev/null @@ -1,38 +0,0 @@ -/** - * `no-variable-conflict` (C2): a PL/pgSQL body must not use a - * `#variable_conflict` directive. - * - * The directive papers over an ambiguity between a column name and a PL/pgSQL - * variable; the house style is to remove the ambiguity (rename the variable, - * qualify the column) rather than declare a winner. The directive is a - * compiler pragma that must start a line at the top of the body, so a line - * scan is exact — it never appears inside an expression or string. - */ - -import type { LintRule } from '../types'; - -const RE = /^\s*#variable_conflict\b\s*(\S+)?/i; - -export const noVariableConflict: LintRule = { - id: 'no-variable-conflict', - code: 'C2', - title: 'Function must not use #variable_conflict', - reasonRequired: false, - run(unit) { - if (unit.language.toLowerCase() !== 'plpgsql') return []; - const out = []; - for (let i = 0; i < unit.lines.length; i++) { - const m = RE.exec(unit.lines[i]); - if (!m) continue; - const mode = m[1] ?? ''; - out.push({ - ruleId: 'no-variable-conflict', - line: i + 1, - message: `Function uses #variable_conflict${mode ? ` ${mode}` : ''}`, - hint: 'Remove the directive and disambiguate explicitly: rename the variable or qualify the column reference.', - context: { mode } - }); - } - return out; - } -}; diff --git a/packages/safegres/src/lint/rules/require-qualified-refs.ts b/packages/safegres/src/lint/rules/require-qualified-refs.ts deleted file mode 100644 index c815a5cb2..000000000 --- a/packages/safegres/src/lint/rules/require-qualified-refs.ts +++ /dev/null @@ -1,67 +0,0 @@ -/** - * `require-qualified-refs` (C3): every relation reference must be - * schema-qualified. - * - * Banning `SET search_path` (C1) only removes the footgun; it does not make - * name resolution safe on its own. This is the rule that actually enforces the - * discipline: an unqualified `FROM users` resolves against whatever - * search_path happens to be, so it must be `FROM app_public.users`. - * - * v1 covers relation references (`RangeVar`). Names introduced by a CTE in the - * same query are excluded — they are not schema objects. Unqualified *function* - * calls are deferred (they need a built-in allowlist to avoid flagging - * `now()`, `count()`, …). - */ - -import { findAll } from '../../ast/walk'; -import type { LintProblem, LintRule, SqlFragment } from '../types'; - -function cteNames(ast: unknown): Set { - const out = new Set(); - for (const cte of findAll(ast, 'CommonTableExpr')) { - if (typeof cte.ctename === 'string') out.add(cte.ctename); - } - return out; -} - -function fragmentProblems(fragment: SqlFragment): LintProblem[] { - const out: LintProblem[] = []; - const ctes = cteNames(fragment.ast); - const seen = new Set(); - for (const rv of findAll(fragment.ast, 'RangeVar')) { - const relname = typeof rv.relname === 'string' ? rv.relname : undefined; - if (!relname) continue; - if (typeof rv.schemaname === 'string' && rv.schemaname.length > 0) continue; - if (ctes.has(relname)) continue; - const loc = typeof rv.location === 'number' ? rv.location : -1; - const line = fragment.lineForOffset(loc >= 0 ? loc : 0); - const key = `${line}:${relname}`; - if (seen.has(key)) continue; - seen.add(key); - out.push({ - ruleId: 'require-qualified-refs', - line, - message: `Unqualified relation reference "${relname}"`, - hint: 'Schema-qualify the reference (e.g. `app_public.' + relname + '`). Unqualified names resolve against search_path.', - context: { relation: relname } - }); - } - return out; -} - -export const requireQualifiedRefs: LintRule = { - id: 'require-qualified-refs', - code: 'C3', - title: 'Relation references must be schema-qualified', - reasonRequired: false, - run(unit) { - const byKey = new Map(); - for (const fragment of unit.fragments) { - for (const p of fragmentProblems(fragment)) { - const key = `${p.line}:${(p.context as { relation: string }).relation}`; - if (!byKey.has(key)) byKey.set(key, p); - } - } - return [...byKey.values()].sort((a, b) => a.line - b.line); - } -}; diff --git a/packages/safegres/src/lint/suppressions.ts b/packages/safegres/src/lint/suppressions.ts deleted file mode 100644 index c199fc902..000000000 --- a/packages/safegres/src/lint/suppressions.ts +++ /dev/null @@ -1,162 +0,0 @@ -/** - * ESLint / Prettier-style suppression comments, embedded in the SQL body as - * `--` line comments. Because they live in the function source, a waiver - * authored in a migration survives `pg_get_functiondef` and is visible to the - * live-database audit — the comment is the single source of truth. - * - * Grammar (a `--` comment containing): - * - * safegres-disable-next-line […] [-- ] next physical line - * safegres-disable-line […] [-- ] this physical line - * safegres-disable […] [-- ] until a matching enable - * safegres-enable […] closes a disable range - * safegres-disable-file […] [-- ] the whole definition - * - * With no rule listed a directive applies to every rule. A reason follows a - * second `--` (ESLint style) or a `:`. Rules whose metadata requires a reason - * are *not* silenced by a reasonless directive — the finding stands, so a - * waiver is never silent. - */ - -import type { SuppressionScope } from './types'; - -interface LineDirective { - scope: 'next-line' | 'line'; - targetLine: number; - rules: Set | null; - reason: string | null; -} - -interface Interval { - rule: string | null; - start: number; - end: number; - reason: string | null; -} - -interface FileDirective { - rules: Set | null; - reason: string | null; -} - -export interface SuppressionMatch { - scope: SuppressionScope; - reason: string | null; -} - -export interface SuppressionResolution { - /** The directive silences the finding. */ - suppressed: boolean; - scope?: SuppressionScope; - reason?: string | null; - /** A directive matched but lacked a required reason, so it does not apply. */ - invalidMissingReason?: boolean; -} - -const DIRECTIVE_RE = - /safegres-(disable-next-line|disable-line|disable-file|disable|enable)\b[ \t]*([^\r\n]*)/i; - -function splitReason(rest: string): { ruleSpec: string; reason: string | null } { - const dashIdx = rest.indexOf('--'); - if (dashIdx >= 0) { - return { ruleSpec: rest.slice(0, dashIdx), reason: normalizeReason(rest.slice(dashIdx + 2)) }; - } - const colonIdx = rest.indexOf(':'); - if (colonIdx >= 0) { - return { ruleSpec: rest.slice(0, colonIdx), reason: normalizeReason(rest.slice(colonIdx + 1)) }; - } - return { ruleSpec: rest, reason: null }; -} - -function normalizeReason(s: string): string | null { - const t = s.trim().replace(/\*\/\s*$/, '').trim(); - return t.length > 0 ? t : null; -} - -function parseRules(ruleSpec: string): Set | null { - const parts = ruleSpec.split(/[\s,]+/).map((p) => p.trim()).filter((p) => p.length > 0); - return parts.length > 0 ? new Set(parts) : null; -} - -/** Parsed suppression state for one definition, queryable by (rule, line). */ -export class Suppressions { - private readonly lineDirectives: LineDirective[] = []; - private readonly intervals: Interval[] = []; - private readonly fileDirectives: FileDirective[] = []; - - constructor(lines: string[]) { - // Range bookkeeping: an open disable per rule (and one for "all"). - const open = new Map(); - - lines.forEach((text, i) => { - const line = i + 1; - const m = DIRECTIVE_RE.exec(text); - if (!m) return; - const kind = m[1].toLowerCase(); - const { ruleSpec, reason } = splitReason(m[2] ?? ''); - const rules = parseRules(ruleSpec); - - switch (kind) { - case 'disable-next-line': - this.lineDirectives.push({ scope: 'next-line', targetLine: line + 1, rules, reason }); - break; - case 'disable-line': - this.lineDirectives.push({ scope: 'line', targetLine: line, rules, reason }); - break; - case 'disable-file': - this.fileDirectives.push({ rules, reason }); - break; - case 'disable': { - const keys: Array = rules ? [...rules] : [null]; - for (const k of keys) if (!open.has(k)) open.set(k, { start: line, reason }); - break; - } - case 'enable': { - const keys: Array = rules ? [...rules] : [...open.keys()]; - for (const k of keys) { - const o = open.get(k); - if (o) { - this.intervals.push({ rule: k, start: o.start, end: line, reason: o.reason }); - open.delete(k); - } - } - break; - } - } - }); - - for (const [rule, o] of open) { - this.intervals.push({ rule, start: o.start, end: Number.POSITIVE_INFINITY, reason: o.reason }); - } - } - - private match(ruleId: string, line: number): SuppressionMatch | null { - for (const f of this.fileDirectives) { - if (f.rules === null || f.rules.has(ruleId)) return { scope: 'file', reason: f.reason }; - } - for (const d of this.lineDirectives) { - if (d.targetLine === line && (d.rules === null || d.rules.has(ruleId))) { - return { scope: d.scope, reason: d.reason }; - } - } - for (const iv of this.intervals) { - if ((iv.rule === null || iv.rule === ruleId) && line >= iv.start && line < iv.end) { - return { scope: 'range', reason: iv.reason }; - } - } - return null; - } - - /** - * Resolve whether a finding is suppressed. `reasonRequired` rules are only - * silenced by a directive that carries a reason. - */ - resolve(ruleId: string, line: number, reasonRequired: boolean): SuppressionResolution { - const m = this.match(ruleId, line); - if (!m) return { suppressed: false }; - if (reasonRequired && (m.reason === null || m.reason.length === 0)) { - return { suppressed: false, invalidMissingReason: true, scope: m.scope }; - } - return { suppressed: true, scope: m.scope, reason: m.reason }; - } -} diff --git a/packages/safegres/src/lint/types.ts b/packages/safegres/src/lint/types.ts deleted file mode 100644 index c1d71f046..000000000 --- a/packages/safegres/src/lint/types.ts +++ /dev/null @@ -1,94 +0,0 @@ -/** - * Source-level SQL/PL/pgSQL linter — types. - * - * This module is deliberately free of any `pg` / catalog dependency: it takes - * a function *definition* (the `CREATE FUNCTION …` text, as `pg_get_functiondef` - * returns it, or as authored in a migration) and returns findings. That keeps - * it mechanically liftable into a standalone `@pgsql/lint` package if a second - * consumer ever appears; safegres is just the first one. - */ - -/** A parsed function definition, in the coordinate space the linter reports in. */ -export interface LintUnit { - /** The full definition text — the line/column space every finding refers to. */ - text: string; - /** `text` split on `\n`, 1-based when indexed as `lines[line - 1]`. */ - lines: string[]; - /** `sql`, `plpgsql`, `c`, `internal`, … (lower-cased `pg_proc.prolang`). */ - language: string; - /** Display name for messages, e.g. `app.grant_role(text)`. */ - name?: string; - /** The `CreateFunctionStmt` AST node, when the text parsed as one. */ - createFnStmt?: Record; - /** Absolute line (1-based, within `text`) the function body's first char sits on. */ - bodyStartLine?: number; - /** Embedded SQL fragments (body statements / expressions) with a line mapper. */ - fragments: SqlFragment[]; - /** Dynamic-SQL statements found in a PL/pgSQL body, by absolute line. */ - dynamicSql: DynamicSqlSite[]; - /** True when the definition (or its body) could not be parsed. */ - parseError?: string; -} - -/** One embedded SQL statement/expression, with a char-offset → absolute-line mapper. */ -export interface SqlFragment { - /** Parsed SQL AST for this fragment. */ - ast: unknown; - /** Map a char offset within this fragment's source to an absolute line in `text`. */ - lineForOffset: (offset: number) => number; -} - -export interface DynamicSqlSite { - line: number; - /** `EXECUTE`, `EXECUTE … USING`, or `FOR … IN EXECUTE`. */ - form: string; -} - -/** A single lint finding, before suppressions are applied. */ -export interface LintProblem { - ruleId: string; - /** Absolute line (1-based) within the definition. */ - line: number; - message: string; - hint?: string; - context?: Record; -} - -/** A problem that a suppression comment silenced — reported, never dropped. */ -export interface SuppressedProblem extends LintProblem { - /** The reason text from the directive, or null when none was given. */ - reason: string | null; - scope: SuppressionScope; -} - -export type SuppressionScope = 'next-line' | 'line' | 'range' | 'file'; - -/** The result of linting one definition. */ -export interface LintResult { - /** Active findings — not suppressed. */ - problems: LintProblem[]; - /** Suppressed findings, kept for the "accepted risk" report bucket. */ - suppressed: SuppressedProblem[]; -} - -/** Static metadata for a lint rule. */ -export interface LintRuleMeta { - /** ESLint-style stable id, e.g. `no-dynamic-sql`. */ - id: string; - /** safegres registry code this rule maps to, e.g. `C4`. */ - code: string; - title: string; - /** - * Whether a suppression of this rule must carry a reason. When true, a bare - * `safegres-disable*` directive does not suppress — the finding stands — so - * a waiver is never silent. Only `no-dynamic-sql` requires it: it is the one - * rule we expect to be waived (lookup-only / codegen), and the waiver's whole - * value is the documented reason. - */ - reasonRequired: boolean; -} - -/** A lint rule: pure `unit → problems`. */ -export interface LintRule extends LintRuleMeta { - run: (unit: LintUnit) => LintProblem[]; -} diff --git a/packages/safegres/src/lint/util.ts b/packages/safegres/src/lint/util.ts deleted file mode 100644 index ed5ef70b2..000000000 --- a/packages/safegres/src/lint/util.ts +++ /dev/null @@ -1,7 +0,0 @@ -/** Absolute 1-based line of a char offset within `text`. */ -export function lineOfOffset(text: string, offset: number): number { - let n = 0; - const end = Math.min(Math.max(offset, 0), text.length); - for (let i = 0; i < end; i++) if (text.charCodeAt(i) === 10) n++; - return n + 1; -} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fe1e84b80..b3012704e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2648,6 +2648,9 @@ importers: '@pgpmjs/logger': specifier: workspace:^ version: link:../../pgpm/logger/dist + '@pgsql/lint': + specifier: ^18.1.0 + version: 18.1.0 '@pgsql/traverse': specifier: ^18.7.1 version: 18.7.1 @@ -6516,6 +6519,13 @@ packages: integrity: sha512-XO7cAxhnTZl0Yggq6jOgjiOHhbgcO4NqFqwSmQpjK3b6TEE6Uj/jfSk6wzYyemh3+I0sHirKSetjQwn5cZktFw==, } + '@pgsql/lint@18.1.0': + resolution: + { + integrity: sha512-lMKacnInGGNo76vjIrV9o8aNUB6sheWorGpMsXZJlA7UJ2UhxO78/yOCjDxExFdFLZZjbVpck7zdBm1eGEi2QQ==, + } + hasBin: true + '@pgsql/quotes@18.2.0': resolution: { @@ -17841,6 +17851,16 @@ snapshots: dependencies: '@noble/hashes': 1.8.0 + '@pgsql/lint@18.1.0': + dependencies: + '@pgsql/traverse': 18.7.1 + chalk: 4.1.2 + libpg-query: 18.1.2 + minimist: 1.2.8 + pgsql-parser: 18.2.1 + transitivePeerDependencies: + - supports-color + '@pgsql/quotes@18.2.0': {} '@pgsql/scripts@18.3.2':