diff --git a/apps/cli/package.json b/apps/cli/package.json index 5ef53bb..e32f243 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -1,6 +1,6 @@ { "name": "@profullstack/threatcrush", - "version": "0.7.1", + "version": "0.7.2", "description": "All-in-one security agent daemon — monitor, detect, scan, and protect servers in real-time", "bin": { "threatcrush": "./dist/index.js" @@ -36,7 +36,6 @@ "dependencies": { "@iarna/toml": "^2.2.5", "@sentry/node": "^8.45.0", - "@threatcrush/scan": "workspace:*", "better-sqlite3": "^11.7.0", "blessed": "^0.1.81", "blessed-contrib": "^4.11.0", @@ -49,6 +48,7 @@ "react-blessed-contrib": "^0.2.1" }, "devDependencies": { + "@threatcrush/scan": "workspace:*", "@types/better-sqlite3": "^7.6.12", "@types/blessed": "^0.1.25", "@types/node": "^22.19.17", diff --git a/packages/scan/package.json b/packages/scan/package.json index 1670cec..4645517 100644 --- a/packages/scan/package.json +++ b/packages/scan/package.json @@ -1,6 +1,6 @@ { "name": "@threatcrush/scan", - "version": "0.7.1", + "version": "0.7.2", "description": "ThreatCrush scan rules and engine, shared by the CLI, web, desktop and extension.", "license": "MIT", "type": "module", diff --git a/packages/scan/src/__tests__/code-rules.test.ts b/packages/scan/src/__tests__/code-rules.test.ts index 7d78268..27faeab 100644 --- a/packages/scan/src/__tests__/code-rules.test.ts +++ b/packages/scan/src/__tests__/code-rules.test.ts @@ -58,6 +58,28 @@ describe('SQL injection', () => { expect(ruleIds('a.rb', `User.where("id = '#{id}'")`)).toContain('rb-sql-interpolation'); expect(ruleIds('a.rb', `User.where('id = ?', params[:id])`)).toHaveLength(0); }); + + // A SQL verb is not SQL until it has the clause that makes it a statement. + // These are template literals whose only SQL-ness is an English word that + // happens to be a verb — real findings from ShortsStudio and capacitor. + it('does not read a verb-shaped English word as SQL', () => { + // `insert` in a React key; `INSERT` alone no longer qualifies — it needs INTO. + expect(ruleIds('a.jsx', 'key={`insert-${insertIndex}`}')).toHaveLength(0); + // `Update`/`Delete`/`drop` as prose or identifiers. + expect(ruleIds('a.js', 'log.info(`Update finished in ${ms}ms`);')).toHaveLength(0); + expect(ruleIds('a.js', 'const cls = `dropdown-${open ? "open" : "shut"}`;')).toHaveLength(0); + expect(ruleIds('a.ts', 'const label = `Delete ${count} items?`;')).toHaveLength(0); + }); + + it('still flags a genuine INSERT INTO and DROP TABLE built by interpolation', () => { + // The structured forms must survive — the fix narrows, it does not disable. + expect(ruleIds('a.js', 'db.query(`INSERT INTO users (name) VALUES (\'${name}\')`);')).toContain( + 'sql-template-interpolation', + ); + expect(ruleIds('a.js', 'db.query(`DROP TABLE ${table}`);')).toContain( + 'sql-template-interpolation', + ); + }); }); describe('command injection', () => { diff --git a/packages/scan/src/code-rules.ts b/packages/scan/src/code-rules.ts index 9d90569..930abb6 100644 --- a/packages/scan/src/code-rules.ts +++ b/packages/scan/src/code-rules.ts @@ -239,7 +239,28 @@ const EXFIL_SINK = /\bconsole\s*\.\s*(?:log|debug|info|warn|error)\s*\(|\bfetch\ * as an argument) leaves a comma after the closing quote and matches none of * them — which is exactly how the safe counterparts stay unflagged. */ -const SQL_KEYWORDS = 'SELECT|INSERT\\s+INTO|INSERT|UPDATE|DELETE\\s+FROM|DELETE|DROP|UNION\\s+SELECT'; +// Each verb requires the clause that makes it SQL rather than an English word. +// +// The bare forms — `INSERT`, `UPDATE`, `DELETE`, `DROP`, a lone `SELECT` — were +// the source of this rule's false positives: `\bINSERT\b` matches the `insert` +// in a React key `` `insert-${i}` ``, and `\bUPDATE\b` matches `Update` in a +// log line `` `Update finished in ${ms}ms` ``. Both read as SQL injection. +// +// Real SQL pairs the verb with structure — `SELECT … FROM`, `INSERT INTO`, +// `UPDATE … SET`, `DELETE FROM`, `DROP TABLE`. Requiring it keeps every +// injection shape the corpus and the unit tests exercise (all of which are +// `SELECT … FROM` or `DELETE FROM`) while a verb standing alone as prose no +// longer qualifies. The `SELECT`/`UPDATE` look-aheads stay inside one string +// literal — the character class excludes quotes and backticks — so the clause +// must live in the same statement, not merely somewhere later on the line. +const SQL_KEYWORDS = + "SELECT\\b(?=[^`'\"\\n]*\\bFROM\\b)" + + "|INSERT\\s+INTO" + + "|UPDATE\\b(?=[^`'\"\\n]*\\bSET\\b)" + + "|DELETE\\s+FROM" + + "|DROP\\s+(?:TABLE|DATABASE|INDEX|VIEW|SCHEMA)" + + "|TRUNCATE\\s+TABLE" + + "|UNION\\s+SELECT"; /** * A quoted string containing a SQL verb. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index de405b2..76fb1bd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -27,9 +27,6 @@ importers: '@sentry/node': specifier: ^8.45.0 version: 8.55.1 - '@threatcrush/scan': - specifier: workspace:* - version: link:../../packages/scan better-sqlite3: specifier: ^11.7.0 version: 11.10.0 @@ -61,6 +58,9 @@ importers: specifier: ^0.2.1 version: 0.2.1(blessed@0.1.81)(react-blessed@0.7.2(blessed@0.1.81)(react@18.3.1))(react@18.3.1) devDependencies: + '@threatcrush/scan': + specifier: workspace:* + version: link:../../packages/scan '@types/better-sqlite3': specifier: ^7.6.12 version: 7.6.13