Skip to content

ci: scan every pull request with ThreatCrush - #1

Merged
ralyodio merged 2 commits into
masterfrom
ci/threatcrush-security-scan
Aug 10, 2026
Merged

ci: scan every pull request with ThreatCrush#1
ralyodio merged 2 commits into
masterfrom
ci/threatcrush-security-scan

Conversation

@ralyodio

Copy link
Copy Markdown
Owner

Adds a ThreatCrush security scan on every pull request, with SARIF uploaded so findings are annotated on the diff and collected under Security → Code scanning.

Blocked on a release. The pin is 0.6.0, which is not published yet — it is the version that adds the shell support this repository needs (profullstack/threatcrush#87). Merging before that release will fail the job. Draft until then.

Why the version matters here

debtap is one bash script in a file with no extension. ThreatCrush 0.5.1 picks files by extension alone, so it never opens it:

$ threatcrush scan .
✔ Scanned 1 files          ← LICENSE / README.md
✓ No security issues found!

$ threatcrush scan debtap
✔ Scanned 0 files
✓ No security issues found!

A clean scan that scanned nothing is worse than no scan. profullstack/threatcrush#87 adds shebang detection and a shell rule set; with it, this repository scans properly.

What it finds today

8 findings, all genuine, all the same class — package sources fetched without a trustworthy channel:

line rule what
103 sh-insecure-transport-flag curl -k -s https://packages.ubuntu.com
104, 106, 108 sh-plaintext-download http://ftp.debian.org/debian/dists/sid/…
111, 113 sh-plaintext-download http://archive.ubuntu.com, http://ports.ubuntu.com
120 sh-insecure-transport-flag curl -k -L https://github.com/helixarch/virtual-packages-list-generator/…master.tar.gz
128 sh-insecure-transport-flag curl -k -C - -f https://aur.archlinux.org/packages.gz

Lines 120 and 128 are the ones worth a look: an archive and a package list fetched over HTTPS with certificate verification disabled, written into /var/cache/debtap/, and then used to build packages. Anyone able to intercept those connections chooses what gets packaged. Dropping -k on those three calls costs nothing — all three hosts present valid certificates.

The plain-HTTP fetches are inherited from upstream and are a larger change, since Debian's Contents-*.gz is served over HTTP by default. They are worth tracking rather than fixing in this PR.

Nothing is auto-fixed here; the scan is advisory.

Safety of the workflow itself

  • pull_request, not pull_request_target. The job checks out contributor code, so it must not hold this repository's secrets or write access. Forks get neither.
  • persist-credentials: false — the scanner has no reason to push.
  • permissions: limited to contents: read and security-events: write.
  • Pinned version. A scanner running on every patch is a dependency; @latest would be an unreviewed one.
  • No --fail-on. Advisory: it annotates, it does not block a merge.

What does fail the job

Only the two cases where a green tick would be false:

  1. the scanner produced no SARIF — the diff was not scanned;
  2. the scanner read zero files.

The second is specific to this repository, for the reason above. If it ever fires, it means the script stopped being recognised — which is not the same as the code being clean, and is precisely the failure this PR exists to avoid.

Runs on pull requests and pushes to master, uploads SARIF so findings are
annotated on the diff and collected under Security -> Code scanning.

Uses `pull_request` rather than `pull_request_target`, so a contributor's
code is never executed with this repository's secrets or write access, and
checks out with `persist-credentials: false`. The version is pinned: a
scanner that runs on every patch is a dependency, and an unpinned one takes
whatever the registry serves that morning.

The scan is advisory — no `--fail-on` — so it annotates without blocking a
merge. Two things do fail the job, both of them cases where a green tick
would be a lie:

  - the scanner produced no SARIF, so the diff was not scanned;
  - the scanner read zero files.

The second guard is specific to this repository. Everything here is one
bash script in a file with no extension, and a scanner that picks files by
extension alone reads none of it and still exits 0. If that check fires it
means the file stopped being recognised, which is not the same as the code
being clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
GitHub runs a `run:` block with `bash -e`, so the step died on the first
npx failure and every check below it — the ones that explain whether the
diff went unscanned — never executed. The log showed a bare npm error with
no indication of what it meant for the scan.

Capture the exit codes instead, and fail with a message naming the likely
cause when neither report is produced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ralyodio
ralyodio merged commit c4c8c94 into master Aug 10, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant