From ec15d6e50d0fd93d6cb8c006df8c1b4128fb3c9c Mon Sep 17 00:00:00 2001 From: Denis Cuciuc Date: Thu, 10 Sep 2026 01:36:18 +0300 Subject: [PATCH 1/6] fix: quote SQL identifiers, verify TLS, and guard destructive commands Schema, table and index names reached SQL text unquoted or with quotes that were never doubled. VACUUM of a table whose name contains a double quote failed outright, and generate-drop-sql emitted SQL that errors when pasted into psql. Route every identifier through quoteIdentifier. --ssl passed rejectUnauthorized: false, so it encrypted without authenticating the server. It now verifies by default, with --ssl-ca and an explicit --ssl-no-verify opt-out. run-vacuum and the pg_stat_statements DDL commands ran immediately from the CLI while the interactive menus prompted first. They now require --yes, and --dry-run reports what they would do. Also: add a pool error listener so an idle-client failure no longer kills watch mode, reject unknown commands instead of silently running a full analysis, handle SIGTERM and a second SIGINT in the watch loop, clear its pending countdown timer, and set process.exitCode rather than calling process.exit, which could truncate piped --json output. --- index.ts | 34 ++++++++++++++++++-- src/analyzers/index-analyzer.ts | 3 +- src/analyzers/table-analyzer.ts | 3 +- src/cli/options.ts | 30 +++++++++++++++++- src/cli/runner.ts | 56 +++++++++++++++++++++++++++++++-- src/constants.ts | 10 ++++++ src/interactive/display.ts | 3 +- src/utils/sql.ts | 15 +++++++++ src/watch/runner.ts | 54 +++++++++++++++++++++++++++++-- 9 files changed, 197 insertions(+), 11 deletions(-) create mode 100644 src/utils/sql.ts diff --git a/index.ts b/index.ts index f557744..65e29de 100644 --- a/index.ts +++ b/index.ts @@ -1,3 +1,5 @@ +import { readFileSync } from "node:fs"; +import type { ConnectionOptions } from "node:tls"; import { Pool } from "pg"; import { parseOptions, toAnalyzerOptions } from "./src/cli/options"; import { executeCommand } from "./src/cli/runner"; @@ -24,6 +26,25 @@ function resolveValue( return envValue ?? profileValue ?? cliValue ?? fallbackValue; } +function buildSslConfig( + enabled: boolean, + caPath: string | undefined, + noVerify: boolean, +): ConnectionOptions | undefined { + if (!enabled) { + return undefined; + } + + if (caPath) { + return { ca: readFileSync(caPath, "utf8") }; + } + + // Opting out of verification has to be explicit: --ssl alone encrypts *and* + // authenticates, so a misconfigured server fails loudly instead of silently + // leaving the connection open to interception. + return noVerify ? { rejectUnauthorized: false } : {}; +} + async function main(): Promise { const options = parseOptions(); const config = loadConfig(options.config); @@ -83,7 +104,14 @@ async function main(): Promise { "", preferProfile, ), - ssl: ssl ? { rejectUnauthorized: false } : undefined, + ssl: buildSslConfig(ssl, options.sslCa, options.sslNoVerify), + }); + + // pg emits 'error' on idle clients; without a listener Node treats it as an + // unhandled error event and kills the process. That is near-certain in watch + // mode, which holds the pool open for hours. + pool.on("error", (error) => { + console.warn(`Postgres pool error: ${error.message}`); }); const runtimeOptions = { @@ -143,5 +171,7 @@ async function main(): Promise { main().catch((error) => { const message = error instanceof Error ? error.message : String(error); console.error("Error during analysis:", message); - process.exit(1); + // Set exitCode rather than calling process.exit, which can truncate buffered + // stdout — e.g. a large --json report being piped to a file. + process.exitCode = 1; }); diff --git a/src/analyzers/index-analyzer.ts b/src/analyzers/index-analyzer.ts index 0037df5..ee89aa0 100644 --- a/src/analyzers/index-analyzer.ts +++ b/src/analyzers/index-analyzer.ts @@ -9,6 +9,7 @@ import type { UnusedIndex, } from "../types"; import { formatBytes } from "../utils/format"; +import { quoteQualifiedName } from "../utils/sql"; export class IndexAnalyzer { constructor( @@ -63,7 +64,7 @@ export class IndexAnalyzer { isPrimary: row.is_primary, usageStatus: row.usage_status, indexDefinition: row.index_definition, - dropStatement: `DROP INDEX ${row.schema}.${row.index};`, + dropStatement: `DROP INDEX ${quoteQualifiedName(row.schema, row.index)};`, })); } diff --git a/src/analyzers/table-analyzer.ts b/src/analyzers/table-analyzer.ts index e52e9d7..7c21ba4 100644 --- a/src/analyzers/table-analyzer.ts +++ b/src/analyzers/table-analyzer.ts @@ -9,6 +9,7 @@ import type { VacuumSummary, VacuumTarget, } from "../types"; +import { quoteQualifiedName } from "../utils/sql"; export class TableAnalyzer { constructor( @@ -299,7 +300,7 @@ export class TableAnalyzer { options: { analyze?: boolean; full?: boolean } = { analyze: true }, ): Promise { const startTime = Date.now(); - const fullTableName = `"${schema}"."${table}"`; + const fullTableName = quoteQualifiedName(schema, table); try { let command = "VACUUM"; diff --git a/src/cli/options.ts b/src/cli/options.ts index 836c8c0..8f26233 100644 --- a/src/cli/options.ts +++ b/src/cli/options.ts @@ -8,6 +8,10 @@ export interface ParsedOptions extends AnalyzerOptions { user: string; password?: string; ssl: boolean; + sslCa?: string; + sslNoVerify: boolean; + yes: boolean; + dryRun: boolean; profile?: string; config?: string; schemas?: string[]; @@ -31,6 +35,9 @@ export function parseOptions(argv = process.argv.slice(2)): ParsedOptions { database: DEFAULTS.database, user: DEFAULTS.user, ssl: false, + sslNoVerify: false, + yes: false, + dryRun: false, command: "full", json: false, quiet: false, @@ -66,6 +73,21 @@ export function parseOptions(argv = process.argv.slice(2)): ParsedOptions { case "--ssl": options.ssl = true; break; + case "--ssl-ca": + options.ssl = true; + options.sslCa = argv[++index]; + break; + case "--ssl-no-verify": + options.ssl = true; + options.sslNoVerify = true; + break; + case "--yes": + case "-y": + options.yes = true; + break; + case "--dry-run": + options.dryRun = true; + break; case "--output": case "-o": options.outputDir = argv[++index]; @@ -172,7 +194,9 @@ Connection options: -d, --database Database name (env: DB_NAME/PGDATABASE) -U, --user Database user (env: DB_USER/PGUSER) -W, --password Database password (env: DB_PASSWORD/PGPASSWORD) - --ssl Enable SSL (env: DB_SSL=true) + --ssl Enable SSL with certificate verification (env: DB_SSL=true) + --ssl-ca Enable SSL and verify against this CA bundle + --ssl-no-verify Enable SSL without verifying the server certificate --profile Use named profile from .analyzerrc.json --config Use a custom config file path @@ -192,6 +216,10 @@ Output options: -i, --interactive Interactive mode with menu start Alias for --interactive +Safety options: + -y, --yes Confirm destructive commands (run-vacuum, DDL) + --dry-run Report what a destructive command would do, without running it + Commands: -c, --command Run a specific analysis command diff --git a/src/cli/runner.ts b/src/cli/runner.ts index 3e5d9ef..7f62b09 100644 --- a/src/cli/runner.ts +++ b/src/cli/runner.ts @@ -5,11 +5,13 @@ import { IndexAnalyzer } from "../analyzers/index-analyzer"; import { QueryAnalyzer } from "../analyzers/query-analyzer"; import { TableAnalyzer } from "../analyzers/table-analyzer"; import { StatsCollector } from "../collectors/stats-collector"; +import { COMMANDS, DESTRUCTIVE_COMMANDS } from "../constants"; import { DiffReporter } from "../reporters/diff-reporter"; import { ReportGenerator } from "../reporters/report-generator"; import { calculateHealthScore } from "../thresholds"; import type { AnalysisReport, FullReport } from "../types"; import { formatBytes } from "../utils/format"; +import { quoteQualifiedName } from "../utils/sql"; import type { ParsedOptions } from "./options"; import { toAnalyzerOptions } from "./options"; @@ -76,6 +78,28 @@ export async function buildFullReport( }; } +function assertKnownCommand(command: string): void { + if (!(COMMANDS as readonly string[]).includes(command)) { + throw new Error( + `Unknown command: ${command}. Run --help for the list of commands.`, + ); + } +} + +function assertConfirmedIfDestructive(options: ParsedOptions): void { + if (!DESTRUCTIVE_COMMANDS.has(options.command as never)) { + return; + } + + if (options.dryRun || options.yes) { + return; + } + + throw new Error( + `${options.command} changes server state. Re-run with --yes to confirm, or --dry-run to preview.`, + ); +} + export async function executeCommand( pool: Pool, options: ParsedOptions, @@ -83,6 +107,9 @@ export async function executeCommand( const log = options.quiet || options.json ? () => {} : console.log; const services = createServices(pool, options); + assertKnownCommand(options.command); + assertConfirmedIfDestructive(options); + if (options.command !== "full") { const result = await runCommand(pool, services, options, log); console.log(JSON.stringify(result, null, 2)); @@ -228,6 +255,15 @@ async function runCommand( }; } case "run-vacuum": { + if (options.dryRun) { + const candidates = await services.tables.getTablesNeedingVacuum(); + return { + dryRun: true, + tables: candidates, + message: `Would VACUUM ANALYZE ${candidates.length} table(s). Re-run with --yes to execute.`, + }; + } + log("Running VACUUM ANALYZE on tables that need it...\n"); const summary = await services.tables.autoVacuum({ analyze: true, @@ -257,6 +293,13 @@ async function runCommand( alreadyExists: true, }; } + if (options.dryRun) { + return { + dryRun: true, + message: + "Would run CREATE EXTENSION pg_stat_statements. Re-run with --yes to execute.", + }; + } await pool.query("CREATE EXTENSION IF NOT EXISTS pg_stat_statements"); return { success: true, @@ -274,6 +317,13 @@ async function runCommand( notExists: true, }; } + if (options.dryRun) { + return { + dryRun: true, + message: + "Would run DROP EXTENSION pg_stat_statements. Re-run with --yes to execute.", + }; + } await pool.query("DROP EXTENSION IF EXISTS pg_stat_statements"); return { success: true, @@ -289,7 +339,7 @@ async function runCommand( scans: index.indexScans, sql: index.dropStatement ?? - `DROP INDEX CONCURRENTLY IF EXISTS ${index.schema}.${index.index};`, + `DROP INDEX CONCURRENTLY IF EXISTS ${quoteQualifiedName(index.schema, index.index)};`, })); const totalSize = indexes.reduce( (accumulator, index) => accumulator + index.sizeBytes, @@ -307,8 +357,10 @@ async function runCommand( } case "server-info": return { serverInfo: await services.stats.getServerInfo() }; - default: + case "full": return buildFullReport(pool, options); + default: + throw new Error(`Unsupported command: ${options.command}`); } } diff --git a/src/constants.ts b/src/constants.ts index 69ce0c7..b012d64 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -48,6 +48,16 @@ export const WATCH_ALLOWED = new Set([ "vacuum-needed", ]); +/** + * Commands that change server state. Running one non-interactively requires + * --yes; the interactive menus prompt for confirmation instead. + */ +export const DESTRUCTIVE_COMMANDS = new Set([ + "run-vacuum", + "create-pg-stat-statements", + "drop-pg-stat-statements", +]); + export const WATCH_BLOCKED = new Set([ "run-vacuum", "generate-drop-sql", diff --git a/src/interactive/display.ts b/src/interactive/display.ts index dd97007..ccb3829 100644 --- a/src/interactive/display.ts +++ b/src/interactive/display.ts @@ -9,6 +9,7 @@ import { printSeparator, printSubBullet, } from "../utils/print"; +import { quoteQualifiedName } from "../utils/sql"; type HealthMetrics = { databaseSize: string; @@ -249,7 +250,7 @@ export function showDropSql(indexes: UnusedIndex[]): void { for (const index of indexes) { console.log( - ` ${index.dropStatement ?? `DROP INDEX CONCURRENTLY IF EXISTS ${index.schema}.${index.index};`}`, + ` ${index.dropStatement ?? `DROP INDEX CONCURRENTLY IF EXISTS ${quoteQualifiedName(index.schema, index.index)};`}`, ); } } diff --git a/src/utils/sql.ts b/src/utils/sql.ts new file mode 100644 index 0000000..a5df32b --- /dev/null +++ b/src/utils/sql.ts @@ -0,0 +1,15 @@ +/** + * Quote a PostgreSQL identifier for safe interpolation into SQL text. + * + * Identifiers cannot be passed as bind parameters, so anywhere a schema, table + * or index name reaches SQL it must be quoted here first. Embedded double + * quotes are doubled, which is what makes the result injection-safe. + */ +export function quoteIdentifier(name: string): string { + return `"${name.replace(/"/g, '""')}"`; +} + +/** Quote a schema-qualified name, e.g. `public.users` -> `"public"."users"`. */ +export function quoteQualifiedName(schema: string, name: string): string { + return `${quoteIdentifier(schema)}.${quoteIdentifier(name)}`; +} diff --git a/src/watch/runner.ts b/src/watch/runner.ts index 8bdafd4..cc61f80 100644 --- a/src/watch/runner.ts +++ b/src/watch/runner.ts @@ -25,12 +25,52 @@ export function validateWatchCommand(command: string): void { export async function runWatchLoop(options: WatchOptions): Promise { validateWatchCommand(options.command); + if ( + !Number.isFinite(options.intervalSeconds) || + options.intervalSeconds <= 0 + ) { + throw new Error(`Invalid watch interval: ${options.intervalSeconds}`); + } + let stopped = false; - const handleSigInt = () => { + let pendingTimer: NodeJS.Timeout | undefined; + let signalCount = 0; + + // The loop stops at the next checkpoint rather than mid-query, so the first + // signal is a graceful stop. A second one means the user is waiting on a slow + // round-trip and wants out now — restore the default behaviour so Ctrl+C + // actually exits instead of appearing to hang. + const requestStop = (signal: NodeJS.Signals) => { stopped = true; + signalCount++; + + if (pendingTimer) { + clearTimeout(pendingTimer); + pendingTimer = undefined; + } + + if (signalCount === 1) { + process.stdout.write( + "\nStopping after the current iteration (press again to exit now)...\n", + ); + return; + } + + detach(); + process.kill(process.pid, signal); }; + const handleSigInt = () => requestStop("SIGINT"); + const handleSigTerm = () => requestStop("SIGTERM"); + + function detach(): void { + process.off("SIGINT", handleSigInt); + process.off("SIGTERM", handleSigTerm); + } + process.on("SIGINT", handleSigInt); + // Without this, `docker stop` skips the caller's connection cleanup. + process.on("SIGTERM", handleSigTerm); try { while (!stopped) { @@ -56,7 +96,12 @@ export async function runWatchLoop(options: WatchOptions): Promise { } process.stdout.write(`\rNext update in ${remaining}s... `); - await new Promise((resolve) => setTimeout(resolve, 1000)); + await new Promise((resolve) => { + pendingTimer = setTimeout(() => { + pendingTimer = undefined; + resolve(); + }, 1000); + }); } if (!stopped) { @@ -64,7 +109,10 @@ export async function runWatchLoop(options: WatchOptions): Promise { } } } finally { - process.off("SIGINT", handleSigInt); + if (pendingTimer) { + clearTimeout(pendingTimer); + } + detach(); process.stdout.write("\nWatch stopped.\n"); } } From 1dfab134f1306353585af345606e9485fd5d423d Mon Sep 17 00:00:00 2001 From: Denis Cuciuc Date: Thu, 10 Sep 2026 01:43:30 +0300 Subject: [PATCH 2/6] refactor: validate commands before opening a connection Move the command and destructive-command checks out of executeCommand into src/cli/validate.ts, called from main before the pool is created, so an invalid command fails immediately instead of after a connection attempt. --- index.ts | 9 +++++++++ src/cli/runner.ts | 26 -------------------------- src/cli/validate.ts | 29 +++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 26 deletions(-) create mode 100644 src/cli/validate.ts diff --git a/index.ts b/index.ts index 65e29de..3d493a6 100644 --- a/index.ts +++ b/index.ts @@ -3,6 +3,10 @@ import type { ConnectionOptions } from "node:tls"; import { Pool } from "pg"; import { parseOptions, toAnalyzerOptions } from "./src/cli/options"; import { executeCommand } from "./src/cli/runner"; +import { + assertConfirmedIfDestructive, + assertKnownCommand, +} from "./src/cli/validate"; import { loadConfig, resolveProfile } from "./src/config/loader"; import { DEFAULTS } from "./src/constants"; import { InteractiveCLI } from "./src/interactive"; @@ -55,6 +59,11 @@ async function main(): Promise { throw new Error("--watch cannot be combined with --json."); } + // Validate before opening a connection so a typo fails immediately rather + // than after a connection timeout. + assertKnownCommand(options.command); + assertConfirmedIfDestructive(options); + const envSsl = process.env.DB_SSL === "true" || process.env.PGSSLMODE === "require" ? true diff --git a/src/cli/runner.ts b/src/cli/runner.ts index 7f62b09..2fe484b 100644 --- a/src/cli/runner.ts +++ b/src/cli/runner.ts @@ -5,7 +5,6 @@ import { IndexAnalyzer } from "../analyzers/index-analyzer"; import { QueryAnalyzer } from "../analyzers/query-analyzer"; import { TableAnalyzer } from "../analyzers/table-analyzer"; import { StatsCollector } from "../collectors/stats-collector"; -import { COMMANDS, DESTRUCTIVE_COMMANDS } from "../constants"; import { DiffReporter } from "../reporters/diff-reporter"; import { ReportGenerator } from "../reporters/report-generator"; import { calculateHealthScore } from "../thresholds"; @@ -78,28 +77,6 @@ export async function buildFullReport( }; } -function assertKnownCommand(command: string): void { - if (!(COMMANDS as readonly string[]).includes(command)) { - throw new Error( - `Unknown command: ${command}. Run --help for the list of commands.`, - ); - } -} - -function assertConfirmedIfDestructive(options: ParsedOptions): void { - if (!DESTRUCTIVE_COMMANDS.has(options.command as never)) { - return; - } - - if (options.dryRun || options.yes) { - return; - } - - throw new Error( - `${options.command} changes server state. Re-run with --yes to confirm, or --dry-run to preview.`, - ); -} - export async function executeCommand( pool: Pool, options: ParsedOptions, @@ -107,9 +84,6 @@ export async function executeCommand( const log = options.quiet || options.json ? () => {} : console.log; const services = createServices(pool, options); - assertKnownCommand(options.command); - assertConfirmedIfDestructive(options); - if (options.command !== "full") { const result = await runCommand(pool, services, options, log); console.log(JSON.stringify(result, null, 2)); diff --git a/src/cli/validate.ts b/src/cli/validate.ts new file mode 100644 index 0000000..2aa63d4 --- /dev/null +++ b/src/cli/validate.ts @@ -0,0 +1,29 @@ +import { COMMANDS, DESTRUCTIVE_COMMANDS } from "../constants"; +import type { ParsedOptions } from "./options"; + +/** Reject an unrecognised --command instead of silently running a full analysis. */ +export function assertKnownCommand(command: string): void { + if (!(COMMANDS as readonly string[]).includes(command)) { + throw new Error( + `Unknown command: ${command}. Run --help for the list of commands.`, + ); + } +} + +/** + * Require an explicit --yes before a command that changes server state. The + * interactive menus prompt instead, so this only gates non-interactive runs. + */ +export function assertConfirmedIfDestructive(options: ParsedOptions): void { + if (!DESTRUCTIVE_COMMANDS.has(options.command as never)) { + return; + } + + if (options.dryRun || options.yes) { + return; + } + + throw new Error( + `${options.command} changes server state. Re-run with --yes to confirm, or --dry-run to preview.`, + ); +} From 955524c99340371897b424b381a119e41507bedd Mon Sep 17 00:00:00 2001 From: Denis Cuciuc Date: Thu, 10 Sep 2026 03:24:37 +0300 Subject: [PATCH 3/6] feat: add a programmatic API and split it from the CLI entry point The README documented `new DatabaseAnalyzer({...})` with `.analyze()/.printSummary()/.generateReport()/.close()`. No such symbol existed anywhere in the codebase, and the package exported nothing at all: index.ts ended in a bare main(), so requiring the package connected to Postgres and ran an analysis as a side effect of the import, while the generated .d.ts was empty. The CLI moves to src/cli/main.ts (the bin target) and src/index.ts becomes a side-effect-free library entry exporting a real PostgresAnalyzer plus the analyzers, collectors, reporters and report types. Adds an `exports` map and `sideEffects: false`. PostgresAnalyzer keeps `vacuum()` separate from `analyze()`, since it changes server state, and can wrap a pool the caller already has, in which case `close()` leaves it alone. Its SSL options mirror the CLI's: verification on by default, with `sslCa` and an explicit `sslRejectUnauthorized: false` opt-out. Also corrects the architecture tree, which still listed `src/interactive.ts` and omitted `src/cli/`, `src/config/`, `src/watch/` and `src/utils/`. --- README.md | 81 ++++++++++--- package.json | 61 +++++----- scripts/add-shebang.js | 8 +- src/api.ts | 221 ++++++++++++++++++++++++++++++++++++ index.ts => src/cli/main.ts | 17 ++- src/index.ts | 47 ++++++++ tsconfig.json | 14 ++- 7 files changed, 392 insertions(+), 57 deletions(-) create mode 100644 src/api.ts rename index.ts => src/cli/main.ts (91%) create mode 100644 src/index.ts diff --git a/README.md b/README.md index 9a67b06..f9a57da 100644 --- a/README.md +++ b/README.md @@ -348,24 +348,67 @@ Output: ## Programmatic usage +The package has two entry points. Importing it gives you the library and does nothing else; +the CLI is reached through the `pg-analyzer` binary. + ```ts -import { DatabaseAnalyzer } from "@deniscuciuc/pg-analyzer"; +import { PostgresAnalyzer } from "@deniscuciuc/pg-analyzer"; -const analyzer = new DatabaseAnalyzer({ +const analyzer = new PostgresAnalyzer({ host: "localhost", port: 5432, database: "mydb", user: "postgres", - password: "password", - ssl: { rejectUnauthorized: false }, + password: process.env.PGPASSWORD, + outputDir: "./reports", }); -const report = await analyzer.analyze(); -analyzer.printSummary(report); -await analyzer.generateReport(report); -await analyzer.close(); +try { + const report = await analyzer.analyze(); + + console.log(`Health score: ${analyzer.healthScore(report)}`); + for (const index of report.unusedIndexes) { + console.log(`Unused: ${index.schema}.${index.index} (${index.size})`); + } + + // "markdown" (default), "json" or "html"; returns the path written. + const path = await analyzer.generateReport("json", report); + console.log(`Report written to ${path}`); +} finally { + await analyzer.close(); +} ``` +### `PostgresAnalyzer` + +| Member | Description | +|---|---| +| `new PostgresAnalyzer(options?)` | Opens no connection. See the option table below. | +| `connect()` | Creates the pool and verifies it can reach the server. Called automatically by `analyze()`; call it directly to surface a connection failure early. | +| `analyze()` | Runs a full analysis and resolves to an `AnalysisReport`. | +| `healthScore(report)` | Returns the health score, 0 to 100. | +| `vacuum({ full? })` | Runs `VACUUM ANALYZE` on the tables that need it. Separate from `analyze()` because it changes server state. | +| `generateReport(format?, report?)` | Writes a report and resolves to the file path. | +| `close()` | Ends the pool. Does nothing to a pool you supplied yourself. | + +| Option | Default | Description | +|---|---|---| +| `host` / `port` | `localhost` / `5432` | | +| `database` / `user` | `postgres` / `postgres` | | +| `password` | — | | +| `ssl` | `false` | TLS with the server certificate verified | +| `sslCa` | — | Path to a CA bundle; implies `ssl` | +| `sslRejectUnauthorized` | `true` | Set to `false` to skip verification — this leaves the connection open to interception | +| `schemas` / `tables` | all | Restrict analysis | +| `outputDir` | `./reports` | Where `generateReport` writes | +| `slowQueryThresholdMs` | `100` | | +| `minIndexScans` | `50` | Below this, an index is considered unused | +| `thresholds` | built-in | Override the health thresholds | +| `pool` | — | Use an existing `pg.Pool`; you keep ownership and `close()` will not end it | + +The analyzers, collectors, reporters and every report type are exported too, so you can +assemble a different pipeline — see [`src/index.ts`](src/index.ts) for the full surface. + --- ## Managed PostgreSQL providers @@ -405,21 +448,29 @@ Or use the npm script: `pnpm pg-stat-statements:create`. ## Architecture -``` +```text db-analyzer-postgres/ -├── index.ts # Entry point + CLI -├── package.json ├── src/ -│ ├── types.ts # Shared types +│ ├── cli/main.ts # CLI entry point (the `pg-analyzer` binary) +│ ├── index.ts # Library entry point, no side effects +│ ├── api.ts # PostgresAnalyzer, the programmatic API +│ ├── cli/{options,runner,validate}.ts +│ ├── config/loader.ts # Config loading and profile resolution +│ ├── constants.ts │ ├── queries.ts # SQL queries -│ ├── interactive.ts # Interactive CLI +│ ├── filter-helpers.ts # Parameterized schema/table filters +│ ├── thresholds.ts # Health scoring +│ ├── types.ts # Shared types │ ├── analyzers/ │ │ ├── index-analyzer.ts │ │ ├── query-analyzer.ts │ │ └── table-analyzer.ts │ ├── collectors/stats-collector.ts -│ └── reporters/report-generator.ts -├── .github/copilot-instructions.md # AI agent workflow +│ ├── interactive/{index,display,menus}.ts +│ ├── reporters/{report-generator,html-reporter,diff-reporter}.ts +│ ├── utils/{format,print,sql}.ts # sql.ts quotes identifiers +│ └── watch/runner.ts # Watch mode loop +├── tests/ # Automated tests ├── .env.example └── reports/ # Generated reports (gitignored) ``` diff --git a/package.json b/package.json index f5f8cf8..f11eff0 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@deniscuciuc/pg-analyzer", "version": "1.0.0", - "description": "CLI tool that analyzes PostgreSQL databases for performance issues — indexes, slow queries, table bloat, connections and cache health.", + "description": "CLI tool that analyzes PostgreSQL databases for performance issues \u2014 indexes, slow queries, table bloat, connections and cache health.", "keywords": [ "postgresql", "postgres", @@ -29,44 +29,45 @@ "access": "public" }, "bin": { - "pg-analyzer": "./dist/index.js" + "pg-analyzer": "./dist/src/cli/main.js" }, - "main": "./dist/index.js", - "types": "./dist/index.d.ts", + "main": "./dist/src/index.js", + "types": "./dist/src/index.d.ts", "files": [ - "dist/", + "dist/src/", "README.md", "LICENSE", "CHANGELOG.md", "analyzerrc.example.json" ], "scripts": { - "start": ". ./.env && npx ts-node index.ts start", - "analyze": ". ./.env && npx ts-node index.ts", - "analyze:help": "npx ts-node index.ts --help", - "analyze:health": ". ./.env && npx ts-node index.ts -j -c health", - "analyze:indexes": ". ./.env && npx ts-node index.ts -j -c unused-indexes", - "analyze:queries": ". ./.env && npx ts-node index.ts -j -c slow-queries", - "analyze:tables": ". ./.env && npx ts-node index.ts -j -c tables", - "analyze:fk": ". ./.env && npx ts-node index.ts -j -c fk-without-indexes", - "analyze:connections": ". ./.env && npx ts-node index.ts -j -c connections", - "analyze:config": ". ./.env && npx ts-node index.ts -j -c config", - "analyze:html": ". ./.env && npx ts-node index.ts --html -c full", - "analyze:watch": ". ./.env && npx ts-node index.ts -c health --watch 30", - "server:info": ". ./.env && npx ts-node index.ts -j -c server-info", - "indexes:drop-sql": ". ./.env && npx ts-node index.ts -j -c generate-drop-sql", - "vacuum": ". ./.env && npx ts-node index.ts -c run-vacuum", - "vacuum:check": ". ./.env && npx ts-node index.ts -j -c vacuum-needed", - "pg-stat-statements:create": ". ./.env && npx ts-node index.ts -j -c create-pg-stat-statements", - "pg-stat-statements:drop": ". ./.env && npx ts-node index.ts -j -c drop-pg-stat-statements", - "build": "tsc", + "start": ". ./.env && npx ts-node src/cli/main.ts start", + "analyze": ". ./.env && npx ts-node src/cli/main.ts", + "analyze:help": "npx ts-node src/cli/main.ts --help", + "analyze:health": ". ./.env && npx ts-node src/cli/main.ts -j -c health", + "analyze:indexes": ". ./.env && npx ts-node src/cli/main.ts -j -c unused-indexes", + "analyze:queries": ". ./.env && npx ts-node src/cli/main.ts -j -c slow-queries", + "analyze:tables": ". ./.env && npx ts-node src/cli/main.ts -j -c tables", + "analyze:fk": ". ./.env && npx ts-node src/cli/main.ts -j -c fk-without-indexes", + "analyze:connections": ". ./.env && npx ts-node src/cli/main.ts -j -c connections", + "analyze:config": ". ./.env && npx ts-node src/cli/main.ts -j -c config", + "analyze:html": ". ./.env && npx ts-node src/cli/main.ts --html -c full", + "analyze:watch": ". ./.env && npx ts-node src/cli/main.ts -c health --watch 30", + "server:info": ". ./.env && npx ts-node src/cli/main.ts -j -c server-info", + "indexes:drop-sql": ". ./.env && npx ts-node src/cli/main.ts -j -c generate-drop-sql", + "vacuum": ". ./.env && npx ts-node src/cli/main.ts -c run-vacuum", + "vacuum:check": ". ./.env && npx ts-node src/cli/main.ts -j -c vacuum-needed", + "pg-stat-statements:create": ". ./.env && npx ts-node src/cli/main.ts -j -c create-pg-stat-statements", + "pg-stat-statements:drop": ". ./.env && npx ts-node src/cli/main.ts -j -c drop-pg-stat-statements", + "build": "rm -rf dist && tsc && node scripts/add-shebang.js", "postbuild": "node scripts/add-shebang.js", "lint": "biome check .", "lint:fix": "biome check --write .", "prepublishOnly": "pnpm lint && pnpm build", "release:patch": "npm version patch && git push && git push --tags", "release:minor": "npm version minor && git push && git push --tags", - "release:major": "npm version major && git push && git push --tags" + "release:major": "npm version major && git push && git push --tags", + "test": "node --test dist/tests/*.test.js" }, "dependencies": { "@inquirer/prompts": "^8.1.0", @@ -82,5 +83,13 @@ "engines": { "node": ">=20", "pnpm": ">=10" - } + }, + "exports": { + ".": { + "types": "./dist/src/index.d.ts", + "require": "./dist/src/index.js", + "default": "./dist/src/index.js" + } + }, + "sideEffects": false } diff --git a/scripts/add-shebang.js b/scripts/add-shebang.js index b19cb69..07e1a19 100644 --- a/scripts/add-shebang.js +++ b/scripts/add-shebang.js @@ -1,13 +1,15 @@ #!/usr/bin/env node "use strict"; +// tsc does not preserve a shebang, so the compiled CLI entry gets one here. const fs = require("node:fs"); const path = require("node:path"); -const file = path.join(__dirname, "..", "dist", "index.js"); +const file = path.join(__dirname, "..", "dist", "src", "cli", "main.js"); +const relative = path.relative(path.join(__dirname, ".."), file); if (!fs.existsSync(file)) { - console.error("dist/index.js not found - run pnpm build first"); + console.error(`${relative} not found - run pnpm build first`); process.exit(1); } @@ -21,4 +23,4 @@ if (process.platform !== "win32") { fs.chmodSync(file, 0o755); } -console.log("shebang added to dist/index.js"); +console.log(`shebang added to ${relative}`); diff --git a/src/api.ts b/src/api.ts new file mode 100644 index 0000000..fc618b3 --- /dev/null +++ b/src/api.ts @@ -0,0 +1,221 @@ +import { readFileSync } from "node:fs"; +import type { ConnectionOptions } from "node:tls"; +import { Pool } from "pg"; +import { buildFullReport } from "./cli/runner"; +import { DEFAULTS } from "./constants"; +import { ReportGenerator } from "./reporters/report-generator"; +import { calculateHealthScore } from "./thresholds"; +import type { + AnalysisReport, + AnalyzerOptions, + ThresholdOverrides, + VacuumSummary, +} from "./types"; + +/** Connection and analysis settings for {@link PostgresAnalyzer}. */ +export interface PostgresAnalyzerOptions { + /** Host name. Defaults to `localhost`. */ + host?: string; + /** Port. Defaults to `5432`. */ + port?: number; + /** Database name. Defaults to `postgres`. */ + database?: string; + /** User name. Defaults to `postgres`. */ + user?: string; + /** Password, if the server requires one. */ + password?: string; + /** + * Enable TLS. The server certificate is verified unless {@link sslRejectUnauthorized} + * is set to `false`. Defaults to `false`. + */ + ssl?: boolean; + /** Path to a CA bundle used to verify the server certificate. Implies {@link ssl}. */ + sslCa?: string; + /** + * Set to `false` to skip certificate verification. Leaves the connection open to + * interception, so it must be an explicit choice. + */ + sslRejectUnauthorized?: boolean; + /** Restrict analysis to these schemas. */ + schemas?: string[]; + /** Restrict analysis to these tables. */ + tables?: string[]; + /** Directory that {@link PostgresAnalyzer.generateReport} writes into. Defaults to `./reports`. */ + outputDir?: string; + /** Queries slower than this many milliseconds are reported. Defaults to `100`. */ + slowQueryThresholdMs?: number; + /** Below this many scans an index is considered unused. Defaults to `50`. */ + minIndexScans?: number; + /** Override the built-in health thresholds. */ + thresholds?: ThresholdOverrides; + /** + * Use this pool instead of creating one. The caller keeps ownership: `close()` will not + * end a pool it did not create. + */ + pool?: Pool; +} + +/** + * Programmatic entry point for the PostgreSQL analyzer. + * + * Importing this module has no side effects — unlike the CLI entry point, which connects and + * runs an analysis on import. Always `close()` when finished, ideally in a `finally`. + * + * ```ts + * import { PostgresAnalyzer } from "@deniscuciuc/pg-analyzer"; + * + * const analyzer = new PostgresAnalyzer({ database: "app", user: "postgres" }); + * try { + * const report = await analyzer.analyze(); + * console.log(analyzer.healthScore(report)); + * } finally { + * await analyzer.close(); + * } + * ``` + */ +export class PostgresAnalyzer { + private readonly options: PostgresAnalyzerOptions; + private readonly analyzerOptions: AnalyzerOptions; + private readonly ownsPool: boolean; + private pool: Pool | undefined; + + constructor(options: PostgresAnalyzerOptions = {}) { + this.options = options; + + this.analyzerOptions = { + slowQueryThresholdMs: + options.slowQueryThresholdMs ?? DEFAULTS.slowQueryThreshold, + minIndexScans: options.minIndexScans ?? DEFAULTS.minIndexScans, + topQueriesLimit: 50, + outputDir: options.outputDir ?? DEFAULTS.output, + schemas: options.schemas, + tables: options.tables, + thresholds: options.thresholds, + }; + + this.pool = options.pool; + this.ownsPool = options.pool === undefined; + } + + /** + * Creates the connection pool and verifies it can reach the server. Called automatically + * by {@link analyze}; call it directly to surface a connection failure early. + */ + async connect(): Promise { + if (!this.pool) { + this.pool = new Pool({ + host: this.options.host ?? DEFAULTS.host, + port: this.options.port ?? DEFAULTS.port, + database: this.options.database ?? DEFAULTS.database, + user: this.options.user ?? DEFAULTS.user, + password: this.options.password, + ssl: this.buildSslConfig(), + }); + + // pg emits 'error' on idle-client failures; with no listener Node treats it as an + // unhandled error event and terminates the process. + this.pool.on("error", () => {}); + } + + try { + await this.pool.query("SELECT 1"); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error(`Cannot connect to PostgreSQL: ${message}`); + } + } + + /** Runs a full analysis and returns the report. */ + async analyze(): Promise { + await this.connect(); + + return buildFullReport(this.requirePool(), { + ...this.analyzerOptions, + command: "full", + } as never); + } + + /** Computes the health score for a report, 0 to 100. */ + healthScore(report: AnalysisReport): number { + return calculateHealthScore(report, this.analyzerOptions.thresholds); + } + + /** + * Runs `VACUUM ANALYZE` on the tables that need it. + * + * This changes server state, which is why it is a separate method rather than part of + * {@link analyze}. + */ + async vacuum(options: { full?: boolean } = {}): Promise { + await this.connect(); + + const { TableAnalyzer } = await import("./analyzers/table-analyzer"); + const tables = new TableAnalyzer(this.requirePool(), this.analyzerOptions); + + return tables.autoVacuum({ analyze: true, full: options.full }); + } + + /** + * Writes a report to {@link PostgresAnalyzerOptions.outputDir}. + * + * @param format Output format. Defaults to `markdown`. + * @param report A report from {@link analyze}; one is generated if omitted. + * @returns The path of the file written. + */ + async generateReport( + format: "markdown" | "json" | "html" = "markdown", + report?: AnalysisReport, + ): Promise { + const resolved = report ?? (await this.analyze()); + const generator = new ReportGenerator( + this.analyzerOptions.outputDir ?? DEFAULTS.output, + this.analyzerOptions, + ); + + switch (format) { + case "json": + return generator.generateJsonReport(resolved); + case "html": + return generator.generateHtmlReport(resolved); + default: + return generator.generateFullReport(resolved); + } + } + + /** + * Closes the pool. Does nothing when the pool was supplied by the caller, who retains + * ownership of it. + */ + async close(): Promise { + if (!this.pool || !this.ownsPool) { + return; + } + + const pool = this.pool; + this.pool = undefined; + await pool.end(); + } + + private buildSslConfig(): ConnectionOptions | undefined { + const { ssl, sslCa, sslRejectUnauthorized } = this.options; + + if (!ssl && !sslCa) { + return undefined; + } + + if (sslCa) { + return { ca: readFileSync(sslCa, "utf8") }; + } + + // Verification is on unless explicitly disabled: TLS without it encrypts but does + // not authenticate the server. + return sslRejectUnauthorized === false ? { rejectUnauthorized: false } : {}; + } + + private requirePool(): Pool { + if (!this.pool) { + throw new Error("Not connected. Call connect() first."); + } + return this.pool; + } +} diff --git a/index.ts b/src/cli/main.ts similarity index 91% rename from index.ts rename to src/cli/main.ts index 3d493a6..f9feaeb 100644 --- a/index.ts +++ b/src/cli/main.ts @@ -1,16 +1,13 @@ import { readFileSync } from "node:fs"; import type { ConnectionOptions } from "node:tls"; import { Pool } from "pg"; -import { parseOptions, toAnalyzerOptions } from "./src/cli/options"; -import { executeCommand } from "./src/cli/runner"; -import { - assertConfirmedIfDestructive, - assertKnownCommand, -} from "./src/cli/validate"; -import { loadConfig, resolveProfile } from "./src/config/loader"; -import { DEFAULTS } from "./src/constants"; -import { InteractiveCLI } from "./src/interactive"; -import { runWatchLoop } from "./src/watch/runner"; +import { loadConfig, resolveProfile } from "../config/loader"; +import { DEFAULTS } from "../constants"; +import { InteractiveCLI } from "../interactive"; +import { runWatchLoop } from "../watch/runner"; +import { parseOptions, toAnalyzerOptions } from "./options"; +import { executeCommand } from "./runner"; +import { assertConfirmedIfDestructive, assertKnownCommand } from "./validate"; function resolveValue( cliValue: T | undefined, diff --git a/src/index.ts b/src/index.ts new file mode 100644 index 0000000..89f536f --- /dev/null +++ b/src/index.ts @@ -0,0 +1,47 @@ +/** + * Library entry point for `@deniscuciuc/pg-analyzer`. + * + * Importing this module has no side effects. The CLI lives in `src/cli/main.ts` and is + * reached through the `pg-analyzer` binary — importing the package used to run an analysis + * as a side effect of `require()`, because the CLI was the package entry point. + */ + +export { IndexAnalyzer } from "./analyzers/index-analyzer"; +export { QueryAnalyzer } from "./analyzers/query-analyzer"; +export { TableAnalyzer } from "./analyzers/table-analyzer"; +export type { PostgresAnalyzerOptions } from "./api"; +export { PostgresAnalyzer } from "./api"; +export { StatsCollector } from "./collectors/stats-collector"; +export type { Command } from "./constants"; +export { + COMMANDS, + DEFAULTS, + DESTRUCTIVE_COMMANDS, + THRESHOLDS, +} from "./constants"; +export { DiffReporter } from "./reporters/diff-reporter"; +export { HtmlReporter } from "./reporters/html-reporter"; +export { ReportGenerator } from "./reporters/report-generator"; +export { calculateHealthScore } from "./thresholds"; +export type { + AnalysisReport, + AnalyzerOptions, + BloatedTable, + DatabaseConfig, + DatabaseMetrics, + DuplicateIndex, + FullReport, + IndexInfo, + LockInfo, + MissingIndex, + QueryStats, + ReportThreshold, + SlowQuery, + TableStats, + ThresholdOverrides, + UnusedIndex, + VacuumResult, + VacuumSummary, + VacuumTarget, +} from "./types"; +export { quoteIdentifier, quoteQualifiedName } from "./utils/sql"; diff --git a/tsconfig.json b/tsconfig.json index 1f49e83..42c7230 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -2,7 +2,9 @@ "compilerOptions": { "target": "ES2022", "module": "CommonJS", - "lib": ["ES2022"], + "lib": [ + "ES2022" + ], "outDir": "dist", "rootDir": ".", "strict": true, @@ -15,6 +17,12 @@ "sourceMap": false, "moduleResolution": "node" }, - "include": ["index.ts", "src/**/*.ts"], - "exclude": ["node_modules", "dist", "**/*.test.ts"] + "include": [ + "src/**/*.ts", + "tests/**/*.ts" + ], + "exclude": [ + "node_modules", + "dist" + ] } From 4b28616afe5fa455726e4a6b05d19f69f7b7103d Mon Sep 17 00:00:00 2001 From: Denis Cuciuc Date: Thu, 10 Sep 2026 03:38:42 +0300 Subject: [PATCH 4/6] test: add a test suite, and tighten the settings that let the bugs through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repo had no tests at all — no tests/, no runner, no test script, and no CI step. tsconfig even excluded **/*.test.ts for tests that did not exist. Adds 40 tests covering the CLI parser, the config loader and profiles, SQL identifier quoting, the health score, the report writers' content rather than just their file extension, and the programmatic API. Includes a regression test that a hostile table name cannot reach the generated HTML unescaped. Bugs the tests found: - `--watch -1` was mistaken for a flag and silently replaced by the default interval instead of being rejected. The parser now distinguishes a negative number from a flag. - Numeric flags ran through Number.parseInt with no check, so `--port abc` produced NaN and a driver error far from the actual mistake. Each flag now reports its own name. - A flag at the end of argv stored undefined rather than failing. - An unknown flag was ignored, so a typo like `--jsno` silently produced human-readable output instead of JSON. Tooling: enable noUncheckedIndexedAccess — the missing flag that made `argv[++index]` look like a `string` when it is `string | undefined`, which is the root cause of the last two — plus noImplicitOverride, noFallthroughCasesInSwitch, noUnusedLocals and noUnusedParameters, and fix what they found. Biome's noUnusedVariables and noUnusedImports were warnings, so `biome check` exited 0 on dead code; both are now errors, and the lint scope covers tests/ and scripts/, which were unchecked. --- .github/CODEOWNERS | 2 + .github/workflows/ci.yml | 41 +++++- .github/workflows/publish.yml | 68 +++++++--- CODE_OF_CONDUCT.md | 113 ++++++++++++++++ SECURITY.md | 62 +++++++++ biome.json | 9 +- scripts/add-shebang.js | 1 - src/analyzers/index-analyzer.ts | 2 +- src/analyzers/table-analyzer.ts | 4 +- src/cli/options.ts | 73 ++++++++-- src/reporters/report-generator.ts | 3 +- tests/config-and-sql.test.ts | 108 +++++++++++++++ tests/options.test.ts | 217 ++++++++++++++++++++++++++++++ tests/reporters-and-api.test.ts | 169 +++++++++++++++++++++++ tsconfig.json | 7 +- 15 files changed, 828 insertions(+), 51 deletions(-) create mode 100644 .github/CODEOWNERS create mode 100644 CODE_OF_CONDUCT.md create mode 100644 SECURITY.md create mode 100644 tests/config-and-sql.test.ts create mode 100644 tests/options.test.ts create mode 100644 tests/reporters-and-api.test.ts diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000..048a850 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,2 @@ +# Default owner for everything in this repository. +* @deniscuciuc diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0b47beb..94ac3a3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,28 +1,39 @@ name: CI on: + # Every branch, not just main and develop: a feature branch previously ran no CI at all, + # so a push had to become a PR before anything was checked. push: - branches: [main, develop] + branches: ['**'] pull_request: - branches: [main, develop] + branches: [main] + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read jobs: ci: - name: Lint & Build + name: Lint, build & test (Node ${{ matrix.node-version }}) runs-on: ubuntu-latest strategy: + fail-fast: false matrix: - node-version: ["20", "22"] + # 20.6 is the floor (the dev scripts use --env-file); 24 is current. + node-version: ["20", "22", "24"] steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - uses: pnpm/action-setup@v4 with: version: 10 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v5 with: node-version: ${{ matrix.node-version }} cache: "pnpm" @@ -36,5 +47,21 @@ jobs: - name: Build run: pnpm build + - name: Test + run: pnpm test + - name: Smoke test - help output - run: node dist/index.js --help + run: node dist/src/cli/main.js --help + + dependency-review: + name: Dependency review + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + permissions: + contents: read + pull-requests: write + steps: + - uses: actions/checkout@v5 + - uses: actions/dependency-review-action@v4 + with: + fail-on-severity: moderate diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index f3dab41..e1238ea 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -5,16 +5,21 @@ on: tags: - "v*" -permissions: - contents: write +permissions: {} jobs: publish: name: Publish to npm & create GitHub Release runs-on: ubuntu-latest + # Publishing a version to npm is irreversible. Configure the `npm` environment with a + # required reviewer in repository settings to gate it. + environment: npm + permissions: + contents: write # Create the GitHub release + id-token: write # npm provenance steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 with: fetch-depth: 0 @@ -22,35 +27,62 @@ jobs: with: version: 10 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v5 with: - node-version: "20" + node-version: "22" registry-url: "https://registry.npmjs.org" cache: "pnpm" + - name: Extract version from tag + id: version + run: echo "VERSION=${GITHUB_REF_NAME#v}" >> "$GITHUB_OUTPUT" + + - name: Verify the tag matches package.json + env: + TAG_VERSION: ${{ steps.version.outputs.VERSION }} + run: | + PKG_VERSION="$(node -p "require('./package.json').version")" + if [ "$PKG_VERSION" != "$TAG_VERSION" ]; then + echo "::error::Tag v$TAG_VERSION does not match package.json version $PKG_VERSION" + exit 1 + fi + echo "Version $PKG_VERSION matches the tag." + - name: Install dependencies run: pnpm install --frozen-lockfile + - name: Lint + run: pnpm lint + - name: Build run: pnpm build - - name: Publish to npm - run: pnpm publish --access public --no-git-checks - env: - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} - - - name: Extract version from tag - id: version - run: echo "VERSION=${GITHUB_REF_NAME#v}" >> "$GITHUB_OUTPUT" + - name: Test + run: pnpm test - name: Extract CHANGELOG section for this version + env: + TAG_VERSION: ${{ steps.version.outputs.VERSION }} run: | - VERSION="${{ steps.version.outputs.VERSION }}" - NOTES=$(awk "/^## \[$VERSION\]/{found=1; next} found && /^## \[/{exit} found{print}" CHANGELOG.md) - if [ -z "$NOTES" ]; then - NOTES="See [CHANGELOG.md](./CHANGELOG.md) for details." + # awk reads the version from the environment rather than interpolating it into the + # script, so the tag name is never treated as code. + awk -v v="$TAG_VERSION" ' + $0 ~ "^## \\[" v "\\]" { found = 1; next } + found && /^## \[/ { exit } + found { print } + ' CHANGELOG.md > /tmp/release-notes.md + if [ ! -s /tmp/release-notes.md ]; then + echo "::error::No CHANGELOG.md section for $TAG_VERSION" + exit 1 fi - echo "$NOTES" > /tmp/release-notes.md + cat /tmp/release-notes.md + + - name: Publish to npm + # --provenance publishes a signed attestation linking the tarball to this workflow + # run, which needs the id-token permission above. + run: pnpm publish --access public --provenance --no-git-checks + env: + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} - name: Create GitHub Release uses: softprops/action-gh-release@v2 diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..db89c4b --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,113 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +We as members, contributors, and leaders pledge to make participation in our +community a harassment-free experience for everyone, regardless of age, body +size, visible or invisible disability, ethnicity, sex characteristics, gender +identity and expression, level of experience, education, socio-economic status, +nationality, personal appearance, race, religion, or sexual identity and +orientation. + +We pledge to act and interact in ways that contribute to an open, welcoming, +diverse, inclusive, and healthy community. + +## Our Standards + +Examples of behavior that contributes to a positive environment: + +* Demonstrating empathy and kindness toward other people +* Being respectful of differing opinions, viewpoints, and experiences +* Giving and gracefully accepting constructive feedback +* Accepting responsibility and apologizing to those affected by our mistakes, + and learning from the experience +* Focusing on what is best for the overall community + +Examples of unacceptable behavior: + +* The use of sexualized language or imagery, and sexual attention or advances of + any kind +* Trolling, insulting or derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or email address, + without their explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Enforcement Responsibilities + +Project maintainers are responsible for clarifying and enforcing our standards of +acceptable behavior and will take appropriate and fair corrective action in +response to any behavior that they deem inappropriate, threatening, offensive, +or harmful. + +Project maintainers have the right and responsibility to remove, edit, or reject +comments, commits, code, issues, and other contributions that are not aligned to +this Code of Conduct, and will communicate reasons for moderation decisions when +appropriate. + +## Scope + +This Code of Conduct applies within all community spaces, and also applies when +an individual is officially representing the community in public spaces. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported to the project maintainers at denis.cuciuc@zelqonworks.com. All +complaints will be reviewed and investigated promptly and fairly. + +All project maintainers are obligated to respect the privacy and security of the +reporter of any incident. + +## Enforcement Guidelines + +Project maintainers will follow these Community Impact Guidelines in determining +the consequences for any action they deem in violation of this Code of Conduct: + +### 1. Correction + +**Community Impact**: Use of inappropriate language or other behavior deemed +unprofessional or unwelcome in the community. + +**Consequence**: A private, written warning from project maintainers, providing +clarity around the nature of the violation and an explanation of why the +behavior was inappropriate. A public apology may be requested. + +### 2. Warning + +**Community Impact**: A violation through a single incident or series of +actions. + +**Consequence**: A warning with consequences for continued behavior. No +interaction with the people involved for a specified period of time. Violating +these terms may lead to a temporary or permanent ban. + +### 3. Temporary Ban + +**Community Impact**: A serious violation of community standards, including +sustained inappropriate behavior. + +**Consequence**: A temporary ban from any sort of interaction or public +communication with the community for a specified period of time. Violating +these terms may lead to a permanent ban. + +### 4. Permanent Ban + +**Community Impact**: Demonstrating a pattern of violation of community +standards, including sustained inappropriate behavior, harassment of an +individual, or aggression toward or disparagement of classes of individuals. + +**Consequence**: A permanent ban from any sort of public interaction within the +community. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], +version 2.1, available at +https://www.contributor-covenant.org/version/2/1/code_of_conduct.html. + +Community Impact Guidelines were inspired by +[Mozilla's code of conduct enforcement ladder](https://github.com/mozilla/inclusion). + +[homepage]: https://www.contributor-covenant.org diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..3a0a21c --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,62 @@ +# Security Policy + +## Supported Versions + +| Version | Supported | +|---------|--------------------| +| 1.x | :white_check_mark: | +| < 1.0 | :x: | + +Only the latest release receives security fixes. + +## Reporting a Vulnerability + +**Please do not open a public issue for a security vulnerability.** + +Report it through GitHub's private vulnerability reporting, which is the preferred channel: + + + +If you cannot use GitHub, email **denis.cuciuc@zelqonworks.com** instead. + +Please include a description and impact, steps to reproduce, the affected version, and any +suggested mitigation. + +## What to Expect + +| Stage | Target | +|---|---| +| Acknowledgement | Within 48 hours | +| Initial assessment | Within 5 working days | +| Fix for a high or critical issue | Within 30 days of triage | +| Fix for a moderate or low issue | Next scheduled release | + +## Responsible Disclosure + +Please give us a reasonable opportunity to release a fix before disclosing publicly. We will +credit you in the advisory and the changelog unless you prefer otherwise. + +## Scope + +This tool connects to a PostgreSQL server with credentials you supply and can, for some +commands, change server state. In scope: + +- Anything that could execute unintended commands against your database +- Credential handling, including anything that could write a password to a log, a report or + the terminal +- Anything that could cause a destructive command to run without the confirmation it requires +- Output escaping in the generated HTML reports + +Out of scope: vulnerabilities in PostgreSQL itself or in its client library — report those +upstream, though we would still like to know so we can pin or work around an affected +version. + +## Operational Notes + +- Prefer a read-only role. Only the `run-vacuum`-style commands and the profiler and + extension commands need write access, and those require `--yes`. +- Passing a password on the command line puts it in your shell history and in `ps` output. + Prefer the environment variables documented in the README, or a `.analyzerrc.json` + profile with restrictive file permissions. +- `.analyzerrc.json` may contain credentials. It is listed in `.gitignore`, but that is + the only thing protecting it — do not commit it. diff --git a/biome.json b/biome.json index 341fe4e..b0eac9f 100644 --- a/biome.json +++ b/biome.json @@ -1,17 +1,14 @@ { "$schema": "https://biomejs.dev/schemas/2.3.11/schema.json", "files": { - "includes": ["*.ts", "src/**/*.ts"] + "includes": ["src/**/*.ts", "tests/**/*.ts", "scripts/**/*.js"] }, "linter": { "enabled": true, "rules": { - "suspicious": { - "noExplicitAny": "off" - }, "correctness": { - "noUnusedVariables": "warn", - "noUnusedImports": "warn" + "noUnusedVariables": "error", + "noUnusedImports": "error" }, "style": { "noNonNullAssertion": "off" diff --git a/scripts/add-shebang.js b/scripts/add-shebang.js index 07e1a19..afc325d 100644 --- a/scripts/add-shebang.js +++ b/scripts/add-shebang.js @@ -1,5 +1,4 @@ #!/usr/bin/env node -"use strict"; // tsc does not preserve a shebang, so the compiled CLI entry gets one here. const fs = require("node:fs"); diff --git a/src/analyzers/index-analyzer.ts b/src/analyzers/index-analyzer.ts index ee89aa0..17f0490 100644 --- a/src/analyzers/index-analyzer.ts +++ b/src/analyzers/index-analyzer.ts @@ -180,7 +180,7 @@ export class IndexAnalyzer { private extractColumnsFromDefinition(definition: string): string { const match = definition.match(/\(([^)]+)\)/); - return match ? match[1] : ""; + return match?.[1] ?? ""; } private extractTypeFromDefinition(definition: string): string { diff --git a/src/analyzers/table-analyzer.ts b/src/analyzers/table-analyzer.ts index 7c21ba4..3559146 100644 --- a/src/analyzers/table-analyzer.ts +++ b/src/analyzers/table-analyzer.ts @@ -345,8 +345,8 @@ export class TableAnalyzer { const results: VacuumResult[] = []; const startTime = Date.now(); - for (let i = 0; i < tables.length; i++) { - const { schema, table } = tables[i]; + for (const [i, target] of tables.entries()) { + const { schema, table } = target; const result = await this.vacuumTable(schema, table, { analyze: options.analyze, full: options.full, diff --git a/src/cli/options.ts b/src/cli/options.ts index 8f26233..258dcf0 100644 --- a/src/cli/options.ts +++ b/src/cli/options.ts @@ -52,23 +52,23 @@ export function parseOptions(argv = process.argv.slice(2)): ParsedOptions { switch (argv[index]) { case "--host": case "-h": - options.host = argv[++index]; + options.host = requireValue("--host", argv[++index]); break; case "--port": case "-p": - options.port = Number.parseInt(argv[++index], 10); + options.port = parseNumericFlag("--port", argv[++index]); break; case "--database": case "-d": - options.database = argv[++index]; + options.database = requireValue("--database", argv[++index]); break; case "--user": case "-U": - options.user = argv[++index]; + options.user = requireValue("--user", argv[++index]); break; case "--password": case "-W": - options.password = argv[++index]; + options.password = requireValue("--password", argv[++index]); break; case "--ssl": options.ssl = true; @@ -90,13 +90,13 @@ export function parseOptions(argv = process.argv.slice(2)): ParsedOptions { break; case "--output": case "-o": - options.outputDir = argv[++index]; + options.outputDir = requireValue("--output", argv[++index]); break; case "--profile": - options.profile = argv[++index]; + options.profile = requireValue("--profile", argv[++index]); break; case "--config": - options.config = argv[++index]; + options.config = requireValue("--config", argv[++index]); break; case "--schemas": options.schemas = parseList(argv[++index]); @@ -105,14 +105,17 @@ export function parseOptions(argv = process.argv.slice(2)): ParsedOptions { options.tables = parseList(argv[++index]); break; case "--compare": - options.compare = argv[++index]; + options.compare = requireValue("--compare", argv[++index]); break; case "--html": options.html = true; break; case "--watch": { const nextValue = argv[index + 1]; - if (nextValue && !nextValue.startsWith("-")) { + // A bare "-" prefix is not enough to tell a flag from a negative number: + // `--watch -1` used to be read as a flag and silently fall back to the + // default instead of being rejected. + if (nextValue !== undefined && /^-?\d+$/.test(nextValue)) { options.watch = Number.parseInt(nextValue, 10); index++; } else { @@ -121,10 +124,16 @@ export function parseOptions(argv = process.argv.slice(2)): ParsedOptions { break; } case "--slow-query-threshold": - options.slowQueryThreshold = Number.parseInt(argv[++index], 10); + options.slowQueryThreshold = parseNumericFlag( + "--slow-query-threshold", + argv[++index], + ); break; case "--min-index-scans": - options.minIndexScans = Number.parseInt(argv[++index], 10); + options.minIndexScans = parseNumericFlag( + "--min-index-scans", + argv[++index], + ); break; case "--help": printHelp(); @@ -140,13 +149,23 @@ export function parseOptions(argv = process.argv.slice(2)): ParsedOptions { break; case "--command": case "-c": - options.command = argv[++index]; + options.command = requireValue("--command", argv[++index]); break; case "--interactive": case "-i": case "start": options.interactive = true; break; + default: { + // Previously ignored, so a typo like --jsno silently produced human output. + const token = argv[index]; + if (token?.startsWith("-")) { + throw new Error( + `Unknown option: ${token}. Run --help for the list of options.`, + ); + } + break; + } } } @@ -171,6 +190,34 @@ export function toAnalyzerOptions(options: ParsedOptions): AnalyzerOptions { }; } +/** + * Parses a numeric flag value, rejecting a missing or non-numeric one. + * + * `Number.parseInt` returns NaN for both, which used to flow into the driver and surface + * as a confusing error far from the actual mistake. + */ +function parseNumericFlag(flag: string, value: string | undefined): number { + if (value === undefined || value.startsWith("-")) { + throw new Error(`${flag} requires a value.`); + } + + const parsed = Number.parseInt(value, 10); + if (!Number.isFinite(parsed) || parsed <= 0) { + throw new Error(`${flag} must be a positive number, got '${value}'.`); + } + + return parsed; +} + +/** Parses a string flag value, rejecting a missing one. */ +function requireValue(flag: string, value: string | undefined): string { + if (value === undefined) { + throw new Error(`${flag} requires a value.`); + } + + return value; +} + function parseList(value?: string): string[] | undefined { const entries = value ?.split(",") diff --git a/src/reporters/report-generator.ts b/src/reporters/report-generator.ts index 92f06e6..5a4b5bd 100644 --- a/src/reporters/report-generator.ts +++ b/src/reporters/report-generator.ts @@ -307,8 +307,7 @@ ${issues.length > 0 ? issues.join("\n") : "- No critical issues found"} .sort((a, b) => b.totalTime - a.totalTime) .slice(0, 10); - for (let i = 0; i < topByTime.length; i++) { - const q = topByTime[i]; + for (const [i, q] of topByTime.entries()) { content += `#### ${i + 1}. Query (Total: ${formatMs(q.totalTime)}, Avg: ${formatMs(q.meanTime)}, Calls: ${q.calls.toLocaleString()})\n\n`; content += "```sql\n"; content += `${q.query}\n`; diff --git a/tests/config-and-sql.test.ts b/tests/config-and-sql.test.ts new file mode 100644 index 0000000..070690d --- /dev/null +++ b/tests/config-and-sql.test.ts @@ -0,0 +1,108 @@ +import assert from "node:assert/strict"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; +import { loadConfig, resolveProfile } from "../src/config/loader"; +import { quoteIdentifier, quoteQualifiedName } from "../src/utils/sql"; + +function writeConfig(contents: string): string { + const dir = mkdtempSync(join(tmpdir(), "pg-config-")); + const path = join(dir, "config.json"); + writeFileSync(path, contents); + return path; +} + +test("loadConfig reads an explicit config path", () => { + const path = writeConfig( + JSON.stringify({ + output: "./out", + slowQueryThreshold: 250, + profiles: { staging: { host: "staging.example.com", port: 6543 } }, + }), + ); + + const config = loadConfig(path); + + assert.equal(config.output, "./out"); + assert.equal(config.slowQueryThreshold, 250); + assert.equal(config.profiles?.staging?.host, "staging.example.com"); +}); + +test("a missing explicit config path is an error, not a silent fallback", () => { + assert.throws( + () => loadConfig(join(tmpdir(), "does-not-exist-9f3a.json")), + /Config file not found/, + ); +}); + +test("malformed JSON at an explicit path names the file", () => { + const path = writeConfig("{ not json"); + + assert.throws(() => loadConfig(path), /Could not parse config file/); + assert.throws( + () => loadConfig(path), + new RegExp(path.replace(/[/\\]/g, "\\$&")), + ); +}); + +test("resolveProfile returns the named profile", () => { + const config = { + profiles: { + prod: { host: "prod.example.com", port: 5432 }, + staging: { host: "staging.example.com" }, + }, + }; + + assert.equal(resolveProfile(config, "prod").host, "prod.example.com"); + assert.equal(resolveProfile(config, "staging").host, "staging.example.com"); +}); + +test("resolveProfile falls back to defaultProfile", () => { + const config = { + defaultProfile: "staging", + profiles: { staging: { host: "staging.example.com" } }, + }; + + assert.equal(resolveProfile(config, undefined).host, "staging.example.com"); +}); + +test("an unknown profile name is an error", () => { + const config = { profiles: { prod: { host: "prod.example.com" } } }; + + assert.throws(() => resolveProfile(config, "nope"), /nope/); +}); + +test("no profiles at all resolves to an empty profile", () => { + assert.deepEqual(resolveProfile({}, undefined), {}); +}); + +test("quoteIdentifier wraps and escapes", () => { + assert.equal(quoteIdentifier("users"), '"users"'); + assert.equal(quoteIdentifier("MixedCase"), '"MixedCase"'); + assert.equal(quoteIdentifier("with space"), '"with space"'); +}); + +test("quoteIdentifier doubles an embedded double quote", () => { + // This is what makes interpolation safe: an unescaped quote would let the rest of the + // identifier close the quoted string and continue as SQL. + assert.equal(quoteIdentifier('a"b'), '"a""b"'); + assert.equal( + quoteIdentifier('evil" ; DROP TABLE users; --'), + '"evil"" ; DROP TABLE users; --"', + ); +}); + +test("quoteQualifiedName quotes both parts independently", () => { + assert.equal(quoteQualifiedName("public", "users"), '"public"."users"'); + assert.equal(quoteQualifiedName('sch"ema', 'ta"ble'), '"sch""ema"."ta""ble"'); +}); + +test("a quoted identifier round-trips through a naive unquote", () => { + // Sanity check on the escaping rule: unquoting must recover the original name. + for (const name of ["users", 'a"b', '""', 'a""""b', "DROP TABLE x; --"]) { + const quoted = quoteIdentifier(name); + const unquoted = quoted.slice(1, -1).replace(/""/g, '"'); + assert.equal(unquoted, name); + } +}); diff --git a/tests/options.test.ts b/tests/options.test.ts new file mode 100644 index 0000000..694840c --- /dev/null +++ b/tests/options.test.ts @@ -0,0 +1,217 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { parseOptions, toAnalyzerOptions } from "../src/cli/options"; +import { + assertConfirmedIfDestructive, + assertKnownCommand, +} from "../src/cli/validate"; +import { DEFAULTS } from "../src/constants"; + +test("parseOptions returns the documented defaults", () => { + const options = parseOptions([]); + + assert.equal(options.host, DEFAULTS.host); + assert.equal(options.port, DEFAULTS.port); + assert.equal(options.database, DEFAULTS.database); + assert.equal(options.user, DEFAULTS.user); + assert.equal(options.command, "full"); + assert.equal(options.ssl, false); + assert.equal(options.json, false); + assert.equal(options.yes, false); + assert.equal(options.dryRun, false); +}); + +test("parseOptions reads connection flags", () => { + const options = parseOptions([ + "--host", + "db.example.com", + "--port", + "6543", + "--database", + "app", + "--user", + "reader", + "--password", + "secret", + ]); + + assert.equal(options.host, "db.example.com"); + assert.equal(options.port, 6543); + assert.equal(options.database, "app"); + assert.equal(options.user, "reader"); + assert.equal(options.password, "secret"); +}); + +test("parseOptions supports the short flag aliases", () => { + const options = parseOptions([ + "-h", + "h", + "-p", + "1234", + "-d", + "d", + "-U", + "u", + "-W", + "w", + "-j", + "-q", + ]); + + assert.equal(options.host, "h"); + assert.equal(options.port, 1234); + assert.equal(options.database, "d"); + assert.equal(options.user, "u"); + assert.equal(options.password, "w"); + assert.equal(options.json, true); + assert.equal(options.quiet, true); +}); + +test("--ssl enables TLS with verification left on", () => { + const options = parseOptions(["--ssl"]); + + assert.equal(options.ssl, true); + assert.equal(options.sslNoVerify, false); + assert.equal(options.sslCa, undefined); +}); + +test("--ssl-ca implies --ssl and records the bundle path", () => { + const options = parseOptions(["--ssl-ca", "/etc/ssl/rds.pem"]); + + assert.equal(options.ssl, true); + assert.equal(options.sslCa, "/etc/ssl/rds.pem"); +}); + +test("--ssl-no-verify must be explicit", () => { + const options = parseOptions(["--ssl-no-verify"]); + + assert.equal(options.ssl, true); + assert.equal(options.sslNoVerify, true); +}); + +test("--watch defaults its interval and accepts one", () => { + assert.equal(parseOptions(["--watch"]).watch, DEFAULTS.watchInterval); + assert.equal(parseOptions(["--watch", "5"]).watch, 5); + // A following flag must not be consumed as the interval. + assert.equal( + parseOptions(["--watch", "--json"]).watch, + DEFAULTS.watchInterval, + ); +}); + +test("a non-positive watch interval is rejected", () => { + assert.throws(() => parseOptions(["--watch", "0"]), /Invalid watch interval/); + // A negative value used to be mistaken for a flag and silently replaced by the default. + assert.throws( + () => parseOptions(["--watch", "-1"]), + /Invalid watch interval/, + ); +}); + +test("a non-numeric flag value is rejected with the flag named", () => { + assert.throws( + () => parseOptions(["--port", "abc"]), + /--port must be a positive number/, + ); + assert.throws( + () => parseOptions(["-p", "0"]), + /--port must be a positive number/, + ); + assert.throws( + () => parseOptions(["--slow-query-threshold", "fast"]), + /--slow-query-threshold must be a positive number/, + ); + assert.throws( + () => parseOptions(["--min-index-scans", "-5"]), + /--min-index-scans requires a value/, + ); +}); + +test("a flag at the end of argv is rejected rather than storing undefined", () => { + assert.throws(() => parseOptions(["--host"]), /--host requires a value/); + assert.throws( + () => parseOptions(["--command"]), + /--command requires a value/, + ); + assert.throws(() => parseOptions(["--port"]), /--port requires a value/); +}); + +test("an unknown flag is rejected rather than ignored", () => { + // --jsno used to silently produce human-readable output instead of JSON. + assert.throws(() => parseOptions(["--jsno"]), /Unknown option: --jsno/); + assert.throws(() => parseOptions(["-Z"]), /Unknown option: -Z/); +}); + +test("list flags are split and trimmed", () => { + const options = parseOptions([ + "--schemas", + "public, app ,", + "--tables", + "users,orders", + ]); + + assert.deepEqual(options.schemas, ["public", "app"]); + assert.deepEqual(options.tables, ["users", "orders"]); +}); + +test("an empty list flag is treated as unset", () => { + assert.equal(parseOptions(["--schemas", " , "]).schemas, undefined); +}); + +test("start is an alias for --interactive", () => { + assert.equal(parseOptions(["start"]).interactive, true); + assert.equal(parseOptions(["-i"]).interactive, true); +}); + +test("toAnalyzerOptions maps threshold names the analyzers expect", () => { + const options = parseOptions([ + "--slow-query-threshold", + "250", + "--min-index-scans", + "7", + ]); + const analyzerOptions = toAnalyzerOptions(options); + + // The CLI field is slowQueryThreshold; the analyzers read slowQueryThresholdMs. + assert.equal(analyzerOptions.slowQueryThresholdMs, 250); + assert.equal(analyzerOptions.minIndexScans, 7); +}); + +test("assertKnownCommand rejects an unrecognised command", () => { + assert.throws(() => assertKnownCommand("helth"), /Unknown command: helth/); + assert.doesNotThrow(() => assertKnownCommand("health")); + assert.doesNotThrow(() => assertKnownCommand("full")); +}); + +test("a destructive command needs --yes or --dry-run", () => { + const base = parseOptions(["-c", "run-vacuum"]); + + assert.throws( + () => assertConfirmedIfDestructive(base), + /run-vacuum changes server state/, + ); + assert.doesNotThrow(() => + assertConfirmedIfDestructive({ ...base, yes: true }), + ); + assert.doesNotThrow(() => + assertConfirmedIfDestructive({ ...base, dryRun: true }), + ); +}); + +test("a read-only command needs no confirmation", () => { + assert.doesNotThrow(() => + assertConfirmedIfDestructive(parseOptions(["-c", "health"])), + ); +}); + +test("the pg_stat_statements DDL commands are treated as destructive", () => { + for (const command of [ + "create-pg-stat-statements", + "drop-pg-stat-statements", + ]) { + assert.throws( + () => assertConfirmedIfDestructive(parseOptions(["-c", command])), + new RegExp(`${command} changes server state`), + ); + } +}); diff --git a/tests/reporters-and-api.test.ts b/tests/reporters-and-api.test.ts new file mode 100644 index 0000000..10b7cf6 --- /dev/null +++ b/tests/reporters-and-api.test.ts @@ -0,0 +1,169 @@ +import assert from "node:assert/strict"; +import { mkdtempSync, readFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; +import { PostgresAnalyzer } from "../src/api"; +import * as api from "../src/index"; +import { ReportGenerator } from "../src/reporters/report-generator"; +import type { AnalysisReport } from "../src/types"; + +function report(overrides: Partial = {}): AnalysisReport { + return { + generatedAt: new Date("2026-09-10T00:00:00Z"), + databaseName: "analyze", + metrics: { + databaseSize: "100 MB", + databaseSizeBytes: 104_857_600, + totalConnections: 5, + activeConnections: 3, + idleConnections: 2, + cacheHitRatio: 99.5, + indexHitRatio: 98.2, + deadTuplesRatio: 1, + bloatEstimate: "2 MB", + }, + unusedIndexes: [], + missingIndexes: [], + duplicateIndexes: [], + tableStats: [], + slowQueries: [], + bloatedTables: [], + recommendations: [], + ...overrides, + }; +} + +test("the library entry point exports the analyzer and helpers", () => { + assert.equal(typeof api.PostgresAnalyzer, "function"); + assert.equal(typeof api.quoteIdentifier, "function"); + assert.equal(typeof api.calculateHealthScore, "function"); + assert.equal(typeof api.ReportGenerator, "function"); + assert.equal(typeof api.IndexAnalyzer, "function"); + assert.ok(Array.isArray(api.COMMANDS)); +}); + +test("constructing an analyzer opens no connection", () => { + const analyzer = new PostgresAnalyzer({ host: "203.0.113.1", port: 1 }); + assert.ok(analyzer instanceof PostgresAnalyzer); +}); + +test("analyze reports a connection failure clearly", async () => { + const analyzer = new PostgresAnalyzer({ host: "127.0.0.1", port: 1 }); + + await assert.rejects( + () => analyzer.analyze(), + (error: Error) => { + assert.match(error.message, /Cannot connect to PostgreSQL/); + return true; + }, + ); + + await analyzer.close(); +}); + +test("close is safe when never connected, and idempotent", async () => { + const analyzer = new PostgresAnalyzer(); + await analyzer.close(); + await analyzer.close(); +}); + +test("a caller-supplied pool is not ended by close", async () => { + let endCalls = 0; + const fakePool = { + on: () => fakePool, + query: async () => ({ rows: [{ "?column?": 1 }] }), + end: async () => { + endCalls++; + }, + }; + + const analyzer = new PostgresAnalyzer({ pool: fakePool as never }); + await analyzer.connect(); + await analyzer.close(); + + assert.equal(endCalls, 0, "close must not end a pool it does not own"); +}); + +test("healthScore returns a number for a healthy report", () => { + const analyzer = new PostgresAnalyzer(); + + const score = analyzer.healthScore(report()); + + assert.equal(typeof score, "number"); + assert.ok(score > 0 && score <= 100, `score was ${score}`); +}); + +test("healthScore drops when the cache hit ratio is poor", () => { + const analyzer = new PostgresAnalyzer(); + + const healthy = analyzer.healthScore(report()); + const unhealthy = analyzer.healthScore( + report({ + metrics: { + ...report().metrics, + cacheHitRatio: 60, + indexHitRatio: 60, + deadTuplesRatio: 25, + }, + }), + ); + + assert.ok(unhealthy < healthy, `${unhealthy} should be below ${healthy}`); +}); + +test("the markdown report contains the database name and metrics", async () => { + const outputDir = mkdtempSync(join(tmpdir(), "pg-report-")); + const generator = new ReportGenerator(outputDir, {}); + + const path = await generator.generateFullReport(report()); + const contents = readFileSync(path, "utf8"); + + assert.ok(path.endsWith(".md"), path); + // Asserting on content, not just that a file appeared with the right extension. + assert.match(contents, /analyze/); + assert.match(contents, /100 MB/); +}); + +test("the json report is valid JSON carrying the report", async () => { + const outputDir = mkdtempSync(join(tmpdir(), "pg-report-")); + const generator = new ReportGenerator(outputDir, {}); + + const path = await generator.generateJsonReport(report()); + const parsed = JSON.parse(readFileSync(path, "utf8")); + + assert.ok(path.endsWith(".json"), path); + assert.equal(parsed.databaseName ?? parsed.report?.databaseName, "analyze"); +}); + +test("the html report escapes a hostile identifier", async () => { + const outputDir = mkdtempSync(join(tmpdir(), "pg-report-")); + const generator = new ReportGenerator(outputDir, {}); + + const path = await generator.generateHtmlReport( + report({ + unusedIndexes: [ + { + schema: "public", + table: "", + index: "idx_evil", + size: "1 MB", + sizeBytes: 1_048_576, + indexScans: 0, + isUnique: false, + isPrimary: false, + definition: "CREATE INDEX ...", + reason: "Never scanned", + } as never, + ], + }), + ); + const contents = readFileSync(path, "utf8"); + + assert.ok(path.endsWith(".html"), path); + assert.ok( + !contents.includes(""), + "a table name must not be interpolated into the HTML unescaped", + ); + assert.match(contents, /<script>/); +}); diff --git a/tsconfig.json b/tsconfig.json index 42c7230..926e5c9 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -15,7 +15,12 @@ "declaration": true, "declarationMap": true, "sourceMap": false, - "moduleResolution": "node" + "moduleResolution": "node", + "noUncheckedIndexedAccess": true, + "noImplicitOverride": true, + "noFallthroughCasesInSwitch": true, + "noUnusedLocals": true, + "noUnusedParameters": true }, "include": [ "src/**/*.ts", From 3607ffdaba651bc3a15004df79e651c3e2bd3722 Mon Sep 17 00:00:00 2001 From: Denis Cuciuc Date: Thu, 10 Sep 2026 03:39:31 +0300 Subject: [PATCH 5/6] ci: run tests on every branch, add provenance, and gate publishing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI only ran on pushes to main and develop, so a feature branch was checked by nothing until it became a PR. It now runs on every branch, cancels superseded runs, tests on Node 24 as well as 20 and 22, and runs `pnpm test` — which the mongo and postgres workflows never did because there were no tests. publish.yml had no `id-token: write`, so releases carried no npm provenance, and nothing verified that the tag matched package.json — a mismatched tag would have published the wrong version. It now checks that first, lints, builds and tests before publishing, publishes with `--provenance`, and reads the release notes from CHANGELOG.md with awk taking the version from the environment rather than interpolating it into the script. The job is gated on an `npm` environment so an irreversible publish can require approval. Adds the missing SECURITY.md — with the operational notes this tool needs, since it takes database credentials and can change server state — plus CODE_OF_CONDUCT.md and CODEOWNERS. Bumps the version to 1.1.0 with a changelog entry. --- CHANGELOG.md | 40 ++++++++++++++++++++++++++++++++++++++++ CONTRIBUTING.md | 2 +- package.json | 2 +- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49c8479..e25b668 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,46 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.1.0] - 2026-09-10 + +### Added + +- A programmatic API. `PostgresAnalyzer` is exported from the package root, alongside the + analyzers, collectors, reporters and every report type. Importing the package now has + no side effects — previously `require("@deniscuciuc/pg-analyzer")` connected to the + database and ran an analysis, because the CLI was the package entry point and + `main`/`types` pointed at it. The CLI moved to `src/cli/main.ts` and is still reached + through the `pg-analyzer` binary. +- A test suite. `pnpm test` runs it, and CI runs it on Node 20, 22 and 24. +- `SECURITY.md` and `CODE_OF_CONDUCT.md`. + +### Fixed + +- `--watch -1` was mistaken for a flag and silently replaced by the default interval + instead of being rejected. +- Numeric flags were parsed with no validation, so `--port abc` produced `NaN` and + surfaced as a confusing driver error far from the actual mistake. Each flag now reports + its own name. +- A flag at the end of the argument list stored `undefined` rather than failing. +- An unknown flag was silently ignored, so a typo like `--jsno` produced human-readable + output instead of JSON. +- The connection is no longer torn down by an unhandled `error` event: with no listener + attached, an idle-connection failure terminated the process, which was near-certain in + `--watch` mode. +- Ctrl+C during a query did nothing, and a second Ctrl+C did nothing either, so the CLI + appeared to hang. A second signal now exits, and `SIGTERM` is handled so `docker stop` + reaches the connection cleanup. +- `process.exitCode` is set instead of calling `process.exit`, which could truncate + buffered output when piping `--json` to a file. + +### Changed + +- The npm tarball no longer contains the compiled tests. +- Stricter TypeScript (`noUncheckedIndexedAccess` and friends) and Biome rules; unused + variables and imports are errors rather than warnings. +- CI runs on every branch, not just `main` and `develop`, and cancels superseded runs. +- Publishing now emits npm provenance and verifies the tag matches `package.json`. + ## [1.0.0] - 2026-06-13 ### Added diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 919356e..82e34f8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,7 +37,7 @@ pnpm lint:fix ## Coding standards -- TypeScript strict mode - no `any` +- TypeScript strict mode, with `noUncheckedIndexedAccess` — and no `any`, which Biome enforces - Biome formatting (tab indent, enforced by CI) - No new runtime dependencies without discussion in an issue first diff --git a/package.json b/package.json index f11eff0..0de4b0a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@deniscuciuc/pg-analyzer", - "version": "1.0.0", + "version": "1.1.0", "description": "CLI tool that analyzes PostgreSQL databases for performance issues \u2014 indexes, slow queries, table bloat, connections and cache health.", "keywords": [ "postgresql", From 4c164b2621e0e1441df078755c53e1098eb8e802 Mon Sep 17 00:00:00 2001 From: Denis Cuciuc Date: Thu, 10 Sep 2026 03:41:55 +0300 Subject: [PATCH 6/6] ci: drop the dependency-review job It needs the repository's Dependency graph enabled, which is a settings change rather than something the workflow can do, so the job failed with "Dependency review is not supported on this repository". Removed so CI is green; worth adding back once Dependency graph and Dependabot security updates are switched on in repository settings. --- .github/workflows/ci.yml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 94ac3a3..3cd0b32 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -53,15 +53,3 @@ jobs: - name: Smoke test - help output run: node dist/src/cli/main.js --help - dependency-review: - name: Dependency review - runs-on: ubuntu-latest - if: github.event_name == 'pull_request' - permissions: - contents: read - pull-requests: write - steps: - - uses: actions/checkout@v5 - - uses: actions/dependency-review-action@v4 - with: - fail-on-severity: moderate