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/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/security-scan/README.md b/.github/security-scan/README.md new file mode 100644 index 0000000000..55f4988d39 --- /dev/null +++ b/.github/security-scan/README.md @@ -0,0 +1,61 @@ +# Scanning changed files for disguised content + +`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 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. + +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`. + +## Running it + +```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 +``` + +`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 + +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`. + +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. + +To accept a finding, dismiss the alert in the Security tab. That is per finding and persists, +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 new file mode 100644 index 0000000000..8a3846c830 --- /dev/null +++ b/.github/workflows/security-scan.yml @@ -0,0 +1,67 @@ +name: Security Scan + +# 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: + branches: + - '**' + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +# No token scopes by default; each job opts into only what it needs. +permissions: {} + +jobs: + scan-changed-files: + name: "Scan changed files" + # 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) + runs-on: ubuntu-latest + timeout-minutes: 10 + + permissions: + contents: read + security-events: write # upload-sarif + + steps: + - name: Checkout repository + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + # 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 + + # 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: python3 .github/scripts/tests/test-scan-changed-files.py + + - name: Scan changed files + env: + SARIF_OUT: ${{ runner.temp }}/results.sarif + # 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 + + # 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 diff --git a/flake.nix b/flake.nix index ad9d1f31d4..852cf21b2b 100644 --- a/flake.nix +++ b/flake.nix @@ -179,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 =