fix: sanitize threshold values in complexity SQL queries (#136)

* fix: sanitize threshold values in complexity SQL queries

Coerce threshold warn values through Number() and guard with isNaN
before interpolating into SQL HAVING clauses, preventing malformed
queries when non-numeric values are provided in config.

Impact: 1 functions changed, 4 affected

* test: add regression tests for non-numeric threshold sanitization

* fix: strict type validation for threshold values in complexity queries

Replace Number() coercion + isNaN with typeof === 'number' && isFinite()
to reject values like Number(""), Number(null), Number(true) that silently
coerce to valid numbers. Add maintainabilityIndex to default thresholds.
Update regression tests to verify exceeds arrays and summary.aboveWarn.

Addresses Greptile review on #136.

Impact: 2 functions changed, 1 affected

* test: verify exceeds arrays are empty with invalid thresholds

Assert that no function has exceeds when thresholds are non-numeric
strings, complementing the summary.aboveWarn === 0 assertions.

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Author: carlos-alm

src/complexity.js

@@ -673,6 +673,7 @@ export function complexityData(customDbPath, opts = {}) {
     cognitive: { warn: 15, fail: null },
     cyclomatic: { warn: 10, fail: null },
     maxNesting: { warn: 4, fail: null },
+    maintainabilityIndex: { warn: 20, fail: null },
   };
 
   // Build query
@@ -699,19 +700,21 @@ export function complexityData(customDbPath, opts = {}) {
     params.push(kindFilter);
   }
 
+  const isValidThreshold = (v) => typeof v === 'number' && Number.isFinite(v);
+
   let having = '';
   if (aboveThreshold) {
     const conditions = [];
-    if (thresholds.cognitive?.warn != null) {
+    if (isValidThreshold(thresholds.cognitive?.warn)) {
       conditions.push(`fc.cognitive >= ${thresholds.cognitive.warn}`);
     }
-    if (thresholds.cyclomatic?.warn != null) {
+    if (isValidThreshold(thresholds.cyclomatic?.warn)) {
       conditions.push(`fc.cyclomatic >= ${thresholds.cyclomatic.warn}`);
     }
-    if (thresholds.maxNesting?.warn != null) {
+    if (isValidThreshold(thresholds.maxNesting?.warn)) {
       conditions.push(`fc.max_nesting >= ${thresholds.maxNesting.warn}`);
     }
-    if (thresholds.maintainabilityIndex?.warn != null) {
+    if (isValidThreshold(thresholds.maintainabilityIndex?.warn)) {
       conditions.push(
         `fc.maintainability_index > 0 AND fc.maintainability_index <= ${thresholds.maintainabilityIndex.warn}`,
       );
@@ -758,14 +761,17 @@ export function complexityData(customDbPath, opts = {}) {
 
   const functions = filtered.map((r) => {
     const exceeds = [];
-    if (thresholds.cognitive?.warn != null && r.cognitive >= thresholds.cognitive.warn)
+    if (isValidThreshold(thresholds.cognitive?.warn) && r.cognitive >= thresholds.cognitive.warn)
       exceeds.push('cognitive');
-    if (thresholds.cyclomatic?.warn != null && r.cyclomatic >= thresholds.cyclomatic.warn)
+    if (isValidThreshold(thresholds.cyclomatic?.warn) && r.cyclomatic >= thresholds.cyclomatic.warn)
       exceeds.push('cyclomatic');
-    if (thresholds.maxNesting?.warn != null && r.max_nesting >= thresholds.maxNesting.warn)
+    if (
+      isValidThreshold(thresholds.maxNesting?.warn) &&
+      r.max_nesting >= thresholds.maxNesting.warn
+    )
       exceeds.push('maxNesting');
     if (
-      thresholds.maintainabilityIndex?.warn != null &&
+      isValidThreshold(thresholds.maintainabilityIndex?.warn) &&
       r.maintainability_index > 0 &&
       r.maintainability_index <= thresholds.maintainabilityIndex.warn
     )
@@ -817,10 +823,13 @@ export function complexityData(customDbPath, opts = {}) {
         minMI: +Math.min(...miValues).toFixed(1),
         aboveWarn: allRows.filter(
           (r) =>
-            (thresholds.cognitive?.warn != null && r.cognitive >= thresholds.cognitive.warn) ||
-            (thresholds.cyclomatic?.warn != null && r.cyclomatic >= thresholds.cyclomatic.warn) ||
-            (thresholds.maxNesting?.warn != null && r.max_nesting >= thresholds.maxNesting.warn) ||
-            (thresholds.maintainabilityIndex?.warn != null &&
+            (isValidThreshold(thresholds.cognitive?.warn) &&
+              r.cognitive >= thresholds.cognitive.warn) ||
+            (isValidThreshold(thresholds.cyclomatic?.warn) &&
+              r.cyclomatic >= thresholds.cyclomatic.warn) ||
+            (isValidThreshold(thresholds.maxNesting?.warn) &&
+              r.max_nesting >= thresholds.maxNesting.warn) ||
+            (isValidThreshold(thresholds.maintainabilityIndex?.warn) &&
               r.maintainability_index > 0 &&
               r.maintainability_index <= thresholds.maintainabilityIndex.warn),
         ).length,

tests/integration/complexity.test.js

@@ -9,10 +9,15 @@ import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
 import Database from 'better-sqlite3';
-import { afterAll, beforeAll, describe, expect, test } from 'vitest';
+import { afterAll, beforeAll, describe, expect, test, vi } from 'vitest';
 import { complexityData } from '../../src/complexity.js';
+import { loadConfig } from '../../src/config.js';
 import { initSchema } from '../../src/db.js';
 
+vi.mock('../../src/config.js', () => ({
+  loadConfig: vi.fn(() => ({})),
+}));
+
 // ─── Helpers ───────────────────────────────────────────────────────────
 
 function insertNode(db, name, kind, file, line, endLine = null) {
@@ -320,4 +325,50 @@ describe('complexityData', () => {
       expect(typeof fn.maintainabilityIndex).toBe('number');
     }
   });
+
+  // ─── Threshold sanitization (regression) ────────────────────────────
+
+  test('non-numeric threshold values do not crash SQL query', () => {
+    vi.mocked(loadConfig).mockReturnValueOnce({
+      manifesto: {
+        rules: {
+          cognitive: { warn: 'abc' },
+          cyclomatic: { warn: '123xyz' },
+          maxNesting: { warn: undefined },
+        },
+      },
+    });
+    // Should not throw — invalid thresholds are silently skipped
+    const data = complexityData(dbPath, { aboveThreshold: true });
+    expect(data.functions).toBeDefined();
+    expect(Array.isArray(data.functions)).toBe(true);
+    // With all thresholds invalid, no filtering occurs — all functions returned
+    expect(data.functions.length).toBeGreaterThanOrEqual(4);
+    expect(data.summary.aboveWarn).toBe(0);
+    // No function should have exceeds when all thresholds are invalid
+    for (const fn of data.functions) {
+      expect(fn.exceeds).toBeUndefined();
+    }
+  });
+
+  test('string-numeric thresholds are rejected (strict type check)', () => {
+    vi.mocked(loadConfig).mockReturnValueOnce({
+      manifesto: {
+        rules: {
+          cognitive: { warn: '15' },
+          cyclomatic: { warn: '10' },
+          maxNesting: { warn: '4' },
+        },
+      },
+    });
+    const data = complexityData(dbPath, { aboveThreshold: true });
+    // String thresholds fail typeof === 'number' — treated as no threshold
+    // so all functions are returned (no HAVING filter applied)
+    expect(data.functions.length).toBeGreaterThanOrEqual(4);
+    expect(data.summary.aboveWarn).toBe(0);
+    // No exceeds when thresholds are strings
+    for (const fn of data.functions) {
+      expect(fn.exceeds).toBeUndefined();
+    }
+  });
 });