diff --git a/.claude/skills/codetrial-triage/pulls.md b/.claude/skills/codetrial-triage/pulls.md index 41a78aa26..26b768d02 100644 --- a/.claude/skills/codetrial-triage/pulls.md +++ b/.claude/skills/codetrial-triage/pulls.md @@ -335,15 +335,20 @@ says why it reads the whole log rather than `--log-failed`, and which markers count. A check outside Actions, cubic among them, comes back with its link only; read it from the PR's checks tab. -A `cargo-audit` advisory that `main` also has is the maintainer's, and the PR -must not carry a lockfile bump to clear it: #304 was told it "needs nothing from -you", and #305 to drop its unrelated `Cargo.lock` change. Once `main` lands the +For historical runs with `cargo-audit` in the portable lane, an advisory that +`main` also has is the maintainer's, and the PR must not carry a lockfile bump +to clear it: #304 was told it "needs nothing from you", and #305 to drop its +unrelated `Cargo.lock` change. Once `main` lands the fix (#331 moved to zip 9), a PR still failing on that advisory has a stale base, and the sweep puts it in `rebase` under the same limits as a conflicting one, so a request in any wording on its head, such as #326's own, keeps it out. A PR that touches a Cargo file is never a stale base: read whether its lockfile change brought the advisory before asking for anything. +The independent `audit` job compares the actual merge against its base tip; +inherited findings do not fail it. Read its summary for newly introduced +vulnerabilities or an incomplete scan instead of classifying it as stale. + A clean merge does not establish readiness. For a dependent draft, check whether its prerequisite merged and whether the author still needs to rebase, remove prerequisite commits, or mark it ready. Compare overlapping PRs by behavior as diff --git a/.claude/skills/codetrial-verify/SKILL.md b/.claude/skills/codetrial-verify/SKILL.md index fdbfda36a..278466eb2 100644 --- a/.claude/skills/codetrial-verify/SKILL.md +++ b/.claude/skills/codetrial-verify/SKILL.md @@ -15,12 +15,16 @@ make check # every gate, plus a live Gemini credential check CI runs one `gates ()` job per lane in parallel: `gates (rust)`, `gates (portable)` and `gates (browser)`. The `check` job only collects their -results and the mutation jobs', so its log does not say what failed: open the -failing lane, or, when every lane is green, the failing `plan-mutants` job or +results, the independent `audit` job and the mutation jobs', so its log does +not say what failed: open the failing lane, `audit`, `plan-mutants` job or `mutants` shard. On a pull request the portable lane checks the commit messages first, and a bad message skips the rest of that lane only. A gate names its lane where it is declared, one naming none is portable, and a test -proves the three lanes together cover the full run. +proves the three lanes plus the independent audit cover the full run. +`cargo-audit` runs only in the default local gate; CI uses `scripts/audit.py` +for merge-relative PR checks and full main checks. See +[dependency security checks](../../../docs/development.md#dependency-security-checks). +Its regression suite is `tests/test_audit.py`. Reproduce a failure with the same lane locally. `scripts/test.sh` is a list of named gates that all run before it reports, so @@ -149,16 +153,22 @@ What this adds is one call that reads all of it at the PR's head: Read its `failing` entries in this order. `current` false is history, not the current failure: say so and stop there. `current` or `on_main` null means its lookup failed, and `gates`, `errors` and `excerpt` null mean the job log could -not be read; check either with the raw calls below. `on_main` true means the latest -finished run on `main` fails the same job, so the failure is the maintainer's -and the author is not asked to fix it. Otherwise `gates` names what -`scripts/test.sh` failed and `errors` the lines that say why; rerun that lane -locally with the same head. A `cargo-audit` failure on a PR whose base predates -the advisory fix is a stale base, answered by a rebase (#334), not a code -change; `stale_base` (defined in the script's header) says exactly that, and -the PR sweep lists such a PR in `rebase` on its own. The aggregate `check` job -only collects results, so its entry points at the failing job it collected, -as in the lanes section above. A check outside Actions has only its link, and +not be read; check either with the raw calls below. `on_main` true means the +latest finished run on `main` fails the same job. Read whether the cause is +shared before asking the author for a fix: the PR `audit` compares added +findings, while main runs a full scan, so both can fail for different reasons. +Otherwise `gates` names what `scripts/test.sh` failed and `errors` the lines +that say why; rerun that lane locally with the same head. For historical runs +that still put `cargo-audit` in the portable lane, a failure on a PR whose base +predates the advisory fix is a stale base, answered by a rebase (#334), not a +code change; `stale_base` (defined in the script's header) says exactly that, +and the PR sweep lists such a historical PR in `rebase` on its own. +The independent `audit` job already compares the proposed merge with the base +tip: inherited findings are informational, while added findings and incomplete +dependency-change scans require investigation rather than an automatic rebase. +The aggregate `check` job only collects results, so its entry points at the +failing job it collected, as in the lanes section above. A check outside +Actions has only its link, and `held` checks have no log until a maintainer approves the run. The raw calls behind it, when one answer needs checking: diff --git a/.claude/skills/codetrial-verify/ci-failure.sh b/.claude/skills/codetrial-verify/ci-failure.sh index 882b7bb52..f192f589d 100755 --- a/.claude/skills/codetrial-verify/ci-failure.sh +++ b/.claude/skills/codetrial-verify/ci-failure.sh @@ -20,7 +20,9 @@ # Actions, its link only. current and on_main are null when the # lookup behind them failed, and gates, errors and excerpt when # the job log could not be read. -# stale_base true when every failing lane failed only gates that read +# stale_base legacy portable-audit runs only; the independent audit job +# reports inherited findings without failing. True when every +# failing lane failed only gates that read # outside data (the table in the last jq below: cargo-audit # reads the advisory database), on the current head, main's # latest run started after those runs and succeeded in each of diff --git a/.github/actions/install-audit/action.yml b/.github/actions/install-audit/action.yml new file mode 100644 index 000000000..762ababfa --- /dev/null +++ b/.github/actions/install-audit/action.yml @@ -0,0 +1,14 @@ +name: Install cargo-audit +description: Install the pinned, digest-checked RustSec release binary. +runs: + using: composite + steps: + - shell: sh + env: + GH_TOKEN: ${{ github.token }} + CARGO_AUDIT_VERSION: 0.22.2 + run: | + name="cargo-audit-x86_64-unknown-linux-musl-v$CARGO_AUDIT_VERSION" + ./scripts/install-release-binary.sh rustsec/rustsec \ + "cargo-audit/v$CARGO_AUDIT_VERSION" "$name.tgz" "$name/cargo-audit" + cargo audit --version | grep -qxF "cargo-audit-audit $CARGO_AUDIT_VERSION" diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml new file mode 100644 index 000000000..6dab2f13d --- /dev/null +++ b/.github/workflows/audit.yml @@ -0,0 +1,76 @@ +name: security audit + +on: + schedule: + - cron: '43 4 * * *' + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: security-audit + +jobs: + audit: + # Only trusted main code may update the repository's tracking issue. + if: github.ref == 'refs/heads/main' + runs-on: ubuntu-24.04 + timeout-minutes: 10 + permissions: + contents: read + issues: write + steps: + - uses: actions/checkout@v7 + with: + persist-credentials: false + - name: Install cargo-audit + uses: ./.github/actions/install-audit + - name: Audit main + id: audit + run: python3 scripts/audit.py --report "$RUNNER_TEMP/audit.md" + - name: Track remediation + if: ${{ !cancelled() && steps.audit.outputs.findings != '' }} + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + FINDINGS: ${{ steps.audit.outputs.findings }} + run: | + title='Dependency security audit' + fingerprint() { sed -n '/^Findings fingerprint:/p'; } + # gh refuses --slurp with --jq, so filter the slurped pages with jq. + # Closed issues count too: reopening keeps existing subscribers. + found=$(gh api --paginate --slurp --method GET "repos/$GH_REPO/issues" \ + -f state=all -f 'creator=github-actions[bot]' -f per_page=100 \ + | jq -r --arg title "$title" '[.[][] | select(.pull_request == null and .user.login == "github-actions[bot]" and .title == $title)][0] // empty | "\(.number) \(.state) \(.closed_at // "")"') + read -r issue state closed <<< "$found" + if [ "$FINDINGS" != true ]; then + # The clean body replaces the old fingerprint, so findings that + # return later count as a change and notify subscribers again. + if [ "$state" = open ]; then + gh issue edit "$issue" --body-file "$RUNNER_TEMP/audit.md" + gh issue close "$issue" --reason completed + fi + exit 0 + fi + if [ -z "$issue" ]; then + gh issue create --title "$title" --body-file "$RUNNER_TEMP/audit.md" + exit 0 + fi + current=$(fingerprint < "$RUNNER_TEMP/audit.md") + previous=$(gh issue view "$issue" --json body --jq .body | fingerprint) + # Keep the old fingerprint until notification succeeds so failures + # retry, but skip the comment if a failed edit left it posted. Only + # comments since a close count, so recurring findings notify again. + if [ "$previous" != "$current" ]; then + notified=$(gh api --paginate --slurp "repos/$GH_REPO/issues/$issue/comments" \ + | jq -r --arg since "$closed" '[.[][] | select(.user.login == "github-actions[bot]" and .created_at > $since)] | last | .body // ""' \ + | fingerprint) + if [ "$notified" != "$current" ]; then + gh issue comment "$issue" --body-file "$RUNNER_TEMP/audit.md" + fi + fi + gh issue edit "$issue" --body-file "$RUNNER_TEMP/audit.md" + if [ "$state" = closed ]; then + gh issue reopen "$issue" + fi diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 1253044ec..90ed89613 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -54,23 +54,51 @@ env: # install, and a guard skips the install whenever a cached binary exists: # bumping only the install line would leave the old key matching, the old # binary restored, and CI running a version nobody asked for. - CARGO_AUDIT_VERSION: 0.22.2 CARGO_MUTANTS_VERSION: 27.1.0 - # Pinned for the same reason the two above are: a linter that moves under you + # Pinned for the same reason as the one above: a linter that moves under you # turns an unrelated push red, and the fix is then a version bump nobody chose # to make. Kept in step with whatever contributors run locally, which is # whatever `ruff` is on their PATH; the gate skips rather than fails when they # have none. RUFF_VERSION: 0.15.2 - # The workflow's own linter, pinned like the three above. Installed rather + # The workflow's own linter, pinned like the tools above. Installed rather # than assumed: `scripts/test.sh` skips this gate when the binary is absent, # and absent is what it was here, so the file that describes every other # check was the one file nothing checked. ACTIONLINT_VERSION: 1.7.12 jobs: + audit: + if: github.event_name != 'schedule' && github.event_name != 'workflow_dispatch' + runs-on: ubuntu-24.04 + timeout-minutes: 10 + steps: + - uses: actions/checkout@v7 + with: + fetch-depth: 2 + persist-credentials: false + - name: Install cargo-audit + uses: ./.github/actions/install-audit + - name: Audit the tested merge or main + env: + EVENT: ${{ github.event_name }} + GROUP_BASE: ${{ github.event.merge_group.base_sha }} + run: | + case "$EVENT" in + pull_request) + base=$(git rev-parse HEAD^1) + python3 scripts/audit.py --base "$base" + ;; + merge_group) + git cat-file -e "$GROUP_BASE^{commit}" || git fetch --no-tags --depth=1 origin "$GROUP_BASE" + python3 scripts/audit.py --base "$GROUP_BASE" + ;; + push) python3 scripts/audit.py ;; + *) exit 2 ;; + esac + gates: name: gates (${{ matrix.lane }}) if: github.event_name != 'schedule' && github.event_name != 'workflow_dispatch' @@ -225,25 +253,6 @@ jobs: id: chromium uses: ./.github/actions/chromium - # The gate skips this audit when the binary is missing, which is right for - # a contributor's laptop and wrong for CI: a silent skip here means the - # dependency surface stops being checked and nothing says so. Installed - # ahead of the gate so the skip branch is never the one CI takes. - # - # The upstream release binary, checked against the digest its release - # records, rather than `cargo install`: that compiled the crate graph for - # minutes on every cache miss. musl, so the binary does not depend on the - # runner's glibc. The version check proves the pin is what installed. - - name: Install cargo-audit - if: matrix.lane == 'portable' - env: - GH_TOKEN: ${{ github.token }} - run: | - name="cargo-audit-x86_64-unknown-linux-musl-v$CARGO_AUDIT_VERSION" - ./scripts/install-release-binary.sh rustsec/rustsec \ - "cargo-audit/v$CARGO_AUDIT_VERSION" "$name.tgz" "$name/cargo-audit" - cargo audit --version | grep -qxF "cargo-audit-audit $CARGO_AUDIT_VERSION" - # The Python here generates the problem bank and drives two integration # harnesses, and `ruff check` is its only linter. The gate skips when the # binary is missing, which is right on a laptop and wrong here. pipx is on @@ -366,7 +375,7 @@ jobs: target key: ${{ steps.cargo-cache.outputs.key }} - # The one status: every gate lane passed, and on a pull request the mutation + # The one status: every gate lane and audit passed, and on a PR the mutation # plan and every shard did too. Planning only runs on pull requests, and the # shards only after a plan that found mutants, so a skipped shard job after a # successful plan means the plan counted none. A push or a merge group plans @@ -377,7 +386,7 @@ jobs: # reaches this job and fails it. Not on a schedule or a dispatch, which run # no gates. check: - needs: [gates, plan-mutants, mutants] + needs: [gates, audit, plan-mutants, mutants] if: ${{ !cancelled() && github.event_name != 'schedule' && github.event_name != 'workflow_dispatch' }} runs-on: ubuntu-24.04 timeout-minutes: 5 @@ -385,6 +394,7 @@ jobs: - name: Require every gate lane and mutation shard env: GATES: ${{ needs.gates.result }} + AUDIT: ${{ needs.audit.result }} PLAN: ${{ needs.plan-mutants.result }} MUTANTS: ${{ needs.mutants.result }} run: | @@ -392,6 +402,10 @@ jobs: echo "::error::gate lanes: $GATES. This job only collects them; the failed gates lane has the cause." exit 1 fi + if [ "$AUDIT" != success ]; then + echo "::error::security audit: $AUDIT. See the audit job for new vulnerabilities or an incomplete scan." + exit 1 + fi case "$MUTANTS/$PLAN" in success/success | skipped/success | skipped/skipped) ;; *) echo "::error::mutants: $MUTANTS (plan: $PLAN)"; exit 1 ;; @@ -653,11 +667,11 @@ jobs: # compiles `web/` (including its WASM) into it. Native runners avoid a # cross-platform linker or packaging toolchain. # - # After `gates` rather than `check`: on a push `check` adds nothing, and the - # mutation jobs it waits on are skipped, which the default `success()` reads + # After `gates` and `audit` rather than `check`: on a push `check` adds + # nothing, and its mutation jobs are skipped, which the default `success()` reads # as a reason to skip everything downstream. build: - needs: gates + needs: [gates, audit] if: github.event_name == 'push' && github.ref == 'refs/heads/main' name: ${{ matrix.name }} timeout-minutes: 45 @@ -970,7 +984,7 @@ jobs: # One rolling release for the current main build. GitHub releases require a # tag; `latest` avoids exposing an implementation SHA as a release name. release: - needs: [gates, build] + needs: [gates, audit, build] if: github.event_name == 'push' && github.ref == 'refs/heads/main' runs-on: ubuntu-24.04 timeout-minutes: 15 diff --git a/README.md b/README.md index b47054515..0465e604e 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,7 @@ rather than guessing. | Phase | Required | Notes | |---|---|---| | Build time | Rust stable, `curl` or `wget`, and `sha256sum` or `shasum`; Linux also needs Clang 21+ and GLib development headers | Cargo resolves application crates from `Cargo.lock`. `make build` fetches the checksum-pinned browser assets on the first build. | -| Test time | Build-time tools, Node.js 18+, and Python 3.9+ | Node runs browser and fixture checks; Python verifies generated problem-bank files. `npm ci` adds pinned Prettier, ESLint and Playwright for the full local browser gate. `ruff`, `shellcheck`, `shfmt`, `commentflow`, `actionlint`, and `cargo-audit` are optional: each gate reports that it skipped rather than failing without them. CI installs all of them, so those lanes are enforced on a pull request whatever a checkout can run; locally the `actionlint` lane also takes its container image. | +| Test time | Build-time tools, Node.js 18+, and Python 3.9+ | Node runs browser and fixture checks; Python verifies generated problem-bank files. `npm ci` adds pinned Prettier, ESLint and Playwright for the full local browser gate. `ruff`, `shellcheck`, `shfmt`, `commentflow`, `actionlint`, and `cargo-audit` are optional: each gate reports that it skipped rather than failing without them; locally the `actionlint` lane also takes its container image. CI installs these tools and enforces its test lanes whatever a checkout can run. The independent [dependency security check](docs/development.md#dependency-security-checks) blocks newly introduced vulnerabilities on PRs and runs full audits on main. | | Runtime | The compiled `codetrial` binary and a config file | No Node.js, Python, or `node_modules` is required. Rust dependencies are compiled into the binary; browser dependencies are vendored, checksum-pinned, and embedded, so a `web/` directory is optional and only overrides what is already inside. | On Linux x86_64, `make` downloads the checksum-pinned WebRTC compiler into diff --git a/docs/development.md b/docs/development.md index 711825453..219ecf973 100644 --- a/docs/development.md +++ b/docs/development.md @@ -5,6 +5,55 @@ time actually goes. `make` targets and the one-line summary of each are in [README.md](../README.md#development); everything here is the detail behind them. +## Dependency security checks + +A new RustSec advisory must reach maintainers without making an unrelated +feature look broken. The independent `audit` job uses `scripts/audit.py`: + +- Pull requests compare the tested merge commit against its first parent, the + event's base tip. Merge groups compare against `merge_group.base_sha`. + Never use the old branch's merge-base: that can hide a reintroduced weakness. +- Both lockfiles use one freshly fetched advisory database snapshot. Findings + are compared by advisory ID, crate and version, so replacing one vulnerable + dependency with another cannot pass by keeping the same count. +- An unchanged lockfile gets one informational scan. Inherited vulnerabilities + and scan errors appear in the job summary without failing the PR. With a + changed lockfile, new findings or an incomplete scan fail the audit. Removing + a finding passes; upgrading to another affected version remains blocking. +- Pushes to main run a full audit. Builds and the rolling release wait for it, + so a known vulnerability or scan error blocks publication. This can delay + unrelated fixes while the previous release remains vulnerable; prioritize + the dependency repair rather than bypassing the audit. + +Checkout and tool-installation failures still fail the job. The informational +exception covers scanner and database errors after the tool is installed. + +The separate `security audit` workflow runs daily and on manual dispatch from +main. It maintains one bot-authored issue titled `Dependency security audit`, +updating its findings or closing it after a clean scan, and reopening the same +issue when findings return so its subscribers keep following it. Changes to the +findings also leave a comment to notify subscribers, once even when a retry +follows a failed update; a database refresh alone does not. +An incomplete scan fails without closing the issue. Only this trusted-main job +has `issues: write`; fork PRs use read permissions and job summaries, without +comments or secrets. Maintainers should subscribe to the tracking issue and +workflow failures, assign an owner and a repair deadline, and link the repair +PR in the issue. A recurring scan is not a repair deadline. + +Enable Dependabot alerts and security updates in repository settings; the +existing `.github/dependabot.yml` groups coupled security updates. RustSec +findings may precede GitHub's advisory import, so the daily audit remains +necessary. Review security PRs promptly and link them to the audit issue. +Do not add blanket advisory ignores. Any proposed exception needs a maintainer, +reason, expiry and tracking issue; no exception is implemented by this change. + +`./scripts/test.sh` retains the full local `cargo audit` when installed. Its +three CI lanes omit that gate because the separate audit job owns CI policy. +To reproduce CI locally, run `python3 scripts/audit.py` for a full scan or +`python3 scripts/audit.py --base ` on the tested merge tree. +The scanner tests run with +`python3 scripts/run-python-tests.py tests/test_audit.py`. + ## What the gate checks, and what it will not `./scripts/test.sh` finishes by printing what it did not check, because several @@ -22,7 +71,7 @@ cargo install cargo-audit # the dependency advisory scan CI runs `scripts/test.sh rust`, `scripts/test.sh portable` and `scripts/test.sh browser` in parallel, with fail-fast disabled so a failure in one lane does not hide the others. Clippy and Rust tests share the Rust lane's -build cache; formatting, audits, Python and shell checks run in the portable +build cache; formatting, Python and shell checks run in the portable lane; Node browser tests have their own lane, except the face-detector suite, which needs the Rust binary and runs after its build. All lanes fetch and verify vendor assets; each restores the vendored-asset cache, and only the @@ -32,18 +81,18 @@ clone only `HEAD`; the portable lane keeps full history for the commit-message check. The Rust lane deletes the runner's unused toolchains only when less than 40 GB is free. Running `./scripts/test.sh` locally still runs every gate. -`check` is the only status a branch rule for main should require. It needs -every lane to succeed, and on a pull request the mutation plan and every shard -too; shards may be skipped only when the plan found no mutants. A push to main -or a merge queue entry plans no mutants, so `check` reports as soon as the -lanes finish. The workflow also runs on `merge_group`, so once main has a merge -queue, it tests each pull request on top of the main it will land on; two pull -requests that pass against their own bases can still break main together. A -merge group saves no caches, since its queue ref is gone once it lands. -Mutation planning checks the Rust diff before installing its tools, so a PR -with no Rust changes starts no mutation shards and pays no mutation-tool setup -cost. Rust diffs still use the complete diff-selected mutant set and the -existing exclusions. +`check` is the only status a branch rule for main should require. It needs every +lane and the independent security audit to succeed, and on a pull request the +mutation plan and every shard too; shards may be skipped only when the plan +found no mutants. A push to main or a merge queue entry plans no mutants, so +`check` reports as soon as the lanes finish. The workflow also runs on +`merge_group`, so once main has a merge queue, it tests each pull request on top +of the main it will land on; two pull requests that pass against their own bases +can still break main together. A merge group saves no caches, since its queue +ref is gone once it lands. Mutation planning checks the Rust diff before +installing its tools, so a PR with no Rust changes starts no mutation shards and +pays no mutation-tool setup cost. Rust diffs still use the complete +diff-selected mutant set and the existing exclusions. Every skip the gate sees joins its `not checked by this run:` summary: a gate's own skip, a Node browser test's skip or todo, a missing formatter in diff --git a/scripts/audit.py b/scripts/audit.py new file mode 100644 index 000000000..39cdd7ba7 --- /dev/null +++ b/scripts/audit.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python3 +"""Audit the full lockfile, or only vulnerabilities a proposed merge adds.""" + +import argparse +import hashlib +import json +import os +import subprocess +import sys +import tempfile +from pathlib import Path + + +def git(root, *args): + return subprocess.check_output(["git", "-C", str(root), *args]) + + +def audit(lockfile, database, fetch): + command = [ + "cargo", + "audit", + "--json", + "--db", + str(database), + "--file", + str(lockfile), + ] + if not fetch: + command.append("--no-fetch") + # A single configuration must govern both trees; do not load PR ignore rules. + result = subprocess.run( + command, cwd=lockfile.parent, capture_output=True, text=True + ) + if result.stderr: + print(result.stderr, file=sys.stderr, end="") + try: + report = json.loads(result.stdout) + vulnerabilities = report["vulnerabilities"] + warnings = report.get("warnings", {}) + if ( + not isinstance(vulnerabilities["list"], list) + or type(vulnerabilities["found"]) is not bool + or type(vulnerabilities["count"]) is not int + or not isinstance(warnings, dict) + or not all(isinstance(items, list) for items in warnings.values()) + ): + raise ValueError("invalid audit report schema") + findings = { + ( + item["advisory"]["id"], + item["package"]["name"], + item["package"]["version"], + ) + for item in vulnerabilities["list"] + } + valid = ( + vulnerabilities["found"] == bool(findings) + and vulnerabilities["count"] == len(vulnerabilities["list"]) + and result.returncode == int(bool(findings)) + and all( + isinstance(value, str) and value for key in findings for value in key + ) + ) + if not valid: + raise ValueError("inconsistent audit report or exit status") + return findings, warnings + except (KeyError, TypeError, ValueError) as error: + raise ValueError(f"audit did not complete: {error}") from error + + +def scan(head, previous, informational): + """Return head findings, those the base already had, warnings and the DB revision.""" + with tempfile.TemporaryDirectory(prefix="codetrial-audit-") as directory: + scratch = Path(directory) + database = scratch / "advisory-db" + lockfile = scratch / "Cargo.lock" + lockfile.write_bytes(head) + findings, warnings = audit(lockfile, database, True) + revision = git(database, "rev-parse", "HEAD").decode().strip() + if informational: + inherited = findings + elif previous is not None: + lockfile.write_bytes(previous) + inherited, _ = audit(lockfile, database, False) + else: + inherited = set() + return findings, inherited, warnings, revision + + +def render(lines, findings, added, warnings, revision): + lines.append(f"RustSec database: `{revision}`") + lines.append(f"\nVulnerabilities: {len(findings)}; blocking: {len(added)}") + fingerprint = hashlib.sha256(json.dumps(sorted(findings)).encode()).hexdigest() + lines.append(f"Findings fingerprint: `{fingerprint}`") + for finding in sorted(findings): + advisory, crate, version = finding + blocking = finding in added + lines.append( + f"- [{advisory}](https://rustsec.org/advisories/{advisory}.html): " + f"`{crate} {version}` ({'blocking' if blocking else 'inherited'})" + ) + if blocking: + message = ( + f"{advisory} {crate} {version}".replace("%", "%25") + .replace("\r", "%0D") + .replace("\n", "%0A") + ) + print(f"::error::{message}", file=sys.stderr) + if warnings: + lines.append( + "\nNon-blocking cargo-audit warnings (details in job log): " + + ", ".join(f"{kind}: {len(items)}" for kind, items in warnings.items()) + ) + print(json.dumps(warnings, indent=2), file=sys.stderr) + if findings: + lines.append( + "\nMaintainers track remediation in [the security audit issue]" + "(https://github.com/sysprog21/codetrial/issues?q=" + "is%3Aissue+is%3Aopen+author%3Aapp%2Fgithub-actions+" + "in%3Atitle+%22Dependency+security+audit%22) " + "and [Dependabot alerts](https://github.com/sysprog21/codetrial/security/dependabot)." + ) + + +def run(root, base, lines): + head = (root / "Cargo.lock").read_bytes() + # Failing to read the base is fatal: it decides whether errors may pass. + previous = git(root, "show", f"{base}:Cargo.lock") if base else None + informational = previous == head + if informational: + mode = "informational (unchanged lockfile)" + else: + mode = "new vulnerabilities in proposed merge" if base else "full" + lines.append("Mode: " + mode) + try: + findings, inherited, warnings, revision = scan(head, previous, informational) + except (OSError, ValueError, subprocess.CalledProcessError) as error: + if not informational: + raise + lines.append( + f"\nScan incomplete: {error}. No dependency change; informational only." + ) + print( + "::warning::Security scan incomplete; unchanged lockfile, see audit summary." + ) + return 0, None + added = findings - inherited + render(lines, findings, added, warnings, revision) + return int(bool(added)), bool(findings) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--base", help="base tip SHA of the tested merge, not its merge-base" + ) + parser.add_argument( + "--report", type=Path, help="write a Markdown report for security tracking" + ) + args = parser.parse_args() + if args.base == "": + parser.error("--base must name the tested merge's base commit") + root = Path(__file__).resolve().parents[1] + lines = ["# Dependency security audit", ""] + findings = None + try: + status, findings = run(root, args.base, lines) + except (OSError, ValueError, subprocess.CalledProcessError) as error: + lines.append(f"\nScan incomplete: {error}") + print( + "::error::Security scan did not complete; see audit summary.", + file=sys.stderr, + ) + status = 2 + report = "\n".join(lines) + "\n" + print(report) + if args.report: + # GitHub issue bodies accept at most 65,536 characters. + args.report.write_text( + report + if len(report) <= 60000 + else report[:60000] + + "\n\nReport truncated; see the security audit workflow log for all findings.\n" + ) + if summary_path := os.environ.get("GITHUB_STEP_SUMMARY"): + with open(summary_path, "a") as summary: + summary.write(report) + output_path = os.environ.get("GITHUB_OUTPUT") + if findings is not None and output_path: + with open(output_path, "a") as output: + output.write(f"findings={str(findings).lower()}\n") + return status + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test.sh b/scripts/test.sh index 3f38c70e9..d8d9dbe15 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -307,11 +307,10 @@ ruff_gate() # gate. Repo-wide false positives are answered once in `.shellcheckrc`; the few # local ones carry a directive naming why, beside the code it is about. # -# Optional the way cargo-audit is: CI installs it, and a contributor without it -# gets a note rather than a failure they cannot act on. That note counts the -# scripts off the same glob the loop walks, because the number was typed into -# the sentence once and adding a script left it describing a tree that no longer -# existed. +# Optional: CI installs it, and a contributor without it gets a note rather than +# a failure they cannot act on. That note counts the scripts off the same glob +# the loop walks, because the number was typed into the sentence once and adding +# a script left it describing a tree that no longer existed. shell_syntax() { status=0 @@ -464,6 +463,7 @@ gate study-plan-guards "$PYTHON" "$ROOT/scripts/check-study-plan-guards.py" gate calibration-fixtures "$PYTHON" "$ROOT/scripts/gen-calibration-fixtures.py" --check gate calibration-tests unittest_gate "$ROOT/tests/test_calibrate_framework.py" gate gemini-usage-tests unittest_gate "$ROOT/tests/test_analyze_gemini_usage.py" +gate audit-tests unittest_gate "$ROOT/tests/test_audit.py" gate test-gate-tests unittest_gate "$ROOT/tests/test_test_gate.py" gate mutants-plan-tests unittest_gate "$ROOT/tests/test_mutants_plan.py" gate mutants-install-tests unittest_gate "$ROOT/tests/test_mutants_install.py" @@ -477,7 +477,7 @@ gate skill-refs "$PYTHON" "$ROOT/scripts/check-skill-refs.py" gate skill-scripts-tests unittest_gate "$ROOT/tests/test_skill_scripts.py" gate --lane browser browser-tests browser_tests static gate eslint eslint_gate -gate cargo-audit cargo_audit_gate +gate --lane all cargo-audit cargo_audit_gate gate vendor-patches vendor_patches_gate gate actionlint actionlint_gate diff --git a/tests/test_audit.py b/tests/test_audit.py new file mode 100644 index 000000000..5b501d705 --- /dev/null +++ b/tests/test_audit.py @@ -0,0 +1,462 @@ +"""Exercise audits with real base commits and a deterministic cargo boundary.""" + +import importlib.util +import json +import os +import re +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +SPEC = importlib.util.spec_from_file_location( + "support", Path(__file__).with_name("support.py") +) +support = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(support) + +CARGO = """#!/usr/bin/env python3 +import json, os, subprocess, sys +from pathlib import Path +args = sys.argv +db = Path(args[args.index("--db") + 1]) +with open(os.environ["AUDIT_CALLS"], "a") as log: + log.write(json.dumps(args[1:]) + "\\n") +if "--no-fetch" not in args: + db.mkdir() + subprocess.run(["git", "init", "--quiet", str(db)], check=True) + subprocess.run(["git", "-C", str(db), "-c", "user.name=Test", + "-c", "user.email=test@example.org", "-c", "core.hooksPath=/dev/null", + "commit", "--quiet", "--allow-empty", "-m", "snapshot"], check=True) +data = json.loads(Path(args[args.index("--file") + 1]).read_text()) +if "raw" in data: + print(data["raw"]) + sys.exit(data.get("exit", 1)) +items = [{"advisory": {"id": a}, "package": {"name": n, "version": v}} + for a, n, v in data["findings"]] +print(json.dumps({"vulnerabilities": {"found": bool(items), "count": len(items), + "list": items}, "warnings": data.get("warnings", {})})) +sys.exit(data.get("exit", int(bool(items)))) +""" + +GH = """#!/usr/bin/env python3 +import json, os, subprocess, sys +args = sys.argv[1:] +action = "api" if args[0] == "api" else args[1] +with open(os.environ["GH_CALLS"], "a") as log: + log.write(json.dumps(args) + "\\n") +if action == os.environ["FAIL_ACTION"]: + sys.exit(1) +if "--slurp" in args and ("--jq" in args or "--template" in args): + sys.exit("the `--slurp` option is not supported with `--jq` or `--template`") +if action == "api": + comments = any(arg.endswith("/comments") for arg in args) + print(os.environ["COMMENT_PAGES" if comments else "ISSUE_PAGES"]) +if action == "view": + print(os.environ["OLD_BODY"]) +""" + +A = ["RUSTSEC-2026-0385", "ctutils", "0.4.3"] +B = ["RUSTSEC-2026-9999", "other", "1.0.0"] +ISSUE = { + "number": 42, + "title": "Dependency security audit", + "state": "open", + "user": {"login": "github-actions[bot]"}, +} + + +class AuditTests(unittest.TestCase): + def test_empty_base_cannot_fall_back_to_a_full_scan(self): + result = subprocess.run( + [sys.executable, str(support.ROOT / "scripts/audit.py"), "--base", ""], + capture_output=True, + text=True, + timeout=60, + ) + self.assertEqual(result.returncode, 2) + self.assertIn("--base must name", result.stderr) + + def run_tracking( + self, pages, *, findings="true", old="", current="", fail="", comments=() + ): + workflow = (support.ROOT / ".github/workflows/audit.yml").read_text() + tracking = support.step(support.job("audit", workflow), "Track remediation") + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + (root / "audit.md").write_text(current) + support.write_executable(root / "gh", GH) + result = subprocess.run( + # Actions runs bash steps with errexit and pipefail. + ["bash", "-euo", "pipefail", "-c", support.run_block(tracking)], + env={ + **os.environ, + "PATH": str(root) + os.pathsep + os.environ["PATH"], + "GH_CALLS": str(root / "calls"), + "RUNNER_TEMP": str(root), + "ISSUE_PAGES": json.dumps(pages), + "COMMENT_PAGES": json.dumps([list(comments)]), + "OLD_BODY": old, + "FAIL_ACTION": fail, + "FINDINGS": findings, + "GH_REPO": "sysprog21/codetrial", + }, + capture_output=True, + text=True, + timeout=60, + ) + calls = [ + json.loads(line) for line in (root / "calls").read_text().splitlines() + ] + self.assertIn("creator=github-actions[bot]", calls[0]) + self.assertIn("--paginate", calls[0]) + self.assertIn("--slurp", calls[0]) + self.assertIn("state=all", calls[0]) + for call in calls[1:]: + if "--body-file" in call: + self.assertEqual( + Path(call[call.index("--body-file") + 1]).read_text(), current + ) + return result, calls + + def test_only_trusted_main_runs_with_issue_write_access(self): + workflow = (support.ROOT / ".github/workflows/audit.yml").read_text() + job = support.job("audit", workflow) + tracking = support.step(job, "Track remediation") + self.assertIn("steps.audit.outputs.findings != ''", tracking) + self.assertIn("if: github.ref == 'refs/heads/main'", job) + self.assertIn( + " permissions:\n contents: read\n issues: write\n", job + ) + self.assertIn("\npermissions:\n contents: read\n\n", workflow) + triggers = support.section( + workflow, "\non:\n", "\npermissions:", "audit triggers" + ) + self.assertNotIn("pull_request", triggers) + + @unittest.skipUnless(shutil.which("jq"), "jq is not installed") + def test_tracking_filters_real_json_including_other_authors_prs_and_pages(self): + bot = {key: value for key, value in ISSUE.items() if key != "number"} + decoys = [ + {**bot, "number": 1, "user": {"login": "outsider"}}, + {**bot, "number": 2, "pull_request": {}}, + {**bot, "number": 3, "title": "Other issue"}, + ] + for pages, expected in ( + ([[]], None), + ([decoys], None), + ([decoys, [{**bot, "number": 42}, {**bot, "number": 43}]], "42"), + ): + with self.subTest(pages=pages): + result, calls = self.run_tracking(pages, findings="false") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual( + [call[:3] for call in calls[1:]], + [["issue", "edit", expected], ["issue", "close", expected]] + if expected + else [], + ) + + @unittest.skipUnless(shutil.which("jq"), "jq is not installed") + def test_tracking_notifies_only_when_findings_change_and_preserves_retry(self): + issue = ISSUE + closed = {**issue, "state": "closed", "closed_at": "2026-01-03T00:00:00Z"} + unchanged = "Findings fingerprint: `same`\n" + changed = "Findings fingerprint: `new`\n" + bot = {"login": "github-actions[bot]"} + before = "2026-01-02T00:00:00Z" + after = "2026-01-04T00:00:00Z" + posted = [{"user": bot, "created_at": before, "body": "Report\n" + changed}] + reposted = [{**posted[0], "created_at": after}] + stale = [ + {"user": bot, "created_at": before, "body": "Report\n" + changed}, + {"user": bot, "created_at": before, "body": "Report\n" + unchanged}, + {"user": {"login": "outsider"}, "created_at": after, "body": changed}, + ] + notify = ["view", "comments", "comment", "edit"] + for pages, findings, old, current, fail, comments, actions in ( + ([[]], "true", "", unchanged, "", (), ["create"]), + ( + [[issue]], + "true", + unchanged, + unchanged + "New database revision\n", + "", + (), + ["view", "edit"], + ), + ([[issue]], "true", unchanged, changed, "", (), notify), + ([[issue]], "true", "Legacy report", changed, "", (), notify), + ([[issue]], "true", unchanged, changed, "", stale, notify), + # A comment whose follow-up edit failed is not posted twice. + ( + [[issue]], + "true", + unchanged, + changed, + "", + posted, + ["view", "comments", "edit"], + ), + # Recurring findings reopen the issue its subscribers follow. + ( + [[closed]], + "true", + unchanged, + unchanged, + "", + (), + ["view", "edit", "reopen"], + ), + # Findings that return after a clean close notify again, even when + # they match the comment from before the close. + ( + [[closed]], + "true", + "Clean report", + changed, + "", + posted, + notify + ["reopen"], + ), + # A comment posted after the close, whose edit then failed, is not + # posted again. + ( + [[closed]], + "true", + "Clean report", + changed, + "", + reposted, + ["view", "comments", "edit", "reopen"], + ), + ([[issue]], "false", unchanged, "Clean report", "", (), ["edit", "close"]), + ([[closed]], "false", unchanged, "Clean report", "", (), []), + ([[]], "false", "", "Clean report", "", (), []), + ([[issue]], "true", unchanged, changed, "api", (), []), + ([[issue]], "true", unchanged, changed, "comment", (), notify[:3]), + ([[issue]], "true", unchanged, changed, "edit", (), notify), + ): + with self.subTest( + pages=pages, findings=findings, old=old, fail=fail, actions=actions + ): + result, calls = self.run_tracking( + pages, + findings=findings, + old=old, + current=current, + fail=fail, + comments=comments, + ) + self.assertEqual(result.returncode, int(bool(fail)), result.stderr) + names = [ + "comments" if call[0] == "api" else call[1] for call in calls[1:] + ] + self.assertEqual(names, actions) + for call in calls[1:]: + if call[0] == "api": + self.assertIn( + "repos/sysprog21/codetrial/issues/42/comments", call + ) + elif call[1] != "create": + self.assertEqual(call[2], "42") + + def run_audit(self, base, head, *, full=False, invalid_base=False): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + (root / "scripts").mkdir() + script = root / "scripts/audit.py" + script.write_bytes((support.ROOT / "scripts/audit.py").read_bytes()) + support.write_executable(root / "cargo", CARGO) + + def git(*args): + return subprocess.check_output(["git", "-C", str(root), *args]) + + git("init", "--quiet") + git("config", "user.name", "Test") + git("config", "user.email", "test@example.org") + git("config", "core.hooksPath", "/dev/null") + lockfile = root / "Cargo.lock" + lockfile.write_text(json.dumps(base)) + git("add", "Cargo.lock") + git("commit", "--quiet", "-m", "base") + sha = git("rev-parse", "HEAD").decode().strip() + lockfile.write_text(json.dumps(head)) + command = [sys.executable, str(script), "--report", str(root / "report.md")] + if not full: + command += ["--base", "missing" if invalid_base else sha] + env = { + **os.environ, + "PATH": str(root) + os.pathsep + os.environ["PATH"], + "AUDIT_CALLS": str(root / "calls"), + "GITHUB_OUTPUT": str(root / "outputs"), + "GITHUB_STEP_SUMMARY": str(root / "summary"), + } + result = subprocess.run( + command, env=env, capture_output=True, text=True, timeout=30 + ) + report = (root / "report.md").read_text() + summary = (root / "summary").read_text() + if len(summary) <= 60000: + self.assertEqual(report, summary) + else: + self.assertTrue(report.startswith(summary[:60000])) + self.assertIn("Report truncated", report) + outputs = ( + (root / "outputs").read_text() if (root / "outputs").exists() else "" + ) + calls = ( + [json.loads(line) for line in (root / "calls").read_text().splitlines()] + if (root / "calls").exists() + else [] + ) + return result, report, outputs, calls + + def test_unchanged_lockfile_reports_inherited_findings_with_one_scan(self): + result, report, outputs, calls = self.run_audit( + {"findings": [A]}, {"findings": [A]} + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("ctutils 0.4.3` (inherited)", report) + self.assertIn("blocking: 0", report) + self.assertNotIn("::error::", result.stderr) + self.assertEqual(outputs, "findings=true\n") + self.assertEqual(len(calls), 1) + + def test_changed_lockfile_uses_one_snapshot_and_allows_inherited_findings(self): + result, report, _, calls = self.run_audit( + {"findings": [A]}, {"findings": [A], "change": True} + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("(inherited)", report) + self.assertEqual(len(calls), 2) + self.assertNotIn("--no-fetch", calls[0]) + self.assertIn("--no-fetch", calls[1]) + self.assertEqual( + calls[0][calls[0].index("--db") + 1], calls[1][calls[1].index("--db") + 1] + ) + + def test_equal_counts_do_not_hide_replacement_or_version_changes(self): + for finding in (B, [*A[:2], "0.4.2"]): + with self.subTest(finding=finding): + result, report, _, _ = self.run_audit( + {"findings": [A]}, {"findings": [finding]} + ) + self.assertEqual(result.returncode, 1, result.stderr) + self.assertIn("blocking: 1", report) + + def test_fixed_current_base_blocks_a_vulnerability_from_an_old_branch(self): + result, report, _, _ = self.run_audit({"findings": []}, {"findings": [A]}) + self.assertEqual(result.returncode, 1, result.stderr) + self.assertIn("(blocking)", report) + self.assertIn("::error::RUSTSEC-2026-0385 ctutils 0.4.3", result.stderr) + + def test_annotations_escape_workflow_command_control_characters(self): + finding = [A[0], "crate%\r\n::warning::injected", "1.0.0"] + result, _, _, _ = self.run_audit( + {"findings": []}, {"findings": [finding]}, full=True + ) + self.assertEqual(result.returncode, 1) + self.assertIn( + "::error::RUSTSEC-2026-0385 crate%25%0D%0A::warning::injected 1.0.0", + result.stderr, + ) + self.assertNotIn("\n::warning::", result.stderr) + + def test_fingerprint_tracks_findings_not_order_or_database_and_warning_changes( + self, + ): + fingerprints = [] + for findings, warnings in ( + ([A, B], {}), + ([B, A], {"unmaintained": ["example"]}), + ([A], {}), + ): + result, report, _, _ = self.run_audit( + {"findings": []}, + {"findings": findings, "warnings": warnings}, + full=True, + ) + self.assertEqual(result.returncode, 1, result.stderr) + match = re.search(r"^Findings fingerprint: `([0-9a-f]{64})`$", report, re.M) + self.assertIsNotNone(match) + fingerprints.append(match[1]) + self.assertEqual(fingerprints[0], fingerprints[1]) + self.assertNotEqual(fingerprints[0], fingerprints[2]) + + def test_full_scan_blocks_findings_but_accepts_a_patch(self): + for findings, status in (([A], 1), ([], 0)): + with self.subTest(findings=findings): + result, _, outputs, calls = self.run_audit( + {"findings": []}, {"findings": findings}, full=True + ) + self.assertEqual(result.returncode, status, result.stderr) + self.assertEqual(outputs, f"findings={str(bool(findings)).lower()}\n") + self.assertEqual(len(calls), 1) + + def test_incomplete_scans_never_claim_a_clean_result(self): + # Every malformed report fails the same schema check, so one per mode + # suffices beyond the diff mode that tries them all. + cases = [ + {"raw": "not JSON"}, + {"raw": "{}"}, + { + "raw": json.dumps( + {"vulnerabilities": {"found": False, "count": 0, "list": {}}} + ), + "exit": 0, + }, + {"findings": [], "warnings": []}, + {"findings": [], "exit": 2}, + {"findings": [A], "exit": 0}, + ] + runs = [(broken, "diff") for broken in cases] + runs += [(cases[0], "full"), (cases[-1], "informational")] + for broken, mode in runs: + with self.subTest(broken=broken, mode=mode): + result, report, outputs, _ = self.run_audit( + broken if mode == "informational" else {"findings": []}, + broken, + full=mode == "full", + ) + self.assertEqual(result.returncode, 0 if mode == "informational" else 2) + self.assertIn("Scan incomplete", report) + self.assertEqual(outputs, "") + + def test_base_scan_errors_and_missing_bases_block(self): + for base, invalid in (({"raw": "broken"}, False), ({"findings": []}, True)): + with self.subTest(base=base, invalid=invalid): + result, report, outputs, _ = self.run_audit( + base, {"findings": []}, invalid_base=invalid + ) + self.assertEqual(result.returncode, 2) + self.assertIn("Scan incomplete", report) + self.assertEqual(outputs, "") + + def test_warnings_remain_visible_without_blocking(self): + result, report, _, _ = self.run_audit( + {"findings": []}, + {"findings": [], "warnings": {"unmaintained": ["example"]}}, + full=True, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("unmaintained", report) + + def test_large_issue_reports_are_bounded_without_losing_the_full_summary(self): + findings = [ + ["RUSTSEC-2026-9999", f"crate-{index}-" + "x" * 200, "1.0.0"] + for index in range(300) + ] + result, report, outputs, _ = self.run_audit( + {"findings": []}, {"findings": findings}, full=True + ) + self.assertEqual(result.returncode, 1, result.stderr) + self.assertLess(len(report), 65536) + self.assertIn("Report truncated", report) + self.assertEqual(outputs, "findings=true\n") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_skill_scripts.py b/tests/test_skill_scripts.py index 433f48984..d585ab47e 100644 --- a/tests/test_skill_scripts.py +++ b/tests/test_skill_scripts.py @@ -1314,6 +1314,30 @@ def test_ci_failure_names_the_gates_and_what_main_shares(self): # indent failed beside cargo-audit, so a rebase would not clear it. self.assertIs(found["stale_base"], False) + def test_ci_failure_reports_the_advisory_from_an_audit_annotation(self): + checks = [ + { + "name": "audit", + "state": "FAILURE", + "bucket": "fail", + "link": "https://github.com/sysprog21/codetrial/actions/runs/7/job/70", + } + ] + message = "RUSTSEC-2026-0385 ctutils 0.4.3" + result = run_fake( + VERIFY / "ci-failure.sh", + [5], + FAKE_GH_CI, + { + "FAKE_CHECKS": json.dumps(checks), + "FAKE_LOG": f"audit\tscan\t2026-10-22T00:00:00.0Z ##[error]{message}\n", + }, + ) + self.assertEqual(result.returncode, 0, result.stderr) + found = json.loads(result.stdout) + self.assertEqual(found["failing"][0]["errors"], [message]) + self.assertFalse(found["stale_base"]) + def test_an_audit_failure_main_has_fixed_is_a_stale_base(self): checks = [ { diff --git a/tests/test_test_gate.py b/tests/test_test_gate.py index 2606f4db3..6d9cd5ba0 100644 --- a/tests/test_test_gate.py +++ b/tests/test_test_gate.py @@ -168,7 +168,8 @@ def test_parallel_lanes_cover_the_default_gate(self): self.assertEqual(rust & browser, shared) self.assertEqual(portable & browser, shared) self.assertEqual(browser - shared, {"browser-tests"}) - self.assertEqual(rust | portable | browser, full) + self.assertEqual(rust | portable | browser | {"cargo-audit"}, full) + self.assertNotIn("cargo-audit", rust | portable | browser) self.assertEqual(self.run_lane()[1], results["all"][1]) def test_new_gate_is_included_in_portable_lane(self): @@ -221,27 +222,27 @@ def test_ci_refuses_skipped_gates(self): self.assertIn("failed gates: skipped-gates", result.stderr) def run_status(self, command, **env): + passing = dict.fromkeys(("GATES", "AUDIT", "PLAN", "MUTANTS"), "success") return subprocess.run( ["bash", "-e", "-c", command], - env={**os.environ, **env}, + env={**os.environ, **passing, **env}, capture_output=True, check=False, ).returncode def test_check_requires_the_lanes_and_explains_every_mutation_skip(self): check = support.job("check") - self.assertIn(" needs: [gates, plan-mutants, mutants]\n", check) + self.assertIn(" needs: [gates, audit, plan-mutants, mutants]\n", check) self.assertIn(f" if: ${{{{ !cancelled() && {NOT_AVATAR} }}}}\n", check) command = support.run_block(check) states = ("success", "failure", "cancelled", "skipped", "") - # The lanes are checked first, so one passing mutation answer stands in - # for the rest. - for gates in states: - with self.subTest(gates=gates): - status = self.run_status( - command, GATES=gates, PLAN="success", MUTANTS="success" - ) - self.assertEqual(status == 0, gates == "success") + # The lanes and audit are checked first, so one passing mutation answer + # stands in for the rest. + for lane in ("GATES", "AUDIT"): + for state in states: + with self.subTest(lane=lane, state=state): + status = self.run_status(command, **{lane: state}) + self.assertEqual(status == 0, state == "success") accepted = { # Shards ran and passed. ("success", "success"), @@ -253,16 +254,93 @@ def test_check_requires_the_lanes_and_explains_every_mutation_skip(self): for plan in states: for mutants in states: with self.subTest(plan=plan, mutants=mutants): - status = self.run_status( - command, GATES="success", PLAN=plan, MUTANTS=mutants - ) + status = self.run_status(command, PLAN=plan, MUTANTS=mutants) self.assertEqual(status == 0, (plan, mutants) in accepted) + def test_audit_dispatches_events_and_stops_on_git_or_scan_errors(self): + audit = support.job("audit") + command = support.run_block(audit) + self.assertIn("GROUP_BASE: ${{ github.event.merge_group.base_sha }}", audit) + self.assertIn("fetch-depth: 2", audit) + self.assertNotIn("Install cargo-audit", gates_job()) + parent = ["git:rev-parse HEAD^1"] + group = ["git:cat-file -e group-base^{commit}"] + fetch = "git:fetch --no-tags --depth=1 origin group-base" + for event, fail, expected_status, expected_calls in ( + ("push", "", 0, ["python:scripts/audit.py"]), + ( + "pull_request", + "", + 0, + parent + ["python:scripts/audit.py --base actual-parent"], + ), + ( + "merge_group", + "", + 0, + group + ["python:scripts/audit.py --base group-base"], + ), + ( + "merge_group", + "missing-base", + 0, + group + [fetch, "python:scripts/audit.py --base group-base"], + ), + ("merge_group", "fetch", 1, group + [fetch]), + ("pull_request", "rev-parse", 1, parent), + ("push", "scan", 2, ["python:scripts/audit.py"]), + ("unknown", "", 2, []), + ): + with self.subTest(event=event, fail=fail): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + support.write_executable( + root / "git", + """#!/bin/sh +printf 'git:%s\\n' "$*" >> "$CALLS" +case "$1" in + rev-parse) [ "$FAIL" != rev-parse ] || exit 1; echo actual-parent ;; + cat-file) [ "$FAIL" != missing-base ] && [ "$FAIL" != fetch ] ;; + fetch) [ "$FAIL" != fetch ] ;; + *) exit 99 ;; +esac +""", + ) + support.write_executable( + root / "python3", + """#!/bin/sh +printf 'python:%s\\n' "$*" >> "$CALLS" +[ "$FAIL" != scan ] || exit 2 +""", + ) + result = subprocess.run( + ["bash", "-eo", "pipefail", "-c", command], + cwd=root, + env={ + **os.environ, + "PATH": str(root) + os.pathsep + os.environ["PATH"], + "CALLS": str(root / "calls"), + "EVENT": event, + "FAIL": fail, + "GROUP_BASE": "group-base", + }, + capture_output=True, + text=True, + timeout=60, + ) + self.assertEqual(result.returncode, expected_status, result.stderr) + calls = ( + (root / "calls").read_text().splitlines() + if (root / "calls").exists() + else [] + ) + self.assertEqual(calls, expected_calls) + def test_publishing_waits_on_the_lanes_not_on_check(self): # A push skips the mutation jobs `check` waits on, and the default # `success()` would carry that skip through `check` to both of these. - self.assertIn(" needs: gates\n", support.job("build")) - self.assertIn(" needs: [gates, build]\n", support.job("release")) + self.assertIn(" needs: [gates, audit]\n", support.job("build")) + self.assertIn(" needs: [gates, audit, build]\n", support.job("release")) def test_each_lane_has_a_bounded_job(self): timeouts = {lane: int(entry["timeout"]) for lane, entry in lanes().items()} @@ -369,7 +447,7 @@ def test_each_lane_installs_what_its_gates_need(self): set(matrix["portable"]["packages"].split()), {"shellcheck", "shfmt"} ) self.assertEqual(set(matrix["browser"]), {"lane", "timeout"}) - for name in ("ruff", "commentflow", "actionlint", "cargo-audit"): + for name in ("ruff", "commentflow", "actionlint"): with self.subTest(tool=name): self.assertEqual( condition(gates, f"Install {name}"), "matrix.lane == 'portable'"