diff --git a/.changeset/comments-flag.md b/.changeset/comments-flag.md new file mode 100644 index 00000000..26d70904 --- /dev/null +++ b/.changeset/comments-flag.md @@ -0,0 +1,5 @@ +--- +'dotenv-diff': minor +--- + +add `--comment-warnings` flag (opt-in, off by default) that warns about `.env.example` keys lacking a documenting comment. diff --git a/docs/capabilities.md b/docs/capabilities.md index 6fb048ec..a01aefc8 100644 --- a/docs/capabilities.md +++ b/docs/capabilities.md @@ -127,6 +127,10 @@ For example, if your code uses `DATABASE_URL` but your `.env` defines `DATABAS_U Only close matches are suggested (small edit distance, scaled to key length), and only the single closest key is shown per missing variable. Suggestions are advisory — they do **not** affect the exit code or the health score. Disable them with `--no-suggest`. -### 13 Health Score +### 13 Undocumented Example Keys (opt-in) + +With `--comment-warnings`, flags `.env.example` keys that have no documenting comment — either a `#` comment on the line directly above or an inline `#` comment after the value. Off by default. See [`--comment-warnings`](./configuration_and_flags.md#--comment-warnings). + +### 14 Health Score A final score based on scan findings (missing, unused, duplicates, security warnings, and more). diff --git a/docs/configuration_and_flags.md b/docs/configuration_and_flags.md index 44a86a98..7cccf53e 100644 --- a/docs/configuration_and_flags.md +++ b/docs/configuration_and_flags.md @@ -48,6 +48,7 @@ CLI flags always take precedence over configuration file values. - [--no-expire-warnings](#--no-expire-warnings) - [--inconsistent-naming-warnings](#--inconsistent-naming-warnings) - [--no-inconsistent-naming-warnings](#--no-inconsistent-naming-warnings) +- [--comment-warnings](#--comment-warnings) - [--suggest](#--suggest) - [--no-suggest](#--no-suggest) @@ -824,6 +825,42 @@ Usage in the configuration file: } ``` +### `--comment-warnings` + +Warn about `.env.example` keys that lack a documenting comment (disabled by default — opt in with this flag). + +A well documented `.env.example` is the fastest onboarding for a new contributor. + +A key counts as **documented** when either: + +- a `#` comment sits on the line **directly above** it, or +- it has an **inline** `#` comment after the value. + +```dotenv +# Stripe webhook signing secret (dashboard.stripe.com/webhooks) +STRIPE_WEBHOOK_SECRET= # ✓ documented (comment above) +PORT=3000 # server port # ✓ documented (inline comment) +API_KEY= # ✗ reported (undocumented) +``` + +Undocumented keys are listed in the console output and in JSON (`commentWarnings`), count toward the [health score](./capabilities.md), can be suppressed with a [baseline](./baseline.md) (`comment` rule), and cause a non-zero exit under [`--strict`](#--strict). + +> Note: the rule is intentionally simple. A shared section header (e.g. `# === Database ===`) counts only for the first key directly beneath it, so keys further down may still be reported. + +Example usage: + +```bash +dotenv-diff --comment-warnings +``` + +Usage in the configuration file: + +```json +{ + "commentWarnings": true +} +``` + ### `--suggest` Suggests the closest existing key when a missing variable looks like a typo (enabled by default). Missing entries are annotated with a `→ did you mean DATABAS_URL?` hint by cross-referencing the missing key against the keys that already exist (defined keys in scan mode, extra keys in compare mode). Only close matches are shown, and suggestions never change the exit code or health score. diff --git a/packages/cli/src/baseline/scanBaseline.ts b/packages/cli/src/baseline/scanBaseline.ts index fd039e86..42551624 100644 --- a/packages/cli/src/baseline/scanBaseline.ts +++ b/packages/cli/src/baseline/scanBaseline.ts @@ -129,6 +129,10 @@ export function collectBaselineEntries( entries.push({ rule: 'expire', key: warning.key }); } + for (const warning of scanResult.commentWarnings ?? []) { + entries.push({ rule: 'comment', key: warning.key }); + } + // Sort the key pair so the entry is identical regardless of scanner order for (const warning of scanResult.inconsistentNamingWarnings ?? []) { const pair = [warning.key1, warning.key2].sort().join('|'); @@ -205,6 +209,11 @@ export function applyBaselineEntries( }, ), }), + ...(scanResult.commentWarnings != null && { + commentWarnings: scanResult.commentWarnings.filter( + (w) => !has('comment', w.key), + ), + }), }; } diff --git a/packages/cli/src/cli/program.ts b/packages/cli/src/cli/program.ts index ec017275..5f502a5d 100644 --- a/packages/cli/src/cli/program.ts +++ b/packages/cli/src/cli/program.ts @@ -95,6 +95,10 @@ export function createProgram() { '--no-inconsistent-naming-warnings', 'Disable inconsistent naming pattern warnings', ) + .option( + '--comment-warnings', + 'Warn about .env.example keys that lack a documenting # comment', + ) .option( '--suggest', 'Suggest the closest existing key for likely typos (enabled by default)', diff --git a/packages/cli/src/cli/run.ts b/packages/cli/src/cli/run.ts index b7228295..015b5a60 100644 --- a/packages/cli/src/cli/run.ts +++ b/packages/cli/src/cli/run.ts @@ -121,6 +121,7 @@ async function runScanMode(opts: Options): Promise { uppercaseKeys: opts.uppercaseKeys, expireWarnings: opts.expireWarnings, inconsistentNamingWarnings: opts.inconsistentNamingWarnings, + commentWarnings: opts.commentWarnings, listAll: opts.listAll, baseline: opts.baseline, suggest: opts.suggest, diff --git a/packages/cli/src/commands/scanUsage.ts b/packages/cli/src/commands/scanUsage.ts index 6a3dd825..e4a6d6cc 100644 --- a/packages/cli/src/commands/scanUsage.ts +++ b/packages/cli/src/commands/scanUsage.ts @@ -133,6 +133,9 @@ export async function scanUsage(opts: ScanUsageOptions): Promise { scanResult.inconsistentNamingWarnings = result.inconsistentNamingWarnings; } + if (result.commentWarnings) { + scanResult.commentWarnings = result.commentWarnings; + } if ( result.exampleFull && result.comparedAgainst === DEFAULT_EXAMPLE_FILE @@ -204,6 +207,7 @@ export async function scanUsage(opts: ScanUsageOptions): Promise { (w) => w.daysLeft <= EXPIRE_THRESHOLD_DAYS, ).length ?? 0) > 0 || (scanResult.inconsistentNamingWarnings?.length ?? 0) > 0 || + (scanResult.commentWarnings?.length ?? 0) > 0 || (scanResult.exampleWarnings?.length ?? 0) > 0) ), }; @@ -278,6 +282,7 @@ function calculateStats(scanResult: ScanResult): void { (scanResult.uppercaseWarnings?.length ?? 0) + (scanResult.expireWarnings?.length ?? 0) + (scanResult.inconsistentNamingWarnings?.length ?? 0) + + (scanResult.commentWarnings?.length ?? 0) + (scanResult.secrets?.length ?? 0) + scanResult.missing.length + scanResult.unused.length + diff --git a/packages/cli/src/config/options.ts b/packages/cli/src/config/options.ts index abc0593a..f8f748a1 100644 --- a/packages/cli/src/config/options.ts +++ b/packages/cli/src/config/options.ts @@ -57,6 +57,7 @@ export function normalizeOptions(raw: RawOptions): Options { const uppercaseKeys = raw.uppercaseKeys !== false; const expireWarnings = raw.expireWarnings !== false; const inconsistentNamingWarnings = raw.inconsistentNamingWarnings !== false; + const commentWarnings = toBool(raw.commentWarnings); const suggest = raw.suggest !== false; const listAll = toBool(raw.listAll); const explain = @@ -105,6 +106,7 @@ export function normalizeOptions(raw: RawOptions): Options { uppercaseKeys, expireWarnings, inconsistentNamingWarnings, + commentWarnings, listAll, explain, matrix, diff --git a/packages/cli/src/config/types.ts b/packages/cli/src/config/types.ts index c1d81871..863576ed 100644 --- a/packages/cli/src/config/types.ts +++ b/packages/cli/src/config/types.ts @@ -94,6 +94,7 @@ export interface RawOptions { uppercaseKeys?: boolean; expireWarnings?: boolean; inconsistentNamingWarnings?: boolean; + commentWarnings?: boolean; listAll?: boolean; explain?: string; matrix?: boolean | string[]; @@ -141,6 +142,7 @@ export interface Options { uppercaseKeys: boolean; expireWarnings: boolean; inconsistentNamingWarnings: boolean; + commentWarnings: boolean; listAll: boolean; explain: string | undefined; matrix: boolean; @@ -240,6 +242,7 @@ export interface ScanUsageOptions extends ScanOptions { uppercaseKeys?: boolean; expireWarnings?: boolean; inconsistentNamingWarnings?: boolean; + commentWarnings?: boolean; listAll?: boolean; baseline?: boolean; suggest?: boolean; @@ -285,6 +288,7 @@ export interface ScanResult { uppercaseWarnings?: UppercaseWarning[]; expireWarnings?: ExpireWarning[]; inconsistentNamingWarnings?: InconsistentNamingWarning[]; + commentWarnings?: CommentWarning[]; /** Typo suggestions for variables used in code but not defined in the env file */ suggestions?: TypoSuggestion[]; fileContentMap?: Map; @@ -339,6 +343,7 @@ export interface ComparisonOptions { uppercaseKeys?: boolean; expireWarnings?: boolean; inconsistentNamingWarnings?: boolean; + commentWarnings?: boolean; suggest?: boolean; } @@ -453,6 +458,17 @@ export interface ExpireWarning { daysLeft: number; } +/** + * Warning about an `.env.example` key that has no documenting comment. + * fx: `API_KEY=` with no `#` comment on the line above and no inline `#` comment. + */ +export interface CommentWarning { + /** The undocumented environment variable key */ + key: string; + /** 1-based line number of the key in the example file */ + line: number; +} + /** * A "did you mean" suggestion produced when a reported key looks like a typo * of an existing key. @@ -494,7 +510,8 @@ export type BaselineRule = | 'framework' | 'uppercase' | 'expire' - | 'inconsistent-naming'; + | 'inconsistent-naming' + | 'comment'; /** * A single suppressed warning in the baseline file. diff --git a/packages/cli/src/core/scan/computeHealthScore.ts b/packages/cli/src/core/scan/computeHealthScore.ts index a3dbbb6a..130f406c 100644 --- a/packages/cli/src/core/scan/computeHealthScore.ts +++ b/packages/cli/src/core/scan/computeHealthScore.ts @@ -39,6 +39,9 @@ export function computeHealthScore(scan: ScanResult): number { // === 9. Inconsistent naming warnings === score -= (scan.inconsistentNamingWarnings?.length ?? 0) * 3; + // === 9b. Undocumented example keys (advisory) === + score -= (scan.commentWarnings?.length ?? 0) * 2; + // === 10. Duplicate definitions === score -= (scan.duplicates?.env?.length ?? 0) * 10; score -= (scan.duplicates?.example?.length ?? 0) * 10; diff --git a/packages/cli/src/services/detectMissingComments.ts b/packages/cli/src/services/detectMissingComments.ts new file mode 100644 index 00000000..94c4bdb2 --- /dev/null +++ b/packages/cli/src/services/detectMissingComments.ts @@ -0,0 +1,46 @@ +import fs from 'fs'; +import type { CommentWarning } from '../config/types.js'; +import { splitEnvLines, parseEnvLine } from '../core/envLine.js'; + +/** + * Detects `.env.example` keys that lack a documenting comment. + * + * A key counts as documented when either: + * - the line directly above it is a `#` comment, or + * - the key's own line has an inline comment after the value (`KEY=value # ...`). + * + * Everything else (a bare `KEY=` with no neighbouring comment) is reported. + * fx: + * + * # Stripe webhook signing secret + * STRIPE_WEBHOOK_SECRET= ← documented (comment above) + * PORT=3000 # server port ← documented (inline comment) + * API_KEY= ← reported (undocumented) + * + * @param filePath - Path to the `.env.example` file to inspect. + * @returns One warning per undocumented key, in file order. + */ +export function detectMissingComments(filePath: string): CommentWarning[] { + const lines = splitEnvLines(fs.readFileSync(filePath, 'utf8')); + const warnings: CommentWarning[] = []; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]!; + const parsed = parseEnvLine(line); + if (!parsed) continue; // blank line, comment line, or non-key line + + // Inline comment: a `#` anywhere after the first `=`. + const eq = line.indexOf('='); + const hasInlineComment = eq !== -1 && line.slice(eq + 1).includes('#'); + + // Comment directly above: the immediately preceding line is a `#` comment. + const prev = i > 0 ? lines[i - 1]!.trim() : ''; + const hasCommentAbove = prev.startsWith('#'); + + if (!hasInlineComment && !hasCommentAbove) { + warnings.push({ key: parsed.key, line: i + 1 }); + } + } + + return warnings; +} diff --git a/packages/cli/src/services/printScanResult.ts b/packages/cli/src/services/printScanResult.ts index e4ae8f5d..89acf16b 100644 --- a/packages/cli/src/services/printScanResult.ts +++ b/packages/cli/src/services/printScanResult.ts @@ -26,6 +26,7 @@ import { printUppercaseWarning } from '../ui/scan/printUppercaseWarning.js'; import { computeHealthScore } from '../core/scan/computeHealthScore.js'; import { printHealthScore } from '../ui/scan/printHealthScore.js'; import { printExpireWarnings } from '../ui/scan/printExpireWarnings.js'; +import { printCommentWarnings } from '../ui/scan/printCommentWarnings.js'; import { printInconsistentNamingWarning } from '../ui/scan/printInconsistentNamingWarning.js'; import { printListAll } from '../ui/scan/printListAll.js'; @@ -122,6 +123,10 @@ export function printScanResult( if (scanResult.expireWarnings) { printExpireWarnings(scanResult.expireWarnings, opts.strict); } + // Undocumented example keys + if (scanResult.commentWarnings) { + printCommentWarnings(scanResult.commentWarnings, opts.strict); + } // Check for high severity secrets - ALWAYS exit with error const hasHighSeveritySecrets = (scanResult.secrets ?? []).some( (s) => s.severity === 'high', @@ -178,7 +183,8 @@ export function printScanResult( (scanResult.expireWarnings?.filter( (w) => w.daysLeft <= EXPIRE_THRESHOLD_DAYS, ).length ?? 0) > 0 || - (scanResult.inconsistentNamingWarnings?.length ?? 0) > 0; + (scanResult.inconsistentNamingWarnings?.length ?? 0) > 0 || + (scanResult.commentWarnings?.length ?? 0) > 0; if (hasStrictViolations) exitWithError = true; } diff --git a/packages/cli/src/services/processComparisonFile.ts b/packages/cli/src/services/processComparisonFile.ts index 8143ec78..7defbf66 100644 --- a/packages/cli/src/services/processComparisonFile.ts +++ b/packages/cli/src/services/processComparisonFile.ts @@ -9,6 +9,8 @@ import { toUpperSnakeCase } from '../core/helpers/toUpperSnakeCase.js'; import { resolveFromCwd } from '../core/helpers/resolveFromCwd.js'; import { detectEnvExpirations } from './detectEnvExpirations.js'; import { detectInconsistentNaming } from '../core/detectInconsistentNaming.js'; +import { detectMissingComments } from './detectMissingComments.js'; +import { DEFAULT_EXAMPLE_FILE } from '../config/constants.js'; import type { ScanUsageOptions, ScanResult, @@ -18,6 +20,7 @@ import type { ComparisonFile, ExpireWarning, InconsistentNamingWarning, + CommentWarning, FixContext, } from '../config/types.js'; @@ -36,6 +39,7 @@ export interface ProcessComparisonResult { uppercaseWarnings?: UppercaseWarning[]; expireWarnings?: ExpireWarning[]; inconsistentNamingWarnings?: InconsistentNamingWarning[]; + commentWarnings?: CommentWarning[]; error?: { message: string; shouldExit: boolean }; } @@ -60,6 +64,7 @@ export function processComparisonFile( let uppercaseWarnings: UppercaseWarning[] = []; let expireWarnings: ExpireWarning[] = []; let inconsistentNamingWarnings: InconsistentNamingWarning[] = []; + let commentWarnings: CommentWarning[] = []; const fix: FixContext = { fixApplied: false, @@ -134,6 +139,19 @@ export function processComparisonFile( inconsistentNamingWarnings = detectInconsistentNaming(allKeys); } + // Warn about .env.example keys without a documenting comment. Runs on the + // example file (documentation lives there), not the env file — using the + // explicit --example path when given, else the default `.env.example`. + if (opts.commentWarnings) { + const examplePath = resolveFromCwd( + opts.cwd, + opts.examplePath ?? DEFAULT_EXAMPLE_FILE, + ); + if (fs.existsSync(examplePath)) { + commentWarnings = detectMissingComments(examplePath); + } + } + // Apply fixes (both duplicates + missing keys + gitignore) if (opts.fix) { const { changed, result } = applyFixes({ @@ -178,6 +196,7 @@ export function processComparisonFile( uppercaseWarnings, expireWarnings, inconsistentNamingWarnings, + commentWarnings, error: { message: errorMessage, shouldExit: opts.isCiMode ?? false, @@ -197,6 +216,7 @@ export function processComparisonFile( uppercaseWarnings, expireWarnings, inconsistentNamingWarnings, + commentWarnings, }; } diff --git a/packages/cli/src/ui/scan/printCommentWarnings.ts b/packages/cli/src/ui/scan/printCommentWarnings.ts new file mode 100644 index 00000000..90cea180 --- /dev/null +++ b/packages/cli/src/ui/scan/printCommentWarnings.ts @@ -0,0 +1,30 @@ +import type { CommentWarning } from '../../config/types.js'; +import { label, error, warning, divider, header, padLabel } from '../theme.js'; + +/** + * Prints warnings for `.env.example` keys that lack a documenting comment. + * @param warnings Array of undocumented-key warnings + * @param strict Whether strict mode is enabled (colours the row as an error) + * @returns void + */ +export function printCommentWarnings( + warnings: CommentWarning[], + strict: boolean = false, +): void { + if (warnings.length === 0) return; + + const indicator = strict ? error('▸') : warning('▸'); + const rowColor = strict ? error : warning; + + console.log(); + console.log(`${indicator} ${header('Undocumented keys')}`); + console.log(`${divider}`); + + for (const warn of warnings) { + console.log( + `${label(padLabel(warn.key))}${rowColor('missing a documenting # comment')}`, + ); + } + + console.log(`${divider}`); +} diff --git a/packages/cli/src/ui/scan/scanJsonOutput.ts b/packages/cli/src/ui/scan/scanJsonOutput.ts index fccc4b25..68deae33 100644 --- a/packages/cli/src/ui/scan/scanJsonOutput.ts +++ b/packages/cli/src/ui/scan/scanJsonOutput.ts @@ -5,6 +5,7 @@ import type { Duplicate, ExpireWarning, InconsistentNamingWarning, + CommentWarning, UppercaseWarning, FrameworkWarning, ExampleSecretWarning, @@ -51,6 +52,7 @@ interface ScanJsonOutput { }; logged?: EnvUsage[]; expireWarnings?: ExpireWarning[]; + commentWarnings?: CommentWarning[]; uppercaseWarnings?: UppercaseWarning[]; inconsistentNamingWarnings?: InconsistentNamingWarning[]; frameworkWarnings?: FrameworkWarning[]; @@ -189,6 +191,14 @@ export function scanJsonOutput( })); } + // Undocumented example keys + if (scanResult.commentWarnings?.length) { + output.commentWarnings = scanResult.commentWarnings.map((w) => ({ + key: w.key, + line: w.line, + })); + } + const healthScore = computeHealthScore(scanResult); output.healthScore = healthScore; diff --git a/packages/cli/test/e2e/cli.commentWarnings.e2e.test.ts b/packages/cli/test/e2e/cli.commentWarnings.e2e.test.ts new file mode 100644 index 00000000..d5f93945 --- /dev/null +++ b/packages/cli/test/e2e/cli.commentWarnings.e2e.test.ts @@ -0,0 +1,81 @@ +import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest'; +import fs from 'fs'; +import path from 'path'; +import { makeTmpDir, rmrf } from '../utils/fs-helpers.js'; +import { buildOnce, runCli, cleanupBuild } from '../utils/cli-helpers.js'; + +const tmpDirs: string[] = []; + +beforeAll(() => { + buildOnce(); +}); + +afterAll(() => { + cleanupBuild(); +}); + +afterEach(() => { + while (tmpDirs.length) { + const dir = tmpDirs.pop(); + if (dir) rmrf(dir); + } +}); + +/** + * Sets up a clean project where .env and .env.example share the same keys + * (so there are no missing/unused issues), the code uses them all, and .env is + * gitignored — leaving comment warnings as the only variable under test. + */ +function setup(exampleBody: string) { + const cwd = makeTmpDir(); + tmpDirs.push(cwd); + fs.writeFileSync(path.join(cwd, '.gitignore'), '.env\n'); + fs.writeFileSync(path.join(cwd, '.env'), 'API_KEY=x\nDB_HOST=y\nPORT=1\n'); + fs.writeFileSync(path.join(cwd, '.env.example'), exampleBody); + fs.mkdirSync(path.join(cwd, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(cwd, 'src', 'index.js'), + 'console.log(process.env.API_KEY, process.env.DB_HOST, process.env.PORT);', + ); + return cwd; +} + +// API_KEY documented via comment above; PORT via inline; DB_HOST undocumented. +const MIXED_EXAMPLE = + '# API key for the service\nAPI_KEY=\nDB_HOST=\nPORT=3000 # server port\n'; + +describe('Comment Warnings (--comment-warnings)', () => { + it('does not check comments by default (opt-in)', () => { + const cwd = setup(MIXED_EXAMPLE); + const res = runCli(cwd, []); + expect(res.stdout).not.toContain('Undocumented keys'); + }); + + it('warns about an undocumented example key when --comment-warnings is set', () => { + const cwd = setup(MIXED_EXAMPLE); + const res = runCli(cwd, ['--comment-warnings']); + expect(res.stdout).toContain('Undocumented keys'); + expect(res.stdout).toContain('DB_HOST'); + }); + + it('does not warn about keys documented above or inline', () => { + const cwd = setup('# db host\nDB_HOST=\nAPI_KEY=key123 # the key\n'); + const res = runCli(cwd, ['--comment-warnings']); + expect(res.stdout).not.toContain('Undocumented keys'); + }); + + it('lists undocumented keys under commentWarnings in JSON mode', () => { + const cwd = setup(MIXED_EXAMPLE); + const res = runCli(cwd, ['--comment-warnings', '--json']); + expect(res.stdout).toContain('"commentWarnings"'); + expect(res.stdout).toContain('"DB_HOST"'); + expect(res.stdout).not.toContain('Undocumented keys'); // no UI in json mode + }); + + it('exits with error under --strict when an undocumented key exists', () => { + const cwd = setup(MIXED_EXAMPLE); + const res = runCli(cwd, ['--comment-warnings', '--strict']); + expect(res.status).toBe(1); + expect(res.stdout).toContain('Undocumented keys'); + }); +}); diff --git a/packages/cli/test/property/services/detectMissingComments.property.test.ts b/packages/cli/test/property/services/detectMissingComments.property.test.ts new file mode 100644 index 00000000..9a1d4d4f --- /dev/null +++ b/packages/cli/test/property/services/detectMissingComments.property.test.ts @@ -0,0 +1,126 @@ +import { describe, test, expect, beforeAll, afterAll } from 'vitest'; +import fc from 'fast-check'; +import fs from 'fs'; +import path from 'path'; +import os from 'os'; +import { detectMissingComments } from '../../../src/services/detectMissingComments.js'; +import { parseEnvLine, splitEnvLines } from '../../../src/core/envLine.js'; + +/** + * Property-based ("fuzz") tests for the undocumented-key detector. + * + * A key is "documented" when a `#` comment sits directly above it or inline + * after its value; anything else is reported. These feed thousands of random + * `.env.example` bodies through the detector and check it against an independent + * oracle: it never throws, only reports real keys, never reports a key that has + * a comment above or inline, and reports every key that has neither. Property- + * based testing is also what OpenSSF Scorecard recognises as fuzzing for JS/TS. + */ + +let dir: string; +let filePath: string; + +beforeAll(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'comments-prop-')); + filePath = path.join(dir, '.env.example'); +}); +afterAll(() => { + fs.rmSync(dir, { recursive: true, force: true }); +}); + +function run(content: string) { + fs.writeFileSync(filePath, content); + return detectMissingComments(filePath); +} + +/** Independent re-derivation of the documented/undocumented rule. */ +function oracle(content: string): { key: string; line: number }[] { + const lines = splitEnvLines(content); + const out: { key: string; line: number }[] = []; + for (let i = 0; i < lines.length; i++) { + const parsed = parseEnvLine(lines[i]!); + if (!parsed) continue; + const eq = lines[i]!.indexOf('='); + const inline = eq !== -1 && lines[i]!.slice(eq + 1).includes('#'); + const above = i > 0 && lines[i - 1]!.trim().startsWith('#'); + if (!inline && !above) out.push({ key: parsed.key, line: i + 1 }); + } + return out; +} + +const keyName = fc.stringMatching(/^[A-Z_][A-Z0-9_]{0,6}$/); +const value = fc.stringMatching(/^[^\r\n#]{0,6}$/); +const lineArb = fc.oneof( + keyName.map((k) => `${k}=`), // bare key (undocumented unless comment above) + fc.tuple(keyName, value).map(([k, v]) => `${k}=${v}`), + fc.tuple(keyName, value).map(([k, v]) => `${k}=${v} # inline doc`), // inline + fc.stringMatching(/^# [^\r\n]{0,12}$/), // comment line + fc.constant(''), // blank + fc.stringMatching(/^[^\r\n=#]{0,8}$/), // junk (no key) +); + +const bodyArb = fc + .tuple(fc.array(lineArb, { maxLength: 18 }), fc.constantFrom('\n', '\r\n')) + .map(([lines, eol]) => lines.join(eol)); + +describe('detectMissingComments (property-based)', () => { + test('never throws on arbitrary file content', () => { + fc.assert( + fc.property(fc.string({ unit: 'binary' }), (content) => { + run(content); + }), + { numRuns: 1000 }, + ); + }); + + test('matches the documented/undocumented oracle exactly', () => { + fc.assert( + fc.property(bodyArb, (content) => { + expect(run(content)).toEqual(oracle(content)); + }), + { numRuns: 2000 }, + ); + }); + + test('every reported entry is a real key at the reported line', () => { + fc.assert( + fc.property(bodyArb, (content) => { + const lines = splitEnvLines(content); + for (const w of run(content)) { + const parsed = parseEnvLine(lines[w.line - 1]!); + expect(parsed?.key).toBe(w.key); + } + }), + { numRuns: 2000 }, + ); + }); + + test('a key with a comment directly above is never reported', () => { + fc.assert( + fc.property(keyName, value, (k, v) => { + expect(run(`# documented\n${k}=${v}`)).toEqual([]); + }), + { numRuns: 1000 }, + ); + }); + + test('a key with an inline comment is never reported', () => { + fc.assert( + fc.property(keyName, value, (k, v) => { + expect(run(`${k}=${v} # inline`)).toEqual([]); + }), + { numRuns: 1000 }, + ); + }); + + test('a bare undocumented key is always reported', () => { + fc.assert( + fc.property(keyName, value, (k, v) => { + // Preceded by a blank line so there is no comment above. + const result = run(`\n${k}=${v}`); + expect(result).toEqual([{ key: k, line: 2 }]); + }), + { numRuns: 1000 }, + ); + }); +}); diff --git a/packages/cli/test/unit/baseline/scanBaseline.test.ts b/packages/cli/test/unit/baseline/scanBaseline.test.ts index 65be3ef1..280b720a 100644 --- a/packages/cli/test/unit/baseline/scanBaseline.test.ts +++ b/packages/cli/test/unit/baseline/scanBaseline.test.ts @@ -270,6 +270,14 @@ describe('collectBaselineEntries', () => { expect(result).toContainEqual({ rule: 'expire', key: 'OLD_KEY' }); }); + it('collects comment warnings', () => { + const result = collectBaselineEntries({ + ...emptyScanResult, + commentWarnings: [{ key: 'UNDOCUMENTED', line: 2 }], + }); + expect(result).toContainEqual({ rule: 'comment', key: 'UNDOCUMENTED' }); + }); + it('collects inconsistent-naming warnings as sorted key pair', () => { const result = collectBaselineEntries({ ...emptyScanResult, @@ -573,6 +581,22 @@ describe('applyBaselineEntries', () => { expect(after.expireWarnings).toBeUndefined(); }); + it('suppresses comment warning', () => { + const result: ScanResult = { + ...emptyScanResult, + commentWarnings: [{ key: 'UNDOC', line: 2 }], + }; + const entries: BaselineEntry[] = [{ rule: 'comment', key: 'UNDOC' }]; + const after = applyBaselineEntries(result, entries); + expect(after.commentWarnings).toHaveLength(0); + }); + + it('does not touch commentWarnings when field is absent', () => { + const result: ScanResult = { ...emptyScanResult }; + const after = applyBaselineEntries(result, [{ rule: 'comment', key: 'x' }]); + expect(after.commentWarnings).toBeUndefined(); + }); + it('suppresses inconsistent-naming warning (sorted pair)', () => { const result: ScanResult = { ...emptyScanResult, diff --git a/packages/cli/test/unit/cli/run.test.ts b/packages/cli/test/unit/cli/run.test.ts index a3401de5..c28d22bc 100644 --- a/packages/cli/test/unit/cli/run.test.ts +++ b/packages/cli/test/unit/cli/run.test.ts @@ -97,6 +97,7 @@ function createBaseOptions(overrides: Partial = {}): Options { uppercaseKeys: true, expireWarnings: true, inconsistentNamingWarnings: true, + commentWarnings: false, listAll: false, explain: undefined, baseline: false, diff --git a/packages/cli/test/unit/commands/scanUsage.test.ts b/packages/cli/test/unit/commands/scanUsage.test.ts index 415a57e1..7d3117f8 100644 --- a/packages/cli/test/unit/commands/scanUsage.test.ts +++ b/packages/cli/test/unit/commands/scanUsage.test.ts @@ -282,6 +282,37 @@ describe('scanUsage', () => { expect(processComparisonFile).toHaveBeenCalled(); }); + it('attaches commentWarnings from processComparisonFile onto the scan result', async () => { + vi.mocked(determineComparisonFile).mockResolvedValue({ + type: 'found', + file: { path: '/env/.env', name: '.env' }, + }); + vi.mocked(processComparisonFile).mockReturnValue({ + scanResult: { ...baseScanResult }, + comparedAgainst: '.env', + envVariables: {}, + duplicatesFound: false, + dupsEnv: [], + dupsEx: [], + fix: { + fixApplied: false, + removedDuplicates: [], + addedEnv: [], + gitignoreUpdated: false, + }, + commentWarnings: [{ key: 'UNDOC', line: 2 }], + } as ProcessComparisonResult); + + await scanUsage({ ...baseOpts, json: false }); + + expect(printScanResult).toHaveBeenCalledWith( + expect.objectContaining({ commentWarnings: [{ key: 'UNDOC', line: 2 }] }), + expect.anything(), + expect.anything(), + expect.anything(), + ); + }); + it('sets frameworkWarnings on scanResult when frameworkValidator returns results', async () => { const { frameworkValidator } = await import('../../../src/core/frameworks/frameworkValidator.js'); diff --git a/packages/cli/test/unit/services/detectMissingComments.test.ts b/packages/cli/test/unit/services/detectMissingComments.test.ts new file mode 100644 index 00000000..dcfbb9e0 --- /dev/null +++ b/packages/cli/test/unit/services/detectMissingComments.test.ts @@ -0,0 +1,56 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import fs from 'fs'; +import path from 'path'; +import os from 'os'; +import { detectMissingComments } from '../../../src/services/detectMissingComments.js'; + +describe('detectMissingComments', () => { + let dir: string; + let file: string; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'comments-unit-')); + file = path.join(dir, '.env.example'); + }); + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + const run = (content: string) => { + fs.writeFileSync(file, content); + return detectMissingComments(file); + }; + + it('accepts a key documented by a comment on the line above', () => { + expect(run('# Stripe secret\nSTRIPE_KEY=')).toEqual([]); + }); + + it('accepts a key documented by an inline comment', () => { + expect(run('PORT=3000 # server port')).toEqual([]); + }); + + it('reports a bare key with no comment', () => { + expect(run('API_KEY=')).toEqual([{ key: 'API_KEY', line: 1 }]); + }); + + it('reports each undocumented key with its line number', () => { + const content = ['# Database', 'DB_HOST=', 'DB_PORT=', 'SECRET=abc'].join( + '\n', + ); + // DB_HOST is documented (comment above); DB_PORT and SECRET are not. + expect(run(content)).toEqual([ + { key: 'DB_PORT', line: 3 }, + { key: 'SECRET', line: 4 }, + ]); + }); + + it('ignores blank lines and comment lines themselves', () => { + expect(run('\n# just a note\n\n')).toEqual([]); + }); + + it('does not treat a blank line between a comment and a key as documentation', () => { + // The line directly above the key is blank, not a comment. + expect(run('# note\n\nAPI_KEY=')).toEqual([{ key: 'API_KEY', line: 3 }]); + }); +}); diff --git a/packages/cli/test/unit/services/printScanResult.test.ts b/packages/cli/test/unit/services/printScanResult.test.ts index fce386b6..e947d0ee 100644 --- a/packages/cli/test/unit/services/printScanResult.test.ts +++ b/packages/cli/test/unit/services/printScanResult.test.ts @@ -60,6 +60,10 @@ vi.mock('../../../src/ui/scan/printUppercaseWarning.js', () => ({ printUppercaseWarning: vi.fn(), })); +vi.mock('../../../src/ui/scan/printCommentWarnings.js', () => ({ + printCommentWarnings: vi.fn(), +})); + vi.mock('../../../src/ui/scan/printExpireWarnings.js', () => ({ printExpireWarnings: vi.fn(), })); @@ -103,6 +107,7 @@ import { printListAll } from '../../../src/ui/scan/printListAll.js'; import { printExampleWarnings } from '../../../src/ui/scan/printExampleWarnings.js'; import { printSecrets } from '../../../src/ui/scan/printSecrets.js'; import { printExpireWarnings } from '../../../src/ui/scan/printExpireWarnings.js'; +import { printCommentWarnings } from '../../../src/ui/scan/printCommentWarnings.js'; import { printConsolelogWarning } from '../../../src/ui/scan/printConsolelogWarning.js'; import type { SecretFinding } from '../../../src/core/security/secretDetectors.js'; import type { @@ -345,6 +350,19 @@ describe('printScanResult', () => { expect(printExpireWarnings).toHaveBeenCalled(); }); + it('prints comment warnings when present', () => { + printScanResult( + { + ...baseScanResult, + commentWarnings: [{ key: 'DB_HOST', line: 3 }], + }, + baseOpts, + '.env', + ); + + expect(printCommentWarnings).toHaveBeenCalled(); + }); + it('returns exitWithError true when high severity example warning exists', () => { const warning: ExampleSecretWarning = { key: 'DB_PASSWORD', diff --git a/packages/cli/test/unit/services/processComparisonFile.test.ts b/packages/cli/test/unit/services/processComparisonFile.test.ts index 9e23345f..0c17fa18 100644 --- a/packages/cli/test/unit/services/processComparisonFile.test.ts +++ b/packages/cli/test/unit/services/processComparisonFile.test.ts @@ -56,6 +56,10 @@ vi.mock('../../../src/core/detectInconsistentNaming.js', () => ({ ]), })); +vi.mock('../../../src/services/detectMissingComments.js', () => ({ + detectMissingComments: vi.fn(() => [{ key: 'UNDOC', line: 2 }]), +})); + import fs from 'fs'; import { processComparisonFile } from '../../../src/services/processComparisonFile.js'; import { applyFixes } from '../../../src/services/fixEnv.js'; @@ -117,6 +121,27 @@ describe('processComparisonFile', () => { expect(result.uppercaseWarnings?.length).toBeGreaterThan(0); }); + it('detects comment warnings on the example file when enabled', () => { + const result = processComparisonFile(baseScanResult, compareFile, { + ...baseOpts, + commentWarnings: true, + }); + + expect(result.commentWarnings).toEqual([{ key: 'UNDOC', line: 2 }]); + }); + + it('falls back to the default example file and skips when it does not exist', () => { + // examplePath undefined → resolves DEFAULT_EXAMPLE_FILE; existsSync false → no detection. + vi.mocked(fs.existsSync).mockReturnValueOnce(false); + const result = processComparisonFile(baseScanResult, compareFile, { + ...baseOpts, + examplePath: undefined, + commentWarnings: true, + }); + + expect(result.commentWarnings).toEqual([]); + }); + it('detects duplicates when not allowed', () => { const result = processComparisonFile(baseScanResult, compareFile, { ...baseOpts, diff --git a/packages/cli/test/unit/ui/scan/printCommentWarnings.test.ts b/packages/cli/test/unit/ui/scan/printCommentWarnings.test.ts new file mode 100644 index 00000000..865a353d --- /dev/null +++ b/packages/cli/test/unit/ui/scan/printCommentWarnings.test.ts @@ -0,0 +1,45 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { printCommentWarnings } from '../../../../src/ui/scan/printCommentWarnings.js'; +import type { CommentWarning } from '../../../../src/config/types.js'; + +describe('printCommentWarnings', () => { + let consoleLogSpy: ReturnType; + + beforeEach(() => { + consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + consoleLogSpy.mockRestore(); + }); + + it('prints nothing when there are no warnings', () => { + printCommentWarnings([]); + expect(consoleLogSpy).not.toHaveBeenCalled(); + }); + + it('prints the header and each undocumented key', () => { + const warnings: CommentWarning[] = [ + { key: 'DB_HOST', line: 3 }, + { key: 'SECRET', line: 4 }, + ]; + + printCommentWarnings(warnings); + + const out = consoleLogSpy.mock.calls.flat().join(' '); + expect(out).toContain('Undocumented keys'); + expect(out).toContain('DB_HOST'); + expect(out).toContain('SECRET'); + expect(out).toContain('missing a documenting # comment'); + }); + + it('still prints in strict mode (error styling branch)', () => { + const warnings: CommentWarning[] = [{ key: 'API_KEY', line: 1 }]; + + printCommentWarnings(warnings, true); + + const out = consoleLogSpy.mock.calls.flat().join(' '); + expect(out).toContain('Undocumented keys'); + expect(out).toContain('API_KEY'); + }); +}); diff --git a/packages/cli/test/unit/ui/scan/scanJsonOutput.test.ts b/packages/cli/test/unit/ui/scan/scanJsonOutput.test.ts index 627447ef..459e15ff 100644 --- a/packages/cli/test/unit/ui/scan/scanJsonOutput.test.ts +++ b/packages/cli/test/unit/ui/scan/scanJsonOutput.test.ts @@ -320,6 +320,21 @@ describe('scanJsonOutput', () => { }); }); + it('includes comment warnings', () => { + const scanResult = makeScanResult({ + commentWarnings: [ + { key: 'DB_HOST', line: 3 }, + { key: 'SECRET', line: 5 }, + ], + }); + + const result = scanJsonOutput(scanResult, ''); + + expect(result.commentWarnings).toHaveLength(2); + expect(result.commentWarnings?.[0]).toEqual({ key: 'DB_HOST', line: 3 }); + expect(result.commentWarnings?.[1]).toEqual({ key: 'SECRET', line: 5 }); + }); + it('includes healthScore', () => { const scanResult = makeScanResult();