ci: scan pull requests for credentials and injection with ThreatCrush (#3427)
Adds a pull-request workflow that scans the diff for hardcoded credentials, injection, SSRF and unsafe deserialisation. Results go to the Security tab as SARIF and to a comment on the pull request. ### What it does on this repository ``` @profullstack/threatcrush@0.11.0 scan . 6908 files in 27.5s — 4570 finding(s): 38 high, 4061 medium, 471 low confidence: 500 evidence, 4070 pattern ``` **None of that is a claim about your code, and I have not verified any of it.** `confidence: pattern` means a regex matched and nothing more; expect false positives in that tier. It is here because the check on this pull request may never run at all — GitHub withholds workflow runs from first-time contributors, and across 24 open requests elsewhere not one has been approved. Rather than ask you to approve a run to find out what it produces, that is what it produces. Opened alongside the question in https://github.com/Runfusion/Fusion/issues/3426, which is the place to say no or ask for changes. This is only the diff, so it is there to read rather than imagine — closing either one is a fine answer. **This is not a CodeQL replacement, and it is worth saying where it differs.** CodeQL does semantic dataflow analysis and is better at it than this is — a repository already running it is not missing much by closing this. Two gaps it does fill: - Code scanning and secret scanning are free on public repositories, but need paid GitHub Code Security / Secret Protection on private ones. This is MIT and free on both, so the same gate can run across a mixed set of repositories. - CodeQL analyses a fixed set of languages, and among compiled ones it analyses only the language with the most source files unless it's explicitly configured otherwise. In a polyglot repository the rest goes unscanned by default; this reads every file it is pointed at. It is additive and report-only, so running both costs a few CI minutes and changes nothing else. **It is report-only.** `failOn` is empty, so it annotates and never fails a build. A repository with pre-existing findings should get a report on its first install, not a blocked pull request — a gate that fires on everything gets switched off within a day. Tighten it to `critical,high` in the workflow once any backlog is triaged. - `.github/workflows/threatcrush-scan.yml` — the workflow - `.github/scripts/threatcrush-to-sarif.py` — a compatibility shim for CLI versions older than native SARIF output; unused once the installed CLI can emit it itself Permissions are least-privilege (`contents: read`, `pull-requests: write`, `security-events: write`). It runs on `pull_request`, not `pull_request_target`, so contributor code never executes with your secrets in scope. The SARIF upload is `continue-on-error` and degrades quietly where code scanning is unavailable. The CLI is pinned to `@profullstack/threatcrush@0.11.0` and installed with `--ignore-scripts`, and checkout runs with `persist-credentials: false`. A scanner that installs a floating version, runs its dependencies' lifecycle scripts and leaves a token in `.git/config` is asking you to trust more than it is worth, and none of that is needed to read a diff. Bump the pin whenever you like — nothing here updates itself. Disclosure: I maintain [ThreatCrush](https://github.com/profullstack/threatcrush). It is free and MIT, and the workflow installs it from npm — nothing here phones home. If this is not something you want, closing it is the right answer, and I will not send another. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added automated ThreatCrush security scanning for pull requests. * Scan results are converted to a standardized format and uploaded for review. * Findings can update pull request comments and generate downloadable reports and artifacts. * Supports current and legacy scanner output formats. * Adds configurable severity thresholds and verified scanner installation. * **Bug Fixes** * Invalid, incomplete, or unrecognized scan output now fails safely with clear diagnostics. * Scan failures and security findings are reliably reported. * Improved handling of scan completion status and finding details. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Anthony Ettinger <anthony@chovy.com> Co-authored-by: gsxdsm <gsxdsm@users.noreply.github.com>
This commit is contained in:
Anthony Ettinger
256
.github/scripts/threatcrush-to-sarif.py
vendored
Normal file
256
.github/scripts/threatcrush-to-sarif.py
vendored
Normal file
@@ -0,0 +1,256 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Convert ThreatCrush terminal output to SARIF 2.1.0.
|
||||
|
||||
Compatibility shim for CLI versions older than native ``--format sarif``.
|
||||
When the CLI can emit SARIF itself the workflow uses that and never runs this
|
||||
file; parsing a human-readable stream is strictly worse and exists only so a
|
||||
repository is not left unscanned while waiting for a release.
|
||||
|
||||
It **fails closed**. If it cannot recognise the output it exits non-zero and
|
||||
dumps what it saw. Emitting empty SARIF instead would report "0 findings",
|
||||
which is indistinguishable from a clean scan and is the single most expensive
|
||||
thing a security tool can get wrong.
|
||||
|
||||
Three details of the format, each of which is load-bearing:
|
||||
|
||||
* Severity is bare for ``CRITICAL`` and bracketed for ``[HIGH]``/``[MEDIUM]``/
|
||||
``[LOW]``. One regex shape misses half the findings.
|
||||
* ``File:`` paths are relative to the scan root, not the repository root. Left
|
||||
unprefixed, every finding resolves to nothing in the consumer's view of the
|
||||
repo. Hence ``--path-prefix``.
|
||||
* Whole-file findings report line ``:0``. SARIF requires ``startLine >= 1``.
|
||||
|
||||
``Code:`` lines are redacted excerpts of the match. They are skipped rather
|
||||
than parsed, both because matching them would double-count every finding and
|
||||
because a redacted excerpt tells a reader nothing the ``Info:`` line does not.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import json
|
||||
import re
|
||||
import sys
|
||||
|
||||
ANSI = re.compile(r"\x1b\[[0-9;]*[A-Za-z]")
|
||||
|
||||
# ` CRITICAL AWS Access Key` / ` [HIGH] Sensitive File`
|
||||
SEVERITY_LINE = re.compile(r"^\s*(?:\[(CRITICAL|HIGH|MEDIUM|LOW|INFO)\]|(CRITICAL))\s+(.+?)\s*$")
|
||||
FILE_LINE = re.compile(r"^\s*File:\s*(.+?):(\d+)\s*$")
|
||||
INFO_LINE = re.compile(r"^\s*Info:\s*(.+?)\s*$")
|
||||
|
||||
# Proof that a scan ran to completion. Without one of these we are looking at a
|
||||
# crash, a help screen, or an unrecognised release — never at a clean result.
|
||||
#
|
||||
# FNXC:ThreatCrushFooter 2026-08-24-02:14:
|
||||
# The last non-empty line must full-match a documented completion footer.
|
||||
# `match()` plus `.*No security issues found` accepted any line that merely
|
||||
# contained the phrase — a truncated crash quoting the clean message then
|
||||
# produced empty SARIF and a green check. ThreatCrush 0.11.0 `printHuman()`
|
||||
# emits `✓ No security issues found!` or `N issue(s) found across M files`.
|
||||
# Optional bang / missing checkmark stay accepted; `--fail-on` trailer lines
|
||||
# are skipped so a requested failure cannot look like an unrecognised scan.
|
||||
FOOTER = re.compile(
|
||||
r"\s*(?:✓\s+No security issues found!|No security issues found!?|"
|
||||
r"(?P<count>\d+)\s+issue\(s\)\s+found(?:\s+across\s+\d+\s+files)?)\s*"
|
||||
)
|
||||
_FAIL_ON_TRAILER = re.compile(r"^\s*✗\s+findings at or above\b")
|
||||
|
||||
LEVELS = {"CRITICAL": "error", "HIGH": "error", "MEDIUM": "warning", "LOW": "note", "INFO": "none"}
|
||||
SECURITY_SEVERITY = {"CRITICAL": "9.0", "HIGH": "7.0", "MEDIUM": "5.0", "LOW": "3.0", "INFO": "1.0"}
|
||||
RANK = {"info": 0, "low": 1, "medium": 2, "high": 3, "critical": 4}
|
||||
|
||||
|
||||
class Unrecognised(Exception):
|
||||
"""The output did not look like a completed ThreatCrush scan."""
|
||||
|
||||
|
||||
def rule_id(title: str) -> str:
|
||||
"""Derive a stable rule id from a finding title.
|
||||
|
||||
Old CLIs print `AWS Access Key`, not `secret-aws-access-key`. Slugifying
|
||||
keeps SARIF results groupable and keeps fingerprints stable across runs,
|
||||
which is what stops the Security tab treating every run as brand-new alerts.
|
||||
"""
|
||||
slug = re.sub(r"[^a-z0-9]+", "-", title.lower()).strip("-")
|
||||
return f"threatcrush-{slug}" if slug else "threatcrush-finding"
|
||||
|
||||
|
||||
def _completion_line(lines: list[str]) -> str:
|
||||
for line in reversed(lines):
|
||||
if not line.strip() or _FAIL_ON_TRAILER.match(line):
|
||||
continue
|
||||
return line
|
||||
return ""
|
||||
|
||||
|
||||
def parse(text: str) -> list[dict]:
|
||||
lines = ANSI.sub("", text).splitlines()
|
||||
footer = FOOTER.fullmatch(_completion_line(lines))
|
||||
if footer is None:
|
||||
raise Unrecognised("no scan-completion footer found")
|
||||
# "No security issues found" has no number; that branch means zero.
|
||||
expected = int(footer.group("count") or 0)
|
||||
|
||||
findings: list[dict] = []
|
||||
pending: dict | None = None
|
||||
|
||||
for line in lines:
|
||||
severity_match = SEVERITY_LINE.match(line)
|
||||
if severity_match:
|
||||
# FNXC:ThreatCrushParse 2026-08-24-02:14:
|
||||
# A later severity used to overwrite `pending`. An incomplete
|
||||
# earlier block then vanished, and if the remaining complete
|
||||
# findings happened to match the footer count the converter
|
||||
# reported a successful under-count. Fail closed before replace.
|
||||
if pending is not None:
|
||||
raise Unrecognised(
|
||||
f"incomplete finding block: {pending.get('title', 'untitled')!r}"
|
||||
)
|
||||
severity = severity_match.group(1) or severity_match.group(2)
|
||||
pending = {"severity": severity.upper(), "title": severity_match.group(3).strip()}
|
||||
continue
|
||||
|
||||
if pending is None:
|
||||
continue
|
||||
|
||||
file_match = FILE_LINE.match(line)
|
||||
if file_match:
|
||||
pending["file"] = file_match.group(1).strip()
|
||||
pending["line"] = int(file_match.group(2))
|
||||
continue
|
||||
|
||||
info_match = INFO_LINE.match(line)
|
||||
if info_match and "file" in pending:
|
||||
pending["message"] = info_match.group(1).strip()
|
||||
findings.append(pending)
|
||||
pending = None
|
||||
|
||||
# Fail closed on a trailing half-read block. Mid-scan overwrites are
|
||||
# rejected at the next severity line so they cannot be laundered by a
|
||||
# later complete finding that happens to match the footer count.
|
||||
if pending is not None:
|
||||
raise Unrecognised(f"incomplete finding block: {pending.get('title', 'untitled')!r}")
|
||||
if len(findings) != expected:
|
||||
raise Unrecognised(f"footer reported {expected} finding(s), parsed {len(findings)}")
|
||||
|
||||
return findings
|
||||
|
||||
|
||||
def to_sarif(findings: list[dict], prefix: str, version: str) -> dict:
|
||||
rules: dict[str, dict] = {}
|
||||
results = []
|
||||
|
||||
for finding in findings:
|
||||
rid = rule_id(finding["title"])
|
||||
rules.setdefault(
|
||||
rid,
|
||||
{
|
||||
"id": rid,
|
||||
"name": rid,
|
||||
"shortDescription": {"text": finding["title"]},
|
||||
"fullDescription": {"text": finding["title"]},
|
||||
"defaultConfiguration": {"level": LEVELS[finding["severity"]]},
|
||||
"properties": {
|
||||
"tags": ["security", "threatcrush"],
|
||||
"security-severity": SECURITY_SEVERITY[finding["severity"]],
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
# removeprefix, not lstrip. lstrip takes a *set* of characters, so
|
||||
# lstrip("./") eats every leading dot and slash: `.github/workflows/x.yml`
|
||||
# became `github/workflows/x.yml` and `.env` became `env`. Both then point
|
||||
# at a path that does not exist, and `.env` is exactly the sort of file a
|
||||
# credential scanner has findings in.
|
||||
uri = finding["file"].removeprefix("./")
|
||||
if prefix:
|
||||
uri = f"{prefix.strip('/')}/{uri}"
|
||||
|
||||
results.append(
|
||||
{
|
||||
"ruleId": rid,
|
||||
"level": LEVELS[finding["severity"]],
|
||||
"message": {"text": finding.get("message", finding["title"])},
|
||||
"locations": [
|
||||
{
|
||||
"physicalLocation": {
|
||||
"artifactLocation": {"uri": uri, "uriBaseId": "%SRCROOT%"},
|
||||
# Clamped: SARIF rejects 0, and a whole-file finding
|
||||
# has no line to report.
|
||||
"region": {"startLine": max(1, finding["line"])},
|
||||
}
|
||||
}
|
||||
],
|
||||
"partialFingerprints": {
|
||||
"primaryLocationLineHash": f"{rid}:{uri}:{max(1, finding['line'])}"
|
||||
},
|
||||
"properties": {"severity": finding["severity"].lower()},
|
||||
}
|
||||
)
|
||||
|
||||
return {
|
||||
"$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
|
||||
"version": "2.1.0",
|
||||
"runs": [
|
||||
{
|
||||
"tool": {
|
||||
"driver": {
|
||||
"name": "ThreatCrush",
|
||||
"version": version,
|
||||
"informationUri": "https://threatcrush.com",
|
||||
"rules": list(rules.values()),
|
||||
}
|
||||
},
|
||||
"results": results,
|
||||
"columnKind": "utf16CodeUnits",
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--input", required=True, help="captured `threatcrush scan` output")
|
||||
parser.add_argument("--output", required=True, help="SARIF file to write")
|
||||
parser.add_argument("--path-prefix", default="", help="prepended to every file URI")
|
||||
parser.add_argument("--tool-version", default="unknown")
|
||||
parser.add_argument("--fail-on", default="", help="comma-separated severities that exit 1")
|
||||
args = parser.parse_args()
|
||||
|
||||
with open(args.input, encoding="utf-8", errors="replace") as handle:
|
||||
text = handle.read()
|
||||
|
||||
try:
|
||||
findings = parse(text)
|
||||
except Unrecognised as err:
|
||||
print(f"error: unrecognised ThreatCrush output ({err})", file=sys.stderr)
|
||||
print("--- first 40 lines ---", file=sys.stderr)
|
||||
for line in ANSI.sub("", text).splitlines()[:40]:
|
||||
print(line, file=sys.stderr)
|
||||
return 2
|
||||
|
||||
with open(args.output, "w", encoding="utf-8") as handle:
|
||||
json.dump(to_sarif(findings, args.path_prefix, args.tool_version), handle, indent=2)
|
||||
handle.write("\n")
|
||||
|
||||
print(f"converted {len(findings)} finding(s) to {args.output}")
|
||||
|
||||
thresholds = [s.strip().lower() for s in args.fail_on.split(",") if s.strip()]
|
||||
if thresholds:
|
||||
unknown = [s for s in thresholds if s not in RANK]
|
||||
if unknown:
|
||||
# Silently ignoring a typo produces a gate that never fires, which
|
||||
# looks exactly like a passing build.
|
||||
print(f"error: unknown severity in --fail-on: {', '.join(unknown)}", file=sys.stderr)
|
||||
return 2
|
||||
floor = min(RANK[s] for s in thresholds)
|
||||
if any(RANK[f["severity"].lower()] >= floor for f in findings):
|
||||
print(f"::error::findings at or above {args.fail_on}")
|
||||
return 1
|
||||
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
96
.github/scripts/threatcrush-to-sarif.test.py
vendored
Normal file
96
.github/scripts/threatcrush-to-sarif.test.py
vendored
Normal file
@@ -0,0 +1,96 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Behaviour tests for threatcrush-to-sarif.py.
|
||||
|
||||
FNXC:ThreatCrushParse 2026-08-24-02:14:
|
||||
The converter is fail-closed. These cases pin the two review findings that
|
||||
used to report a clean scan: a substring "footer" and an overwritten
|
||||
incomplete finding block.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
_SPEC = importlib.util.spec_from_file_location(
|
||||
"threatcrush_to_sarif",
|
||||
Path(__file__).with_name("threatcrush-to-sarif.py"),
|
||||
)
|
||||
assert _SPEC and _SPEC.loader
|
||||
_mod = importlib.util.module_from_spec(_SPEC)
|
||||
_SPEC.loader.exec_module(_mod)
|
||||
parse = _mod.parse
|
||||
Unrecognised = _mod.Unrecognised
|
||||
|
||||
|
||||
CLEAN = """
|
||||
Scanning . for security issues...
|
||||
✓ No security issues found!
|
||||
"""
|
||||
|
||||
FINDINGS = """
|
||||
Scan Results
|
||||
[HIGH] AWS Access Key
|
||||
File: .env:1
|
||||
Info: hardcoded credential
|
||||
1 issue(s) found across 12 files
|
||||
"""
|
||||
|
||||
|
||||
class ParseFooterTests(unittest.TestCase):
|
||||
def test_clean_scan_uses_documented_footer(self) -> None:
|
||||
self.assertEqual(parse(CLEAN), [])
|
||||
|
||||
def test_findings_footer_with_across_files(self) -> None:
|
||||
findings = parse(FINDINGS)
|
||||
self.assertEqual(len(findings), 1)
|
||||
self.assertEqual(findings[0]["title"], "AWS Access Key")
|
||||
self.assertEqual(findings[0]["file"], ".env")
|
||||
|
||||
def test_rejects_substring_footer_on_last_line(self) -> None:
|
||||
text = "error: failed to write cache: No security issues found in previous run\n"
|
||||
with self.assertRaises(Unrecognised):
|
||||
parse(text)
|
||||
|
||||
def test_rejects_embedded_clean_phrase_when_last_line_is_not_footer(self) -> None:
|
||||
text = (
|
||||
" [HIGH] AWS Access Key\n"
|
||||
" File: .env:1\n"
|
||||
" Info: hardcoded credential\n"
|
||||
"error: No security issues found in cache\n"
|
||||
)
|
||||
with self.assertRaises(Unrecognised):
|
||||
parse(text)
|
||||
|
||||
def test_skips_fail_on_trailer_after_real_footer(self) -> None:
|
||||
text = CLEAN + "\n ✗ findings at or above high — failing as requested by --fail-on\n"
|
||||
self.assertEqual(parse(text), [])
|
||||
|
||||
|
||||
class ParseIncompleteBlockTests(unittest.TestCase):
|
||||
def test_rejects_incomplete_block_before_later_severity_overwrites_it(self) -> None:
|
||||
# Footer count would match the one complete finding if the incomplete
|
||||
# CRITICAL block were silently dropped.
|
||||
text = """
|
||||
CRITICAL Incomplete Secret
|
||||
[HIGH] AWS Access Key
|
||||
File: .env:1
|
||||
Info: hardcoded credential
|
||||
1 issue(s) found across 12 files
|
||||
"""
|
||||
with self.assertRaisesRegex(Unrecognised, "incomplete finding block"):
|
||||
parse(text)
|
||||
|
||||
def test_rejects_trailing_incomplete_block(self) -> None:
|
||||
text = """
|
||||
[HIGH] AWS Access Key
|
||||
File: .env:1
|
||||
1 issue(s) found across 1 files
|
||||
"""
|
||||
with self.assertRaisesRegex(Unrecognised, "incomplete finding block"):
|
||||
parse(text)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user