diff --git a/.github/threatcrush-baseline.json b/.github/threatcrush-baseline.json deleted file mode 100644 index 710f33b..0000000 --- a/.github/threatcrush-baseline.json +++ /dev/null @@ -1,398 +0,0 @@ -{ - "_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 deleted file mode 100755 index 2d43b8a..0000000 --- a/.github/threatcrush-gate.py +++ /dev/null @@ -1,160 +0,0 @@ -#!/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 index ada18d5..8e87de6 100644 --- a/.github/workflows/security-gate.yml +++ b/.github/workflows/security-gate.yml @@ -5,12 +5,32 @@ # 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. +# This started out as a reviewed-baseline gate, because ThreatCrush 0.3.0 +# reported 56 findings here and all 56 were false positives — six of them +# high-severity, so `--fail-on high` would have blocked every pull request. +# Those were rule bugs rather than anything in this repository, and they are +# fixed in 0.4.0 (profullstack/threatcrush#76): static `innerHTML` assignments, +# short escaper aliases like `esc()`, `searchParams.set` counted as untrusted +# input, and test fixtures read as live credentials. This repository now has +# zero high-severity findings, so the plain gate does the job and ~200 lines of +# baseline machinery went away with it. +# +# The remaining findings are medium and deliberately do not block: they are +# `innerHTML` sinks in multi-line templates whose interpolations are escaped on +# a different line than the assignment, which a line-oriented scanner cannot +# see. +# +# Know what this does not catch. Severity depends on whether the scanner can +# see the taint source near the sink, so the same XSS is rated differently +# depending on how the code is arranged: +# +# innerHTML = '' + new URL(location).searchParams.get('q') + '' high, blocks +# innerHTML = '' + q + '' // q is a parameter medium, does not +# +# Both were measured. The gate therefore catches a vulnerability written in +# one place and misses one whose source sits in another function. Raising it to +# `--fail-on medium` would close that gap and currently costs 31 false +# positives; the honest position is that this gate is a floor, not a proof. name: security gate on: @@ -23,7 +43,7 @@ permissions: jobs: gate: - name: no unreviewed findings + name: no high-severity findings runs-on: ubuntu-latest timeout-minutes: 15 @@ -49,17 +69,31 @@ jobs: echo "::error::ThreatCrush install failed after 3 attempts" exit 1 - - name: Scan + - name: Record the version that ran + run: threatcrush --version + + # Fails closed in both directions. `--fail-on high` exits 1 when a + # high-or-critical finding exists; any other non-zero exit is a scan that + # did not complete, and an unexamined diff is not a clean one — so the + # step fails either way rather than treating "no output" as "no findings". + - name: Gate on high-severity findings 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 + code=0 + threatcrush scan . --fail-on high --format json --output threatcrush.json || code=$? + + if [ ! -s threatcrush.json ]; then + echo "::error::ThreatCrush produced no findings file (exit ${code}) — this diff was NOT scanned" + exit 1 + fi + + total=$(python3 -c "import json;d=json.load(open('threatcrush.json'));print(len(d['findings'] if isinstance(d,dict) else d))") + echo "ThreatCrush: ${total} finding(s), exit ${code}." >> "$GITHUB_STEP_SUMMARY" - # 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 + if [ "$code" -ne 0 ]; then + echo "::error::high-severity finding(s) present — see the Security tab or the artifact below" + exit "$code" + fi - name: Upload findings if: always() diff --git a/README.md b/README.md index ee65fe1..39ab535 100644 --- a/README.md +++ b/README.md @@ -487,36 +487,62 @@ 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 +- **`security-gate.yml`** decides whether findings stop the merge. It runs + `threatcrush scan . --fail-on high`, so a high-or-critical finding fails the + build. + +It fails closed in both directions: a scan that produces no findings file is +reported as *not scanned* rather than as clean, because an unexamined diff is +not a clean one. + +### Why the gate is `high` and not `medium` + +The scan reported **56 findings here, all 56 false positives**, six of them +high-severity — which made any `--fail-on` setting unusable, since it would +have blocked every pull request. Triaging them showed the fault was in the +rules rather than in this repository, and the fixes shipped in ThreatCrush +0.4.0 ([threatcrush#76](https://github.com/profullstack/threatcrush/pull/76)): + +- static `innerHTML` assignments reported as XSS +- the escaper guard matching `escapeHtml(` but not `esc(`, so the code that + escapes most rigorously was reported most often +- `searchParams` counted as untrusted input even when *writing* an outbound + URL, which fired the SSRF rule on constant hosts +- credentials in test fixtures treated as live + +That took this repository to **zero high-severity findings**, so the plain gate +now works and the reviewed-baseline machinery it replaced (~200 lines) is gone. + +The findings that remain are medium and deliberately do not block. They are +`innerHTML` sinks inside multi-line templates whose interpolations *are* +escaped, just on a different line from the assignment — which a line-oriented +scanner cannot see. + +### What this gate does not catch + +Severity depends on whether the scanner can see the taint source near the sink, +so the *same* vulnerability is rated differently depending on how the code is +arranged. Both of these were measured against this repository: + +| shape | severity | blocks? | +| --- | --- | --- | +| `innerHTML = '' + new URL(location).searchParams.get('q') + ''` | high | yes | +| `innerHTML = '' + q + ''`, where `q` is a parameter | medium | **no** | + +So the gate stops a vulnerability written in one place and misses one whose +source sits in another function. `--fail-on medium` would close the gap and +today costs 31 false positives, which is why it is not set. Treat this as a +floor, not a proof — it is not a substitute for review. + +To silence a finding you have established is safe, use the scanner's own +directive on the line above it, with the rule named so a *different* rule +firing there still surfaces: + +```js +// threatcrush-disable-next-line js-unescaped-html-sink +el.innerHTML = template; ``` -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: