From 18837ab6b664a7b9173961241a89854fdd44028b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 26 Feb 2026 16:46:29 -0700 Subject: [PATCH 1/4] 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 --- src/complexity.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/complexity.js b/src/complexity.js index 6830201e..f687554a 100644 --- a/src/complexity.js +++ b/src/complexity.js @@ -703,13 +703,16 @@ export function complexityData(customDbPath, opts = {}) { if (aboveThreshold) { const conditions = []; if (thresholds.cognitive?.warn != null) { - conditions.push(`fc.cognitive >= ${thresholds.cognitive.warn}`); + const val = Number(thresholds.cognitive.warn); + if (!Number.isNaN(val)) conditions.push(`fc.cognitive >= ${val}`); } if (thresholds.cyclomatic?.warn != null) { - conditions.push(`fc.cyclomatic >= ${thresholds.cyclomatic.warn}`); + const val = Number(thresholds.cyclomatic.warn); + if (!Number.isNaN(val)) conditions.push(`fc.cyclomatic >= ${val}`); } if (thresholds.maxNesting?.warn != null) { - conditions.push(`fc.max_nesting >= ${thresholds.maxNesting.warn}`); + const val = Number(thresholds.maxNesting.warn); + if (!Number.isNaN(val)) conditions.push(`fc.max_nesting >= ${val}`); } if (thresholds.maintainabilityIndex?.warn != null) { conditions.push( From f8cba1f4d2e5db11de3a48d06d963a0aae1be91d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 26 Feb 2026 18:16:50 -0700 Subject: [PATCH 2/4] test: add regression tests for non-numeric threshold sanitization --- tests/integration/complexity.test.js | 45 +++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/tests/integration/complexity.test.js b/tests/integration/complexity.test.js index 20fec19c..2a59f831 100644 --- a/tests/integration/complexity.test.js +++ b/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,42 @@ 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); + }); + + 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); + }); }); From 674a48afd468d6880ef27b97d9bb90524c2384b8 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 26 Feb 2026 19:07:21 -0700 Subject: [PATCH 3/4] 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 --- src/complexity.js | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/src/complexity.js b/src/complexity.js index f687554a..34614a49 100644 --- a/src/complexity.js +++ b/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,22 +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) { - const val = Number(thresholds.cognitive.warn); - if (!Number.isNaN(val)) conditions.push(`fc.cognitive >= ${val}`); + if (isValidThreshold(thresholds.cognitive?.warn)) { + conditions.push(`fc.cognitive >= ${thresholds.cognitive.warn}`); } - if (thresholds.cyclomatic?.warn != null) { - const val = Number(thresholds.cyclomatic.warn); - if (!Number.isNaN(val)) conditions.push(`fc.cyclomatic >= ${val}`); + if (isValidThreshold(thresholds.cyclomatic?.warn)) { + conditions.push(`fc.cyclomatic >= ${thresholds.cyclomatic.warn}`); } - if (thresholds.maxNesting?.warn != null) { - const val = Number(thresholds.maxNesting.warn); - if (!Number.isNaN(val)) conditions.push(`fc.max_nesting >= ${val}`); + 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}`, ); @@ -761,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 ) @@ -820,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, From 2ab45feb45921e8895b494d0105d11c2a9b3ecb2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 26 Feb 2026 19:07:53 -0700 Subject: [PATCH 4/4] 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. --- tests/integration/complexity.test.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/integration/complexity.test.js b/tests/integration/complexity.test.js index 2a59f831..850cf019 100644 --- a/tests/integration/complexity.test.js +++ b/tests/integration/complexity.test.js @@ -345,6 +345,10 @@ describe('complexityData', () => { // 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)', () => { @@ -362,5 +366,9 @@ describe('complexityData', () => { // 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(); + } }); });