From 5b14e73e766ba578720a3c0aa432bb35c915a967 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Tue, 4 Aug 2026 18:10:33 +0200 Subject: [PATCH 1/5] ci: report disguised file content to code scanning Nothing in CI inspects the content of files a change adds. A payload named like an inert asset is skipped by any scanner that selects files by extension, which is how an implant force-pushed onto the head branch of an open pull request went unexamined. Classify every changed file by content with magika and report any file whose bytes are code or an executable while its extension declares a binary asset. Findings are emitted as SARIF and uploaded to code scanning, so severity, per-finding dismissal and merge blocking are configured in a ruleset rather than hand-maintained here. The script therefore exits 0 on a finding, and reserves a non-zero exit for being unable to scan at all. Cannot-scan is treated as failure throughout, because each case otherwise looks identical to a clean run: a base or head commit missing from the clone makes the diff empty, magika exits non-zero for a file it could not read, and a path holding a byte above 0x80 is quoted by git and drops out of the file list unless the diff is read with -z. Fork pull requests cap security-events at read, so the upload cannot run for them; those fail the job directly and annotate the offending lines instead. --- .github/scripts/fetch-scanners.sh | 47 ++++++ .github/scripts/find-type-mismatches.py | 145 ++++++++++++++++ .github/scripts/scan-changed-files.sh | 101 +++++++++++ .../scripts/tests/test-scan-changed-files.sh | 157 ++++++++++++++++++ .github/security-scan/README.md | 75 +++++++++ .github/workflows/security-scan.yml | 113 +++++++++++++ flake.nix | 10 +- nix/magika.nix | 74 +++++++++ 8 files changed, 721 insertions(+), 1 deletion(-) create mode 100755 .github/scripts/fetch-scanners.sh create mode 100644 .github/scripts/find-type-mismatches.py create mode 100755 .github/scripts/scan-changed-files.sh create mode 100755 .github/scripts/tests/test-scan-changed-files.sh create mode 100644 .github/security-scan/README.md create mode 100644 .github/workflows/security-scan.yml create mode 100644 nix/magika.nix diff --git a/.github/scripts/fetch-scanners.sh b/.github/scripts/fetch-scanners.sh new file mode 100755 index 0000000000..e18d0f2f39 --- /dev/null +++ b/.github/scripts/fetch-scanners.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +# +# Downloads the scanner the security scan needs, verifying its checksum before +# use. +# +# .github/scripts/fetch-scanners.sh +# +# CI only, and x86_64 Linux only. Locally the same version comes from the nix dev +# shell (nix/magika.nix) - keep the version here in step with that file. +# +# Downloading rather than installing from a package manager: apt + pipx cost ~16s +# of a 26s job, against roughly 1s for one checksummed download. + +set -euo pipefail + +# Release assets are maintainer-mutable - a tag can be deleted and re-uploaded - +# so the digest is what protects this job, not the tag. Never use a floating tag +# such as magika's `cli-latest`. +# +# magika versions its CLI separately from its Python package, and the CLI number +# is what `magika --version` reports. +readonly MAGIKA_VERSION="1.1.0" +readonly MAGIKA_SHA256="6b4c1010c84d1f4f06205ccef4597f1690bcd7744f46d841eee26426bc100485" + +dest="${1:?usage: fetch-scanners.sh }" + +if [[ "$(uname -s)-$(uname -m)" != "Linux-x86_64" ]]; then + echo "This script only handles Linux x86_64 (CI). Use 'nix develop' locally." >&2 + exit 2 +fi + +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT + +mkdir -p "$dest" + +curl --fail --silent --show-error --location --output "$tmp/magika.tar.xz" \ + "https://github.com/google/magika/releases/download/cli/v${MAGIKA_VERSION}/magika-cli-x86_64-unknown-linux-gnu.tar.xz" +printf '%s %s\n' "$MAGIKA_SHA256" "$tmp/magika.tar.xz" | sha256sum -c - >/dev/null + +# The archive nests the binary one directory deep. +tar xJf "$tmp/magika.tar.xz" -C "$dest" --strip-components=1 + +"$dest/magika" --version | grep -q "$MAGIKA_VERSION" \ + || { echo "magika is not version $MAGIKA_VERSION" >&2; exit 1; } + +echo "Fetched magika $MAGIKA_VERSION into $dest" >&2 diff --git a/.github/scripts/find-type-mismatches.py b/.github/scripts/find-type-mismatches.py new file mode 100644 index 0000000000..9af86f147d --- /dev/null +++ b/.github/scripts/find-type-mismatches.py @@ -0,0 +1,145 @@ +#!/usr/bin/env python3 +"""Report files whose content is executable but whose extension claims an asset. + +Reads magika JSONL on stdin, writes a SARIF 2.1.0 run on stdout for upload to +GitHub code scanning. Always writes a run, empty when there is nothing to +report, so an upload clears alerts that a previous commit raised. + +Used by scan-changed-files.sh; see .github/security-scan/README.md. +""" + +import json +import sys + +RULE_ID = "content-extension-mismatch" + +ASSET_EXTENSIONS = frozenset( + """ + woff woff2 ttf otf eot + png jpg jpeg gif bmp ico webp avif tiff heic psd + pdf docx xlsx pptx + zip gz bz2 xz 7z rar tar zst br lz4 cab deb rpm dmg msi + wasm so dylib dll exe o a lib bin dat db sqlite + mp3 mp4 wav mov mkv webm ogg flac avi + pack idx class pyc pyo jar img iso + """.split() +) + +EXECUTABLE_GROUPS = frozenset({"code", "executable"}) + +RULE = { + "id": RULE_ID, + "name": "ContentExtensionMismatch", + "shortDescription": {"text": "Executable content under a binary-asset extension"}, + "fullDescription": { + "text": ( + "The content is code or an executable while the extension declares " + "an inert binary asset. Scanners that pick files by extension never " + "open such a file, which is what makes it a convenient hiding place." + ) + }, + "defaultConfiguration": {"level": "error"}, + "help": { + "text": ( + "Confirm what the file really is. If it is a genuine asset, magika " + "would report a matching type, so a mismatch means either the file " + "is misnamed or its content is not what the name claims." + ) + }, +} + + +def extension_of(path: str) -> str: + name = path.rsplit("/", 1)[-1] + return name.rsplit(".", 1)[-1].lower() if "." in name else "" + + +def is_mismatch(path: str, detected: dict) -> bool: + expected = {e.lower() for e in detected.get("extensions", ())} + # The last condition spares assets that are executable by nature: a real + # .wasm is code, and magika lists "wasm" as an expected extension for it. + return ( + detected.get("group") in EXECUTABLE_GROUPS + and extension_of(path) in ASSET_EXTENSIONS + and extension_of(path) not in expected + ) + + +def make_result(path: str, detected: dict) -> dict: + return { + "ruleId": RULE_ID, + "ruleIndex": 0, + "level": "error", + "message": { + "text": ( + f"Content is {detected.get('label', 'unknown')}, but the " + f".{extension_of(path)} extension declares a binary asset." + ) + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": {"uri": path}, + "region": {"startLine": 1}, + } + } + ], + } + + +def build_sarif(results: list) -> dict: + return { + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "disguised-content", + "informationUri": "https://github.com/google/magika", + "rules": [RULE], + } + }, + "results": results, + } + ], + } + + +def main() -> int: + results = [] + + for line in sys.stdin: + line = line.strip() + if not line: + continue + try: + entry = json.loads(line) + except ValueError: + # Format drift, not a clean file. + print(f"unparseable magika output: {line[:120]}", file=sys.stderr) + return 2 + + result = entry.get("result", {}) + if result.get("status") != "ok": + # Unclassifiable, so this file leaves the check unexamined. Say so + # rather than letting it drop out silently. + print( + f"magika could not classify {entry.get('path', '?')}: " + f"{result.get('status', 'unknown')}", + file=sys.stderr, + ) + continue + + path = entry.get("path", "") + detected = result.get("value", {}).get("output", {}) + if is_mismatch(path, detected): + results.append(make_result(path, detected)) + + json.dump(build_sarif(results), sys.stdout, indent=2) + sys.stdout.write("\n") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/scripts/scan-changed-files.sh b/.github/scripts/scan-changed-files.sh new file mode 100755 index 0000000000..5a22a54d1d --- /dev/null +++ b/.github/scripts/scan-changed-files.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +# +# Scans changed files for executable content disguised as a binary asset, and +# annotates edits to files that run on repo open or build. +# +# BASE_SHA= HEAD_SHA= [SARIF_OUT=] scan-changed-files.sh +# +# Writes a SARIF run to SARIF_OUT and exits 0 even when it reported something: +# blocking a merge is code scanning merge protection's job, not this script's. +# Exits 2 when it cannot scan properly, which must never look like a clean run. + +set -euo pipefail + +SARIF_OUT="${SARIF_OUT:-results.sarif}" + +SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" +readonly SCRIPT_DIR +readonly MISMATCH_SCRIPT="$SCRIPT_DIR/find-type-mismatches.py" + +# Runs on repo open or build without being invoked. Annotated, never blocked: +# editing these is routine here. +readonly AUTO_EXEC_PATTERN='^\.(vscode|devcontainer|githooks|cursor|claude|idea|github)/|\.code-workspace$|(^|/)build\.rs$|^\.cargo/config\.toml$|^(Makefile\.toml|justfile|flake\.nix|shell\.nix)$|\.(bat|cmd|ps1)$' + +log() { printf '%s\n' "$*" >&2; } +warn() { printf '::warning::%s\n' "$*"; } +die() { log "$*"; exit 2; } + +# Reads magika JSONL on stdin. Staged in the workdir so a failure part-way cannot +# leave a truncated SARIF behind, which would upload as "nothing found". +write_sarif() { + python3 "$MISMATCH_SCRIPT" > "$workdir/sarif" || die "Could not write SARIF." + mv "$workdir/sarif" "$SARIF_OUT" +} + +command -v magika >/dev/null 2>&1 || die "magika is not installed; see .github/security-scan/README.md." +command -v python3 >/dev/null 2>&1 || die "python3 is not installed." + +[[ -n "${BASE_SHA:-}" ]] || die "BASE_SHA must be set." +[[ -n "${HEAD_SHA:-}" ]] || die "HEAD_SHA must be set." + +# A commit missing from the clone makes the diff empty, which would pass, so +# refuse. Both ends need this: a head sha goes missing when a fork PR's branch is +# force-pushed between event dispatch and checkout. +for sha in "$BASE_SHA" "$HEAD_SHA"; do + git cat-file -e "${sha}^{commit}" 2>/dev/null \ + || die "Commit ${sha} is not in this clone; needs actions/checkout with fetch-depth: 0." +done + +workdir="$(mktemp -d)" +trap 'rm -rf "$workdir"' EXIT + +# -z is required: without it core.quotePath quotes any path holding a byte above +# 0x80, the literal fails the -f test below, and a homoglyph-named payload is +# dropped from the scan silently. Staged through a file because a process +# substitution's exit status is invisible to mapfile. +git diff --name-only --diff-filter=ACMR -z "$BASE_SHA" "$HEAD_SHA" > "$workdir/changed" \ + || die "git diff ${BASE_SHA}..${HEAD_SHA} failed." +mapfile -d '' -t changed < "$workdir/changed" + +files=() +for path in "${changed[@]}"; do + if [[ -f "$path" ]]; then + # magika reports each path back in its JSONL, so a path containing a + # newline would be indistinguishable from two entries. Git permits such + # paths, so refuse rather than mis-scan. + [[ "$path" == *$'\n'* ]] && die "Path contains a newline, which cannot be scanned safely: ${path@Q}" + files+=("$path") + else + # ACMR excludes deletions and a rename reports only its destination, so + # anything else here is unexpected rather than routine. + log "Skipping ${path}: not a regular file." + fi +done + +# An empty run still has to be written: uploading it is what clears alerts an +# earlier commit raised, so skipping the write would leave them showing forever. +if (( ${#files[@]} == 0 )); then + log "No files to scan." + write_sarif < /dev/null + exit 0 +fi + +log "Scanning ${#files[@]} changed file(s)." + +# Status checked separately from the conversion below, whose status would be +# python's: a magika crash would otherwise yield no findings and pass. magika +# also exits non-zero when a listed file could not be read, reporting the reason +# in its JSONL rather than on stderr, so both streams go into the message. +magika --jsonl -- "${files[@]}" > "$workdir/detected" 2> "$workdir/magika.err" \ + || die "magika failed: $(cat "$workdir/magika.err"; head -c 300 "$workdir/detected")" +[[ -s "$workdir/magika.err" ]] && log "magika stderr: $(cat "$workdir/magika.err")" + +write_sarif < "$workdir/detected" + +for path in "${files[@]}"; do + [[ "$path" =~ $AUTO_EXEC_PATTERN ]] \ + && warn "$path: runs automatically when the repo is opened or built - review as code" +done + +log "Wrote $SARIF_OUT" +exit 0 diff --git a/.github/scripts/tests/test-scan-changed-files.sh b/.github/scripts/tests/test-scan-changed-files.sh new file mode 100755 index 0000000000..44adae41dc --- /dev/null +++ b/.github/scripts/tests/test-scan-changed-files.sh @@ -0,0 +1,157 @@ +#!/usr/bin/env bash +# +# Tests for scan-changed-files.sh, run against throwaway git repos. +# +# .github/scripts/tests/test-scan-changed-files.sh +# +# Needs magika, python3, jq and git on PATH. Exit 0 when every case passes. +# +# Locally: nix develop --command .github/scripts/tests/test-scan-changed-files.sh + +set -euo pipefail + +SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" +readonly SCRIPT_DIR +readonly SCAN="$SCRIPT_DIR/../scan-changed-files.sh" + +# JavaScript behind a long run of spaces, saved as a font: content that is code, a +# name claiming an inert asset, and a payload pushed off-screen in a diff. magika +# still types this as javascript, which is what the check keys on. +make_payload() { + { printf '%*s' 1700 '' + printf "globalThis['r']=require;\n" + printf "function go(x){return x*2};\nmodule.exports={go};\n" + } > "$1" +} + +passed=0 +failed=0 + +check() { + local name="$1" expected="$2" actual="$3" + if [[ "$expected" == "$actual" ]]; then + printf 'ok %s\n' "$name" + passed=$((passed + 1)) + else + printf 'FAIL %s (expected %s, got %s)\n' "$name" "$expected" "$actual" + failed=$((failed + 1)) + fi +} + +# A repo with one commit on main. +new_repo() { + local dir; dir="$(mktemp -d)" + git -C "$dir" init -q -b main + git -C "$dir" config user.email t@example.com + git -C "$dir" config user.name Test + echo seed > "$dir/README.md" + git -C "$dir" add -A + git -C "$dir" commit -qm seed + printf '%s' "$dir" +} + +# Runs the scan with SARIF written outside the repo under test, so the report +# cannot show up in a later diff. Sets SARIF for findings() to read. +SARIF="" +scan_in() { + local dir="$1" base="$2" head="$3" + SARIF="$(mktemp)" + ( cd "$dir" && BASE_SHA="$base" HEAD_SHA="$head" SARIF_OUT="$SARIF" "$SCAN" >/dev/null 2>&1 ) +} + +findings() { + jq '[.runs[].results[]] | length' "$SARIF" +} + +rev() { git -C "$1" rev-parse "$2"; } + +# --- the payload is reported, under an ASCII name and a homoglyph one ---------- +# The Cyrillic 'a' matters: git quotes such paths unless the diff is read -z, and +# a quoted path silently drops out of the file list. +for name in 'fa-solid-400.woff2' "$(printf 'f\xd0\xb0-solid-400.woff2')"; do + repo="$(new_repo)" + mkdir -p "$repo/public/fonts" + make_payload "$repo/public/fonts/$name" + git -C "$repo" add -A + git -C "$repo" commit -qm payload + rc=0; scan_in "$repo" "$(rev "$repo" HEAD~1)" "$(rev "$repo" HEAD)" || rc=$? + check "scan succeeds: $name" 0 "$rc" + check "payload reported: $name" 1 "$(findings)" + rm -rf "$repo" +done + +# --- a genuine font is left alone -------------------------------------------- +repo="$(new_repo)" +mkdir -p "$repo/public/fonts" +printf 'wOF2\x00\x01\x00\x00' > "$repo/public/fonts/real.woff2" +head -c 4000 /dev/urandom >> "$repo/public/fonts/real.woff2" +git -C "$repo" add -A +git -C "$repo" commit -qm font +rc=0; scan_in "$repo" "$(rev "$repo" HEAD~1)" "$(rev "$repo" HEAD)" || rc=$? +check "genuine woff2 exits 0" 0 "$rc" +check "genuine woff2 reports nothing" 0 "$(findings)" +rm -rf "$repo" + +# --- a payload in an earlier commit of a multi-commit range is still found ---- +# Regression for the base-commit fallback: scanning only the tip missed this. +repo="$(new_repo)" +mkdir -p "$repo/public/fonts" +make_payload "$repo/public/fonts/fa-solid-400.woff2" +git -C "$repo" add -A +git -C "$repo" commit -qm payload +echo later > "$repo/later.txt" +git -C "$repo" add -A +git -C "$repo" commit -qm later +scan_in "$repo" "$(rev "$repo" HEAD~2)" "$(rev "$repo" HEAD)" +check "payload in a non-tip commit reported" 1 "$(findings)" +# ... and confirm the narrow range really would have missed it, so the case bites +scan_in "$repo" "$(rev "$repo" HEAD~1)" "$(rev "$repo" HEAD)" +check "tip-only range misses it (why the fallback matters)" 0 "$(findings)" +rm -rf "$repo" + +# --- refusing to scan nothing ------------------------------------------------ +repo="$(new_repo)" +head="$(rev "$repo" HEAD)" +missing=deadbeefdeadbeefdeadbeefdeadbeefdeadbeef + +rc=0; scan_in "$repo" "$head" "$missing" || rc=$? +check "absent head sha exits 2" 2 "$rc" + +rc=0; scan_in "$repo" "$missing" "$head" || rc=$? +check "absent base sha exits 2" 2 "$rc" + +sarif_out="$(mktemp)" +rc=0; ( cd "$repo" && BASE_SHA="$head" SARIF_OUT="$sarif_out" "$SCAN" >/dev/null 2>&1 ) || rc=$? +check "missing HEAD_SHA exits 2" 2 "$rc" + +# An empty diff must still write a run: uploading it is what clears alerts an +# earlier commit raised. +rc=0; scan_in "$repo" "$head" "$head" || rc=$? +check "empty diff exits 0" 0 "$rc" +check "empty diff still writes an empty run" 0 "$(findings)" +rm -rf "$repo" + +# --- a file the scanner cannot read must not pass ----------------------------- +repo="$(new_repo)" +echo secret > "$repo/unreadable.txt" +git -C "$repo" add -A +git -C "$repo" commit -qm unreadable +chmod 000 "$repo/unreadable.txt" +rc=0; scan_in "$repo" "$(rev "$repo" HEAD~1)" "$(rev "$repo" HEAD)" || rc=$? +chmod 644 "$repo/unreadable.txt" +check "unreadable file exits 2" 2 "$rc" +rm -rf "$repo" + +# --- a path containing a newline must be refused, not silently mis-listed ----- +repo="$(new_repo)" +printf 'x\n' > "$repo/$(printf 'we\nird.txt')" 2>/dev/null || true +if git -C "$repo" add -A 2>/dev/null && git -C "$repo" commit -qm newline 2>/dev/null; then + rc=0; scan_in "$repo" "$(rev "$repo" HEAD~1)" "$(rev "$repo" HEAD)" || rc=$? + check "newline in path exits 2" 2 "$rc" +else + printf 'skip newline case (filesystem rejected the name)\n' +fi +rm -rf "$repo" + +printf '\n%d passed, %d failed\n' "$passed" "$failed" +(( failed == 0 )) diff --git a/.github/security-scan/README.md b/.github/security-scan/README.md new file mode 100644 index 0000000000..8118d772e8 --- /dev/null +++ b/.github/security-scan/README.md @@ -0,0 +1,75 @@ +# Scanning changed files for disguised content + +`.github/workflows/security-scan.yml` inspects the *content* of every file a push or pull +request touches, and reports any file whose bytes are code or an executable while its +extension claims an inert binary asset: JavaScript named `.woff2`, an ELF named `.png`, and +so on. + +Findings are uploaded as [SARIF][sarif] and become code scanning alerts. Blocking a merge is +configured in a ruleset, not in the script. + +## Why content and not extension + +Selecting files by extension is the blind spot this exists to close. Scanners that filter +candidates by name never open a payload called `fa-solid-400.woff2`, so the disguise works +precisely because tooling declines to look. [magika][magika] classifies by content, so the +name cannot hide anything from it. + +The check fires only on a three-way conjunction, in `find-type-mismatches.py`: magika reports +the content as code or executable, *and* the extension is one that promises inert data, *and* +magika does not list that extension as expected for the type it detected. The third condition +is what spares assets that are executable by nature, such as a genuine `.wasm`. + +## Running it locally + +The dev shell provides magika at the version CI uses: + +```sh +nix develop --command bash -c ' + BASE_SHA=$(git merge-base origin/main HEAD) HEAD_SHA=$(git rev-parse HEAD) \ + .github/scripts/scan-changed-files.sh' +``` + +That writes `results.sarif`. Tests: +`nix develop --command .github/scripts/tests/test-scan-changed-files.sh`. + +## What blocks a merge + +Nothing in the script does. It always exits 0 when it merely *found* something, and exits 2 +only when it could not scan properly. Gating is a **code scanning** rule on the branch +ruleset, with the alerts threshold set to `Errors`, matching the `level` the SARIF emits. + +That rule also blocks when the tool's analysis is still running or is not configured at all, +so a job that never reports cannot be mistaken for a clean result. Using it instead of a +required status check avoids a trap: this workflow skips itself for same-repo pull requests to +avoid scanning twice, the skipped run publishes a check with the same name as the real one, +and GitHub counts a skipped check as satisfying a requirement. + +To accept a finding, dismiss the alert in the repository's Security tab. That is per finding +and it persists, so there is no allowlist file to maintain. + +## Fork pull requests + +Forks cap `security-events` at read, so SARIF cannot be uploaded for them. The workflow fails +the job directly in that case, which annotates the offending lines instead. Same detection, +different reporting surface. + +## Version pinning + +magika is pinned twice and both must move together: + +| | local | CI | +|---|---|---| +| magika | `nix/magika.nix` | `.github/scripts/fetch-scanners.sh` | + +Both take the same upstream release binary, checksummed. Deliberately not from nixpkgs: on +Darwin that build installs a binary with no `LC_RPATH` that cannot resolve +`libonnxruntime.dylib` and aborts on every call. + +CI downloads rather than using nix because nix is the slower path *for this job*: installing +Nix plus restoring its cache is measured at 7s + 22s here, against roughly 1s for one +checksummed download. Release assets are maintainer-mutable, so the pinned digest is the +protection, not the tag. Never use a floating tag such as magika's `cli-latest`. + +[sarif]: https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/uploading-a-sarif-file-to-github +[magika]: https://github.com/google/magika diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml new file mode 100644 index 0000000000..0a124e2c5d --- /dev/null +++ b/.github/workflows/security-scan.yml @@ -0,0 +1,113 @@ +name: Security Scan + +# Runs on every branch, not just PRs. The implant this job exists to catch was +# force-pushed onto the head branches of already-open pull requests, and a push +# to a branch with no open PR fires no `pull_request` event at all. +on: + pull_request: + push: + branches: + - '**' + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + scan-changed-files: + name: "Scan changed files" + # A push covers every same-repo branch, including a push to the branch of an + # already-merged PR, which fires no pull_request event - that is how the + # implant this job exists to catch arrived. Fork PRs produce no push event + # here, so pull_request covers those; the guard keeps same-repo PRs from being + # scanned twice. + if: >- + github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name != github.repository + runs-on: ubuntu-latest + timeout-minutes: 10 + + permissions: + contents: read + security-events: write # github/codeql-action/upload-sarif + + steps: + - name: Checkout repository + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + # Full history: the scan diffs against a base commit, and with a + # shallow clone that object is absent. The script treats a missing + # base as a hard error rather than scanning nothing and passing. + fetch-depth: 0 + persist-credentials: false + + - name: Install the scanner + run: | + .github/scripts/fetch-scanners.sh "$RUNNER_TEMP/bin" + echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" + + - name: Resolve the base commit + id: base + env: + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PUSH_BEFORE: ${{ github.event.before }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + run: | + set -euo pipefail + if [ -n "$PR_BASE_SHA" ]; then + base="$PR_BASE_SHA" + elif git cat-file -e "${PUSH_BEFORE:-missing}^{commit}" 2>/dev/null; then + base="$PUSH_BEFORE" + else + # `before` is all-zeroes when the push created the branch, and on a + # force-push it is the previous tip, which is now orphaned and so not + # fetched even at fetch-depth 0. Both land here, and both are cases + # where the push carries more than one new commit - so fall back to + # the whole branch delta. HEAD~1 would scan only the tip and leave + # the rest of a force-pushed range unexamined. + base="$(git merge-base "origin/${DEFAULT_BRANCH}" HEAD)" + # Pushing to the default branch itself: merge-base is HEAD. + if [ "$base" = "$(git rev-parse HEAD)" ]; then + base="$(git rev-parse HEAD~1)" + fi + fi + echo "sha=$base" >> "$GITHUB_OUTPUT" + + - name: Test the scanner + run: .github/scripts/tests/test-scan-changed-files.sh + + - name: Scan + env: + BASE_SHA: ${{ steps.base.outputs.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + SARIF_OUT: ${{ runner.temp }}/results.sarif + run: .github/scripts/scan-changed-files.sh + + # Findings become code scanning alerts, and a ruleset code scanning rule is + # what blocks a merge on them. See .github/security-scan/README.md. + - name: Upload results to code scanning + if: github.event_name != 'pull_request' + uses: github/codeql-action/upload-sarif@c3400c2f38909e0dcf3c3a41f2030a8217be5d3e # v3 + with: + sarif_file: ${{ runner.temp }}/results.sarif + category: disguised-content + + # Fork PRs cap `security-events` at read, so the upload above cannot run for + # them. Fail the job directly instead, so a fork's findings still surface. + - name: Fail on findings + if: github.event_name == 'pull_request' + env: + SARIF: ${{ runner.temp }}/results.sarif + run: | + set -euo pipefail + jq -r '.runs[].results[] + | "::error file=\(.locations[0].physicalLocation.artifactLocation.uri)::\(.message.text)"' "$SARIF" + count="$(jq '[.runs[].results[]] | length' "$SARIF")" + if [ "$count" -ne 0 ]; then + echo "$count finding(s); see the annotations above." >&2 + exit 1 + fi diff --git a/flake.nix b/flake.nix index ad9d1f31d4..13ea6ae086 100644 --- a/flake.nix +++ b/flake.nix @@ -92,6 +92,7 @@ # Pinned to CI version cargoTools = pkgs.callPackage ./nix/cargo-tools.nix { }; opengrep = pkgs.callPackage ./nix/opengrep.nix { }; + magika = pkgs.callPackage ./nix/magika.nix { }; libcDev = lib.getDev stdenv.cc.libc; @@ -223,7 +224,14 @@ nearTools ++ miscTools ++ buildLibs ++ - [ opengrep ]; + [ + opengrep + # Used by .github/scripts/scan-changed-files.sh and its tests, so + # both are runnable locally. CI installs the same version from a + # pinned release binary instead, because a nix setup costs that + # job more than the download does. + magika + ]; env = envCommon // envDarwin; diff --git a/nix/magika.nix b/nix/magika.nix new file mode 100644 index 0000000000..f3888b532d --- /dev/null +++ b/nix/magika.nix @@ -0,0 +1,74 @@ +{ + lib, + stdenv, + fetchurl, + autoPatchelfHook, +}: + +# nixpkgs ships magika-cli, but it is unusable on Darwin: the binary it installs +# has no LC_RPATH, so it cannot resolve libonnxruntime.dylib and aborts on any +# invocation. Upstream's own release binaries work on both platforms, so take +# those, as nix/opengrep.nix does for the same reason. +let + version = "1.1.0"; + + assets = { + x86_64-linux = { + target = "x86_64-unknown-linux-gnu"; + hash = "sha256-a0wQEMhNH08GIFzO9Fl/FpC813RPRthB7uJkJrwQBIU="; + }; + aarch64-linux = { + target = "aarch64-unknown-linux-gnu"; + hash = "sha256-IYPkdF4fDlnRtwZtH/iP5AmzjJ6/EZqd5qga/RDmqvc="; + }; + aarch64-darwin = { + target = "aarch64-apple-darwin"; + hash = "sha256-VLicWen/AkaZBmF4xF5Y1zGDPMJbfBObQoV9XTG3uAQ="; + }; + }; + + asset = assets.${stdenv.hostPlatform.system} or (throw "magika: unsupported system ${stdenv.hostPlatform.system}"); +in +stdenv.mkDerivation { + pname = "magika"; + inherit version; + + src = fetchurl { + # Releases are tagged cli/vX.Y.Z; that version is also what `magika + # --version` reports, unlike the separately numbered Python package. + url = "https://github.com/google/magika/releases/download/cli/v${version}/magika-cli-${asset.target}.tar.xz"; + inherit (asset) hash; + }; + + # The Linux binary is dynamically linked, so it names an interpreter that does + # not exist under nix and fails with "cannot execute: required file not found" + # without this. + nativeBuildInputs = lib.optionals stdenv.hostPlatform.isLinux [ autoPatchelfHook ]; + buildInputs = lib.optionals stdenv.hostPlatform.isLinux [ stdenv.cc.cc.lib ]; + + # The tarball's single directory becomes the source root, so the binary is at + # the top level here rather than under magika-cli-/. + installPhase = '' + runHook preInstall + install -Dm755 magika $out/bin/magika + runHook postInstall + ''; + + # Runs the binary, so it catches a platform whose prebuilt artifact cannot + # actually start - which is exactly how nixpkgs' Darwin build is broken. + doInstallCheck = true; + installCheckPhase = '' + runHook preInstallCheck + $out/bin/magika --version | grep -q '${version}' + runHook postInstallCheck + ''; + + meta = { + description = "Determines file content types using a deep-learning model"; + homepage = "https://github.com/google/magika"; + license = lib.licenses.asl20; + mainProgram = "magika"; + platforms = builtins.attrNames assets; + sourceProvenance = [ lib.sourceTypes.binaryNativeCode ]; + }; +} From 179fc8501bca39359eafa9dc648b4cbd66f89c5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Tue, 4 Aug 2026 18:37:49 +0200 Subject: [PATCH 2/5] refactor: drop the scan shell scripts for one python script magika ships a Python API, so classifying files no longer needs a CLI binary, JSONL parsing, or exit-code juggling across three shell scripts. Range resolution moves out of inline workflow bash into the scanner, and uv installs the pinned dependency from the script's own inline metadata, which removes the fetch script and the nix derivation along with their duplicated version. The workflow is now only `uses:` steps and single-command `run:` steps. A path with a newline no longer has to be refused: paths are passed as arguments rather than through a newline-delimited list. --- .github/scripts/fetch-scanners.sh | 47 ---- .github/scripts/find-type-mismatches.py | 145 ---------- .github/scripts/scan-changed-files.py | 257 ++++++++++++++++++ .github/scripts/scan-changed-files.sh | 101 ------- .../scripts/tests/test-scan-changed-files.py | 174 ++++++++++++ .../scripts/tests/test-scan-changed-files.sh | 157 ----------- .github/security-scan/README.md | 81 ++---- .github/workflows/security-scan.yml | 85 ++---- flake.nix | 11 +- nix/magika.nix | 74 ----- 10 files changed, 477 insertions(+), 655 deletions(-) delete mode 100755 .github/scripts/fetch-scanners.sh delete mode 100644 .github/scripts/find-type-mismatches.py create mode 100644 .github/scripts/scan-changed-files.py delete mode 100755 .github/scripts/scan-changed-files.sh create mode 100644 .github/scripts/tests/test-scan-changed-files.py delete mode 100755 .github/scripts/tests/test-scan-changed-files.sh delete mode 100644 nix/magika.nix diff --git a/.github/scripts/fetch-scanners.sh b/.github/scripts/fetch-scanners.sh deleted file mode 100755 index e18d0f2f39..0000000000 --- a/.github/scripts/fetch-scanners.sh +++ /dev/null @@ -1,47 +0,0 @@ -#!/usr/bin/env bash -# -# Downloads the scanner the security scan needs, verifying its checksum before -# use. -# -# .github/scripts/fetch-scanners.sh -# -# CI only, and x86_64 Linux only. Locally the same version comes from the nix dev -# shell (nix/magika.nix) - keep the version here in step with that file. -# -# Downloading rather than installing from a package manager: apt + pipx cost ~16s -# of a 26s job, against roughly 1s for one checksummed download. - -set -euo pipefail - -# Release assets are maintainer-mutable - a tag can be deleted and re-uploaded - -# so the digest is what protects this job, not the tag. Never use a floating tag -# such as magika's `cli-latest`. -# -# magika versions its CLI separately from its Python package, and the CLI number -# is what `magika --version` reports. -readonly MAGIKA_VERSION="1.1.0" -readonly MAGIKA_SHA256="6b4c1010c84d1f4f06205ccef4597f1690bcd7744f46d841eee26426bc100485" - -dest="${1:?usage: fetch-scanners.sh }" - -if [[ "$(uname -s)-$(uname -m)" != "Linux-x86_64" ]]; then - echo "This script only handles Linux x86_64 (CI). Use 'nix develop' locally." >&2 - exit 2 -fi - -tmp="$(mktemp -d)" -trap 'rm -rf "$tmp"' EXIT - -mkdir -p "$dest" - -curl --fail --silent --show-error --location --output "$tmp/magika.tar.xz" \ - "https://github.com/google/magika/releases/download/cli/v${MAGIKA_VERSION}/magika-cli-x86_64-unknown-linux-gnu.tar.xz" -printf '%s %s\n' "$MAGIKA_SHA256" "$tmp/magika.tar.xz" | sha256sum -c - >/dev/null - -# The archive nests the binary one directory deep. -tar xJf "$tmp/magika.tar.xz" -C "$dest" --strip-components=1 - -"$dest/magika" --version | grep -q "$MAGIKA_VERSION" \ - || { echo "magika is not version $MAGIKA_VERSION" >&2; exit 1; } - -echo "Fetched magika $MAGIKA_VERSION into $dest" >&2 diff --git a/.github/scripts/find-type-mismatches.py b/.github/scripts/find-type-mismatches.py deleted file mode 100644 index 9af86f147d..0000000000 --- a/.github/scripts/find-type-mismatches.py +++ /dev/null @@ -1,145 +0,0 @@ -#!/usr/bin/env python3 -"""Report files whose content is executable but whose extension claims an asset. - -Reads magika JSONL on stdin, writes a SARIF 2.1.0 run on stdout for upload to -GitHub code scanning. Always writes a run, empty when there is nothing to -report, so an upload clears alerts that a previous commit raised. - -Used by scan-changed-files.sh; see .github/security-scan/README.md. -""" - -import json -import sys - -RULE_ID = "content-extension-mismatch" - -ASSET_EXTENSIONS = frozenset( - """ - woff woff2 ttf otf eot - png jpg jpeg gif bmp ico webp avif tiff heic psd - pdf docx xlsx pptx - zip gz bz2 xz 7z rar tar zst br lz4 cab deb rpm dmg msi - wasm so dylib dll exe o a lib bin dat db sqlite - mp3 mp4 wav mov mkv webm ogg flac avi - pack idx class pyc pyo jar img iso - """.split() -) - -EXECUTABLE_GROUPS = frozenset({"code", "executable"}) - -RULE = { - "id": RULE_ID, - "name": "ContentExtensionMismatch", - "shortDescription": {"text": "Executable content under a binary-asset extension"}, - "fullDescription": { - "text": ( - "The content is code or an executable while the extension declares " - "an inert binary asset. Scanners that pick files by extension never " - "open such a file, which is what makes it a convenient hiding place." - ) - }, - "defaultConfiguration": {"level": "error"}, - "help": { - "text": ( - "Confirm what the file really is. If it is a genuine asset, magika " - "would report a matching type, so a mismatch means either the file " - "is misnamed or its content is not what the name claims." - ) - }, -} - - -def extension_of(path: str) -> str: - name = path.rsplit("/", 1)[-1] - return name.rsplit(".", 1)[-1].lower() if "." in name else "" - - -def is_mismatch(path: str, detected: dict) -> bool: - expected = {e.lower() for e in detected.get("extensions", ())} - # The last condition spares assets that are executable by nature: a real - # .wasm is code, and magika lists "wasm" as an expected extension for it. - return ( - detected.get("group") in EXECUTABLE_GROUPS - and extension_of(path) in ASSET_EXTENSIONS - and extension_of(path) not in expected - ) - - -def make_result(path: str, detected: dict) -> dict: - return { - "ruleId": RULE_ID, - "ruleIndex": 0, - "level": "error", - "message": { - "text": ( - f"Content is {detected.get('label', 'unknown')}, but the " - f".{extension_of(path)} extension declares a binary asset." - ) - }, - "locations": [ - { - "physicalLocation": { - "artifactLocation": {"uri": path}, - "region": {"startLine": 1}, - } - } - ], - } - - -def build_sarif(results: list) -> dict: - return { - "$schema": "https://json.schemastore.org/sarif-2.1.0.json", - "version": "2.1.0", - "runs": [ - { - "tool": { - "driver": { - "name": "disguised-content", - "informationUri": "https://github.com/google/magika", - "rules": [RULE], - } - }, - "results": results, - } - ], - } - - -def main() -> int: - results = [] - - for line in sys.stdin: - line = line.strip() - if not line: - continue - try: - entry = json.loads(line) - except ValueError: - # Format drift, not a clean file. - print(f"unparseable magika output: {line[:120]}", file=sys.stderr) - return 2 - - result = entry.get("result", {}) - if result.get("status") != "ok": - # Unclassifiable, so this file leaves the check unexamined. Say so - # rather than letting it drop out silently. - print( - f"magika could not classify {entry.get('path', '?')}: " - f"{result.get('status', 'unknown')}", - file=sys.stderr, - ) - continue - - path = entry.get("path", "") - detected = result.get("value", {}).get("output", {}) - if is_mismatch(path, detected): - results.append(make_result(path, detected)) - - json.dump(build_sarif(results), sys.stdout, indent=2) - sys.stdout.write("\n") - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/.github/scripts/scan-changed-files.py b/.github/scripts/scan-changed-files.py new file mode 100644 index 0000000000..e7032b738f --- /dev/null +++ b/.github/scripts/scan-changed-files.py @@ -0,0 +1,257 @@ +#!/usr/bin/env python3 +# /// script +# requires-python = ">=3.10" +# dependencies = ["magika==1.0.3"] +# /// +"""Report changed files whose content is executable but whose extension claims an asset. + + uv run scan-changed-files.py + +Range comes from the GitHub event, or from BASE_SHA and HEAD_SHA when both are set. +Writes SARIF to SARIF_OUT (default `results.sarif`). + +Exit 0 even on a finding, since merge protection does the blocking; 1 instead when +FAIL_ON_FINDINGS is set, for fork PRs that cannot upload SARIF; 2 when it could +not scan, which must never look clean. + +See .github/security-scan/README.md. +""" + +import json +import os +import re +import subprocess +import sys +from pathlib import Path + +from magika import Magika + +RULE_ID = "content-extension-mismatch" + +ASSET_EXTENSIONS = frozenset( + """ + woff woff2 ttf otf eot + png jpg jpeg gif bmp ico webp avif tiff heic psd + pdf docx xlsx pptx + zip gz bz2 xz 7z rar tar zst br lz4 cab deb rpm dmg msi + wasm so dylib dll exe o a lib bin dat db sqlite + mp3 mp4 wav mov mkv webm ogg flac avi + pack idx class pyc pyo jar img iso + """.split() +) + +EXECUTABLE_GROUPS = frozenset({"code", "executable"}) + +# Runs on repo open or build without being invoked. Warned, never blocked. +AUTO_EXEC = re.compile( + r"^\.(vscode|devcontainer|githooks|cursor|claude|idea|github)/" + r"|\.code-workspace$" + r"|(^|/)build\.rs$" + r"|^\.cargo/config\.toml$" + r"|^(Makefile\.toml|justfile|flake\.nix|shell\.nix)$" + r"|\.(bat|cmd|ps1)$" +) + +RULE = { + "id": RULE_ID, + "name": "ContentExtensionMismatch", + "shortDescription": {"text": "Executable content under a binary-asset extension"}, + "fullDescription": { + "text": ( + "Content is code or an executable while the extension declares an " + "inert asset. Extension-based scanners never open such a file, which " + "is what makes it a hiding place." + ) + }, + "defaultConfiguration": {"level": "error"}, +} + + +def die(message: str) -> None: + print(message, file=sys.stderr) + sys.exit(2) + + +def note(message: str) -> None: + print(message, file=sys.stderr) + + +def env_flag(name: str) -> bool: + # Workflow expressions render "false", which a plain get() reads as set. + return os.environ.get(name, "").strip().lower() in {"1", "true", "yes"} + + +def git_out(*args: str) -> str: + result = subprocess.run(["git", *args], capture_output=True, text=True) + if result.returncode: + die(f"git {' '.join(args)} failed: {result.stderr.strip()}") + return result.stdout.strip() + + +def has_commit(sha: str) -> bool: + return not subprocess.run( + ["git", "cat-file", "-e", f"{sha}^{{commit}}"], capture_output=True + ).returncode + + +def branch_delta(default_branch: str) -> str: + """Base for a range whose start is unknown or unreachable. + + `before` is zeroes on branch creation and orphaned after a force-push. Both + carry several commits, so HEAD~1 would leave most of the range unexamined. + """ + base = git_out("merge-base", f"origin/{default_branch}", "HEAD") + # Pushing to the default branch itself: merge-base is HEAD. + return git_out("rev-parse", "HEAD~1") if base == git_out("rev-parse", "HEAD") else base + + +def resolve_range() -> tuple[str, str]: + base, head = os.environ.get("BASE_SHA"), os.environ.get("HEAD_SHA") + if base and head: + return base, head + + event_path = os.environ.get("GITHUB_EVENT_PATH") + if not event_path: + die("Set BASE_SHA and HEAD_SHA, or run inside a GitHub event.") + event = json.loads(Path(event_path).read_text()) + + if pull_request := event.get("pull_request"): + return pull_request["base"]["sha"], pull_request["head"]["sha"] + + default_branch = event.get("repository", {}).get("default_branch", "main") + before = event.get("before") + head = os.environ.get("GITHUB_SHA") or git_out("rev-parse", "HEAD") + return (before if before and has_commit(before) else branch_delta(default_branch)), head + + +def require_commit(sha: str, name: str) -> None: + # A commit absent from the clone empties the diff, which would look clean. + if not has_commit(sha): + die(f"{name} {sha} is not in this clone; needs actions/checkout with fetch-depth: 0.") + + +def changed_files(base: str, head: str) -> list[str]: + # -z: core.quotePath would quote any byte above 0x80, and the quoted literal + # then matches no file on disk, dropping a homoglyph-named payload. + result = subprocess.run( + ["git", "diff", "--name-only", "--diff-filter=ACMR", "-z", base, head], + capture_output=True, + ) + if result.returncode: + die(f"git diff {base}..{head} failed: {result.stderr.decode(errors='replace').strip()}") + return [p for p in result.stdout.decode("utf-8", "surrogateescape").split("\0") if p] + + +def extension_of(path: str) -> str: + name = path.rsplit("/", 1)[-1] + return name.rsplit(".", 1)[-1].lower() if "." in name else "" + + +def is_mismatch(path: str, output) -> bool: + # Last condition spares assets that are code by nature: magika lists "wasm" + # as expected for a real .wasm. + extension = extension_of(path) + return ( + output.group in EXECUTABLE_GROUPS + and extension in ASSET_EXTENSIONS + and extension not in {e.lower() for e in output.extensions} + ) + + +def make_result(path: str, output) -> dict: + return { + "ruleId": RULE_ID, + "ruleIndex": 0, + "level": "error", + "message": { + "text": ( + f"Content is {output.label}, but the .{extension_of(path)} " + f"extension declares a binary asset." + ) + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": {"uri": path}, + "region": {"startLine": 1}, + } + } + ], + } + + +def build_sarif(results: list) -> dict: + return { + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "disguised-content", + "informationUri": "https://github.com/google/magika", + "rules": [RULE], + } + }, + "results": results, + } + ], + } + + +def main() -> int: + if bool(os.environ.get("BASE_SHA")) != bool(os.environ.get("HEAD_SHA")): + die("BASE_SHA and HEAD_SHA must be set together, or neither.") + + base, head = resolve_range() + sarif_out = Path(os.environ.get("SARIF_OUT", "results.sarif")) + + require_commit(base, "base") + require_commit(head, "head") + + paths = [] + for path in changed_files(base, head): + if Path(path).is_file(): + paths.append(path) + else: + # ACMR excludes deletions, so anything else here is unexpected. + note(f"Skipping {path}: not a regular file.") + + results = [] + if paths: + note(f"Scanning {len(paths)} changed file(s).") + identified = Magika().identify_paths([Path(p) for p in paths]) + for path, outcome in zip(paths, identified): + # Unread file means unexamined, which must not report as clean. + if not outcome.ok: + die(f"magika could not read {path}: {outcome.status}") + if is_mismatch(path, outcome.output): + results.append(make_result(path, outcome.output)) + else: + note("No files to scan.") + + # Written even when empty: uploading that is what clears an earlier commit's + # alerts. A failed write must exit 2, not surface as a traceback's 1. + try: + sarif_out.write_text(json.dumps(build_sarif(results), indent=2) + "\n") + except OSError as error: + die(f"Could not write {sarif_out}: {error}") + + for result in results: + uri = result["locations"][0]["physicalLocation"]["artifactLocation"]["uri"] + print(f"::error file={uri}::{result['message']['text']}") + + for path in paths: + if AUTO_EXEC.search(path): + print( + f"::warning::{path}: runs automatically when the repo is opened " + "or built - review as code" + ) + + note(f"Wrote {sarif_out} with {len(results)} finding(s).") + # Fork PRs cap security-events at read, so the exit code is the only signal. + return 1 if results and env_flag("FAIL_ON_FINDINGS") else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/scripts/scan-changed-files.sh b/.github/scripts/scan-changed-files.sh deleted file mode 100755 index 5a22a54d1d..0000000000 --- a/.github/scripts/scan-changed-files.sh +++ /dev/null @@ -1,101 +0,0 @@ -#!/usr/bin/env bash -# -# Scans changed files for executable content disguised as a binary asset, and -# annotates edits to files that run on repo open or build. -# -# BASE_SHA= HEAD_SHA= [SARIF_OUT=] scan-changed-files.sh -# -# Writes a SARIF run to SARIF_OUT and exits 0 even when it reported something: -# blocking a merge is code scanning merge protection's job, not this script's. -# Exits 2 when it cannot scan properly, which must never look like a clean run. - -set -euo pipefail - -SARIF_OUT="${SARIF_OUT:-results.sarif}" - -SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" -readonly SCRIPT_DIR -readonly MISMATCH_SCRIPT="$SCRIPT_DIR/find-type-mismatches.py" - -# Runs on repo open or build without being invoked. Annotated, never blocked: -# editing these is routine here. -readonly AUTO_EXEC_PATTERN='^\.(vscode|devcontainer|githooks|cursor|claude|idea|github)/|\.code-workspace$|(^|/)build\.rs$|^\.cargo/config\.toml$|^(Makefile\.toml|justfile|flake\.nix|shell\.nix)$|\.(bat|cmd|ps1)$' - -log() { printf '%s\n' "$*" >&2; } -warn() { printf '::warning::%s\n' "$*"; } -die() { log "$*"; exit 2; } - -# Reads magika JSONL on stdin. Staged in the workdir so a failure part-way cannot -# leave a truncated SARIF behind, which would upload as "nothing found". -write_sarif() { - python3 "$MISMATCH_SCRIPT" > "$workdir/sarif" || die "Could not write SARIF." - mv "$workdir/sarif" "$SARIF_OUT" -} - -command -v magika >/dev/null 2>&1 || die "magika is not installed; see .github/security-scan/README.md." -command -v python3 >/dev/null 2>&1 || die "python3 is not installed." - -[[ -n "${BASE_SHA:-}" ]] || die "BASE_SHA must be set." -[[ -n "${HEAD_SHA:-}" ]] || die "HEAD_SHA must be set." - -# A commit missing from the clone makes the diff empty, which would pass, so -# refuse. Both ends need this: a head sha goes missing when a fork PR's branch is -# force-pushed between event dispatch and checkout. -for sha in "$BASE_SHA" "$HEAD_SHA"; do - git cat-file -e "${sha}^{commit}" 2>/dev/null \ - || die "Commit ${sha} is not in this clone; needs actions/checkout with fetch-depth: 0." -done - -workdir="$(mktemp -d)" -trap 'rm -rf "$workdir"' EXIT - -# -z is required: without it core.quotePath quotes any path holding a byte above -# 0x80, the literal fails the -f test below, and a homoglyph-named payload is -# dropped from the scan silently. Staged through a file because a process -# substitution's exit status is invisible to mapfile. -git diff --name-only --diff-filter=ACMR -z "$BASE_SHA" "$HEAD_SHA" > "$workdir/changed" \ - || die "git diff ${BASE_SHA}..${HEAD_SHA} failed." -mapfile -d '' -t changed < "$workdir/changed" - -files=() -for path in "${changed[@]}"; do - if [[ -f "$path" ]]; then - # magika reports each path back in its JSONL, so a path containing a - # newline would be indistinguishable from two entries. Git permits such - # paths, so refuse rather than mis-scan. - [[ "$path" == *$'\n'* ]] && die "Path contains a newline, which cannot be scanned safely: ${path@Q}" - files+=("$path") - else - # ACMR excludes deletions and a rename reports only its destination, so - # anything else here is unexpected rather than routine. - log "Skipping ${path}: not a regular file." - fi -done - -# An empty run still has to be written: uploading it is what clears alerts an -# earlier commit raised, so skipping the write would leave them showing forever. -if (( ${#files[@]} == 0 )); then - log "No files to scan." - write_sarif < /dev/null - exit 0 -fi - -log "Scanning ${#files[@]} changed file(s)." - -# Status checked separately from the conversion below, whose status would be -# python's: a magika crash would otherwise yield no findings and pass. magika -# also exits non-zero when a listed file could not be read, reporting the reason -# in its JSONL rather than on stderr, so both streams go into the message. -magika --jsonl -- "${files[@]}" > "$workdir/detected" 2> "$workdir/magika.err" \ - || die "magika failed: $(cat "$workdir/magika.err"; head -c 300 "$workdir/detected")" -[[ -s "$workdir/magika.err" ]] && log "magika stderr: $(cat "$workdir/magika.err")" - -write_sarif < "$workdir/detected" - -for path in "${files[@]}"; do - [[ "$path" =~ $AUTO_EXEC_PATTERN ]] \ - && warn "$path: runs automatically when the repo is opened or built - review as code" -done - -log "Wrote $SARIF_OUT" -exit 0 diff --git a/.github/scripts/tests/test-scan-changed-files.py b/.github/scripts/tests/test-scan-changed-files.py new file mode 100644 index 0000000000..0371a28c81 --- /dev/null +++ b/.github/scripts/tests/test-scan-changed-files.py @@ -0,0 +1,174 @@ +#!/usr/bin/env python3 +"""Tests for scan-changed-files.py, run against throwaway git repos. + + python3 .github/scripts/tests/test-scan-changed-files.py + +Needs git and uv on PATH; uv supplies the scanner's own dependency, so this runs +the scanner exactly the way CI does. Exits 0 when every case passes. +""" + +import json +import os +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +SCANNER = Path(__file__).resolve().parent.parent / "scan-changed-files.py" + +passed = 0 +failed = 0 + + +def check(name: str, expected, actual) -> None: + global passed, failed + if expected == actual: + print(f"ok {name}") + passed += 1 + else: + print(f"FAIL {name} (expected {expected!r}, got {actual!r})") + failed += 1 + + +def git(repo: Path, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(repo), *args], capture_output=True, text=True, check=True + ).stdout.strip() + + +def new_repo() -> Path: + repo = Path(tempfile.mkdtemp()) + git(repo, "init", "-q", "-b", "main") + git(repo, "config", "user.email", "t@example.com") + git(repo, "config", "user.name", "Test") + (repo / "README.md").write_text("seed\n") + git(repo, "add", "-A") + git(repo, "commit", "-qm", "seed") + return repo + + +def commit_all(repo: Path, message: str) -> None: + git(repo, "add", "-A") + git(repo, "commit", "-qm", message) + + +def write_payload(path: Path) -> None: + """JavaScript behind a long run of spaces, saved under an asset name. + + Content that is code, a name claiming an inert asset, and a payload pushed + off-screen in a diff. magika still types this as javascript. + """ + path.write_text( + " " * 1700 + + "globalThis['r']=require;\n" + + "function go(x){return x*2};\nmodule.exports={go};\n" + ) + + +def scan(repo: Path, base: str, head: str, drop_head: bool = False) -> tuple[int, Path]: + sarif = Path(tempfile.mkstemp()[1]) + env = {**os.environ, "BASE_SHA": base, "SARIF_OUT": str(sarif)} + if drop_head: + env.pop("HEAD_SHA", None) + else: + env["HEAD_SHA"] = head + completed = subprocess.run( + ["uv", "run", str(SCANNER)], + cwd=repo, + env=env, + capture_output=True, + ) + return completed.returncode, sarif + + +def findings(sarif: Path) -> int: + return len(json.loads(sarif.read_text())["runs"][0]["results"]) + + +# --- the payload is reported, under an ASCII name and a homoglyph one ---------- +# The Cyrillic 'a' matters: git quotes such paths unless the diff is read -z, and +# a quoted path would not match the file on disk. +for label, name in [("ascii", "fa-solid-400.woff2"), ("homoglyph", "fа-solid-400.woff2")]: + repo = new_repo() + (repo / "public" / "fonts").mkdir(parents=True) + write_payload(repo / "public" / "fonts" / name) + commit_all(repo, "payload") + rc, sarif = scan(repo, git(repo, "rev-parse", "HEAD~1"), git(repo, "rev-parse", "HEAD")) + check(f"scan succeeds ({label})", 0, rc) + check(f"payload reported ({label})", 1, findings(sarif)) + shutil.rmtree(repo) + +# --- a genuine font is left alone -------------------------------------------- +repo = new_repo() +(repo / "public" / "fonts").mkdir(parents=True) +(repo / "public" / "fonts" / "real.woff2").write_bytes(b"wOF2\x00\x01\x00\x00" + os.urandom(4000)) +commit_all(repo, "font") +rc, sarif = scan(repo, git(repo, "rev-parse", "HEAD~1"), git(repo, "rev-parse", "HEAD")) +check("genuine woff2 exits 0", 0, rc) +check("genuine woff2 reports nothing", 0, findings(sarif)) +shutil.rmtree(repo) + +# --- a payload in an earlier commit of a multi-commit range is still found ---- +# Regression for the base-commit fallback: scanning only the tip missed this. +repo = new_repo() +(repo / "public" / "fonts").mkdir(parents=True) +write_payload(repo / "public" / "fonts" / "fa-solid-400.woff2") +commit_all(repo, "payload") +(repo / "later.txt").write_text("later\n") +commit_all(repo, "later") +_, sarif = scan(repo, git(repo, "rev-parse", "HEAD~2"), git(repo, "rev-parse", "HEAD")) +check("payload in a non-tip commit reported", 1, findings(sarif)) +_, sarif = scan(repo, git(repo, "rev-parse", "HEAD~1"), git(repo, "rev-parse", "HEAD")) +check("tip-only range misses it (why the fallback matters)", 0, findings(sarif)) +shutil.rmtree(repo) + +# --- refusing to scan nothing ------------------------------------------------ +repo = new_repo() +head = git(repo, "rev-parse", "HEAD") +missing = "deadbeef" * 5 + +rc, _ = scan(repo, head, missing) +check("absent head sha exits 2", 2, rc) + +rc, _ = scan(repo, missing, head) +check("absent base sha exits 2", 2, rc) + +rc, _ = scan(repo, head, head, drop_head=True) +check("missing HEAD_SHA exits 2", 2, rc) + +# An empty diff must still write a run: uploading it is what clears alerts an +# earlier commit raised. +rc, sarif = scan(repo, head, head) +check("empty diff exits 0", 0, rc) +check("empty diff still writes an empty run", 0, findings(sarif)) +shutil.rmtree(repo) + +# --- a file the scanner cannot read must not pass ----------------------------- +repo = new_repo() +unreadable = repo / "unreadable.txt" +unreadable.write_text("secret\n") +commit_all(repo, "unreadable") +unreadable.chmod(0o000) +rc, _ = scan(repo, git(repo, "rev-parse", "HEAD~1"), git(repo, "rev-parse", "HEAD")) +unreadable.chmod(0o644) +check("unreadable file exits 2", 2, rc) +shutil.rmtree(repo) + +# --- a path containing a newline is scanned, not dropped ----------------------- +# The shell version had to refuse these because both scanners took newline +# delimited file lists. Passing paths as arguments removes that limitation. +repo = new_repo() +try: + weird = repo / "we\nird.woff2" + write_payload(weird) + commit_all(repo, "newline") +except OSError: + print("skip newline case (filesystem rejected the name)") +else: + _, sarif = scan(repo, git(repo, "rev-parse", "HEAD~1"), git(repo, "rev-parse", "HEAD")) + check("newline in path is still scanned", 1, findings(sarif)) +shutil.rmtree(repo) + +print(f"\n{passed} passed, {failed} failed") +sys.exit(1 if failed else 0) diff --git a/.github/scripts/tests/test-scan-changed-files.sh b/.github/scripts/tests/test-scan-changed-files.sh deleted file mode 100755 index 44adae41dc..0000000000 --- a/.github/scripts/tests/test-scan-changed-files.sh +++ /dev/null @@ -1,157 +0,0 @@ -#!/usr/bin/env bash -# -# Tests for scan-changed-files.sh, run against throwaway git repos. -# -# .github/scripts/tests/test-scan-changed-files.sh -# -# Needs magika, python3, jq and git on PATH. Exit 0 when every case passes. -# -# Locally: nix develop --command .github/scripts/tests/test-scan-changed-files.sh - -set -euo pipefail - -SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" -readonly SCRIPT_DIR -readonly SCAN="$SCRIPT_DIR/../scan-changed-files.sh" - -# JavaScript behind a long run of spaces, saved as a font: content that is code, a -# name claiming an inert asset, and a payload pushed off-screen in a diff. magika -# still types this as javascript, which is what the check keys on. -make_payload() { - { printf '%*s' 1700 '' - printf "globalThis['r']=require;\n" - printf "function go(x){return x*2};\nmodule.exports={go};\n" - } > "$1" -} - -passed=0 -failed=0 - -check() { - local name="$1" expected="$2" actual="$3" - if [[ "$expected" == "$actual" ]]; then - printf 'ok %s\n' "$name" - passed=$((passed + 1)) - else - printf 'FAIL %s (expected %s, got %s)\n' "$name" "$expected" "$actual" - failed=$((failed + 1)) - fi -} - -# A repo with one commit on main. -new_repo() { - local dir; dir="$(mktemp -d)" - git -C "$dir" init -q -b main - git -C "$dir" config user.email t@example.com - git -C "$dir" config user.name Test - echo seed > "$dir/README.md" - git -C "$dir" add -A - git -C "$dir" commit -qm seed - printf '%s' "$dir" -} - -# Runs the scan with SARIF written outside the repo under test, so the report -# cannot show up in a later diff. Sets SARIF for findings() to read. -SARIF="" -scan_in() { - local dir="$1" base="$2" head="$3" - SARIF="$(mktemp)" - ( cd "$dir" && BASE_SHA="$base" HEAD_SHA="$head" SARIF_OUT="$SARIF" "$SCAN" >/dev/null 2>&1 ) -} - -findings() { - jq '[.runs[].results[]] | length' "$SARIF" -} - -rev() { git -C "$1" rev-parse "$2"; } - -# --- the payload is reported, under an ASCII name and a homoglyph one ---------- -# The Cyrillic 'a' matters: git quotes such paths unless the diff is read -z, and -# a quoted path silently drops out of the file list. -for name in 'fa-solid-400.woff2' "$(printf 'f\xd0\xb0-solid-400.woff2')"; do - repo="$(new_repo)" - mkdir -p "$repo/public/fonts" - make_payload "$repo/public/fonts/$name" - git -C "$repo" add -A - git -C "$repo" commit -qm payload - rc=0; scan_in "$repo" "$(rev "$repo" HEAD~1)" "$(rev "$repo" HEAD)" || rc=$? - check "scan succeeds: $name" 0 "$rc" - check "payload reported: $name" 1 "$(findings)" - rm -rf "$repo" -done - -# --- a genuine font is left alone -------------------------------------------- -repo="$(new_repo)" -mkdir -p "$repo/public/fonts" -printf 'wOF2\x00\x01\x00\x00' > "$repo/public/fonts/real.woff2" -head -c 4000 /dev/urandom >> "$repo/public/fonts/real.woff2" -git -C "$repo" add -A -git -C "$repo" commit -qm font -rc=0; scan_in "$repo" "$(rev "$repo" HEAD~1)" "$(rev "$repo" HEAD)" || rc=$? -check "genuine woff2 exits 0" 0 "$rc" -check "genuine woff2 reports nothing" 0 "$(findings)" -rm -rf "$repo" - -# --- a payload in an earlier commit of a multi-commit range is still found ---- -# Regression for the base-commit fallback: scanning only the tip missed this. -repo="$(new_repo)" -mkdir -p "$repo/public/fonts" -make_payload "$repo/public/fonts/fa-solid-400.woff2" -git -C "$repo" add -A -git -C "$repo" commit -qm payload -echo later > "$repo/later.txt" -git -C "$repo" add -A -git -C "$repo" commit -qm later -scan_in "$repo" "$(rev "$repo" HEAD~2)" "$(rev "$repo" HEAD)" -check "payload in a non-tip commit reported" 1 "$(findings)" -# ... and confirm the narrow range really would have missed it, so the case bites -scan_in "$repo" "$(rev "$repo" HEAD~1)" "$(rev "$repo" HEAD)" -check "tip-only range misses it (why the fallback matters)" 0 "$(findings)" -rm -rf "$repo" - -# --- refusing to scan nothing ------------------------------------------------ -repo="$(new_repo)" -head="$(rev "$repo" HEAD)" -missing=deadbeefdeadbeefdeadbeefdeadbeefdeadbeef - -rc=0; scan_in "$repo" "$head" "$missing" || rc=$? -check "absent head sha exits 2" 2 "$rc" - -rc=0; scan_in "$repo" "$missing" "$head" || rc=$? -check "absent base sha exits 2" 2 "$rc" - -sarif_out="$(mktemp)" -rc=0; ( cd "$repo" && BASE_SHA="$head" SARIF_OUT="$sarif_out" "$SCAN" >/dev/null 2>&1 ) || rc=$? -check "missing HEAD_SHA exits 2" 2 "$rc" - -# An empty diff must still write a run: uploading it is what clears alerts an -# earlier commit raised. -rc=0; scan_in "$repo" "$head" "$head" || rc=$? -check "empty diff exits 0" 0 "$rc" -check "empty diff still writes an empty run" 0 "$(findings)" -rm -rf "$repo" - -# --- a file the scanner cannot read must not pass ----------------------------- -repo="$(new_repo)" -echo secret > "$repo/unreadable.txt" -git -C "$repo" add -A -git -C "$repo" commit -qm unreadable -chmod 000 "$repo/unreadable.txt" -rc=0; scan_in "$repo" "$(rev "$repo" HEAD~1)" "$(rev "$repo" HEAD)" || rc=$? -chmod 644 "$repo/unreadable.txt" -check "unreadable file exits 2" 2 "$rc" -rm -rf "$repo" - -# --- a path containing a newline must be refused, not silently mis-listed ----- -repo="$(new_repo)" -printf 'x\n' > "$repo/$(printf 'we\nird.txt')" 2>/dev/null || true -if git -C "$repo" add -A 2>/dev/null && git -C "$repo" commit -qm newline 2>/dev/null; then - rc=0; scan_in "$repo" "$(rev "$repo" HEAD~1)" "$(rev "$repo" HEAD)" || rc=$? - check "newline in path exits 2" 2 "$rc" -else - printf 'skip newline case (filesystem rejected the name)\n' -fi -rm -rf "$repo" - -printf '\n%d passed, %d failed\n' "$passed" "$failed" -(( failed == 0 )) diff --git a/.github/security-scan/README.md b/.github/security-scan/README.md index 8118d772e8..9c7d330702 100644 --- a/.github/security-scan/README.md +++ b/.github/security-scan/README.md @@ -1,75 +1,44 @@ # Scanning changed files for disguised content -`.github/workflows/security-scan.yml` inspects the *content* of every file a push or pull -request touches, and reports any file whose bytes are code or an executable while its -extension claims an inert binary asset: JavaScript named `.woff2`, an ELF named `.png`, and -so on. - -Findings are uploaded as [SARIF][sarif] and become code scanning alerts. Blocking a merge is -configured in a ruleset, not in the script. +`security-scan.yml` classifies the *content* of every file a push or PR touches and reports +any file whose bytes are code or an executable while its extension claims an inert asset: +JavaScript named `.woff2`, an ELF named `.png`. ## Why content and not extension -Selecting files by extension is the blind spot this exists to close. Scanners that filter -candidates by name never open a payload called `fa-solid-400.woff2`, so the disguise works -precisely because tooling declines to look. [magika][magika] classifies by content, so the -name cannot hide anything from it. - -The check fires only on a three-way conjunction, in `find-type-mismatches.py`: magika reports -the content as code or executable, *and* the extension is one that promises inert data, *and* -magika does not list that extension as expected for the type it detected. The third condition -is what spares assets that are executable by nature, such as a genuine `.wasm`. +Selecting files by extension is the blind spot this closes. A scanner that filters candidates +by name never opens `fa-solid-400.woff2`, so the disguise works because tooling declines to +look. [magika][magika] classifies by content, so the name hides nothing. -## Running it locally +The check needs all three of: magika reports code or an executable, the extension promises +inert data, and magika does not list that extension as expected for the detected type. The +last condition spares assets that are code by nature, such as a real `.wasm`. -The dev shell provides magika at the version CI uses: +## Running it ```sh -nix develop --command bash -c ' - BASE_SHA=$(git merge-base origin/main HEAD) HEAD_SHA=$(git rev-parse HEAD) \ - .github/scripts/scan-changed-files.sh' +nix develop --command uv run .github/scripts/scan-changed-files.py # writes results.sarif +nix develop --command python3 .github/scripts/tests/test-scan-changed-files.py ``` -That writes `results.sarif`. Tests: -`nix develop --command .github/scripts/tests/test-scan-changed-files.sh`. +`uv` reads the pinned magika version from the inline script metadata, so CI and local runs +resolve the same dependency and there is nothing to keep in step. ## What blocks a merge -Nothing in the script does. It always exits 0 when it merely *found* something, and exits 2 -only when it could not scan properly. Gating is a **code scanning** rule on the branch -ruleset, with the alerts threshold set to `Errors`, matching the `level` the SARIF emits. - -That rule also blocks when the tool's analysis is still running or is not configured at all, -so a job that never reports cannot be mistaken for a clean result. Using it instead of a -required status check avoids a trap: this workflow skips itself for same-repo pull requests to -avoid scanning twice, the skipped run publishes a check with the same name as the real one, -and GitHub counts a skipped check as satisfying a requirement. - -To accept a finding, dismiss the alert in the repository's Security tab. That is per finding -and it persists, so there is no allowlist file to maintain. - -## Fork pull requests - -Forks cap `security-events` at read, so SARIF cannot be uploaded for them. The workflow fails -the job directly in that case, which annotates the offending lines instead. Same detection, -different reporting surface. - -## Version pinning - -magika is pinned twice and both must move together: +Not the script. It exits 0 on a finding and 2 only when it could not scan. Gating is a +**code scanning** rule on the branch ruleset with the alerts threshold at `Errors`, matching +the SARIF `level`. -| | local | CI | -|---|---|---| -| magika | `nix/magika.nix` | `.github/scripts/fetch-scanners.sh` | +That rule also blocks while analysis is running or if the tool is not configured, so a job +that never reports cannot look clean. It is used instead of a required status check because +this workflow skips itself for same-repo PRs, the skipped run publishes a check with the same +name as the real one, and GitHub counts a skipped check as satisfying a requirement. -Both take the same upstream release binary, checksummed. Deliberately not from nixpkgs: on -Darwin that build installs a binary with no `LC_RPATH` that cannot resolve -`libonnxruntime.dylib` and aborts on every call. +To accept a finding, dismiss the alert in the Security tab. That is per finding and persists, +so there is no allowlist to maintain. -CI downloads rather than using nix because nix is the slower path *for this job*: installing -Nix plus restoring its cache is measured at 7s + 22s here, against roughly 1s for one -checksummed download. Release assets are maintainer-mutable, so the pinned digest is the -protection, not the tag. Never use a floating tag such as magika's `cli-latest`. +Fork PRs cap `security-events` at read and cannot upload SARIF, so the job fails directly and +annotates the lines instead. -[sarif]: https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/uploading-a-sarif-file-to-github [magika]: https://github.com/google/magika diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 0a124e2c5d..64159f2936 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -1,8 +1,8 @@ name: Security Scan -# Runs on every branch, not just PRs. The implant this job exists to catch was -# force-pushed onto the head branches of already-open pull requests, and a push -# to a branch with no open PR fires no `pull_request` event at all. +# Every branch, not just PRs: the implant this catches was force-pushed onto the +# head branch of an open PR, and a push to a branch with no PR fires no +# `pull_request` event. on: pull_request: push: @@ -20,11 +20,8 @@ permissions: jobs: scan-changed-files: name: "Scan changed files" - # A push covers every same-repo branch, including a push to the branch of an - # already-merged PR, which fires no pull_request event - that is how the - # implant this job exists to catch arrived. Fork PRs produce no push event - # here, so pull_request covers those; the guard keeps same-repo PRs from being - # scanned twice. + # Fork PRs produce no push event, so `pull_request` covers those; the guard + # keeps same-repo PRs from being scanned twice. if: >- github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.repository @@ -33,81 +30,37 @@ jobs: permissions: contents: read - security-events: write # github/codeql-action/upload-sarif + security-events: write # upload-sarif steps: - name: Checkout repository uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: - # Full history: the scan diffs against a base commit, and with a - # shallow clone that object is absent. The script treats a missing - # base as a hard error rather than scanning nothing and passing. + # The scan diffs against a base commit that a shallow clone omits, and + # treats a missing base as a hard error rather than scanning nothing. fetch-depth: 0 persist-credentials: false - - name: Install the scanner - run: | - .github/scripts/fetch-scanners.sh "$RUNNER_TEMP/bin" - echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" - - - name: Resolve the base commit - id: base - env: - PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} - PUSH_BEFORE: ${{ github.event.before }} - DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} - run: | - set -euo pipefail - if [ -n "$PR_BASE_SHA" ]; then - base="$PR_BASE_SHA" - elif git cat-file -e "${PUSH_BEFORE:-missing}^{commit}" 2>/dev/null; then - base="$PUSH_BEFORE" - else - # `before` is all-zeroes when the push created the branch, and on a - # force-push it is the previous tip, which is now orphaned and so not - # fetched even at fetch-depth 0. Both land here, and both are cases - # where the push carries more than one new commit - so fall back to - # the whole branch delta. HEAD~1 would scan only the tip and leave - # the rest of a force-pushed range unexamined. - base="$(git merge-base "origin/${DEFAULT_BRANCH}" HEAD)" - # Pushing to the default branch itself: merge-base is HEAD. - if [ "$base" = "$(git rev-parse HEAD)" ]; then - base="$(git rev-parse HEAD~1)" - fi - fi - echo "sha=$base" >> "$GITHUB_OUTPUT" + # Installs the scanner's dependency from the inline metadata in the script, + # so its version lives in one place. + - name: Install uv + uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 - name: Test the scanner - run: .github/scripts/tests/test-scan-changed-files.sh + run: python3 .github/scripts/tests/test-scan-changed-files.py - - name: Scan + - name: Scan changed files env: - BASE_SHA: ${{ steps.base.outputs.sha }} - HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }} SARIF_OUT: ${{ runner.temp }}/results.sarif - run: .github/scripts/scan-changed-files.sh + # Fork PRs cannot upload SARIF, so the exit code carries the finding. + FAIL_ON_FINDINGS: ${{ github.event_name == 'pull_request' }} + run: uv run .github/scripts/scan-changed-files.py - # Findings become code scanning alerts, and a ruleset code scanning rule is - # what blocks a merge on them. See .github/security-scan/README.md. + # A ruleset code scanning rule blocks merges on these alerts. + # See .github/security-scan/README.md. - name: Upload results to code scanning if: github.event_name != 'pull_request' uses: github/codeql-action/upload-sarif@c3400c2f38909e0dcf3c3a41f2030a8217be5d3e # v3 with: sarif_file: ${{ runner.temp }}/results.sarif category: disguised-content - - # Fork PRs cap `security-events` at read, so the upload above cannot run for - # them. Fail the job directly instead, so a fork's findings still surface. - - name: Fail on findings - if: github.event_name == 'pull_request' - env: - SARIF: ${{ runner.temp }}/results.sarif - run: | - set -euo pipefail - jq -r '.runs[].results[] - | "::error file=\(.locations[0].physicalLocation.artifactLocation.uri)::\(.message.text)"' "$SARIF" - count="$(jq '[.runs[].results[]] | length' "$SARIF")" - if [ "$count" -ne 0 ]; then - echo "$count finding(s); see the annotations above." >&2 - exit 1 - fi diff --git a/flake.nix b/flake.nix index 13ea6ae086..852cf21b2b 100644 --- a/flake.nix +++ b/flake.nix @@ -92,7 +92,6 @@ # Pinned to CI version cargoTools = pkgs.callPackage ./nix/cargo-tools.nix { }; opengrep = pkgs.callPackage ./nix/opengrep.nix { }; - magika = pkgs.callPackage ./nix/magika.nix { }; libcDev = lib.getDev stdenv.cc.libc; @@ -180,6 +179,7 @@ procps # pgrep, used by the kill-orphan-mpc-nodes cargo-make task pprof graphviz + uv # runs .github/scripts/*.py with their pinned inline dependencies ]; buildLibs = @@ -224,14 +224,7 @@ nearTools ++ miscTools ++ buildLibs ++ - [ - opengrep - # Used by .github/scripts/scan-changed-files.sh and its tests, so - # both are runnable locally. CI installs the same version from a - # pinned release binary instead, because a nix setup costs that - # job more than the download does. - magika - ]; + [ opengrep ]; env = envCommon // envDarwin; diff --git a/nix/magika.nix b/nix/magika.nix deleted file mode 100644 index f3888b532d..0000000000 --- a/nix/magika.nix +++ /dev/null @@ -1,74 +0,0 @@ -{ - lib, - stdenv, - fetchurl, - autoPatchelfHook, -}: - -# nixpkgs ships magika-cli, but it is unusable on Darwin: the binary it installs -# has no LC_RPATH, so it cannot resolve libonnxruntime.dylib and aborts on any -# invocation. Upstream's own release binaries work on both platforms, so take -# those, as nix/opengrep.nix does for the same reason. -let - version = "1.1.0"; - - assets = { - x86_64-linux = { - target = "x86_64-unknown-linux-gnu"; - hash = "sha256-a0wQEMhNH08GIFzO9Fl/FpC813RPRthB7uJkJrwQBIU="; - }; - aarch64-linux = { - target = "aarch64-unknown-linux-gnu"; - hash = "sha256-IYPkdF4fDlnRtwZtH/iP5AmzjJ6/EZqd5qga/RDmqvc="; - }; - aarch64-darwin = { - target = "aarch64-apple-darwin"; - hash = "sha256-VLicWen/AkaZBmF4xF5Y1zGDPMJbfBObQoV9XTG3uAQ="; - }; - }; - - asset = assets.${stdenv.hostPlatform.system} or (throw "magika: unsupported system ${stdenv.hostPlatform.system}"); -in -stdenv.mkDerivation { - pname = "magika"; - inherit version; - - src = fetchurl { - # Releases are tagged cli/vX.Y.Z; that version is also what `magika - # --version` reports, unlike the separately numbered Python package. - url = "https://github.com/google/magika/releases/download/cli/v${version}/magika-cli-${asset.target}.tar.xz"; - inherit (asset) hash; - }; - - # The Linux binary is dynamically linked, so it names an interpreter that does - # not exist under nix and fails with "cannot execute: required file not found" - # without this. - nativeBuildInputs = lib.optionals stdenv.hostPlatform.isLinux [ autoPatchelfHook ]; - buildInputs = lib.optionals stdenv.hostPlatform.isLinux [ stdenv.cc.cc.lib ]; - - # The tarball's single directory becomes the source root, so the binary is at - # the top level here rather than under magika-cli-/. - installPhase = '' - runHook preInstall - install -Dm755 magika $out/bin/magika - runHook postInstall - ''; - - # Runs the binary, so it catches a platform whose prebuilt artifact cannot - # actually start - which is exactly how nixpkgs' Darwin build is broken. - doInstallCheck = true; - installCheckPhase = '' - runHook preInstallCheck - $out/bin/magika --version | grep -q '${version}' - runHook postInstallCheck - ''; - - meta = { - description = "Determines file content types using a deep-learning model"; - homepage = "https://github.com/google/magika"; - license = lib.licenses.asl20; - mainProgram = "magika"; - platforms = builtins.attrNames assets; - sourceProvenance = [ lib.sourceTypes.binaryNativeCode ]; - }; -} From e8ecc94a336e60162059d4027fe75b8c1361997e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Tue, 4 Aug 2026 18:58:50 +0200 Subject: [PATCH 3/5] ci: tighten permissions and document the per-tool convention permissions: {} at the top level, granted per job, matching astral-sh/uv and apache/iceberg-rust. Records the conventions a second tool has to follow, notably a SARIF category unique per tool, since code scanning keeps one analysis per category. --- .github/security-scan/README.md | 17 +++++++++++++++++ .github/workflows/security-scan.yml | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/.github/security-scan/README.md b/.github/security-scan/README.md index 9c7d330702..55f4988d39 100644 --- a/.github/security-scan/README.md +++ b/.github/security-scan/README.md @@ -41,4 +41,21 @@ so there is no allowlist to maintain. Fork PRs cap `security-events` at read and cannot upload SARIF, so the job fails directly and annotates the lines instead. +## Adding another tool + +One workflow file per tool, as `apache/iceberg-rust`, `sigstore/sigstore-rs` and +`matrix-org/matrix-rust-sdk` do, so each arrives as a self-contained change. Conventions to +match: + +- `permissions: {}` at the top level, granted per job, and `security-events: write` only on a + job that uploads +- `persist-credentials: false` on checkout +- actions pinned by commit SHA with a trailing `# vX.Y.Z` +- **a `category` unique to the tool** on `upload-sarif`. Code scanning keeps one analysis per + category, so a shared value would make one tool overwrite another's alerts +- upload a run even when nothing was found, which is what clears alerts an earlier commit + raised + +Each tool is then separately requireable in the ruleset's code scanning rule. + [magika]: https://github.com/google/magika diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 64159f2936..47a10ac238 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -14,8 +14,8 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true -permissions: - contents: read +# Granted per job instead, so a step only ever holds what it needs. +permissions: {} jobs: scan-changed-files: From 5f41af13370cc2567781d2d2393dcdd442315d2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Tue, 4 Aug 2026 19:26:39 +0200 Subject: [PATCH 4/5] docs: state the job guard as a single skip condition --- .github/workflows/security-scan.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 47a10ac238..b4b44a9781 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -20,11 +20,12 @@ permissions: {} jobs: scan-changed-files: name: "Scan changed files" - # Fork PRs produce no push event, so `pull_request` covers those; the guard - # keeps same-repo PRs from being scanned twice. + # Skip a same-repo PR: the push to its branch already scanned these commits. + # Everything else runs, which leaves pushes and fork PRs, the latter having + # no push event of their own. if: >- - github.event_name != 'pull_request' || - github.event.pull_request.head.repo.full_name != github.repository + !(github.event_name == 'pull_request' && + github.event.pull_request.head.repo.full_name == github.repository) runs-on: ubuntu-latest timeout-minutes: 10 From b6171419c27497bfeb37d29087859670780bf849 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20B=C4=99za?= Date: Tue, 4 Aug 2026 19:28:27 +0200 Subject: [PATCH 5/5] docs: correct the permissions comment to job scope --- .github/workflows/security-scan.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index b4b44a9781..8a3846c830 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -14,7 +14,7 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true -# Granted per job instead, so a step only ever holds what it needs. +# No token scopes by default; each job opts into only what it needs. permissions: {} jobs: