diff --git a/.changeset/diffmatrix-prototype-keys.md b/.changeset/diffmatrix-prototype-keys.md new file mode 100644 index 00000000..f50d9db1 --- /dev/null +++ b/.changeset/diffmatrix-prototype-keys.md @@ -0,0 +1,5 @@ +--- +'dotenv-diff': patch +--- + +fix matrix mode (`--matrix`) misreporting keys that share a name with a built in object member diff --git a/packages/cli/src/core/diffMatrix.ts b/packages/cli/src/core/diffMatrix.ts index e32a3d01..eab766b6 100644 --- a/packages/cli/src/core/diffMatrix.ts +++ b/packages/cli/src/core/diffMatrix.ts @@ -48,8 +48,10 @@ function buildRow( files: MatrixFileInput[], checkValues: boolean, ): MatrixRow { - const presence = files.map((f) => key in f.values); - const values = files.map((f) => f.values[key]); + const presence = files.map((f) => Object.hasOwn(f.values, key)); + const values = files.map((f) => + Object.hasOwn(f.values, key) ? f.values[key] : undefined, + ); const hasMismatch = checkValues && new Set(values.filter((_, i) => presence[i])).size > 1; diff --git a/packages/cli/test/property/core/diffMatrix.property.test.ts b/packages/cli/test/property/core/diffMatrix.property.test.ts new file mode 100644 index 00000000..202e1a27 --- /dev/null +++ b/packages/cli/test/property/core/diffMatrix.property.test.ts @@ -0,0 +1,147 @@ +import { describe, test, expect } from 'vitest'; +import fc from 'fast-check'; +import { diffMatrix } from '../../../src/core/diffMatrix.js'; +import type { MatrixFileInput } from '../../../src/config/types.js'; + +/** + * Property-based ("fuzz") tests for the env matrix diff. + * + * `diffMatrix` builds a key-presence matrix across N files. Because each file's + * values live in a plain object, a naive `key in obj` / `obj[key]` would see + * inherited members (`toString`, `constructor`, …) and report phantom presence. + * These generate thousands of random file sets — deliberately seeded with those + * prototype key names — and check the result against an independent own-property + * oracle: presence is own-property truth, values are the real string or + * undefined (never an inherited function), rows are the sorted key union, and + * `allMatch` follows from the rows. Property-based testing is also what OpenSSF + * Scorecard recognises as fuzzing for JS/TS. + */ + +const PROTO_KEYS = [ + 'toString', + 'constructor', + 'valueOf', + 'hasOwnProperty', + 'isPrototypeOf', + 'propertyIsEnumerable', + 'toLocaleString', + '__proto__', +]; + +const keyArb = fc.oneof( + fc.stringMatching(/^[A-Z_][A-Z0-9_]{0,6}$/), + fc.constantFrom(...PROTO_KEYS), + fc.string(), +); + +const fileArb: fc.Arbitrary = fc.record({ + name: fc.string(), + values: fc + .array(fc.tuple(keyArb, fc.string()), { maxLength: 8 }) + // Object.fromEntries defines *own* properties, so even '__proto__' becomes a + // real own key — matching how the parser would store a literal `__proto__=x`. + .map((pairs) => Object.fromEntries(pairs) as Record), +}); + +const filesArb = fc.array(fileArb, { maxLength: 5 }); + +/** Independent, own-property-correct re-derivation of the whole result. */ +function oracle(files: MatrixFileInput[], checkValues: boolean) { + const keys = [...new Set(files.flatMap((f) => Object.keys(f.values)))].sort(); + const rows = keys.map((key) => { + const presence = files.map((f) => Object.hasOwn(f.values, key)); + const values = files.map((f, i) => + presence[i] ? f.values[key] : undefined, + ); + const present = values.filter((_, i) => presence[i]); + const hasMismatch = checkValues && new Set(present).size > 1; + return { key, presence, values, hasMismatch }; + }); + const allMatch = rows.every( + (r) => r.presence.every(Boolean) && !r.hasMismatch, + ); + return { files: files.map((f) => f.name), rows, allMatch }; +} + +describe('diffMatrix (property-based)', () => { + test('never throws on arbitrary file sets', () => { + fc.assert( + fc.property(filesArb, fc.boolean(), (files, checkValues) => { + diffMatrix(files, checkValues); + }), + { numRuns: 3000 }, + ); + }); + + test('matches the own-property oracle exactly', () => { + fc.assert( + fc.property(filesArb, fc.boolean(), (files, checkValues) => { + expect(diffMatrix(files, checkValues)).toEqual( + oracle(files, checkValues), + ); + }), + { numRuns: 5000 }, + ); + }); + + test('presence and values never leak inherited prototype members', () => { + fc.assert( + fc.property(filesArb, fc.boolean(), (files, checkValues) => { + const res = diffMatrix(files, checkValues); + for (const row of res.rows) { + row.presence.forEach((present, i) => { + // A file is only "present" if it owns the key. + expect(present).toBe(Object.hasOwn(files[i]!.values, row.key)); + // Values are strings or undefined — never a leaked function. + const v = row.values[i]; + expect(v === undefined || typeof v === 'string').toBe(true); + if (!present) expect(v).toBeUndefined(); + }); + } + }), + { numRuns: 4000 }, + ); + }); + + test('rows are the sorted union of all files’ own keys, aligned to file count', () => { + fc.assert( + fc.property(filesArb, (files) => { + const res = diffMatrix(files); + const expectedKeys = [ + ...new Set(files.flatMap((f) => Object.keys(f.values))), + ].sort(); + expect(res.rows.map((r) => r.key)).toEqual(expectedKeys); + expect(res.files).toEqual(files.map((f) => f.name)); + for (const row of res.rows) { + expect(row.presence).toHaveLength(files.length); + expect(row.values).toHaveLength(files.length); + } + }), + { numRuns: 3000 }, + ); + }); + + test('allMatch is exactly "every key present everywhere with no mismatch"', () => { + fc.assert( + fc.property(filesArb, fc.boolean(), (files, checkValues) => { + const res = diffMatrix(files, checkValues); + const expected = res.rows.every( + (r) => r.presence.every(Boolean) && !r.hasMismatch, + ); + expect(res.allMatch).toBe(expected); + }), + { numRuns: 3000 }, + ); + }); + + test('without checkValues, no row is ever flagged as a mismatch', () => { + fc.assert( + fc.property(filesArb, (files) => { + for (const row of diffMatrix(files, false).rows) { + expect(row.hasMismatch).toBe(false); + } + }), + { numRuns: 3000 }, + ); + }); +}); diff --git a/packages/cli/test/property/core/scan/computeHealthScore.property.test.ts b/packages/cli/test/property/core/scan/computeHealthScore.property.test.ts new file mode 100644 index 00000000..b9500013 --- /dev/null +++ b/packages/cli/test/property/core/scan/computeHealthScore.property.test.ts @@ -0,0 +1,184 @@ +import { describe, test, expect } from 'vitest'; +import fc from 'fast-check'; +import type { ScanResult } from '../../../../src/config/types.js'; +import { computeHealthScore } from '../../../../src/core/scan/computeHealthScore.js'; + +/** + * Property-based ("fuzz") tests for the scan health score. + * + * The score starts at 100 and subtracts a fixed weight per issue in each + * category, then clamps to [0, 100]. These generate thousands of random issue + * count combinations to prove the score is always an integer inside [0, 100], a + * fully clean scan scores 100, more issues never raise the score (monotonic), + * only high/medium secrets are penalised, and the total matches the documented + * per-category weights. Property-based testing is also what OpenSSF Scorecard + * recognises as fuzzing for JS/TS. + */ + +interface Counts { + highSecrets: number; + medSecrets: number; + lowSecrets: number; + missing: number; + uppercase: number; + logged: number; + unused: number; + framework: number; + example: number; + expire: number; + inconsistent: number; + dupEnv: number; + dupExample: number; +} + +const fill = (n: number) => Array.from({ length: n }, () => ({})); + +/** Build a minimal ScanResult carrying exactly the requested issue counts. */ +function buildScan(c: Counts): ScanResult { + return { + missing: fill(c.missing), + unused: fill(c.unused), + logged: fill(c.logged), + uppercaseWarnings: fill(c.uppercase), + frameworkWarnings: fill(c.framework), + exampleWarnings: fill(c.example), + expireWarnings: fill(c.expire), + inconsistentNamingWarnings: fill(c.inconsistent), + secrets: [ + ...Array.from({ length: c.highSecrets }, () => ({ severity: 'high' })), + ...Array.from({ length: c.medSecrets }, () => ({ severity: 'medium' })), + ...Array.from({ length: c.lowSecrets }, () => ({ severity: 'low' })), + ], + duplicates: { env: fill(c.dupEnv), example: fill(c.dupExample) }, + } as unknown as ScanResult; +} + +/** Independent re-derivation of the documented weighting. */ +function expectedScore(c: Counts): number { + const raw = + 100 - + c.highSecrets * 20 - + c.medSecrets * 10 - + c.missing * 20 - + c.uppercase * 2 - + c.logged * 10 - + c.unused * 1 - + c.framework * 5 - + c.example * 10 - + c.expire * 5 - + c.inconsistent * 3 - + c.dupEnv * 10 - + c.dupExample * 10; + return Math.max(0, Math.min(100, raw)); +} + +const countsArb: fc.Arbitrary = fc.record({ + highSecrets: fc.nat({ max: 8 }), + medSecrets: fc.nat({ max: 8 }), + lowSecrets: fc.nat({ max: 8 }), + missing: fc.nat({ max: 8 }), + uppercase: fc.nat({ max: 8 }), + logged: fc.nat({ max: 8 }), + unused: fc.nat({ max: 8 }), + framework: fc.nat({ max: 8 }), + example: fc.nat({ max: 8 }), + expire: fc.nat({ max: 8 }), + inconsistent: fc.nat({ max: 8 }), + dupEnv: fc.nat({ max: 8 }), + dupExample: fc.nat({ max: 8 }), +}); + +describe('computeHealthScore (property-based)', () => { + test('result is always an integer in [0, 100]', () => { + fc.assert( + fc.property(countsArb, (c) => { + const score = computeHealthScore(buildScan(c)); + expect(Number.isInteger(score)).toBe(true); + expect(score).toBeGreaterThanOrEqual(0); + expect(score).toBeLessThanOrEqual(100); + }), + { numRuns: 5000 }, + ); + }); + + test('matches the documented per-category weighting', () => { + fc.assert( + fc.property(countsArb, (c) => { + expect(computeHealthScore(buildScan(c))).toBe(expectedScore(c)); + }), + { numRuns: 5000 }, + ); + }); + + test('a fully clean scan scores 100', () => { + const clean: Counts = { + highSecrets: 0, + medSecrets: 0, + lowSecrets: 0, + missing: 0, + uppercase: 0, + logged: 0, + unused: 0, + framework: 0, + example: 0, + expire: 0, + inconsistent: 0, + dupEnv: 0, + dupExample: 0, + }; + expect(computeHealthScore(buildScan(clean))).toBe(100); + }); + + test('monotonic: adding issues never raises the score', () => { + const nonNeg = fc.record({ + highSecrets: fc.nat({ max: 5 }), + medSecrets: fc.nat({ max: 5 }), + lowSecrets: fc.nat({ max: 5 }), + missing: fc.nat({ max: 5 }), + uppercase: fc.nat({ max: 5 }), + logged: fc.nat({ max: 5 }), + unused: fc.nat({ max: 5 }), + framework: fc.nat({ max: 5 }), + example: fc.nat({ max: 5 }), + expire: fc.nat({ max: 5 }), + inconsistent: fc.nat({ max: 5 }), + dupEnv: fc.nat({ max: 5 }), + dupExample: fc.nat({ max: 5 }), + }); + fc.assert( + fc.property(countsArb, nonNeg, (base, extra) => { + const bigger: Counts = { ...base }; + for (const k of Object.keys(base) as (keyof Counts)[]) { + bigger[k] = base[k] + extra[k]; + } + expect(computeHealthScore(buildScan(bigger))).toBeLessThanOrEqual( + computeHealthScore(buildScan(base)), + ); + }), + { numRuns: 5000 }, + ); + }); + + test('low-severity secrets do not affect the score', () => { + fc.assert( + fc.property(countsArb, fc.nat({ max: 20 }), (c, extraLow) => { + const withLow: Counts = { ...c, lowSecrets: c.lowSecrets + extraLow }; + expect(computeHealthScore(buildScan(withLow))).toBe( + computeHealthScore(buildScan(c)), + ); + }), + { numRuns: 3000 }, + ); + }); + + test('never throws when optional fields are absent', () => { + // Only the non-optional `missing` array is guaranteed present in the type. + fc.assert( + fc.property(fc.array(fc.string(), { maxLength: 30 }), (missing) => { + const score = computeHealthScore({ missing } as unknown as ScanResult); + expect(score).toBe(Math.max(0, 100 - missing.length * 20)); + }), + { numRuns: 2000 }, + ); + }); +}); diff --git a/packages/cli/test/property/core/scan/patterns.property.test.ts b/packages/cli/test/property/core/scan/patterns.property.test.ts new file mode 100644 index 00000000..2b8218e6 --- /dev/null +++ b/packages/cli/test/property/core/scan/patterns.property.test.ts @@ -0,0 +1,263 @@ +import { describe, test, expect } from 'vitest'; +import fc from 'fast-check'; +import { + ENV_PATTERNS, + DOCKER_COMPOSE_PATTERNS, + buildSveltekitAliasPatterns, + isDockerComposeFile, +} from '../../../../src/core/scan/patterns.js'; + +/** + * Property-based ("fuzz") tests for the env-usage detection patterns. + * + * Each pattern is a regex plus an optional processor that turns a match into a + * list of variable names; `scanFile` uses `match[1]` when there is no processor. + * These throw thousands of random source strings — real-looking accessors, + * destructuring shapes, and pure noise — at every pattern to prove extraction is + * sound (every variable it yields is a valid UPPER_SNAKE_CASE name), that it + * never throws, and that reusing the shared `/g` regex objects is stable (no + * leaked `lastIndex`). Property-based testing is also what OpenSSF Scorecard + * recognises as fuzzing for JS/TS. + */ + +/** The canonical env-var name shape the scanner is supposed to emit. */ +const ENV_VAR_NAME = /^[A-Z_][A-Z0-9_]*$/; + +const ALIAS_PATTERNS = buildSveltekitAliasPatterns( + 'privateEnv', + '$env/dynamic/private', +); +const ALL_PATTERNS = [ + ...ENV_PATTERNS, + ...DOCKER_COMPOSE_PATTERNS, + ...ALIAS_PATTERNS, +]; + +type Pat = (typeof ALL_PATTERNS)[number]; + +/** Mirror of how scanFile drives a pattern, using a *fresh* regex each call. */ +function extractFresh(pattern: Pat, content: string): string[] { + const re = new RegExp(pattern.regex.source, pattern.regex.flags); + const out: string[] = []; + let m: RegExpExecArray | null; + let guard = 0; + while ((m = re.exec(content)) !== null && guard++ < 10000) { + const vars = pattern.processor ? pattern.processor(m) : [m[1]!]; + out.push(...vars); + if (m.index === re.lastIndex) re.lastIndex++; + } + return out; +} + +/** Drive a pattern using its shared (module-level) regex object, as scanFile does. */ +function extractShared(pattern: Pat, content: string): string[] { + const re = pattern.regex; + const out: string[] = []; + let m: RegExpExecArray | null; + let guard = 0; + while ((m = re.exec(content)) !== null && guard++ < 10000) { + const vars = pattern.processor ? pattern.processor(m) : [m[1]!]; + out.push(...vars); + if (m.index === re.lastIndex) re.lastIndex++; + } + return out; +} + +// ---- source generators biased to actually hit the patterns ---- + +const upperName = fc.stringMatching(/^[A-Z_][A-Z0-9_]{0,10}$/); +const messyName = fc.oneof( + upperName, + fc.stringMatching(/^[A-Za-z0-9_$]{0,10}$/), + fc.string(), +); + +const destructBody = fc + .array( + fc.oneof( + messyName, + fc.tuple(messyName, messyName).map(([k, a]) => `${k}: ${a}`), + fc.tuple(messyName, messyName).map(([k, d]) => `${k} = ${d}`), + fc.constant(''), + fc.constant('...rest'), + ), + { maxLength: 6 }, + ) + .map((parts) => parts.join(', ')); + +const fragment = fc.oneof( + messyName.map((n) => `process.env.${n}`), + messyName.map((n) => `process.env["${n}"]`), + messyName.map((n) => `process.env['${n}']`), + messyName.map((n) => `import.meta.env.${n}`), + messyName.map((n) => `env.${n}`), + destructBody.map((b) => `const {${b}} = process.env;`), + destructBody.map((b) => `const {${b}} = env;`), + destructBody.map((b) => `const {${b}} = privateEnv;`), + messyName.map((n) => `import { ${n} } from '$env/static/private';`), + messyName.map((n) => `import ${n} from '$env/dynamic/public';`), + messyName.map((n) => `\${${n}}`), + messyName.map((n) => `\${${n}:-default}`), + messyName.map((n) => `$${n}`), + fc.string({ unit: 'binary' }), +); + +const sourceArb = fc + .array(fragment, { maxLength: 20 }) + .map((f) => f.join('\n')); + +describe('pattern extraction (property-based)', () => { + test('never throws on arbitrary source', () => { + fc.assert( + fc.property(fc.string({ unit: 'binary' }), (content) => { + for (const p of ALL_PATTERNS) extractFresh(p, content); + }), + { numRuns: 2000 }, + ); + }); + + test('every extracted variable is a valid UPPER_SNAKE_CASE name (soundness)', () => { + fc.assert( + fc.property(sourceArb, (content) => { + for (const p of ALL_PATTERNS) { + for (const v of extractFresh(p, content)) { + expect(typeof v).toBe('string'); + expect(ENV_VAR_NAME.test(v)).toBe(true); + } + } + }), + { numRuns: 4000 }, + ); + }); + + test('reusing the shared /g regex objects is stable (no leaked lastIndex)', () => { + fc.assert( + fc.property(sourceArb, (content) => { + for (const p of ALL_PATTERNS) { + const a = extractShared(p, content); + const b = extractShared(p, content); + expect(b).toEqual(a); + } + }), + { numRuns: 3000 }, + ); + }); +}); + +describe('destructuring processors as pure functions (property-based)', () => { + // Only the destructuring processors (regex captures a `{ ... }` body) do their + // own validation; the simple accessor processors rely on their regex group to + // constrain the value, so they are covered by the full-extraction test above. + const processors = ALL_PATTERNS.filter( + (p) => p.processor && p.regex.source.includes('{'), + ).map((p) => p.processor!); + + const fakeMatch = (group1: string | undefined): RegExpExecArray => + Object.assign([group1 === undefined ? '' : group1, group1], { + index: 0, + input: '', + }) as unknown as RegExpExecArray; + + test('never throw and only ever yield valid env-var names', () => { + fc.assert( + fc.property(fc.string({ unit: 'binary' }), (raw) => { + for (const processor of processors) { + const out = processor(fakeMatch(raw)); + expect(Array.isArray(out)).toBe(true); + for (const v of out) expect(ENV_VAR_NAME.test(v)).toBe(true); + } + }), + { numRuns: 4000 }, + ); + }); +}); + +describe('buildSveltekitAliasPatterns (property-based)', () => { + // The only real caller derives the alias from `(?\w+)`, so this is the + // contract the function must uphold: any word-char alias builds working + // patterns without throwing. (Regex-special aliases are out of contract.) + const wordAlias = fc.stringMatching(/^[A-Za-z_]\w{0,12}$/); + + test('word-char aliases build usable patterns without throwing', () => { + fc.assert( + fc.property(wordAlias, upperName, (alias, varName) => { + const [dot, destructure] = buildSveltekitAliasPatterns( + alias, + '$env/dynamic/private', + ); + expect(dot!.sourceModule).toBe('$env/dynamic/private'); + expect(destructure!.sourceModule).toBe('$env/dynamic/private'); + // The dot accessor matches `.VAR` and yields the var name. + expect(extractFresh(dot!, `${alias}.${varName}`)).toEqual([varName]); + // It must not fire on a *different* identifier's accessor. + expect(extractFresh(dot!, `not${alias}.${varName}`)).toEqual([]); + }), + { numRuns: 2000 }, + ); + }); +}); + +describe('isDockerComposeFile (property-based)', () => { + test('never throws on arbitrary paths', () => { + fc.assert( + fc.property(fc.string({ unit: 'binary' }), (p) => { + isDockerComposeFile(p); + }), + { numRuns: 3000 }, + ); + }); + + test('decision is case-insensitive', () => { + fc.assert( + fc.property(fc.string({ unit: 'binary' }), (p) => { + expect(isDockerComposeFile(p.toUpperCase())).toBe( + isDockerComposeFile(p.toLowerCase()), + ); + }), + { numRuns: 3000 }, + ); + }); + + test('depends only on the basename, not on leading directories', () => { + const base = fc.oneof( + fc.constantFrom( + 'compose.yml', + 'docker-compose.yaml', + 'compose.prod.yml', + 'composer.yml', + 'app.yaml', + 'notcompose.yml', + ), + fc.stringMatching(/^[a-z.-]{1,15}\.ya?ml$/), + ); + const dir = fc.stringMatching(/^[a-z]{1,6}(\/[a-z]{1,6}){0,3}$/); + fc.assert( + fc.property(base, dir, (b, d) => { + expect(isDockerComposeFile(`${d}/${b}`)).toBe(isDockerComposeFile(b)); + }), + { numRuns: 3000 }, + ); + }); + + test('matches documented positives and rejects documented negatives', () => { + for (const p of [ + 'compose.yml', + 'compose.yaml', + 'docker-compose.yml', + 'docker-compose.dev.yml', + 'a/b/compose.prod.yaml', + 'C:\\proj\\docker-compose.yml', + ]) { + expect(isDockerComposeFile(p)).toBe(true); + } + for (const p of [ + 'composer.yml', + 'app.yml', + 'compose.txt', + 'docker-compose.yml.bak', + 'not-compose.yml', + ]) { + expect(isDockerComposeFile(p)).toBe(false); + } + }); +}); diff --git a/packages/cli/test/unit/core/diffMatrix.test.ts b/packages/cli/test/unit/core/diffMatrix.test.ts index 7667271b..70dc6b63 100644 --- a/packages/cli/test/unit/core/diffMatrix.test.ts +++ b/packages/cli/test/unit/core/diffMatrix.test.ts @@ -111,6 +111,40 @@ describe('diffMatrix', () => { expect(result.allMatch).toBe(true); }); + it('does not report a prototype-named key as present in files that omit it', () => { + // Regression: `toString` is inherited from Object.prototype, so `key in + // values` / `values[key]` used to report phantom presence (and leak the + // inherited function as the value) for the file that never defined it. + const result = diffMatrix( + [ + { name: 'a.env', values: { toString: 'foo' } }, + { name: 'b.env', values: { OTHER: 'x' } }, + ], + true, + ); + + const row = result.rows.find((r) => r.key === 'toString')!; + expect(row.presence).toEqual([true, false]); + expect(row.values).toEqual(['foo', undefined]); + expect(row.hasMismatch).toBe(false); + }); + + it('treats a shared prototype-named key like any other matching key', () => { + const result = diffMatrix( + [ + { name: 'a.env', values: { constructor: '1', valueOf: 'x' } }, + { name: 'b.env', values: { constructor: '1', valueOf: 'x' } }, + ], + true, + ); + + expect(result.allMatch).toBe(true); + for (const row of result.rows) { + expect(row.presence).toEqual([true, true]); + expect(row.values.every((v) => typeof v === 'string')).toBe(true); + } + }); + it('supports comparing 3+ files at once', () => { const result = diffMatrix([ { name: '.env.production', values: { DATABASE_URL: 'a', API_KEY: 'k' } },