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..3cd0b32 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,9 @@ 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 + 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/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/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/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/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/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/package.json b/package.json index f5f8cf8..0de4b0a 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.", + "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", "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..afc325d 100644 --- a/scripts/add-shebang.js +++ b/scripts/add-shebang.js @@ -1,13 +1,14 @@ #!/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 +22,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/analyzers/index-analyzer.ts b/src/analyzers/index-analyzer.ts index 0037df5..17f0490 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)};`, })); } @@ -179,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 e52e9d7..3559146 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"; @@ -344,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/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 64% rename from index.ts rename to src/cli/main.ts index f557744..f9feaeb 100644 --- a/index.ts +++ b/src/cli/main.ts @@ -1,10 +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 { 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, @@ -24,6 +27,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); @@ -34,6 +56,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 @@ -83,7 +110,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 +177,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/cli/options.ts b/src/cli/options.ts index 836c8c0..258dcf0 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, @@ -45,36 +52,51 @@ 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; 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]; + 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]); @@ -83,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 { @@ -99,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(); @@ -118,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; + } } } @@ -149,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(",") @@ -172,7 +241,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 +263,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..2fe484b 100644 --- a/src/cli/runner.ts +++ b/src/cli/runner.ts @@ -10,6 +10,7 @@ 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"; @@ -228,6 +229,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 +267,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 +291,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 +313,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 +331,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/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.`, + ); +} 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/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/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/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/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"); } } 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 1f49e83..926e5c9 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, @@ -13,8 +15,19 @@ "declaration": true, "declarationMap": true, "sourceMap": false, - "moduleResolution": "node" + "moduleResolution": "node", + "noUncheckedIndexedAccess": true, + "noImplicitOverride": true, + "noFallthroughCasesInSwitch": true, + "noUnusedLocals": true, + "noUnusedParameters": true }, - "include": ["index.ts", "src/**/*.ts"], - "exclude": ["node_modules", "dist", "**/*.test.ts"] + "include": [ + "src/**/*.ts", + "tests/**/*.ts" + ], + "exclude": [ + "node_modules", + "dist" + ] }