diff --git a/.github/dependabot.yml b/.github/dependabot.yml index fb6eeb3d..b22989ab 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -25,12 +25,15 @@ updates: open-pull-requests-limit: 10 # Supply-chain cooldown: age a fresh release before opening a ROUTINE update PR, to dodge a # package compromised shortly after publish. Security updates ignore cooldown (Dependabot - # behavior), so a real advisory fix still arrives immediately. Pairs with the auto-merge - # workflow: routine patches auto-merge AFTER aging; security patches auto-merge now. + # behavior), so a real advisory fix still opens as a PR immediately. Aging is a PRECONDITION for + # an unattended merge, never a sufficient one — dependabot-auto-merge.yml's allow-set decides + # eligibility, and THIS ecosystem's allow row ships EMPTY, so no uv bump auto-merges on either + # track; every one goes to human review. That workflow owns the decision, so read it there. cooldown: # ~5-day aging window (DEPENDENCY-POSTURE-REVIEW.md) lengthens the malicious-fresh-publish # dodge on the routine VERSION track; the SECURITY track still bypasses cooldown (Dependabot - # design), now backstopped by the published-GHSA gate in dependabot-auto-merge.yml (SEC-007 #2). + # design) and is aged workflow-side instead, behind the published-GHSA gate (SEC-007 #2) and + # the release-age gate (#4) in dependabot-auto-merge.yml. default-days: 5 semver-major-days: 7 # Dependabot WIDENS a declared cap instead of respecting it, so a load-bearing upper bound in @@ -78,7 +81,9 @@ updates: # stops tag mutation, not a malicious new release — Dependabot just bumps the pin to the new SHA. # Only default-days is honored here (this ecosystem does not support the semver-*-days knobs) and # it ages off the tag's commit date rather than the release date, so treat 5 as approximate. - # Security updates bypass cooldown, so an advisory fix is unaffected. Watch codeql-action — a + # Security updates bypass cooldown, so an advisory fix still opens as a PR unaffected — but it + # reaches main by human review rather than unattended, because dependabot-auto-merge.yml's + # advisory gate is pip-keyed and therefore never confirms an action. Watch codeql-action — a # frequent releaser — for stalls after this lands. cooldown: default-days: 5 diff --git a/.github/workflows/dependabot-auto-merge.yml b/.github/workflows/dependabot-auto-merge.yml index ccbc85dd..74089abd 100644 --- a/.github/workflows/dependabot-auto-merge.yml +++ b/.github/workflows/dependabot-auto-merge.yml @@ -8,36 +8,70 @@ name: Dependabot auto-merge # removes the human-latency on the safe, common case, not the safety net. # # IN SCOPE (auto-merged): -# - any PATCH update (incl. security patches — most security fixes are patches) -# - MINOR updates of DEV-only dependencies +# - a PATCH update (or a MINOR of a DEV-only dependency) whose EVERY named dependency sits on its +# own ecosystem's ALLOW row (guardrail #3 below). # OUT OF SCOPE (left for human review, surfaced same-day by the daily security cron + alerts): -# - MINOR/MAJOR updates of runtime deps, and ALL MAJOR updates -# NOTE the uv ecosystem is review-by-default in practice, not by exception: `python-deps` is -# `patterns: ["*"]`, so one weekly batch carries everything, and (a) a grouped update-type is the -# MAX across the batch (fetch-metadata's UPDATE_TYPES_PRIORITY) — one minor demotes every patch -# riding with it — while (b) the deny-list below names at least eight of this project's directly -# declared runtime deps and denies the WHOLE group on any hit. #66 was denied on `fastapi`. -# Auto-merge does fire for github-actions (#75, all-patch batch) and npm (#64). +# - EVERYTHING ELSE, and that is the default by design. An unnamed dependency, an unrecognised +# ecosystem, a MINOR/MAJOR update of a runtime dep, any MAJOR update — every one of them HOLDS. +# - EVERY uv and npm bump, because both allow rows ship EMPTY. `python-deps`/`ide-deps` group on +# `*`, so one weekly batch carries everything and a non-empty row would have to name each of the +# ~98 pinned distributions before it could ever fire. Those two ecosystems were already +# review-by-default in practice; they are now review-by-default BY DESIGN rather than by accident. +# - a github-actions batch carrying any third-party action, since `actions-deps` also groups on `*`. +# Measured: PR #75's five-bump batch carried `pypa/gh-action-pypi-publish`, so that same batch +# would HOLD under this allow-set. Expect auto-merge to fire RARELY — that is the intent of +# "hold unless named", not a regression. # -# Fresh-release supply-chain poisoning is handled upstream by the dependabot.yml `cooldown` -# (routine updates age before a PR opens); SECURITY updates bypass cooldown by design, so a real -# advisory fix still arrives immediately and (if a patch) auto-merges. +# Fresh-release supply-chain poisoning is handled upstream by the dependabot.yml `cooldown`, which +# every configured ecosystem carries: a routine update ages before its PR opens. The per-ecosystem +# windows — and the caveat that `github-actions` ages off the tag's COMMIT date rather than the +# release date — live in `.github/dependabot.yml` beside each value's rationale. Read them there; a +# copy here would rot silently. SECURITY updates bypass cooldown by Dependabot design, so that track +# is aged HERE instead, workflow-side, by guardrail #4. # -# SECURITY-TRACK GUARDRAILS (DEPENDENCY-POSTURE-REVIEW.md #2 and #3 — close SEC-007, CWE-829): -# #3 DENY-LIST — the auth/token/crypto stack (cryptography, argon2-cffi, fastapi, …) is NEVER -# auto-merged, not even for a patch: any such dep routes the PR to MANUAL review. A grouped -# PR that includes ANY denied dep is denied WHOLE (fail-safe — never partial-merge a group). -# #2 PUBLISHED-GHSA GATE — a SECURITY-track PR (which bypasses the cooldown by Dependabot design) -# auto-merges ONLY when a real, published, non-withdrawn advisory is confirmed against the -# dependency's PREVIOUS version (the axios-1.14.1 pattern). The advisory lookup FAILS CLOSED: -# a rate-limit/API error or no-matching-advisory routes to manual review, never auto-merge. -# The non-security VERSION track keeps its cooldown-aged auto-merge unchanged (already aged by -# dependabot.yml, so it does not need the GHSA gate). +# AUTO-MERGE GUARDRAILS. #2 and #3 keep DEPENDENCY-POSTURE-REVIEW.md's numbering (SEC-007, CWE-829), +# with two corrections that document does not yet carry: #3 is INVERTED from the deny-list it +# describes, and #4 is introduced HERE — the posture review does not contain it. Only #2 and #4 are +# security-track-scoped; #3 gates EVERY PR. +# #3 ALLOW-SET — HOLD UNLESS NAMED, on every track. A PR auto-merges only when EVERY dependency it +# touches is on its own ecosystem's allow row; an unnamed dependency, or an ecosystem this step +# does not recognise, routes the PR to MANUAL review. A grouped PR is denied WHOLE on any single +# ineligible member (fail-safe — never partial-merge a group). This INVERTS the former +# deny-list, whose default was merge-unless-named across every ecosystem at once. +# #2 PUBLISHED-GHSA GATE (security track only) — a SECURITY-track PR (which bypasses the cooldown +# by Dependabot design) auto-merges ONLY when a real, published, non-withdrawn advisory is +# confirmed against the dependency's PREVIOUS version (the axios-1.14.1 pattern). The advisory +# lookup FAILS CLOSED: a rate-limit/API error or no-matching-advisory routes to manual review, +# never auto-merge. The non-security VERSION track keeps its cooldown-aged auto-merge unchanged +# (already aged by dependabot.yml, so it does not need the GHSA gate). +# #4 RELEASE AGE (security track only) — a SECURITY-track PR additionally requires every candidate +# version to have been published at least MIN_RELEASE_AGE_HOURS ago, because that track is the +# one the upstream cooldown cannot age. FAILS CLOSED identically to #2: an API error, a missing +# or unparseable upload timestamp, a package name/version that does not match its expected +# shape, or an ecosystem with no publish-date source wired all route to manual review. +# READ THIS BEFORE CITING #4 AS A CONTROL: it IS INERT with respect to the merge decision as +# shipped, and recording it as operating would be exactly the false premise +# docs/Secure_Development_Standards.md §3 forbids. +# `age_ok=true` is reachable only for uv/pip (no publish-date source is wired for any +# other ecosystem), while `eligible=true` is reachable only for github-actions (the uv/pip/npm +# allow rows ship EMPTY). Those two sets are DISJOINT, so no PR can satisfy both conjuncts the +# merge `if` requires. #4 is a FORWARD guard — built, tested and gated now, load-bearing the day +# a Python allow row is populated or the advisory gate is made ecosystem-aware. Populating an +# allow row is the owner's call, not this workflow's. # -# RESIDUAL ACCEPTED RISK: a malicious patch that BOTH rides a real concurrent published advisory -# AND is not on the deny-list would still auto-merge. The deny-list shields the highest-value -# (auth/crypto) deps unconditionally; the GHSA gate blocks the no-advisory fast-publish poison on -# the security track; main's required CI (pip-audit/bandit/tests/lock-sync) backstops both. +# RESIDUAL ACCEPTED RISK, stated at the ONE path that can actually auto-merge: a VERSION-track patch +# (or dev-only minor) bumping a GitHub-published action. Its only aging is dependabot.yml's cooldown, +# which that file itself qualifies as approximate for this ecosystem, so a malicious release from a +# compromised GitHub-owned publisher would still auto-merge. Neither #2 nor #4 narrows that path — +# both are security-track-only, and this path is the version track. +# What main's required CI does and does not backstop, stated precisely: pip-audit reads the +# committed LOCKS for already-published advisories, and bandit/semgrep read THIS repository's own +# source — so none of them ever reads a dependency's contents, and a package that was malicious at +# publish time is not a known CVE. What those required checks DO catch is a bump that breaks the +# build or regresses a KNOWN advisory. No REQUIRED check reads a dependency's shipped bytes. +# security.yml's `trivy` job scans the built engine image, but it is advisory (`continue-on-error`) +# and cron/dispatch-only, so it never runs on this PR — and it matches an advisory database +# regardless. Treat the allow-set plus the aging above as the control, not a downstream scanner. # # Why `pull_request` (not pull_request_target): a Dependabot `pull_request` run gets a read-only # GITHUB_TOKEN by default, which the `permissions:` block below elevates to exactly what the merge @@ -70,33 +104,81 @@ jobs: with: github-token: ${{ secrets.GITHUB_TOKEN }} - # Guardrail #3 (DEPENDENCY-POSTURE-REVIEW.md): the auth/token/crypto stack NEVER auto-merges, - # not even a patch. dependency-names is comma-separated for grouped PRs; we deny the WHOLE - # group if ANY token matches (fail-safe — never partial-merge a group). Exact token match on - # the comma-split names (not a naive substring) so 'cffi' does not match 'argon2-cffi-bindings' - # and vice-versa. - - name: Reject security-critical deps (deny-list) - id: denylist + # Guardrail #3 (DEPENDENCY-POSTURE-REVIEW.md), INVERTED from a deny-list to an ALLOW-SET: the + # default is HOLD, and only a dependency named on its own ecosystem's allow row is eligible. + # dependency-names is comma-separated for grouped PRs; the group is denied WHOLE if ANY member + # is ineligible — the same fail-safe the deny-list had, never a partial group merge. Measured + # consequence: PR #75's five-bump batch carried `pypa/gh-action-pypi-publish`, which is NOT on + # the allow row, so that batch would now HOLD — and that action runs in the `id-token: write` + # publish job, which is exactly the case worth holding. + # + # Every input arrives via `env:`; nothing is interpolated into the body. That is zizmor + # template-injection parity, and it is what lets the behavioural tests execute this body + # verbatim under `bash -e` — the shell Actions applies by default, which those tests mirror. + - name: Hold everything not on the auto-merge allow-set + id: allowset env: DEP_NAMES: ${{ steps.meta.outputs.dependency-names }} + DEP_ECOSYSTEM: ${{ steps.meta.outputs.package-ecosystem }} run: | - # Single source of truth for the security-critical deny-list. - denylist="cryptography argon2-cffi argon2-cffi-bindings paramiko ldap3 pyspnego \ - fastapi starlette uvicorn pydantic pydantic-core python-jose pyjwt passlib bcrypt cffi" - deny=false + set -u + # Normalise with bash parameter expansion only (no echo|tr|xargs) so this block depends on + # nothing outside bash itself and stays executable from pytest on any platform. + eco="${DEP_ECOSYSTEM,,}" + eco="${eco//[[:space:]]/}" + + # These tokens are Dependabot BRANCH-NAME forms, not the dependabot.yml spellings: + # fetch-metadata derives packageEcosystem from chunks[1] of the branch name. Measured on + # live PRs — #135 `dependabot/uv/…`, #75 `dependabot/github_actions/…`, #64 + # `dependabot/npm_and_yarn/ide/…`. Both spellings are accepted so a future rename of either + # side still matches; an UNRECOGNISED token falls to `*)` and holds rather than merging. + case "$eco" in + github_actions|github-actions) + # GitHub-published actions only. GitHub already owns the runner and the job's token, so + # these add no trust root that is not already fully trusted. Deliberately NOT here: + # pypa/gh-action-pypi-publish, astral-sh/setup-uv, ossf/scorecard-action, + # contributor-assistant/github-action — every third-party action holds. + allow_prefixes="actions/ github/ dependabot/" + ;; + uv|pip|npm_and_yarn|npm) + # Deliberately EMPTY. `python-deps`/`ide-deps` group on `*`, so one weekly batch carries + # everything and a non-empty row would have to name every one of the ~98 pinned + # distributions before it could ever fire. These ecosystems were already + # review-by-default in practice; this makes it review-by-default BY DESIGN. + allow_prefixes="" + ;; + *) + echo "::notice::unrecognised package ecosystem '${eco:-}' — holding for manual review." + allow_prefixes="" + ;; + esac + + eligible=true + saw_any=false IFS=',' read -ra names <<< "$DEP_NAMES" for raw in "${names[@]}"; do - # trim surrounding whitespace and lowercase - name="$(echo "$raw" | tr '[:upper:]' '[:lower:]' | xargs)" + name="${raw,,}" + name="${name//[[:space:]]/}" [ -z "$name" ] && continue - for crit in $denylist; do - if [ "$name" = "$crit" ]; then - echo "::notice::'$name' is a security-critical dependency — routing to manual review (no auto-merge)." - deny=true - fi + saw_any=true + matched=false + for p in $allow_prefixes; do + # ANCHORED prefix test, and the trailing slash in each prefix is load-bearing. The two + # near-misses fail for DIFFERENT reasons, so both are pinned by tests: a substring + # match would admit `evil/actions/checkout`, and dropping the trailing slash would + # admit `actionsx/foo`. + case "$name" in + "$p"*) matched=true ;; + esac done + if [ "$matched" != true ]; then + echo "::notice::'$name' is not on the '${eco:-}' auto-merge allow-set — routing the WHOLE PR to manual review." + eligible=false + fi done - echo "deny=$deny" >> "$GITHUB_OUTPUT" + # An empty name list cannot CONFIRM eligibility, so it must not be read as confirming it. + [ "$saw_any" = true ] || eligible=false + echo "eligible=$eligible" >> "$GITHUB_OUTPUT" # Guardrail #2 (DEPENDENCY-POSTURE-REVIEW.md): a SECURITY-track PR auto-merges ONLY when a # real, published, non-withdrawn advisory covers the dependency's PREVIOUS version. The @@ -172,15 +254,146 @@ jobs: done <<< "$pairs" echo "advisory_ok=$advisory_ok" >> "$GITHUB_OUTPUT" + # Guardrail #4: the SECURITY track bypasses dependabot.yml's `cooldown` by Dependabot design, + # so it is aged HERE instead — the candidate version must have been published at least + # MIN_RELEASE_AGE_HOURS ago. This defeats the fast-publish-then-yank pattern (axios-1.14.1) that + # a cooldown would have caught on the version track. It costs unattended-merge LATENCY only: a + # held PR stays open and a human can still merge a genuine CVE fix immediately. + # + # The security-track discriminator is CONSUMED from the ghsa step rather than re-derived, so + # there is exactly one place that decides what "security track" means. + # + # The two cheapest fail-closed guards run BEFORE jq deliberately: that keeps them exercisable + # off the ubuntu leg, where jq is not guaranteed to exist. + # + # Gated on the allow-set because the merge `if` ALREADY requires that conjunct, so skipping here + # changes no merge outcome: a skipped step emits no `age_ok`, and the merge `if` reads a missing + # output as empty, which is not 'true'. What it does remove is an UNAUTHENTICATED outbound GET + # made from a job holding `contents: write`, on a PR that was going to be held anyway. + - name: Require the candidate release to have aged (security track) + id: age + if: steps.allowset.outputs.eligible == 'true' + env: + MIN_RELEASE_AGE_HOURS: "24" + SECURITY_TRACK: ${{ steps.ghsa.outputs.security_track }} + DEP_ECOSYSTEM: ${{ steps.meta.outputs.package-ecosystem }} + DEPS_JSON: ${{ steps.meta.outputs.updated-dependencies-json }} + run: | + set -u + if [ "$SECURITY_TRACK" != "true" ]; then + # Version track: already aged upstream by dependabot.yml's `cooldown`, so this gate does + # not apply. Emit FALSE even on this safe path — never `true` — mirroring the ghsa step + # exactly, so that no early exit anywhere in this step can fail OPEN. The merge `if` + # requires age_ok only when security_track is true. + echo "::notice::version track — aged upstream by dependabot.yml's cooldown; the release-age gate does not apply." + echo "age_ok=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + eco="${DEP_ECOSYSTEM,,}" + eco="${eco//[[:space:]]/}" + case "$eco" in + uv|pip) ;; + *) + # NOT a new denial: the ghsa step above queries `ecosystem=pip` only, so every + # non-pip SECURITY PR already routes to manual review today. If that gate is ever made + # ecosystem-aware, wire the matching publish-date source in here at the same time — + # registry.npmjs.org's `.time[version]` for npm, the commit/release date for actions. + # Until then, writing dated code for ecosystems that cannot reach this step would be + # dead code that looks like coverage. + echo "::notice::no publish-date source wired for ecosystem '${eco:-}' — routing to manual review." + echo "age_ok=false" >> "$GITHUB_OUTPUT" + exit 0 + ;; + esac + + pairs="$(printf '%s' "$DEPS_JSON" | jq -r '.[] | [.dependencyName, .newVersion] | @tsv' 2>/dev/null || echo "ERR")" + if [ "$pairs" = "ERR" ] || [ -z "$pairs" ]; then + echo "::warning::no per-dependency metadata — failing closed (manual review)." + echo "age_ok=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + age_ok=true + now="$(date -u +%s)" + while IFS="$(printf '\t')" read -r raw ver; do + name="${raw,,}" + name="${name//[[:space:]]/}" + if [ -z "$name" ]; then continue; fi + # `newVersion` carries the SAME update_metadata.ts `index === 0` fallback the prevVersion + # guard above cites: a non-first dependency can arrive with an empty string. An unknown + # candidate version means the age question cannot be asked, so DENY. + if [ -z "$ver" ]; then + echo "::warning::no new version for '$name' — failing closed (manual review)." + age_ok=false + break + fi + # PEP 503 normalisation: lowercase, runs of -_. collapse to a single '-'. + name="${name//[-_.]/-}" + while [[ "$name" == *--* ]]; do name="${name//--/-}"; done + # Both values are PR-DERIVED and are about to be pasted into a URL, so validate their + # shape rather than trusting them. + case "$name" in + ""|*[!a-z0-9-]*) + echo "::warning::unexpected package name '$raw' — failing closed (manual review)." + age_ok=false + break + ;; + esac + case "$ver" in + *[!A-Za-z0-9._+!-]*) + echo "::warning::unexpected version '$ver' for '$name' — failing closed (manual review)." + age_ok=false + break + ;; + esac + + body="$(curl -sS --fail --max-time 20 --retry 2 "https://pypi.org/pypi/${name}/${ver}/json" 2>/dev/null || echo "ERR")" + if [ "$body" = "ERR" ] || [ -z "$body" ]; then + echo "::warning::PyPI lookup failed for '${name}==${ver}' — failing closed (manual review)." + age_ok=false + break + fi + published="$(printf '%s' "$body" | jq -r '[.urls[].upload_time_iso_8601] | sort | .[0] // empty' 2>/dev/null || echo "ERR")" + if [ "$published" = "ERR" ] || [ -z "$published" ] || [ "$published" = "null" ]; then + echo "::warning::no upload timestamp for '${name}==${ver}' — failing closed (manual review)." + age_ok=false + break + fi + published_epoch="$(date -u -d "$published" +%s 2>/dev/null || echo "ERR")" + case "$published_epoch" in + ""|*[!0-9]*) + echo "::warning::unparseable upload timestamp '$published' for '${name}==${ver}' — failing closed." + age_ok=false + break + ;; + esac + age_hours=$(( (now - published_epoch) / 3600 )) + if [ "$age_hours" -lt 0 ]; then + echo "::warning::'${name}==${ver}' reports a FUTURE upload time (clock skew?) — failing closed." + age_ok=false + break + fi + if [ "$age_hours" -lt "$MIN_RELEASE_AGE_HOURS" ]; then + echo "::warning::'${name}==${ver}' is ${age_hours}h old, under the ${MIN_RELEASE_AGE_HOURS}h minimum — routing to manual review." + age_ok=false + break + fi + echo "::notice::'${name}==${ver}' was published ${age_hours}h ago (>= ${MIN_RELEASE_AGE_HOURS}h)." + done <<< "$pairs" + echo "age_ok=$age_ok" >> "$GITHUB_OUTPUT" + - name: Enable auto-merge for in-scope updates - # Auto-merge only when: (a) the update is an in-scope type (any patch, or a dev-only minor), - # AND (b) no security-critical dep is involved (deny-list #3), AND (c) for the SECURITY track - # ONLY, a published advisory is confirmed (#2). A non-security (version-track) patch keeps its - # cooldown-aged auto-merge — advisory_ok is not required there. + # Auto-merge only when: (a) EVERY dependency is on its ecosystem's allow row (#3), AND (b) + # for the SECURITY track ONLY, a published advisory is confirmed (#2) AND the candidate + # release has aged (#4), AND (c) the update is an in-scope type (any patch, or a dev-only + # minor). A non-security (version-track) patch keeps its cooldown-aged auto-merge — neither + # advisory_ok nor age_ok is required there, which is why both are emitted false on that path. if: >- - steps.denylist.outputs.deny != 'true' && + steps.allowset.outputs.eligible == 'true' && (steps.ghsa.outputs.security_track != 'true' || - steps.ghsa.outputs.advisory_ok == 'true') && + (steps.ghsa.outputs.advisory_ok == 'true' && + steps.age.outputs.age_ok == 'true')) && (steps.meta.outputs.update-type == 'version-update:semver-patch' || (steps.meta.outputs.update-type == 'version-update:semver-minor' && steps.meta.outputs.dependency-type == 'direct:development')) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index ce3594d0..9d0e5b9e 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -3188,12 +3188,14 @@ What is *not* covered is the thing that will grow: `.semgrep/messagefoundry.yml` ## 336. Dependabot auto-merge shields review with a deny-list -> 🔢 **Filed 2026-08-01 — not started.** Value **5/10** · Difficulty **3/10** · _fill-in_. Auto-merge still keys only on `update-type == 'version-update:semver-patch'` behind a 16-name Python deny-list with no ecosystem filter, so npm and `github-actions` — artifacts that execute inside CI holding the job's token — have zero shield coverage, and `tests/test_dependabot_automerge_guardrails.py:107-108` still asserts a cooldown for the `uv` ecosystem alone; the remainder is a deny-to-allow inversion in one workflow, a workflow-side release-age check for the cooldown-bypassing security track, and broadening one test. +> ✅ **SHIPPED 2026-08-04 — guardrail #3 inverted from a 16-name deny-list to an ecosystem-qualified ALLOW-SET (hold unless named); a fail-closed release-age gate added as #4.** Value **5/10** · Difficulty **3/10** · _fill-in_. §1 `.github/workflows/dependabot-auto-merge.yml` now holds any PR whose dependencies are not on their own ecosystem's allow row — `actions/`/`github/`/`dependabot/` for `github-actions`, deliberately EMPTY for `uv` and `npm`, and an unrecognised ecosystem token holds rather than merges — preserving the fail-safe whole-group denial (measured: PR #75's five-bump batch carried `pypa/gh-action-pypi-publish`, so that batch would now HOLD). §4 a new `id: age` step requires every SECURITY-track candidate version to have been published at least 24h, failing closed on an API error, an absent/unparseable upload timestamp, an unexpected name or version shape, or an ecosystem with no publish-date source wired. ⚠️ **#4 is a FORWARD guard and is INERT with respect to the merge decision as shipped** — `age_ok=true` is reachable only for `uv`/`pip`, `eligible=true` only for `github-actions`, and the merge `if` requires both, so the two sets are disjoint. It is recorded that way in the workflow header rather than as an operating control, and gated on the allow-set so it makes no unauthenticated outbound request from the `contents: write` job for a PR that holds regardless; it becomes load-bearing the day a Python allow row is populated (an owner decision) or the advisory gate is made ecosystem-aware. §3 `tests/test_dependabot_automerge_guardrails.py` now asserts a cooldown on EVERY configured ecosystem behind a vacuity floor, and executes the shipped `run:` bodies under `bash -e` — the shell GitHub Actions actually applies — rather than only reading the YAML. §5 the false-premise backstop clause is corrected rather than merely deleted: no REQUIRED check reads a dependency's shipped bytes, and `trivy`, which does read the built image's bytes, is advisory (`continue-on-error`) and cron/dispatch-only, so it never runs on a Dependabot PR at all. **§2 was ALREADY SHIPPED** by the 2026-08-03 amendment (`.github/dependabot.yml` sets `cooldown.default-days: 5` on `github-actions`) and was NOT rebuilt; **§6 is discharged by DELETING the deny-list** rather than pruning it, which removes `python-jose`/`pyjwt`/`passlib` — all three absent from `requirements.lock`'s 98 pinned distributions — along with it. > ⚠️ **AMENDED 2026-08-03 — the `github-actions` cooldown SHIPPED, discharging Proposed §2 in substance and half the false-premise finding with it.** The second measured-at-HEAD bullet asserts that ecosystem *"carries `schedule` + `groups` only; there is no `cooldown:` key"*, but `.github/dependabot.yml:83-84` now sets `default-days: 5` for it, with the rationale at `:75-82` (#75 took two of five bumps to `main` under 24h from publish; `codeql-action` v4.37.4 was 7h old). So the *"Bounding this honestly"* line **"Only `github-actions` is unaged"** no longer holds, and each of the three configured ecosystems now has a cooldown behind the header's claim (now at `.github/workflows/dependabot-auto-merge.yml:22-24`, not `:16-18`). ⚠️ **Read §2 as discharged in substance, not to the letter** — `.github/dependabot.yml:79-80` records that this ecosystem honors `default-days` alone and ages off the **tag's commit date**, "so treat 5 as approximate", which is why §2's *"matching uv's 5/7"* could not be met. > > **The deny-list itself is untouched, so the rest of the item stands:** 16 Python names at `:84-85` gating every ecosystem behind an author-only job condition (`:64`) with no ecosystem qualifier, a merge gate still keying on `version-update:semver-patch` (`:184`), `tests/test_dependabot_automerge_guardrails.py:107-108` still asserting a cooldown for `uv` alone, and `python-jose` / `pyjwt` / `passlib` still absent from `requirements.lock` — so §§1, 3, 4, 5 and 6 are unaffected, as is the Why's other leg (`.github/workflows/security.yml:261-262` still describes pip-audit as *lockfile only* and bandit/semgrep as *source only*). ⚠️ **At least four `dependabot-auto-merge.yml` citations above (`:16-18`, `:58`, `:78-79`, `:155-161`) and all four `dependabot.yml` ones now point at different lines** — re-measure before quoting one; the `tests/` and `security.yml` citations are still exact. +> **AMENDED 2026-08-04 — one clause of the 2026-08-03 note above is superseded by the SHIPPED banner; the dated measurement itself stands and is deliberately left as written.** *"The deny-list itself is untouched, so the rest of the item stands … §§1, 3, 4, 5 and 6 are unaffected"* was accurate when measured. It is not now: the deny-list no longer exists — guardrail #3 is an allow-set — so §6 is discharged by deletion rather than annotation, and §§1, 3, 4 and 5 are built rather than merely unaffected. The 16 names survive only as a PROPERTY under test (`_DENY_PACKAGES` in `tests/test_dependabot_automerge_guardrails.py` asserts none of them reaches any allow row), not as a mechanism. + **Cluster:** Security / Supply chain. **Priority:** P3. **Verdict:** build. **Severity:** low. diff --git a/docs/testing/master-test-plan/01-environments-data-and-tooling.md b/docs/testing/master-test-plan/01-environments-data-and-tooling.md index 920c8c39..bfbf616e 100644 --- a/docs/testing/master-test-plan/01-environments-data-and-tooling.md +++ b/docs/testing/master-test-plan/01-environments-data-and-tooling.md @@ -347,7 +347,7 @@ Sixteen workflows. This is the complete list; the plan cites these rather than r | [`manifest-lint.yml`](.github/workflows/manifest-lint.yml) | kubeconform + ADR-0047 HA-policy grep assertions on `docker/k8s/*.yaml` | push/PR touching manifests or `docker/README.md`, dispatch | E2 | additive, not required | The only automated signal on E11 today — and it is schema/policy lint, **not** an applied deployment | | [`backlog-hygiene.yml`](.github/workflows/backlog-hygiene.yml) | A PR claiming `BACKLOG #N` and touching engine/IDE code must also update `docs/BACKLOG.md` | PR→main | E2 | not required | Keeps the status ledger honest. The structural half rides `tests/test_backlog_status_check.py` in the `test` matrix | | [`cla.yml`](.github/workflows/cla.yml) | CLA Assistant, signatures on the `cla-signatures` branch | `issue_comment`, `pull_request_target` (opened/synchronize) | E2 | **Required** (`CLA Assistant`) | Contribution gate | -| [`dependabot-auto-merge.yml`](.github/workflows/dependabot-auto-merge.yml) | Scoped auto-merge: patches + dev-only minors; an auth/token/crypto **deny-list** and a published-GHSA gate that **fails closed** | `pull_request` | E2 | n/a | Why the required-check set is load-bearing: it is the only thing standing between a dependency bump and `main` | +| [`dependabot-auto-merge.yml`](.github/workflows/dependabot-auto-merge.yml) | Scoped auto-merge: patches + dev-only minors, each held unless EVERY named dependency sits on its ecosystem's **allow-set** row (hold-unless-named, whole-group denial), plus a published-GHSA gate and a release-age gate on the security track — all **failing closed** | `pull_request` | E2 | n/a | Why the required-check set is load-bearing: it is the only thing standing between a dependency bump and `main` | | [`dependabot-lock-resync.yml`](.github/workflows/dependabot-lock-resync.yml) | Re-exports the four lock artifacts onto the Dependabot branch | PR touching `uv.lock` / `pyproject.toml` | E2 | n/a | Must stay in lockstep with security.yml's DEP-1 step — a file the gate diffs but this job does not export is un-fixable by the bot | | [`vuln-metrics.yml`](.github/workflows/vuln-metrics.yml) | NIST SSDF RV.2 KPIs from real Dependabot PRs + CISA KEV + FIRST EPSS | weekly `0 8 * * 1`, dispatch | E2 | no | Evidence artifact, not a detector | | [`release.yml`](.github/workflows/release.yml) | Build + SBOM + Sigstore sign + GitHub release + PyPI Trusted Publishing; separate `release-harness` (gated on the `PUBLISH_HARNESS` repo variable) and a `webconsole-v*` tag namespace | tag `v*` / `webconsole-v*`, dispatch (dry-run: builds/signs, never publishes) | E2 | n/a | The release chapter's substrate | diff --git a/tests/test_dependabot_automerge_guardrails.py b/tests/test_dependabot_automerge_guardrails.py index 2872231a..d3494182 100644 --- a/tests/test_dependabot_automerge_guardrails.py +++ b/tests/test_dependabot_automerge_guardrails.py @@ -1,27 +1,56 @@ # SPDX-License-Identifier: AGPL-3.0-or-later # Copyright (C) 2026 MessageFoundry Organization and contributors -"""Structural regression tests for the Dependabot auto-merge security-track guardrails (SEC-007). - -These assert the YAML wiring of ``.github/workflows/dependabot-auto-merge.yml`` — the live GitHub -Actions run is the integration test. They close DEPENDENCY-POSTURE-REVIEW.md guardrails: - #3 a DENY-LIST step keeps the auth/token/crypto stack off the auto-merge path (manual review), and - #2 a published-GHSA step gates the cooldown-bypassing SECURITY track on a real advisory. -Both must be wired into the ``Enable auto-merge`` step's ``if`` so a denied/un-advisoried PR does NOT -auto-merge, while a non-sensitive patch still does (``gh pr merge --auto`` preserved). +"""Structural AND behavioural regression tests for the Dependabot auto-merge guardrails (SEC-007). + +Most of these assert the YAML wiring of ``.github/workflows/dependabot-auto-merge.yml``; the last +three go further and **execute the shipped ``run:`` bodies under ``bash -e``** — the shell GitHub +Actions applies by default on Linux — because the guardrails are shell logic and "the YAML names a +step" is a much weaker claim than "the step holds what it must hold". That is possible only because +both new steps take every input through ``env:`` and interpolate +nothing — asserted by ``test_new_steps_take_every_input_through_env_not_interpolation``, which is a +precondition for the behavioural tests as much as it is zizmor template-injection parity. + +The guardrail numbers below are DEPENDENCY-POSTURE-REVIEW.md's, carried with two corrections that +document does not yet make — do not read them as claims about its text: + #3 an ALLOW-SET step holds every PR whose dependencies are not named on their ecosystem's allow row + (hold-unless-named; the whole group is denied on any single ineligible member). This is + **INVERTED** from the deny-list the posture review describes, and it gates EVERY track, not only + the security one. + #2 a published-GHSA step gates the cooldown-bypassing SECURITY track on a real advisory, and + #4 a release-age step additionally holds a SECURITY-track PR whose candidate version is younger + than ``MIN_RELEASE_AGE_HOURS`` — the track the upstream ``cooldown`` cannot age. **#4 is + introduced in this repository; the posture review does not carry it.** +All three must be wired into the ``Enable auto-merge`` step's ``if`` so a held/un-advisoried/fresh PR +does NOT auto-merge, while an eligible patch still does (``gh pr merge --auto`` preserved). + +⚠️ #4 cannot change a merge decision as the workflow ships, and these tests do not pretend otherwise: +``age_ok=true`` is reachable only for uv/pip, ``eligible=true`` only for github-actions, and the merge +``if`` requires both — disjoint sets. It is tested as a FORWARD guard, so that populating a Python +allow row later is a one-line change to a control that already works, not a control to be written +under pressure. ``test_release_age_is_gated_on_the_allow_set`` pins the conjunction that makes this +true, so the day someone widens it, a test has to be edited deliberately. """ from __future__ import annotations +import datetime as dt +import os +import re +import shutil +import subprocess from pathlib import Path import pytest import yaml -_WORKFLOW = ( - Path(__file__).resolve().parent.parent / ".github" / "workflows" / "dependabot-auto-merge.yml" -) +_ROOT = Path(__file__).resolve().parent.parent +_WORKFLOW = _ROOT / ".github" / "workflows" / "dependabot-auto-merge.yml" +_DEPENDABOT = _ROOT / ".github" / "dependabot.yml" -# Security-critical packages that must NEVER auto-merge, even for a patch (posture-review #3). +# Security-critical packages that must NEVER be auto-merge eligible, even for a patch +# (posture-review #3). These were the 16 names of the former DENY-LIST; the mechanism has inverted to +# an allow-set, but the PROPERTY they encoded is still the thing under test — none of them may appear +# on any allow row. Keeping the tuple keeps that property executable rather than merely intended. _DENY_PACKAGES = ( "cryptography", "argon2-cffi", @@ -33,8 +62,21 @@ "starlette", "uvicorn", "pydantic", + "pydantic-core", + "python-jose", + "pyjwt", + "passlib", + "bcrypt", + "cffi", ) +#: ``$VAR`` / ``${VAR}`` shell references, upper-case only (lower-case names are the scripts' own +#: locals, which are not inputs). +_ENV_REF = re.compile(r"\$\{?([A-Z][A-Z0-9_]*)\}?") + +#: Provided by the Actions runner itself, so a step need not declare it in ``env:``. +_RUNNER_PROVIDED = frozenset({"GITHUB_OUTPUT"}) + def _load() -> dict: return yaml.safe_load(_WORKFLOW.read_text(encoding="utf-8")) @@ -45,29 +87,126 @@ def _steps() -> list[dict]: return doc["jobs"]["auto-merge"]["steps"] +def _step(step_id: str) -> dict: + """The step with ``id: ``, failing loudly rather than returning None.""" + found = next((s for s in _steps() if s.get("id") == step_id), None) + assert found is not None, f"no step with id: {step_id}" + return found + + +def _load_dependabot() -> dict: + """Parse ``.github/dependabot.yml``, ASSERTING it is there rather than skipping when it is not. + + The guard this replaced skipped on the premise that the file is "private-only, deny-listed on the + OSS mirror". That premise is false, and `.gitignore` says so in as many words: the publish + deny-list is RETIRED, and `.github/dependabot.yml` is named in the *"DELIBERATELY NOT LISTED"* + note as content meant to ship. Measured: ``git ls-files --error-unmatch .github/dependabot.yml`` + resolves and ``git check-ignore`` exits 1. + + That matters more after this change than before it. At HEAD the guard sat inline in ONE test; + three now route through here, including ``test_every_configured_ecosystem_has_a_cooldown`` — the + test whose whole reason for existing is that a missing cooldown was invisible to CI. A skip keyed + on a false premise would have made it invisible a second way. tests/test_anon_parity.py:42-45 + records this exact lesson from the other direction: a stale skip path "silently skipped the parity + assertions below, which are the only thing keeping [the tables] identical". + """ + assert _DEPENDABOT.exists(), ( + f"{_DEPENDABOT} is a TRACKED file (see .gitignore's 'DELIBERATELY NOT LISTED' note) — its " + f"absence is the bug, not a reason to skip the cooldown assertions" + ) + return yaml.safe_load(_DEPENDABOT.read_text(encoding="utf-8")) + + +def _run_step_body( + step_id: str, + env: dict[str, str], + tmp_path: Path, + path_prepend: Path | None = None, +) -> tuple[int, dict[str, str]]: + """Run a step's shipped ``run:`` body under ``bash -e``; return ``(rc, $GITHUB_OUTPUT map)``. + + The body is executed VERBATIM — no substitution, no rewriting — which is only sound because the + step interpolates no ``${{ }}`` expressions. That invariant is asserted separately. + + **The shell is ``bash -e``, mirroring the ``bash -e {0}`` that GitHub Actions applies as its + default on Linux.** No ``shell:`` is declared anywhere in the workflow, at step, job or workflow + level — ``test_no_step_overrides_the_default_shell`` pins that, because the moment one is added + this harness stops mirroring CI. Plain ``bash`` was the wrong shell to assert under: it keeps + going after a failing command where CI aborts the step, which is a difference these tests would + have reported as agreement. Measured on the shipped bodies, every row here decides identically + under both — so this is a fidelity fix, not a behaviour change — but the returncode is now + RETURNED rather than asserted here, so an abort path can be expressed as an expected outcome + instead of being indistinguishable from a harness bug. + + Any LITERAL value in the step's own ``env:`` (one carrying no ``${{ }}``) is seeded first, so a + tuning constant like ``MIN_RELEASE_AGE_HOURS`` is exercised at its SHIPPED value rather than at + one this test invented. The caller's ``env`` then overlays the PR-derived inputs. + """ + bash = shutil.which("bash") + assert bash is not None # narrowed by the caller's skipif; keeps mypy honest + step = _step(step_id) + body = str(step["run"]) + assert "${{" not in body, ( + f"the {step_id} body interpolates an Actions expression, so this test is no longer executing " + f"what CI executes" + ) + script = tmp_path / f"{step_id}.sh" + script.write_text(body, encoding="utf-8") + out_file = tmp_path / f"{step_id}.githuboutput" + out_file.write_text("", encoding="utf-8") + + full_env = dict(os.environ) + for key, value in (step.get("env") or {}).items(): + if "${{" not in str(value): + full_env[str(key)] = str(value) + full_env.update(env) + full_env["GITHUB_OUTPUT"] = out_file.as_posix() + if path_prepend is not None: + full_env["PATH"] = f"{path_prepend.as_posix()}{os.pathsep}{full_env.get('PATH', '')}" + + proc = subprocess.run( + [bash, "-e", script.as_posix()], + capture_output=True, + text=True, + timeout=120, + env=full_env, + ) + parsed: dict[str, str] = {} + for line in out_file.read_text(encoding="utf-8").splitlines(): + if "=" in line: + key, _, value = line.partition("=") + parsed[key.strip()] = value.strip() + return proc.returncode, parsed + + +# -------------------------------------------------------------------------------------------------- +# Structural: the workflow's wiring +# -------------------------------------------------------------------------------------------------- + + def test_workflow_is_valid_yaml_and_has_the_automerge_job() -> None: doc = _load() assert "auto-merge" in doc["jobs"] -def test_denylist_step_names_every_security_critical_package() -> None: - """Guardrail #3: the deny-list step body must hard-code each security-critical package.""" - steps = _steps() - deny = next((s for s in steps if s.get("id") == "denylist"), None) - assert deny is not None, "no deny-list step (id: denylist) found" - body = deny.get("run", "") +def test_no_security_critical_package_is_auto_merge_eligible() -> None: + """Guardrail #3, as a PROPERTY rather than a mechanism: no security-critical package may be + reachable through the allow-set. The former deny-list named them to EXCLUDE them; the allow-set + holds by default, so the equivalent assertion is that none of them appears on any allow row.""" + allowset = _step("allowset") + body = str(allowset.get("run", "")) for pkg in _DENY_PACKAGES: - assert pkg in body, f"deny-list missing security-critical package: {pkg}" + assert pkg not in body, ( + f"'{pkg}' appears in the allow-set step — a security-critical package must never be " + f"auto-merge eligible (posture-review #3)" + ) # emits a guard output the merge step can require - assert "deny=" in body and "$GITHUB_OUTPUT" in body + assert "eligible=" in body and "$GITHUB_OUTPUT" in body def test_ghsa_step_queries_the_advisory_api_and_emits_a_guard() -> None: """Guardrail #2: a security-track step must consult the advisory API and emit advisory_ok.""" - steps = _steps() - ghsa = next((s for s in steps if s.get("id") == "ghsa"), None) - assert ghsa is not None, "no published-GHSA step (id: ghsa) found" - body = ghsa.get("run", "") + body = str(_step("ghsa").get("run", "")) # calls the GitHub advisories API (gh api ... /advisories) assert "gh api" in body and "/advisories" in body # produces the advisory guard output and a security-track discriminator @@ -77,32 +216,298 @@ def test_ghsa_step_queries_the_advisory_api_and_emits_a_guard() -> None: assert "advisory_ok=false" in body -def test_enable_automerge_gates_on_both_guards_and_preserves_auto_merge() -> None: - """The merge step's ``if`` must require deny != 'true' AND the security-track advisory guard, - while still invoking ``gh pr merge --auto`` for the non-sensitive path.""" +def test_release_age_step_is_security_track_only_and_fails_closed() -> None: + """Guardrail #4: an ``age`` step must exist AFTER ``ghsa``, consume that step's security-track + discriminator rather than re-deriving it, and never hard-code a PASS on an early exit.""" + ids = [s.get("id") for s in _steps()] + assert "age" in ids, "no release-age step (id: age) found" + assert ids.index("age") > ids.index("ghsa"), ( + "the age step must come AFTER ghsa — it consumes ghsa's security_track output" + ) + + age = _step("age") + env = age.get("env") or {} + assert "MIN_RELEASE_AGE_HOURS" in env, "the age threshold must be a declared, visible input" + assert "steps.ghsa.outputs.security_track" in str(env.get("SECURITY_TRACK", "")), ( + "the security-track discriminator must be CONSUMED from the ghsa step, not re-derived — " + "one source of truth for what 'security track' means" + ) + + body = str(age.get("run", "")) + emits = [ln.strip() for ln in body.splitlines() if "$GITHUB_OUTPUT" in ln and "age_ok" in ln] + assert emits, "the age step emits no age_ok output" + # Fails closed: every literal emission is a DENY; the only PASS is the computed variable, which + # can only be true after the loop has consulted a publish date. + for line in emits: + assert "age_ok=true" not in line, f"an early exit hard-codes a PASS, failing OPEN: {line}" + assert any("age_ok=false" in ln for ln in emits), "no fail-closed emission found" + assert any("age_ok=$age_ok" in ln for ln in emits), "no computed emission found" + + +def test_release_age_is_gated_on_the_allow_set() -> None: + """The age step must not run for a PR the allow-set already holds. + + Two distinct reasons, and the weaker one is the one usually noticed. The privacy/egress reason: + this step makes an UNAUTHENTICATED outbound GET from a job holding ``contents: write``, so making + it for a PR that was going to be held anyway is gratuitous. The honesty reason, which is why the + assertion lives here rather than in a comment: this ``if`` is half of the conjunction that makes + guardrail #4 INERT today (``age_ok=true`` needs uv/pip, ``eligible=true`` needs github-actions). + Pinning it means widening #4 into a live control has to edit a test, not slip through as prose. + """ + age = _step("age") + cond = str(age.get("if", "")) + assert "steps.allowset.outputs.eligible == 'true'" in cond, ( + "the age step is not gated on the allow-set, so it performs a privileged-job network fetch " + f"for PRs that hold regardless; if: {cond!r}" + ) + + +def test_no_step_overrides_the_default_shell() -> None: + """Nothing may declare ``shell:``, at step, job or workflow level. + + Not style. ``_run_step_body`` executes the shipped bodies under ``bash -e`` on the strength of + Actions' documented Linux default (``bash -e {0}``). Declare a ``shell:`` anywhere and that + premise silently stops holding, leaving a behavioural suite that asserts confidently about a + shell CI no longer uses. This is the tripwire for that. + """ + doc = _load() + assert "defaults" not in doc, "a workflow-level defaults: block can change the shell" + job = doc["jobs"]["auto-merge"] + assert "defaults" not in job, "a job-level defaults: block can change the shell" + for step in job["steps"]: + assert "shell" not in step, ( + f"step {step.get('id') or step.get('name')!r} declares an explicit shell:, so the " + f"behavioural tests no longer mirror what CI runs — update _run_step_body with it" + ) + + +def test_enable_automerge_gates_on_all_three_guards_and_preserves_auto_merge() -> None: + """The merge step's ``if`` must require the allow-set guard AND the two security-track guards, + while still invoking ``gh pr merge --auto`` for the eligible path.""" steps = _steps() - merge = next((s for s in steps if "gh pr merge --auto" in s.get("run", "")), None) + merge = next((s for s in steps if "gh pr merge --auto" in str(s.get("run", ""))), None) assert merge is not None, ( "the gh pr merge --auto step was removed (auto-merge must be preserved)" ) - cond = merge.get("if", "") - assert "steps.denylist.outputs.deny != 'true'" in cond, "merge not gated on the deny-list guard" - # security-track PRs require a confirmed advisory; version-track patches stay unchanged + cond = str(merge.get("if", "")) + assert "steps.allowset.outputs.eligible == 'true'" in cond, ( + "merge not gated on the allow-set guard" + ) + # security-track PRs require a confirmed advisory AND an aged release; version-track patches + # keep their cooldown-aged auto-merge unchanged assert "steps.ghsa.outputs.advisory_ok == 'true'" in cond assert "steps.ghsa.outputs.security_track != 'true'" in cond + assert "steps.age.outputs.age_ok == 'true'" in cond, "merge not gated on the release-age guard" # the in-scope update-type gate is still present (any patch / dev-only minor) assert "version-update:semver-patch" in cond + # A half-removal would leave the merge condition referencing a step id that no longer exists — + # an Actions expression against a missing step evaluates to empty, silently dropping the guard. + text = _WORKFLOW.read_text(encoding="utf-8") + assert "steps.denylist" not in text, "dangling reference to the removed deny-list step" + assert not any(s.get("id") == "denylist" for s in steps), "the deny-list step is still present" + + +def test_new_steps_take_every_input_through_env_not_interpolation() -> None: + """Both new steps must read their inputs from ``env:``, never from an inlined ``${{ }}``. + + This is zizmor template-injection parity asserted in-tree — a PR-controlled value pasted into a + shell body is a code-execution seam — and it is simultaneously the precondition that lets the + behavioural tests below execute the shipped bodies verbatim.""" + for step_id in ("allowset", "age"): + step = _step(step_id) + body = str(step.get("run", "")) + assert "${{" not in body, ( + f"the {step_id} step interpolates an Actions expression into its shell body; pass it " + f"through env: instead" + ) + declared = set((step.get("env") or {}).keys()) + referenced = set(_ENV_REF.findall(body)) - _RUNNER_PROVIDED + undeclared = referenced - declared + assert not undeclared, f"{step_id} reads undeclared env var(s): {sorted(undeclared)}" + + +# -------------------------------------------------------------------------------------------------- +# Structural: the upstream cooldown configuration +# -------------------------------------------------------------------------------------------------- + def test_cooldown_aging_window_lengthened() -> None: """Posture-review step 3: the uv-ecosystem routine cooldown is widened to >= 5 days.""" - dependabot = Path(__file__).resolve().parent.parent / ".github" / "dependabot.yml" - if not dependabot.exists(): - # Private-only: .github/dependabot.yml is deny-listed on the OSS mirror (dependency updates - # are managed on the private source repo). Skip where it's absent — the sibling tests assert - # the shipped dependabot-auto-merge.yml workflow and still run on the mirror. Mirrors the - # skip-if-absent guard in tests/test_anon_parity.py and tests/test_load_config.py. - pytest.skip("dependabot.yml is private-only (OSS-mirror deny-list)") - doc = yaml.safe_load(dependabot.read_text(encoding="utf-8")) + doc = _load_dependabot() uv = next(u for u in doc["updates"] if u["package-ecosystem"] == "uv") assert uv["cooldown"]["default-days"] >= 5 + + +def test_every_configured_ecosystem_has_a_cooldown() -> None: + """EVERY configured ecosystem must be cooldown-aged, not just uv. + + The sibling test above asserts uv alone, which is precisely why a missing ``cooldown`` on + ``github-actions`` — the ecosystem whose artifacts execute INSIDE CI — was invisible to CI. This + is the test that would have caught it.""" + doc = _load_dependabot() + updates = doc["updates"] + scanned = [u["package-ecosystem"] for u in updates] + print(f"ecosystems scanned for a cooldown: {scanned}") + # Liveness: a pass must mean "checked all of them", never "found none to check". + assert len(updates) >= 3, ( + f"only {len(updates)} update entries found ({scanned}) — the dependabot.yml walk is probably " + f"broken, which would make the per-ecosystem assertion below vacuous" + ) + for entry in updates: + eco = entry["package-ecosystem"] + assert "cooldown" in entry, f"ecosystem '{eco}' has no cooldown — a fresh release is unaged" + days = entry["cooldown"].get("default-days") + assert isinstance(days, int) and days >= 1, ( + f"ecosystem '{eco}' has a non-positive/absent cooldown default-days: {days!r}" + ) + + +def test_ci_executing_ecosystem_is_aged_at_least_five_days() -> None: + """``github-actions`` specifically must age >= 5 days: unlike a Python distribution, a bumped + action executes INSIDE CI holding whatever token the job carries, so a fresh malicious release is + an immediate code-execution primitive rather than a dependency to be imported later.""" + doc = _load_dependabot() + actions = next(u for u in doc["updates"] if u["package-ecosystem"] == "github-actions") + assert actions["cooldown"]["default-days"] >= 5 + + +# -------------------------------------------------------------------------------------------------- +# Behavioural: execute the shipped shell +# -------------------------------------------------------------------------------------------------- + + +@pytest.mark.skipif(shutil.which("bash") is None, reason="needs bash to execute the run: body") +@pytest.mark.parametrize( + ("ecosystem", "names", "expected"), + [ + # The two eligible rows are what make this test non-vacuous: an always-false implementation + # (or an empty github-actions allow row) FAILS here rather than passing everything else. + ("github_actions", "actions/checkout", "true"), + ("github_actions", "actions/checkout,github/codeql-action/init", "true"), + # Measured PR #75: a five-bump batch carrying a third-party action holds the WHOLE group. + ("github_actions", "actions/checkout,pypa/gh-action-pypi-publish", "false"), + # Anchored prefix, not substring: neither of these is actions/. + ("github_actions", "actionsx/foo", "false"), + ("github_actions", "evil/actions/checkout", "false"), + # An empty name list cannot confirm eligibility. + ("github_actions", "", "false"), + # uv/npm allow rows ship empty, so every bump on them holds. + ("uv", "certifi", "false"), + ("uv", "cryptography", "false"), + ("npm_and_yarn", "@types/node", "false"), + ("pip", "requests", "false"), + # An unrecognised (here: empty) ecosystem token must fall to the hold arm. + ("", "actions/checkout", "false"), + ], +) +def test_allowset_holds_everything_not_named( + ecosystem: str, names: str, expected: str, tmp_path: Path +) -> None: + """Execute the shipped allow-set body and check what it actually decides.""" + rc, out = _run_step_body( + "allowset", + {"DEP_NAMES": names, "DEP_ECOSYSTEM": ecosystem}, + tmp_path, + ) + assert rc == 0, f"the allowset body aborted under `bash -e` (rc={rc}) — CI would fail the step" + assert out.get("eligible") == expected, ( + f"ecosystem={ecosystem!r} names={names!r} -> eligible={out.get('eligible')!r}, " + f"expected {expected!r}" + ) + + +@pytest.mark.skipif(shutil.which("bash") is None, reason="needs bash to execute the run: body") +@pytest.mark.parametrize( + ("security_track", "ecosystem"), + [ + # Version track: aged upstream by dependabot.yml's cooldown, so this gate emits false and the + # merge condition simply does not require it. + ("false", "uv"), + ("", "uv"), + # Security track on an ecosystem with no publish-date source wired. + ("true", "github_actions"), + ("true", "npm_and_yarn"), + ("true", ""), + ], +) +def test_release_age_holds_the_version_track_and_undatable_ecosystems( + security_track: str, ecosystem: str, tmp_path: Path +) -> None: + """The two cheapest fail-closed guards, exercised WITHOUT jq or network. + + Every row here must emit ``age_ok=false``. On its own that is not self-certifying — an + unconditionally-false step would also pass — so the discriminating ``true`` case is supplied by + ``test_release_age_passes_an_aged_release_and_holds_a_fresh_one`` below. These two tests are a + pair; this one runs everywhere, that one needs jq.""" + rc, out = _run_step_body( + "age", + { + "SECURITY_TRACK": security_track, + "DEP_ECOSYSTEM": ecosystem, + "DEPS_JSON": '[{"dependencyName":"requests","newVersion":"2.32.3"}]', + }, + tmp_path, + ) + assert rc == 0, f"the age body aborted under `bash -e` (rc={rc}) — CI would fail the step" + assert out.get("age_ok") == "false", ( + f"security_track={security_track!r} ecosystem={ecosystem!r} -> " + f"age_ok={out.get('age_ok')!r}, expected 'false'" + ) + + +def _curl_stub(tmp_path: Path, payload: str | None) -> Path: + """A ``curl`` on PATH that ignores its arguments. ``payload=None`` makes it fail like a network + or HTTP error would (``--fail`` exits non-zero), which must route to manual review.""" + stub_dir = tmp_path / "stub" + stub_dir.mkdir() + stub = stub_dir / "curl" + if payload is None: + stub.write_text("#!/usr/bin/env bash\nexit 1\n", encoding="utf-8") + else: + stub.write_text(f"#!/usr/bin/env bash\ncat <<'JSON'\n{payload}\nJSON\n", encoding="utf-8") + stub.chmod(0o755) + return stub_dir + + +def _iso(hours_ago: float) -> str: + when = dt.datetime.now(dt.UTC) - dt.timedelta(hours=hours_ago) + return when.strftime("%Y-%m-%dT%H:%M:%S.%f") + "Z" + + +@pytest.mark.skipif( + shutil.which("bash") is None or shutil.which("jq") is None, + reason="needs bash + jq. It runs WHEREVER both exist — the ubuntu leg AND the two REQUIRED " + "windows-2022/windows-2025 legs, whose runner images ship jq and Git Bash — and skips on the " + "maintainer's box, where Git Bash carries no jq. A red here is therefore not necessarily a " + "ubuntu-only red", +) +@pytest.mark.parametrize( + ("label", "payload", "expected"), + [ + # The discriminating PASS — without this row the whole release-age suite would be satisfied + # by a step that denies unconditionally. + ("aged 30 days", f'{{"urls":[{{"upload_time_iso_8601":"{_iso(24 * 30)}"}}]}}', "true"), + ("published 1 hour ago", f'{{"urls":[{{"upload_time_iso_8601":"{_iso(1)}"}}]}}', "false"), + ("no dateable artifact", '{"urls":[]}', "false"), + ("PyPI error", None, "false"), + ], +) +def test_release_age_passes_an_aged_release_and_holds_a_fresh_one( + label: str, payload: str | None, expected: str, tmp_path: Path +) -> None: + """The security-track happy path and its three failure modes, against a stubbed PyPI.""" + rc, out = _run_step_body( + "age", + { + "SECURITY_TRACK": "true", + "DEP_ECOSYSTEM": "uv", + "DEPS_JSON": '[{"dependencyName":"requests","newVersion":"2.32.3"}]', + }, + tmp_path, + path_prepend=_curl_stub(tmp_path, payload), + ) + assert rc == 0, f"the age body aborted under `bash -e` (rc={rc}) — CI would fail the step" + assert out.get("age_ok") == expected, ( + f"{label} -> age_ok={out.get('age_ok')!r}, expected {expected!r}" + )