From b36f7f9c1a2f8f7f09959dd16eed90b603de60e2 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sat, 8 Aug 2026 01:07:33 +0000 Subject: [PATCH] Triage every ThreatCrush finding, and gate on new ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things: establish whether the 56 findings are real, and make a genuine vulnerability stop the merge. TRIAGE. All 56 are false positives. Verified individually rather than assumed: - js-unescaped-html-sink (44). Extracted all 59 interpolations reaching an HTML sink across public/*.js and classified them; the 17 that could not be proven safe mechanically were read by hand. Every one is escaped through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or comes from a hardcoded constant. The HIGH one is a static string with no interpolation at all. Credit values are unescaped but the server coerces them with Number() and the packages are a literal array. - js-ssrf-outbound-request (3). URLs are a constant base plus a code-literal path; caller input reaches only URLSearchParams values, which are encoded and cannot alter scheme, host or port. - secret-* (2). Test fixtures. The auth one is a deliberately real-looking fake whose test exists to prove a real-looking key cannot send under NODE_ENV=test. - sql-template-interpolation (4). Hardcoded table names, the static ADDED_COLUMNS map, and '?' placeholders with bound arguments. - redos-nested-quantifier (1). Measured, not argued: the quantifiers are disjoint on their first character, and 60k-char adversarial input matches in 2.6ms with linear growth. - js-dynamic-code-execution (1). The jsdom harness eval'ing our own files. - insecure-temp-file (1). A string literal in a fixture. GATE. ThreatCrush 0.3.0 has no ignore file and no inline suppression; the only control is --fail-on . With every current finding a false positive, enabling it would block every PR and be switched off within a day. So the gate is "no NEW findings": the triaged 56 are recorded in threatcrush-baseline.json with justifications, and anything not listed fails. Verified by injecting a real XSS (unescaped query text into innerHTML) and confirming the gate fails with exit 1 naming that line, then removing it and confirming green. Keyed by rule + file + hash of the offending line rather than line number, so edits above a finding do not spuriously fail; per-file counts are checked too, so a second identical-looking sink is still caught. Findings located in the baseline file itself are ignored — it quotes source lines so reviewers can see what they are signing off, and those quotes are otherwise scanned as code. Lives in a new workflow rather than in threatcrush-scan.yml, which is managed by the sh1pt Actions Fleet and carries a content hash that a pack update would restore over any local edit. No product code changed: 490 tests pass, tsc clean. Co-Authored-By: Claude Opus 5 (1M context) --- .github/threatcrush-baseline.json | 398 ++++++++++++++++++++++++++++ .github/threatcrush-gate.py | 160 +++++++++++ .github/workflows/security-gate.yml | 71 +++++ README.md | 38 +++ 4 files changed, 667 insertions(+) create mode 100644 .github/threatcrush-baseline.json create mode 100755 .github/threatcrush-gate.py create mode 100644 .github/workflows/security-gate.yml diff --git a/.github/threatcrush-baseline.json b/.github/threatcrush-baseline.json new file mode 100644 index 0000000..710f33b --- /dev/null +++ b/.github/threatcrush-baseline.json @@ -0,0 +1,398 @@ +{ + "_comment": "Reviewed ThreatCrush findings. A finding not listed here fails CI. Add an entry only with a justification that says why it is not exploitable; regenerate with: threatcrush scan . --format json --output scan.json && .github/threatcrush-gate.py scan.json --update", + "counts": { + "insecure-temp-file|test/news.test.ts": 1, + "js-dynamic-code-execution|test/dashboard-crypto.test.ts": 1, + "js-ssrf-outbound-request|src/crypto/client.ts": 1, + "js-ssrf-outbound-request|src/crypto/fundamentals.ts": 1, + "js-ssrf-outbound-request|src/providers/alpaca.ts": 1, + "js-unescaped-html-sink|public/app.js": 35, + "js-unescaped-html-sink|public/auth.js": 9, + "redos-nested-quantifier|src/signals/boilerplate.ts": 1, + "secret-generic-api-key|test/auth.test.ts": 1, + "secret-generic-credential|test/credits.test.ts": 1, + "sql-template-interpolation|src/cli.ts": 2, + "sql-template-interpolation|src/server.ts": 1, + "sql-template-interpolation|src/symbols/routes.ts": 1 + }, + "findings": { + "086a84eb64fb29ff": { + "excerpt": "const m = new Mailer({ resendApiKey: \"re_****************\" });", + "file": "test/auth.test.ts", + "justification": "Not a credential: a deliberately real-looking fake in a unit test whose entire purpose is to prove that a real-looking key still cannot send mail while NODE_ENV=test. Replacing it with an obviously-fake string would weaken the test it belongs to.", + "rule": "secret-generic-api-key", + "severity": "high" + }, + "0af813eb989e8fc6": { + "excerpt": "results.innerHTML = items", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "127ef26cc621b98a": { + "excerpt": "if (!el || !LWC || bars.length < 15) { if (el) el.outerHTML = \"\"; return null; }", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "133571480936a372": { + "excerpt": "body.innerHTML = forms[mode] || forms.login;", + "file": "public/auth.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "177f948034fa67b1": { + "excerpt": "const res = await fetch(url, {", + "file": "src/providers/alpaca.ts", + "justification": "Not exploitable: the request URL is built from a constant base (a hardcoded literal or an operator-set env var) plus a code-literal path. Caller input reaches only URLSearchParams values, which are percent-encoded and cannot alter scheme, host or port. No user-supplied value is concatenated into the origin, so there is no request-forgery primitive.", + "rule": "js-ssrf-outbound-request", + "severity": "high" + }, + "295e716e06647157": { + "excerpt": "el.innerHTML = `
", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "32290ebbf68934de": { + "excerpt": "out.innerHTML = `
`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "3ab2abc39323cfa1": { + "excerpt": "out.innerHTML = `
Search failed (${esc(e.message)}).
`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "3c05a19683753b56": { + "excerpt": "/^\\s*(?:exhibit|item)\\s+\\d+(?:\\.\\d+)*/i,", + "file": "src/signals/boilerplate.ts", + "justification": "Not a ReDoS: in /\\d+(?:\\.\\d+)*/ the inner and outer quantifiers are disjoint on their first character (digit vs '.'), so there is no ambiguity for the engine to backtrack through. Measured rather than argued: 60,000-character adversarial inputs match in 2.6ms, and timing grows linearly with input length.", + "rule": "redos-nested-quantifier", + "severity": "medium" + }, + "3fd0f5811f243ecb": { + "excerpt": "list.innerHTML = `
Failed to load watchlist (${esc(e.message)}).
`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "42d86696bbc44ba3": { + "excerpt": "if (el && detailReport) el.innerHTML = reportMetaHtml(detailReport);", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "4b20536fbc556854": { + "excerpt": "el.innerHTML = `
", + "file": "public/auth.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "4e29518498e536c3": { + "excerpt": "if (body) body.innerHTML = `

Email verified

`;", + "file": "public/auth.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "538a9c49e9f3b438": { + "excerpt": "if (!authState.user) { sec.innerHTML = aiPromo(ticker); return; }", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "53bbd9aac183fcfc": { + "excerpt": "grid.innerHTML = rows.length", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "58d03d4b76e97870": { + "excerpt": "list.innerHTML = cs.length ? cs.map(candidateCard).join(\"\") : `
No candidates. Index some transcripts first (CLI: transcripts sync \"<topic>\").
`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "5953edd1cf2c1e01": { + "excerpt": "const rs = await db.execute(`SELECT COUNT(*) AS n FROM ${t}`);", + "file": "src/server.ts", + "justification": "Not injectable: the interpolated values are hardcoded identifiers \u2014 a literal table-name array, the static ADDED_COLUMNS migration map, or a run of '?' placeholders whose values are passed separately as bound arguments. No user input reaches any of these statements.", + "rule": "sql-template-interpolation", + "severity": "medium" + }, + "660538db35715a07": { + "excerpt": "el.innerHTML = `
`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "71a1c321727456a7": { + "excerpt": "if (n) n.innerHTML = `

${aEsc(msg)}

`;", + "file": "public/auth.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "722f6f0ff48f1899": { + "excerpt": "const res = await fetch(url, {", + "file": "src/crypto/client.ts", + "justification": "Not exploitable: the request URL is built from a constant base (a hardcoded literal or an operator-set env var) plus a code-literal path. Caller input reaches only URLSearchParams values, which are percent-encoded and cannot alter scheme, host or port. No user-supplied value is concatenated into the origin, so there is no request-forgery primitive.", + "rule": "js-ssrf-outbound-request", + "severity": "high" + }, + "74d2e58993d2e531": { + "excerpt": "$(\"#topic-list\").innerHTML = (topics || []).map((t) => ``).join(\"\");", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "74f9a4e5b0028e18": { + "excerpt": "sec.outerHTML = analysisBlock(dd);", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "7750701b6d6b5cdf": { + "excerpt": "win.eval([read(\"app.js\"), read(\"auth.js\")].join(\"\\n;\\n\"));", + "file": "test/dashboard-crypto.test.ts", + "justification": "Test-only, no untrusted input: the jsdom harness evaluates this repository's own public/app.js and public/auth.js, read from disk, to drive the real page. They are concatenated and eval'd together because separate eval() calls would each get their own lexical scope, which a browser does not do. Nothing external is executed and this never ships to a client.", + "rule": "js-dynamic-code-execution", + "severity": "medium" + }, + "789442634aa0cd77": { + "excerpt": "console.log(`last update: ${newest ?? \"never\"}`);", + "file": "src/cli.ts", + "justification": "Not injectable: the interpolated values are hardcoded identifiers \u2014 a literal table-name array, the static ADDED_COLUMNS migration map, or a run of '?' placeholders whose values are passed separately as bound arguments. No user input reaches any of these statements.", + "rule": "sql-template-interpolation", + "severity": "medium" + }, + "8e570b3af33107e3": { + "excerpt": "slot.innerHTML = watchlistPromo(ticker);", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "9d3e59d8986f22d0": { + "excerpt": "if (!el || !LWC || bars.length < 35) { if (el) el.outerHTML = \"\"; return null; }", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "9dd837b8d7e2884c": { + "excerpt": "list.innerHTML = items.length", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "a286f4b15c72fff4": { + "excerpt": "if (bars.length < 2) { el.outerHTML = `
No price history available.
`; return null; }", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "a6aa6988a0e4bf90": { + "excerpt": "$(\"#detail-panel\").innerHTML = `", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "a6b67dab22d1c2a0": { + "excerpt": "const rs = await db.execute(`SELECT COUNT(*) AS n FROM ${t}`);", + "file": "src/cli.ts", + "justification": "Not injectable: the interpolated values are hardcoded identifiers \u2014 a literal table-name array, the static ADDED_COLUMNS migration map, or a run of '?' placeholders whose values are passed separately as bound arguments. No user input reaches any of these statements.", + "rule": "sql-template-interpolation", + "severity": "medium" + }, + "ab1c17357feed1ae": { + "excerpt": "$(\"#detail-panel\").innerHTML = `

Loading ${esc(sym)}\u2026

`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "ac74245eefa9b395": { + "excerpt": "list.innerHTML = e.authRequired", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "b0b49f6698d34985": { + "excerpt": "if (slot && detailTicker) slot.innerHTML = detailWatchButton(detailTicker);", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "b1d7c6149e136ed8": { + "excerpt": "const res = await fetch(url, {", + "file": "src/crypto/fundamentals.ts", + "justification": "Not exploitable: the request URL is built from a constant base (a hardcoded literal or an operator-set env var) plus a code-literal path. Caller input reaches only URLSearchParams values, which are percent-encoded and cannot alter scheme, host or port. No user-supplied value is concatenated into the origin, so there is no request-forgery primitive.", + "rule": "js-ssrf-outbound-request", + "severity": "high" + }, + "bd47a1511b440de4": { + "excerpt": "if (body) body.innerHTML = `

Verifying your email\u2026

`;", + "file": "public/auth.js", + "justification": "Not exploitable: this template literal contains no interpolation at all \u2014 it is a static string ('Verifying your email\u2026' / 'Email verified'). There is no data path into it. Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant.", + "rule": "js-unescaped-html-sink", + "severity": "high" + }, + "bf650c7473ab0518": { + "excerpt": "out.innerHTML = `
Failed (${esc(e.message)}).
`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "c566caf2c73989a4": { + "excerpt": "out.innerHTML = r.length", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "d10c0a5c4ac18723": { + "excerpt": "const SECRET = \"whs**************\";", + "file": "test/credits.test.ts", + "justification": "Not a credential: a fixed test secret used to sign and verify an HMAC webhook inside the test suite. It authenticates nothing outside the tests and grants no access.", + "rule": "secret-generic-credential", + "severity": "high" + }, + "da3e2097147b608b": { + "excerpt": "if (!authState.user) { el.innerHTML = \"\"; return; }", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "dc9a3727bb394746": { + "excerpt": "sql: `SELECT ticker FROM reports WHERE ticker IN (${ph})`,", + "file": "src/symbols/routes.ts", + "justification": "Not injectable: the interpolated values are hardcoded identifiers \u2014 a literal table-name array, the static ADDED_COLUMNS migration map, or a run of '?' placeholders whose values are passed separately as bound arguments. No user input reaches any of these statements.", + "rule": "sql-template-interpolation", + "severity": "medium" + }, + "e1fd3e3fd8589d09": { + "excerpt": "if (el) el.innerHTML = `Rebuilding ${esc(ticker)} from live market data\u2026`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "e22e4bc05969eee5": { + "excerpt": "$(\"#detail-panel\").innerHTML = `
Failed to load ${esc(sym)} (${esc(e.message)}).
`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "e6bfa510a3d3f9f7": { + "excerpt": "if (el) el.innerHTML = `${esc(e.message)}`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "e7edfd5c76da40ed": { + "excerpt": "if (!grid.dataset.loaded) grid.innerHTML = `

Could not load crypto prices (${esc(e.message)}).

`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "e9d4fc696bfac23b": { + "excerpt": "eventType: \"news_article\", tickers: [\"VST\"], localPath: \"/tmp/x\", contentType: \"text/plain\",", + "file": "test/news.test.ts", + "justification": "Not a temp file: the flagged '/tmp/x' is a string literal in a test fixture describing a document record. Nothing opens, creates or writes to that path.", + "rule": "insecure-temp-file", + "severity": "medium" + }, + "ebd42867741b60bd": { + "excerpt": "if (!authState.user.emailVerified) { sec.innerHTML = aiVerifyPrompt(); return; }", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "ed859eccf6cc9f31": { + "excerpt": "el.innerHTML = `
Email updates", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "ef937d094678e04d": { + "excerpt": "$(\"#about-stats\").innerHTML = [", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "f21e209518c7ab71": { + "excerpt": "el.innerHTML = u", + "file": "public/auth.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "f2684fc617b5ee81": { + "excerpt": "if (!creditState) { el.innerHTML = \"\"; return; }", + "file": "public/auth.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "f52437d15bc07631": { + "excerpt": "out.innerHTML = s.length", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + }, + "fd36fc855899fdc4": { + "excerpt": "if (!grid.dataset.loaded) grid.innerHTML = `
`;", + "file": "public/app.js", + "justification": "Not exploitable: every interpolation reaching this sink is either passed through esc()/aEsc(), is a number formatted by toFixed/toLocaleString, or is a literal from a hardcoded constant. Verified by extracting all 59 interpolations into HTML sinks across public/*.js and reviewing each; the 17 the extractor could not prove safe were read individually. The rule fires on the innerHTML assignment itself, not on tainted data.", + "rule": "js-unescaped-html-sink", + "severity": "medium" + } + } +} diff --git a/.github/threatcrush-gate.py b/.github/threatcrush-gate.py new file mode 100755 index 0000000..2d43b8a --- /dev/null +++ b/.github/threatcrush-gate.py @@ -0,0 +1,160 @@ +#!/usr/bin/env python3 +"""Fail the build on any ThreatCrush finding that has not been reviewed. + +ThreatCrush 0.3.0 has no ignore file and no inline suppression: the only knob +is `--fail-on `. That is unusable as a gate here, because every one +of this repository's current findings is a false positive — turning it on would +block every pull request and the gate would be switched off within a day. + +So the gate is "no NEW findings" instead of "no findings". Everything already +triaged lives in threatcrush-baseline.json with a written justification; any +finding not in that file fails the job. Adding a genuine vulnerability +therefore stops the merge, while the known-clean 56 do not. + +Findings are keyed by (rule, file, hash of the offending line) rather than by +line number, because line numbers move whenever anything above them is edited +and a line-keyed baseline would spuriously fail on unrelated changes. The +per-(rule, file) count is checked too, so adding a second identical-looking +sink to a file that already has one is still caught. +""" +from __future__ import annotations + +import hashlib +import json +import os +import re +import sys +from collections import Counter +from pathlib import Path + +HERE = Path(__file__).parent +BASELINE_PATH = HERE / "threatcrush-baseline.json" + + +def normalize(excerpt: str) -> str: + """Collapse whitespace so reformatting does not invalidate an entry.""" + return re.sub(r"\s+", " ", (excerpt or "").strip()) + + +def fingerprint(finding: dict) -> str: + payload = "\x1f".join( + [finding.get("ruleId", ""), finding.get("file", ""), normalize(finding.get("excerpt", ""))] + ) + return hashlib.sha256(payload.encode()).hexdigest()[:16] + + +def load_findings(path: Path) -> list[dict]: + data = json.loads(path.read_text()) + if isinstance(data, dict): + data = data.get("findings", []) + # The baseline quotes each finding's source line so a reviewer can see what + # they are signing off. Those quoted lines are themselves scannable, so the + # scanner reports the baseline as vulnerable and every entry added spawns a + # fresh finding. It is inert JSON that is never executed or served, so drop + # findings located in it rather than laundering the excerpts into hashes + # and making the file unreadable to the humans who must review it. + return [f for f in data if Path(f.get("file", "")).name != BASELINE_PATH.name] + + +def describe(f: dict) -> str: + return ( + f" [{f.get('severity', '?'):<6}] {f.get('ruleId', '?')}\n" + f" {f.get('file', '?')}:{f.get('line', '?')}\n" + f" {normalize(f.get('excerpt', ''))[:120]}" + ) + + +def main() -> int: + if len(sys.argv) < 2: + print("usage: threatcrush-gate.py [--update]", file=sys.stderr) + return 2 + scan_path = Path(sys.argv[1]) + updating = "--update" in sys.argv[2:] + + if not scan_path.is_file() or not scan_path.stat().st_size: + # No scan output is not a clean scan. Fail closed — this is the exact + # failure the surrounding workflow is arranged to avoid. + print("::error::no ThreatCrush output to check — this diff was NOT scanned") + return 1 + + findings = load_findings(scan_path) + + if updating: + entries = {} + for f in findings: + entries[fingerprint(f)] = { + "rule": f.get("ruleId"), + "file": f.get("file"), + "severity": f.get("severity"), + "excerpt": normalize(f.get("excerpt", ""))[:200], + "justification": "TODO: explain why this is not exploitable", + } + BASELINE_PATH.write_text( + json.dumps( + { + "_comment": ( + "Reviewed ThreatCrush findings. A finding not listed here fails CI. " + "Add an entry only with a justification that says why it is not " + "exploitable; regenerate with: threatcrush scan . --format json " + "--output scan.json && .github/threatcrush-gate.py scan.json --update" + ), + "counts": dict(Counter(f"{f.get('ruleId')}|{f.get('file')}" for f in findings)), + "findings": entries, + }, + indent=2, + sort_keys=True, + ) + + "\n" + ) + print(f"wrote {len(entries)} baseline entries to {BASELINE_PATH.name}") + return 0 + + if not BASELINE_PATH.is_file(): + print("::error::threatcrush-baseline.json is missing; every finding is unreviewed") + return 1 + + baseline = json.loads(BASELINE_PATH.read_text()) + known = baseline.get("findings", {}) + known_counts = baseline.get("counts", {}) + + new = [f for f in findings if fingerprint(f) not in known] + counts = Counter(f"{f.get('ruleId')}|{f.get('file')}" for f in findings) + grew = [ + (k, counts[k], known_counts.get(k, 0)) + for k in counts + if counts[k] > known_counts.get(k, 0) + ] + # Reported, never fatal: failing a pull request for DELETING a finding + # would punish exactly the change everyone wants people to make. + stale = [k for k in known if k not in {fingerprint(f) for f in findings}] + + lines = [f"ThreatCrush gate: {len(findings)} finding(s), {len(known)} reviewed."] + if stale: + lines.append(f"{len(stale)} baseline entr{'y' if len(stale) == 1 else 'ies'} no longer fire and can be removed.") + + if new or grew: + lines.append("") + lines.append(f"FAILED — {len(new)} unreviewed finding(s).") + for f in new: + lines.append(describe(f)) + for key, now, before in grew: + rule, _, path = key.partition("|") + lines.append(f" count for {rule} in {path} rose {before} -> {now}") + lines.append("") + lines.append( + "If these are real, fix them. If not, review each one and add it to " + ".github/threatcrush-baseline.json with a justification." + ) + + report = "\n".join(lines) + print(report) + summary = os.environ.get("GITHUB_STEP_SUMMARY") + if summary: + with open(summary, "a") as fh: + fh.write(f"### ThreatCrush gate\n\n```\n{report}\n```\n") + + return 1 if (new or grew) else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/security-gate.yml b/.github/workflows/security-gate.yml new file mode 100644 index 0000000..ada18d5 --- /dev/null +++ b/.github/workflows/security-gate.yml @@ -0,0 +1,71 @@ +# Blocking security gate. +# +# Separate from threatcrush-scan.yml on purpose: that file is managed by the +# sh1pt Actions Fleet and carries a content hash, so local edits are liable to +# be overwritten on the next pack update. It reports findings; this decides +# whether they stop the merge. +# +# The decision is "no NEW findings" rather than "no findings". ThreatCrush 0.3.0 +# has no ignore file and no inline suppression, and every finding this +# repository currently produces is a reviewed false positive — so `--fail-on` +# would block every pull request and be switched off within a day. Instead the +# 56 triaged findings live in threatcrush-baseline.json with written +# justifications, and anything not in that file fails here. +name: security gate + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +jobs: + gate: + name: no unreviewed findings + runs-on: ubuntu-latest + timeout-minutes: 15 + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: "20" + + # Same retry shape as the scan workflow: a registry blip is not a + # security signal and must not read like one. + - name: Install ThreatCrush + run: | + for attempt in 1 2 3; do + if npm install -g "@profullstack/threatcrush@latest"; then + exit 0 + fi + delay=$((attempt * 10)) + echo "::warning::ThreatCrush install attempt ${attempt}/3 failed; retrying in ${delay}s" + sleep "${delay}" + done + echo "::error::ThreatCrush install failed after 3 attempts" + exit 1 + + - name: Scan + run: | + set -o pipefail + # Deliberately no --fail-on: the gate below decides, and it needs the + # findings file to exist regardless of what was found. + threatcrush scan . --format json --output threatcrush.json || true + + # Fails closed. An unreadable or absent findings file is not a clean + # scan, and the gate says so rather than passing by default. + - name: Gate on unreviewed findings + run: python3 .github/threatcrush-gate.py threatcrush.json + + - name: Upload findings + if: always() + uses: actions/upload-artifact@v4 + with: + name: threatcrush-findings + path: threatcrush.json + retention-days: 30 + if-no-files-found: warn diff --git a/README.md b/README.md index bd2862d..ee65fe1 100644 --- a/README.md +++ b/README.md @@ -479,6 +479,44 @@ bun test # deterministic unit tests (indicators, parsing) bun run typecheck # tsc --noEmit ``` +## Security scanning + +Every pull request is scanned by ThreatCrush. Two workflows are involved and +they do different jobs: + +- **`threatcrush-scan.yml`** reports findings and uploads SARIF to the Security + tab. It is managed by the sh1pt Actions Fleet and carries a content hash, so + do not edit it locally — a pack update will overwrite it. +- **`security-gate.yml`** decides whether findings stop the merge. + +The gate is **"no new findings"**, not "no findings". ThreatCrush 0.3.0 has no +ignore file and no inline suppression — the only control is `--fail-on +`, and since every finding this repository currently produces is a +reviewed false positive, turning that on would block every pull request and the +gate would be switched off within a day. + +So the 56 triaged findings live in +[`.github/threatcrush-baseline.json`](.github/threatcrush-baseline.json), each +with a written justification saying why it is not exploitable. Anything **not** +in that file fails CI. Introducing a real vulnerability therefore stops the +merge; the known-clean findings do not. + +Findings are keyed by rule + file + a hash of the offending line, not by line +number, so unrelated edits above them do not spuriously fail. The per-file count +is checked too, so adding a second identical-looking sink to a file that already +has one is still caught. + +To review and accept a new finding after establishing it is safe: + +```bash +threatcrush scan . --format json --output scan.json +.github/threatcrush-gate.py scan.json --update # then write real justifications +``` + +Entries whose findings no longer fire are reported as removable, but do not fail +the build — failing a pull request for *deleting* a finding would punish the +change everyone wants people to make. + ## Compliance Every ranking includes: