fix: inconsistent naming warning dropped when hit by a specific char

.changeset/inconsistent-naming-dedup-collision.md

@@ -0,0 +1,5 @@
+---
+'dotenv-diff': patch
+---
+
+fix an inconsistent naming warning being silently dropped when a key contained the character `|`.

packages/cli/src/core/detectInconsistentNaming.ts

@@ -20,8 +20,11 @@ export function detectInconsistentNaming(
       // Skip if either key is undefined
       if (!key1 || !key2) continue;
 
-      // Create a sorted pair key to avoid duplicate checking
-      const pairKey = [key1, key2].sort().join('|');
+      // Create a sorted pair key to avoid duplicate checking. JSON.stringify
+      // keeps the encoding injective — a plain `join('|')` would let keys that
+      // themselves contain the separator collide (e.g. `("||","_")` and
+      // `("|","_|")` both map to `_|||`), silently dropping a real warning.
+      const pairKey = JSON.stringify([key1, key2].sort());
       if (processedPairs.has(pairKey)) continue;
       processedPairs.add(pairKey);
 

packages/cli/test/property/core/detectInconsistentNaming.property.test.ts

@@ -0,0 +1,136 @@
+import { describe, test, expect } from 'vitest';
+import fc from 'fast-check';
+import { detectInconsistentNaming } from '../../../src/core/detectInconsistentNaming.js';
+
+/**
+ * Property-based ("fuzz") tests for the inconsistent-naming detector.
+ *
+ * Two keys are "inconsistently named" when they are equal after lower-casing and
+ * stripping underscores, yet not identical (e.g. `API_KEY` vs `APIKEY`). These
+ * tests throw thousands of random key sets at the detector — heavy on the
+ * characters that create near-collisions (case, underscores, and the `|`
+ * separator the dedup key is built from) — to prove it never throws, only
+ * reports genuine inconsistencies (soundness), reports every one of them
+ * (completeness), and does not depend on the order the keys arrive in.
+ * Property-based testing is also what OpenSSF Scorecard recognises as fuzzing
+ * for JS/TS.
+ */
+
+/** Reference normalizer: the exact notion of "same key" the detector uses. */
+const norm = (k: string) => k.toLowerCase().replace(/_/g, '');
+
+/** Stable, collision-free key for an unordered pair of strings. */
+const pairId = (a: string, b: string) => JSON.stringify([a, b].sort());
+
+/**
+ * Keys drawn from a tiny alphabet on purpose: short strings over {a,A,_,|}
+ * maximise both genuine inconsistencies and delimiter collisions in the dedup
+ * key (`|` is the separator a naive `join('|')` would have used).
+ */
+const trickyKey = fc.stringMatching(/^[aA_|]{0,8}$/);
+
+/** A more realistic env-style key generator. */
+const envKey = fc.stringMatching(/^[A-Za-z][A-Za-z0-9_]{0,10}$/);
+
+// Weight heavily toward the tricky alphabet so collisions are actually hit.
+const keyArb = fc.oneof(
+  { weight: 4, arbitrary: trickyKey },
+  { weight: 1, arbitrary: envKey },
+);
+
+/** All genuinely-inconsistent unordered pairs among the distinct input keys. */
+function expectedPairs(keys: string[]): Set<string> {
+  const uniq = [...new Set(keys.filter((k) => k))];
+  const pairs = new Set<string>();
+  for (let i = 0; i < uniq.length; i++) {
+    for (let j = i + 1; j < uniq.length; j++) {
+      const a = uniq[i]!;
+      const b = uniq[j]!;
+      if (a !== b && norm(a) === norm(b)) pairs.add(pairId(a, b));
+    }
+  }
+  return pairs;
+}
+
+describe('detectInconsistentNaming (property-based)', () => {
+  test('never throws on arbitrary key arrays', () => {
+    fc.assert(
+      fc.property(fc.array(fc.string({ unit: 'binary' })), (keys) => {
+        detectInconsistentNaming(keys);
+      }),
+      { numRuns: 2000 },
+    );
+  });
+
+  test('soundness: every warning is a genuine, distinct inconsistency', () => {
+    fc.assert(
+      fc.property(fc.array(keyArb, { maxLength: 12 }), (keys) => {
+        const set = new Set(keys.filter((k) => k));
+        for (const w of detectInconsistentNaming(keys)) {
+          expect(w.key1).not.toBe(w.key2);
+          expect(norm(w.key1)).toBe(norm(w.key2));
+          expect(set.has(w.key1)).toBe(true);
+          expect(set.has(w.key2)).toBe(true);
+        }
+      }),
+      { numRuns: 3000 },
+    );
+  });
+
+  test('completeness: every distinct inconsistent pair is reported exactly once', () => {
+    fc.assert(
+      fc.property(fc.array(keyArb, { maxLength: 12 }), (keys) => {
+        const out = detectInconsistentNaming(keys);
+        const reported = out.map((w) => pairId(w.key1, w.key2));
+        // No duplicate unordered pairs.
+        expect(new Set(reported).size).toBe(reported.length);
+        // Reported set equals the expected set.
+        expect(new Set(reported)).toEqual(expectedPairs(keys));
+      }),
+      { numRuns: 15000 },
+    );
+  });
+
+  test('order-independence: the set of reported pairs ignores input order', () => {
+    fc.assert(
+      fc.property(
+        fc.array(keyArb, { maxLength: 12 }),
+        fc.array(fc.integer()),
+        (keys, seed) => {
+          // Deterministic shuffle driven by the generated seed array.
+          const shuffled = keys
+            .map((k, i) => ({ k, s: seed[i] ?? i }))
+            .sort((a, b) => a.s - b.s)
+            .map((x) => x.k);
+          const a = new Set(
+            detectInconsistentNaming(keys).map((w) => pairId(w.key1, w.key2)),
+          );
+          const b = new Set(
+            detectInconsistentNaming(shuffled).map((w) =>
+              pairId(w.key1, w.key2),
+            ),
+          );
+          expect(b).toEqual(a);
+        },
+      ),
+      { numRuns: 15000 },
+    );
+  });
+
+  test('suggestion is always one of the two keys, and the underscored one when unambiguous', () => {
+    fc.assert(
+      fc.property(fc.array(keyArb, { maxLength: 12 }), (keys) => {
+        for (const w of detectInconsistentNaming(keys)) {
+          expect([w.key1, w.key2]).toContain(w.suggestion);
+          const has1 = w.key1.includes('_');
+          const has2 = w.key2.includes('_');
+          if (has1 !== has2) {
+            // Exactly one side is snake_case → that side must be suggested.
+            expect(w.suggestion.includes('_')).toBe(true);
+          }
+        }
+      }),
+      { numRuns: 3000 },
+    );
+  });
+});

packages/cli/test/unit/core/detectInconsistentNaming.test.ts

@@ -67,8 +67,7 @@ describe('detectInconsistentNaming', () => {
 
   it('skips undefined keys in the array', () => {
     const keys = ['API_KEY', undefined, 'APIKEY', undefined] as (
-      | string
-      | undefined
+      string | undefined
     )[];
     const warnings = detectInconsistentNaming(keys as string[]);
 
@@ -93,6 +92,17 @@ describe('detectInconsistentNaming', () => {
     expect(warnings[0]!.suggestion).toContain('API_KEY');
   });
 
+  it('does not drop a real inconsistency when keys contain the dedup separator', () => {
+    // Regression: with a `join('|')` dedup key, the pair ("||","_") and the
+    // pair ("|","_|") collided on "_|||", so the genuine "|"/"_|" inconsistency
+    // (both normalise to "|") was silently skipped.
+    const keys = ['||', '|', '_|', '_'];
+    const warnings = detectInconsistentNaming(keys);
+
+    const pairs = warnings.map((w) => [w.key1, w.key2].sort());
+    expect(pairs).toContainEqual(['_|', '|']);
+  });
+
   it('avoids duplicate warnings when same key appears multiple times', () => {
     const keys = ['API_KEY', 'APIKEY', 'API_KEY'];
     const warnings = detectInconsistentNaming(keys);