From 99f2fc6827a8d195697bbfab50a77a2f83cf2931 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 10:54:26 +1200 Subject: [PATCH 01/29] ci(deps): run the OSS Index audit in one job that can prove it scanned The vulnerability audit has been reporting success without running. `ossindex-maven-plugin` sat on `validate` with `fail=false`, got HTTP 401 from ossindex.sonatype.org on every module, and every build stayed green. Credentials now exist as repo secrets; nothing consumed them. Adds `.github/workflows/dependency-audit.yml`, job `deps: ossindex audit`. **The trap this is built around.** The plugin's `fail` parameter covers "vulnerable components were found", not "the request failed". There is no setting that makes an unreachable scanner fatal. Verified end to end with a cold client cache and an expired token: `[WARNING] Failed to fetch component-reports`, HTTP 401, `BUILD SUCCESS`, exit 0. So a green Maven run is never by itself evidence the audit happened, and a token expiry six months from now would degrade silently back to green-but-not-scanning. `bin/check-ossindex-audit.sh` is what makes the job honest. Three fail-closed legs: the failure line in the log; every exported report must carry a non-empty `reports` map (a 401 leaves `{ }` behind, so "the file exists" proves nothing); and one report per module the log says it checked. Leg 2 is deliberately structural rather than textual - it is the leg that still works if Sonatype rewords the warning, which is the realistic way a log-grep guard rots into the defect it was written against. `bin/test-check-ossindex-audit.sh` drives all three with negative controls, and CI runs it before the guard, per the house pattern. Confirmed against the real thing in both directions: the guard turns an exit-0 BUILD SUCCESS red on an expired token, and passes the authenticated run. **Reports, does not block - on purpose.** Findings go to the job summary and do not fail the job; the tree currently has 5 vulnerable components across 12 advisories, top CVSS 9.3, so a blocking job would be red on every PR inside a week and then ignored. Red is reserved for "this check stopped working", which is rare and therefore still readable. Making findings fatal is a one-line change once that backlog is triaged. **Weekly schedule.** The gap nothing else covers: `deps: vulnerabilities` only reviews dependencies a PR changes, and Dependabot misses transitive components outright - netty-codec-http, lz4-java and HdrHistogram all carry advisories here with no alert in any state. A scheduled whole-tree scan is the only thing that sees an unchanged tree acquire a new advisory. Weekly rather than daily because the answer only moves when the advisory database does. This is the sole cron lane in the repo and does not contradict "there is no scheduled build": it re-runs no suite, and what it catches is a function of elapsed time. **No ordering dependency on astubbs/parallel-consumer#259.** `ossindex.skip`, `ossindex.fail` and `ossindex.authId` are user properties of the plugin itself, so the command behaves identically against today's pom (hardcoded `fail=false`, no `authId`) and against the one that PR leaves behind. The pom is deliberately untouched here. `test-compile` rather than `validate`, which cost an experiment: the audit mojo resolves TEST-scope dependencies and the modules depend on `parallel-consumer-core:jar:tests`, so against a repository with the PC snapshots removed a bare `ossindex:audit` dies at the second module with "Could not resolve dependencies". The reactor has to have compiled them. 46s locally for the whole tree. Also worth knowing for anyone experimenting: the OSS Index client keeps an on-disk report cache (macOS `~/Library/Application Support/Sonatype/Ossindex`). A warm cache serves full results even against a bogus base-URL, which invalidated the first two negative controls run here. Clear it before any reachability experiment, and never cache it in CI. Permissions are `contents: read` only - findings go to `$GITHUB_STEP_SUMMARY` specifically so the job never needs `pull-requests: write`, given it runs PR-authored build code beside the token. Skipped for fork PRs, which get no secrets and would 401 forever. --- .github/workflows/dependency-audit.yml | 154 +++++++++++++++++ AGENTS.md | 3 +- bin/check-ossindex-audit.sh | 218 +++++++++++++++++++++++++ bin/test-check-ossindex-audit.sh | 183 +++++++++++++++++++++ docs/inflight/ci-review-agent.md | 12 +- 5 files changed, 565 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/dependency-audit.yml create mode 100755 bin/check-ossindex-audit.sh create mode 100755 bin/test-check-ossindex-audit.sh diff --git a/.github/workflows/dependency-audit.yml b/.github/workflows/dependency-audit.yml new file mode 100644 index 000000000..3b40d0fdf --- /dev/null +++ b/.github/workflows/dependency-audit.yml @@ -0,0 +1,154 @@ +# Whole-tree dependency vulnerability audit against Sonatype OSS Index. +# +# WHAT THIS COVERS THAT NOTHING ELSE DOES +# +# `deps: vulnerabilities` (maven.yml, actions/dependency-review-action) gates PRs, but only on the +# dependencies a PR *changes*. Dependabot alerts cover the standing tree, but miss transitive +# components entirely - `netty-codec-http`, `lz4-java` and `HdrHistogram` all carry advisories here +# and have no Dependabot alert in any state. The gap is a **whole-tree scan, on a schedule**, which +# is what the `schedule:` trigger below is for: an unchanged tree acquires new advisories, and +# nothing else in this repo would ever notice. +# +# This is the one exception to AGENTS.md's "there is no scheduled build, deliberately". That rule +# rejects cron lanes that RE-RUN suites the PR gate already covers. This lane covers something the +# gate cannot: the passage of time. +# +# ONE JOB, DELIBERATELY. `ossindex-maven-plugin` is bound to `validate`, which every Maven job in +# this repo runs, so "enabled in CI" would mean six-plus scans per PR from a single account and a +# burnt rate limit for zero extra signal. It is off by default in the pom +# (`ossindex.skip`, astubbs/parallel-consumer#259) and switched on only here. +# +# A GREEN MAVEN RUN IS NOT EVIDENCE THE SCAN HAPPENED. The plugin's `fail` parameter covers +# "vulnerable components were found", not "the request failed" - with an expired token it prints a +# WARNING and reports BUILD SUCCESS. That silent-green is the exact defect this repo already shipped +# once. `bin/check-ossindex-audit.sh` is what makes the job honest; read its header before changing +# anything here, and note its self-test runs FIRST, per the house pattern in repo-hygiene.yml. +# +# RED MEANS "THE CHECK BROKE", NOT "WE HAVE CVEs". Findings are written to the job summary and do +# not fail the job. The tree has a standing backlog of them; a job red on every PR for known debt is +# ignored within a week. See bin/check-ossindex-audit.sh for the full reasoning. +# +# Required repo secrets: OSSINDEX_USERNAME (the account email), OSSINDEX_TOKEN (API token from +# ossindex.sonatype.org -> account settings). Both already exist. + +name: Dependency Audit + +on: + schedule: + # Weekly, Mondays 06:17 UTC - findings land at the start of the week. Weekly rather than daily + # because the result only moves when the advisory database does, and a digest nobody reads is + # worth less than one they do. Off the hour on purpose: GitHub drops scheduled runs that pile up + # at :00. Cron only fires from the default branch, so this does nothing until it is on master. + - cron: '17 6 * * 1' + pull_request: + workflow_dispatch: + +concurrency: + group: dependency-audit-${{ github.head_ref || github.ref }} + cancel-in-progress: true + +# Read-only. Findings go to the job summary rather than a PR comment specifically so this job never +# needs `pull-requests: write` - it runs PR-authored build code in the same job that holds the OSS +# Index token, and every permission it does not have is one that cannot be turned against it. +# See docs/inflight/ci-review-agent.md for the same exposure question on the review job. +permissions: + contents: read + +jobs: + ossindex: + name: "deps: ossindex audit" + # Fork PRs receive no secrets, so the audit would 401 and the guard would correctly go red on + # every one of them - a permanent false alarm that teaches people to ignore the check. Skip + # instead. Same test the self-hosted lanes use. + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: '17' + # `server-id` must match the plugin's `authId`, which is `ossindex`. setup-java writes a + # settings.xml whose reads these two env var NAMES at Maven runtime, so the + # secrets have to be in `env:` on the Maven step - not here. + server-id: ossindex + server-username: OSSINDEX_USERNAME + server-password: OSSINDEX_TOKEN + # No `cache: maven` - see the caching note at the top of maven.yml. + + # Restore-only, off maven.yml's rotating prepare-deps key. This job must not SAVE a cache: it + # would compete with the key the build lanes own, and it resolves a different (smaller) set. + - name: Restore Maven cache + uses: actions/cache/restore@v4 + with: + path: ~/.m2/repository + key: setup-java-Linux-x64-maven-${{ hashFiles('**/pom.xml') }} + restore-keys: | + setup-java-Linux-x64-maven- + + # Before the guard runs, prove the guard still works - the sequencing repo-hygiene.yml and + # copyright.yml both use. A guard nobody has watched fail is decoration. + - name: Self-test the did-it-actually-scan guard + run: bash bin/test-check-ossindex-audit.sh + + # An empty secret produces a 401, which the guard catches - but it names three possible causes + # where this step names the actual one. Cheap, and it fails before spending a build. + - name: Preflight - credentials must be present + env: + OSSINDEX_USERNAME: ${{ secrets.OSSINDEX_USERNAME }} + OSSINDEX_TOKEN: ${{ secrets.OSSINDEX_TOKEN }} + run: | + if [ -z "${OSSINDEX_USERNAME}" ] || [ -z "${OSSINDEX_TOKEN}" ]; then + echo "::error::OSSINDEX_USERNAME / OSSINDEX_TOKEN are not both set as repo secrets. The audit cannot authenticate, and OSS Index no longer serves anonymous component-report requests." + exit 1 + fi + + # `test-compile`, not `validate`: the audit mojo resolves TEST-scope dependencies and the + # modules depend on `parallel-consumer-core:jar:tests`, so on a runner with no PC snapshots + # installed a bare `ossindex:audit` (or a `validate`-only run) dies at the second module with + # "Could not resolve dependencies". Verified locally against a repository with those snapshots + # removed - the reactor has to have compiled them. + # + # Every property is passed explicitly so this command behaves identically whether or not + # astubbs/parallel-consumer#259 has merged. `ossindex.skip`, `ossindex.fail` and + # `ossindex.authId` are all user properties of the plugin ITSELF, so they work against the + # pom as it stands today (which hardcodes `fail=false` and declares no `authId`) and against + # the one that PR leaves behind (which wires all three to properties, defaulting skip on). + # So this job has no ordering dependency on that PR - and must NOT copy its pom edits here. + - name: Audit dependencies against OSS Index + shell: bash + env: + OSSINDEX_USERNAME: ${{ secrets.OSSINDEX_USERNAME }} + OSSINDEX_TOKEN: ${{ secrets.OSSINDEX_TOKEN }} + run: | + # pipefail, or `tee` swallows a Maven BUILD FAILURE and the step goes green on a broken run. + set -o pipefail + ./mvnw --batch-mode test-compile \ + -Dossindex.skip=false \ + -Dossindex.fail=false \ + -Dossindex.authId=ossindex \ + -Dossindex.reportFile=target/ossindex-report.json \ + | tee ossindex-audit.log + + # The whole point of the job. Renders findings into the run summary, and goes RED only when it + # cannot prove the scan actually happened. + - name: Verify the scan ran, and report what it found + if: always() + shell: bash + run: | + set -o pipefail + bash bin/check-ossindex-audit.sh ossindex-audit.log | tee -a "$GITHUB_STEP_SUMMARY" + + # Kept so a finding can be triaged from the structured report rather than re-run by hand, and + # so a failed run can be diagnosed from the log that produced the verdict. + - name: Upload audit reports + if: always() + uses: actions/upload-artifact@v7 + with: + name: ossindex-reports + path: | + ossindex-audit.log + **/target/ossindex-report.json + if-no-files-found: warn diff --git a/AGENTS.md b/AGENTS.md index e1b519040..ea3241783 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -278,13 +278,14 @@ stack traces (see Testing). - **`.github/workflows/repo-hygiene.yml`** — "Repo Hygiene", on every push/PR plus dispatch. Two independent jobs. `sigpipe` runs `bin/check-shell-sigpipe.sh` (its own self-test first) to catch a `bin/*.sh` piping into `grep -q` under `pipefail`, which silently inverts the script's answer. `actions` runs `bin/check-action-versions.sh`, keeping every GitHub Action pinned to a single version across all workflows. Neither gates the build - they exist because the failures they catch are invisible rather than loud. - **`.github/workflows/check-dependencies.yml`** — "PR Dependency Check". Reads `depends on astubbs/parallel-consumer#N` lines from the PR body and blocks the child until every parent has merged. Produces the **required** check `Check PR Dependencies`, so a stacked PR cannot merge out of order. See [PR Discipline](#pr-discipline) for the syntax. - **`.github/workflows/cancel-closed-pr-runs.yml`** — Cancels a PR's in-flight runs when it closes, so a withdrawn PR stops occupying runners. Housekeeping only; gates nothing. +- **`.github/workflows/dependency-audit.yml`** — "Dependency Audit", job `deps: ossindex audit`. The **only** place `ossindex-maven-plugin` is switched on (`-Dossindex.skip=false`); it is bound to `validate`, so enabling it globally would mean six-plus scans per PR from one account. Runs on every in-repo PR, on dispatch, and **weekly on a schedule** - the schedule is the point, because `deps: vulnerabilities` only reviews dependencies a PR *changes* and Dependabot misses transitive components entirely, so nothing else notices an unchanged tree acquiring a new advisory. (This is the one deliberate exception to "there is no scheduled build" below: it re-runs no suite the gate already covers.) **Findings do not fail it** - they go to the job summary - because the tree carries a standing backlog and a job red on every PR is ignored within a week. It goes red only when `bin/check-ossindex-audit.sh` cannot prove the scan happened. That guard is not optional: the plugin has **no setting that makes an unreachable scanner fatal**, so with an expired token it prints a WARNING and reports `BUILD SUCCESS` - which is the exact silent-green defect this repo already shipped once. `bin/test-check-ossindex-audit.sh` runs first, per the house pattern. Skipped for fork PRs, which receive no secrets and would otherwise 401 forever. - **Self-hosted lanes** (see [`docs/SELF_HOSTED_RUNNER.md`](docs/SELF_HOSTED_RUNNER.md)). None of these gate merging - they are for speed and for work too heavy for a 2-core hosted runner. All are **skipped for PRs from forks** (`head.repo.full_name == github.repository`), because a fork PR must never run on our own hardware. **`highcpu` is the only self-hosted label** - six runners, all online. Declare labels in [`.github/actionlint.yaml`](.github/actionlint.yaml) or actionlint flags them. - `pr-highcpu-fast-feedback.yml` ("highcpu") — on every in-repo PR plus dispatch. The lane that earns the hardware. - `mutation-full-sweep.yml` — dispatch only: the whole-project PIT sweep (`bin/ci-mutation-test.sh -Dverbose=true -Dthreads=N`). The PR-scoped mutation job in `maven.yml` only covers classes changed against the base; this is its exhaustive counterpart. - **There is no scheduled build, deliberately.** Every suite worth re-running is already a required check on each PR and runs again on every push to master, so a cron lane would only repeat covered work. **Do not add a lane for suites the gate already covers.** + **There is no scheduled build, deliberately.** Every suite worth re-running is already a required check on each PR and runs again on every push to master, so a cron lane would only repeat covered work. **Do not add a lane for suites the gate already covers.** The single cron lane in the repo, `dependency-audit.yml`, is not a counter-example: it runs no *suite*, and what it catches - a new advisory published against an unchanged dependency tree - is a function of elapsed time, which no PR-triggered check can ever see. That is the test to apply to any future scheduled lane: does time alone change the answer? **Before pinning a job to a self-hosted label, confirm a runner serves it** - `gh api repos/astubbs/parallel-consumer/actions/runners` lists each runner's labels and online status. A job pinned to a label nothing advertises does not fail; it queues until GitHub cancels it, so the lane reports nothing at all and looks merely quiet. diff --git a/bin/check-ossindex-audit.sh b/bin/check-ossindex-audit.sh new file mode 100755 index 000000000..08cb7b0b9 --- /dev/null +++ b/bin/check-ossindex-audit.sh @@ -0,0 +1,218 @@ +#!/usr/bin/env bash +# +# Copyright (C) 2026 Antony Stubbs and contributors +# + +# Decides whether an ossindex-maven-plugin run actually SCANNED, and renders what it found. +# +# WHY THIS EXISTS +# +# `ossindex-maven-plugin` cannot be configured to fail when the scanner is unreachable. Its `fail` +# parameter covers "vulnerable components were found", not "the request failed". Verified directly, +# with a cold client cache and an expired token: +# +# [WARNING] Failed to fetch component-reports +# org.sonatype.ossindex.service.client.transport.Transport$TransportException: +# Unexpected response; status: HTTP/1.1 401 Unauthorized +# [INFO] BUILD SUCCESS +# +# So a green Maven run is NEVER by itself evidence that the audit ran, and a token that expires six +# months from now degrades silently to green-but-not-scanning. That is the defect this repo already +# shipped once - the audit sat on `validate` with `fail=false`, 401ing on every module while every +# build stayed green. A CI job without this check would rebuild it somewhere more expensive. +# +# Beware the client's own on-disk report cache (macOS: ~/Library/Application Support/Sonatype/ +# Ossindex; Linux: under $XDG_CACHE_HOME). A warm cache serves results even with a bogus base-URL, +# so any experiment about reachability must clear it first, and CI must never cache that directory. +# +# THE SPLIT THIS SCRIPT ENCODES +# +# scan did not run -> exit 1 (red: the check itself is broken, and that IS actionable) +# scan ran, found nothing -> exit 0 +# scan ran, found problems -> exit 0, findings rendered (reporting only, deliberately) +# +# Findings are not fatal today because the tree carries a standing backlog of them: a job that goes +# red on every PR for known debt is ignored inside a week, which costs more than it buys. Red is +# reserved for "this check stopped working", which is rare and therefore still means something. +# Making findings fatal is a one-line change here, once the backlog is triaged - not before. +# +# HOW "DID IT ACTUALLY SCAN" IS DECIDED +# +# Three independent legs, all fail-closed: +# +# 1. NEGATIVE, on the log: `Failed to fetch component-reports` anywhere -> did not scan. +# 2. POSITIVE, structural: every exported report must carry a non-empty `reports` map. This is +# the leg that survives a wording change upstream. On a 401 the plugin still writes the report +# file, but writes `{ }` - so "the file exists" proves nothing and "the file has content" does. +# 3. POSITIVE, coverage: one exported report per module the log says it checked. Catches a +# module that silently produced nothing while its siblings scanned fine. +# +# Leg 2 is the one a token expiry trips. Both directions are exercised by +# bin/test-check-ossindex-audit.sh against real 401-shaped and success-shaped fixtures. +# +# USAGE +# +# bin/check-ossindex-audit.sh [tree-root] +# +# The Maven run must export reports: +# +# ./mvnw --batch-mode test-compile \ +# -Dossindex.skip=false -Dossindex.fail=false -Dossindex.authId=ossindex \ +# -Dossindex.reportFile=target/ossindex-report.json | tee audit.log +# +# `test-compile` rather than `validate`: the audit mojo resolves TEST-scope dependencies, and the +# modules depend on `parallel-consumer-core:jar:tests`, so on a machine without those snapshots +# installed a bare `ossindex:audit` dies with "Could not resolve dependencies". Verified. +# +# Markdown report goes to stdout; CI appends it to $GITHUB_STEP_SUMMARY. Progress goes to stderr. + +set -euo pipefail + +LOG="${1:?usage: check-ossindex-audit.sh [tree-root]}" +ROOT="${2:-$(cd "$(dirname "$0")/.." && pwd)}" + +# The line the plugin logs when the component-report request fails, whatever the cause - 401, DNS, +# timeout. A named constant because the self-test uses it to build its negative fixture. +FETCH_FAILURE_MARKER="Failed to fetch component-reports" +# The line it logs once per module it audits. Feeds the coverage leg only. +MODULE_AUDITED_MARKER="Checking for vulnerabilities;" + +if [ ! -f "$LOG" ]; then + echo "FAIL: Maven log not found: $LOG" >&2 + exit 1 +fi + +# grep with a FILE argument, never `cat ... | grep -q`: an early-exiting reader on a pipe takes the +# writer down with EPIPE, and under `set -o pipefail` a MATCH then reads as failure. See +# bin/AGENTS.md -> "Scripts that guard other scripts". +scan_failed=0 +if grep -qF "$FETCH_FAILURE_MARKER" "$LOG"; then + scan_failed=1 +fi + +modules_in_log=$(grep -cF "$MODULE_AUDITED_MARKER" "$LOG" || true) + +# Collect the exported reports. -print0 / read -d '' so a path containing a space cannot split. +reports=() +while IFS= read -r -d '' f; do + reports+=("$f") +done < <(find "$ROOT" -type f -name 'ossindex-report.json' -print0) + +# Facts out of the JSON, then the findings table. Emitted as `key=value` lines, a `---` sentinel, +# then markdown - so bash keeps the verdict and python keeps only the parsing. +facts=$(python3 - "${reports[@]+"${reports[@]}"}" <<'PY' +import json, sys + +paths = sorted(sys.argv[1:]) +empty = 0 +components = 0 +# coordinates -> {"advisories": {id: score}, "modules": set()} +found = {} + +for p in paths: + try: + with open(p) as fh: + doc = json.load(fh) + except (OSError, ValueError) as e: + print("parse_error=%s: %s" % (p, e), file=sys.stderr) + doc = {} + if not isinstance(doc, dict): + doc = {} + reports = doc.get("reports") or {} + if not reports: + empty += 1 + continue + components += len(reports) + module = p.rsplit("/target/", 1)[0].rsplit("/", 1)[-1] + for coords, report in (doc.get("vulnerable") or {}).items(): + entry = found.setdefault(coords, {"advisories": {}, "modules": set()}) + entry["modules"].add(module) + for vuln in report.get("vulnerabilities") or []: + entry["advisories"][vuln.get("id", "?")] = vuln.get("cvssScore") or 0.0 + +advisories = sum(len(e["advisories"]) for e in found.values()) +scores = [s for e in found.values() for s in e["advisories"].values()] + +print("report_files=%d" % len(paths)) +print("empty_reports=%d" % empty) +print("components=%d" % components) +print("vulnerable=%d" % len(found)) +print("advisories=%d" % advisories) +print("top_score=%s" % (max(scores) if scores else 0)) +print("---") + +if found: + print("| Component | Advisories | Top CVSS | Modules |") + print("|---|---|---|---|") + # Worst first, so the thing to act on is the first thing read. + for coords, e in sorted(found.items(), key=lambda kv: -max(kv[1]["advisories"].values())): + ids = ", ".join("`%s`" % i for i in sorted(e["advisories"])) + print("| `%s` | %s | %s | %s |" % ( + coords, ids, max(e["advisories"].values()), ", ".join(sorted(e["modules"])))) +PY +) + +empty_reports=$(sed -n 's/^empty_reports=//p' <<< "$facts") +report_files=$(sed -n 's/^report_files=//p' <<< "$facts") +components=$(sed -n 's/^components=//p' <<< "$facts") +vulnerable=$(sed -n 's/^vulnerable=//p' <<< "$facts") +advisories=$(sed -n 's/^advisories=//p' <<< "$facts") +top_score=$(sed -n 's/^top_score=//p' <<< "$facts") +findings_table=$(sed -n '/^---$/,$p' <<< "$facts" | tail -n +2) + +problems=() +if [ "$scan_failed" -eq 1 ]; then + problems+=("the plugin logged \"${FETCH_FAILURE_MARKER}\" - the scanner was unreachable, or rejected our credentials") +fi +if [ "$report_files" -eq 0 ]; then + problems+=("no ossindex-report.json was exported anywhere under ${ROOT} - the audit did not run at all") +fi +if [ "$empty_reports" -gt 0 ]; then + problems+=("${empty_reports} of ${report_files} exported report(s) hold zero component reports - that is the shape a 401 leaves behind") +fi +if [ "$report_files" -gt 0 ] && [ "$modules_in_log" -ne "$report_files" ]; then + problems+=("the log says ${modules_in_log} module(s) were checked but ${report_files} report(s) were exported - a module produced nothing") +fi + +if [ "${#problems[@]}" -gt 0 ]; then + echo "## :x: OSS Index audit did not run" + echo + echo "**This is not a vulnerability finding.** The scan did not happen, so an empty result" + echo "means nothing. Likely causes, in order: \`OSSINDEX_TOKEN\` expired or was revoked; the" + echo "\`ossindex\` server id is not reaching Maven; ossindex.sonatype.org is down." + echo + for p in "${problems[@]}"; do + echo "- ${p}" + done + echo + echo "Reproduce locally with an \`ossindex\` \`\` in \`~/.m2/settings.xml\`:" + echo + cat <<'REPRO' +``` +./mvnw --batch-mode test-compile -Dossindex.skip=false -Dossindex.fail=false \ + -Dossindex.authId=ossindex -Dossindex.reportFile=target/ossindex-report.json | tee audit.log +bin/check-ossindex-audit.sh audit.log +``` +REPRO + echo "FAIL: the OSS Index audit did not scan - ${#problems[@]} problem(s), see the report above." >&2 + exit 1 +fi + +if [ "$vulnerable" -eq 0 ]; then + echo "## :white_check_mark: OSS Index audit: no vulnerable components" +else + echo "## :warning: OSS Index audit: ${vulnerable} vulnerable component(s), ${advisories} advisory(ies)" +fi +echo +echo "Scanned **${components}** resolved components across **${report_files}** module(s)." +if [ "$vulnerable" -gt 0 ]; then + echo + echo "Highest CVSS score seen: **${top_score}**." + echo + printf '%s\n' "$findings_table" +fi +echo +echo "_Findings are **reported, not blocking**. This check goes red only when the scan fails to" +echo "run - see the header of \`bin/check-ossindex-audit.sh\` for why._" + +echo "ok: OSS Index audit ran - ${report_files} module report(s), ${components} components, ${vulnerable} vulnerable" >&2 diff --git a/bin/test-check-ossindex-audit.sh b/bin/test-check-ossindex-audit.sh new file mode 100755 index 000000000..0b6c918e2 --- /dev/null +++ b/bin/test-check-ossindex-audit.sh @@ -0,0 +1,183 @@ +#!/usr/bin/env bash +# +# Copyright (C) 2026 Antony Stubbs and contributors +# + +# Self-test for bin/check-ossindex-audit.sh - the "did the vulnerability scan actually run" guard. +# +# The guard exists because ossindex-maven-plugin reports BUILD SUCCESS when it cannot reach the +# scanner, so a token expiry degrades silently to green-but-not-scanning. A guard for that which +# has never been seen to fire is decoration, so every leg gets a negative control here. +# +# Fixtures are the REAL shapes, taken from actual runs, not invented: +# * a 401 leaves `{ }` in the exported report and prints "Failed to fetch component-reports" +# * a successful audit writes {"reports": {...}} plus {"vulnerable": {...}} when it found things +# +# Cases: +# 1. clean scan, no findings -> pass (0) +# 2. clean scan, findings present -> pass (0), findings rendered +# 3. 401: failure line in the log AND `{ }` reports -> FAIL (1) <- the real regression +# 4. token expiry shape with the log line ABSENT -> FAIL (1) <- leg 2 alone +# 5. no report exported at all -> FAIL (1) <- leg 1+2 +# 6. log claims 3 modules checked, only 2 reported -> FAIL (1) <- leg 3 alone +# 7. missing log file -> FAIL (1) +# +# Cases 4 and 6 are the ones worth keeping. Case 4 is the future failure this guard is really for: +# if Sonatype ever reworded the warning, leg 1 goes quiet and only the structural leg is left. Case 6 +# proves the coverage leg is load-bearing rather than decorative - it fires with no failure line in +# the log and every present report perfectly well-formed. +# +# Run: bin/test-check-ossindex-audit.sh (CI runs it BEFORE the guard it protects) + +set -uo pipefail + +GUARD="$(cd "$(dirname "$0")" && pwd)/check-ossindex-audit.sh" +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + +failures=0 + +assert() { # + if [ "$2" = "$3" ]; then + echo "ok: $1" + else + echo "FAIL: $1 (expected exit '$2', got '$3')" + failures=$((failures + 1)) + fi +} + +assert_contains() { # + if grep -qF "$2" <<< "$3"; then + echo "ok: $1" + else + echo "FAIL: $1 (output did not contain '$2')" + failures=$((failures + 1)) + fi +} + +# A report as the plugin exports it when the audit succeeded. $1 = module dir, $2 = "vulnerable" +# to include a finding. +write_report() { # [vulnerable] + mkdir -p "$1/$2/target" + if [ "${3:-}" = "vulnerable" ]; then + cat > "$1/$2/target/ossindex-report.json" <<'JSON' +{ + "reports": { + "org.example:safe:jar:1.0:compile": { "coordinates": "pkg:maven/org.example/safe@1.0" }, + "com.fasterxml.jackson.core:jackson-databind:jar:2.16.2:runtime": { + "coordinates": "pkg:maven/com.fasterxml.jackson.core/jackson-databind@2.16.2", + "vulnerabilities": [ { "id": "CVE-2026-54513", "cvssScore": 9.3 } ] + } + }, + "vulnerable": { + "com.fasterxml.jackson.core:jackson-databind:jar:2.16.2:runtime": { + "coordinates": "pkg:maven/com.fasterxml.jackson.core/jackson-databind@2.16.2", + "vulnerabilities": [ { "id": "CVE-2026-54513", "cvssScore": 9.3 } ] + } + } +} +JSON + else + cat > "$1/$2/target/ossindex-report.json" <<'JSON' +{ "reports": { "org.example:safe:jar:1.0:compile": { "coordinates": "pkg:maven/org.example/safe@1.0" } } } +JSON + fi +} + +# The literal file a 401 leaves behind. Verified against a real expired-token run. +write_empty_report() { # + mkdir -p "$1/$2/target" + printf '{ }' > "$1/$2/target/ossindex-report.json" +} + +# $1 = tree, $2 = how many "Checking for vulnerabilities" lines, $3 = "failed" to add the 401 lines. +write_log() { # [failed] + local log="$1/audit.log" i + : > "$log" + for ((i = 0; i < $2; i++)); do + printf '[INFO] --- ossindex:3.2.0:audit (audit-dependencies) @ module-%d ---\n' "$i" >> "$log" + printf '[INFO] Checking for vulnerabilities; 15 artifacts\n' >> "$log" + if [ "${3:-}" = "failed" ]; then + printf '[WARNING] Failed to fetch component-reports\n' >> "$log" + # shellcheck disable=SC2016 # a literal Java class name, not a shell expansion + printf 'org.sonatype.ossindex.service.client.transport.Transport$TransportException: Unexpected response; status: HTTP/1.1 401 Unauthorized\n' >> "$log" + fi + done + printf '[INFO] BUILD SUCCESS\n' >> "$log" + echo "$log" +} + +# Sets LAST_EC and LAST_OUT. Deliberately NOT `ec=$(run_guard ...)`: a command substitution runs +# in a subshell, so anything it assigns is lost - which is exactly how the first draft of this file +# failed on an unbound LAST_OUT. +run_guard() { # + LAST_EC=0 + LAST_OUT="$("$GUARD" "$1" "$2" 2>/dev/null)" || LAST_EC=$? +} + +# ── 1. clean scan, nothing found ────────────────────────────────────────────── +t="$TMP/clean"; mkdir -p "$t" +write_report "$t" core +write_report "$t" vertx +log=$(write_log "$t" 2) +run_guard "$log" "$t" +assert "clean scan with no findings passes" 0 "$LAST_EC" +assert_contains " ...and says so" "no vulnerable components" "$LAST_OUT" + +# ── 2. clean scan WITH findings - reported, never fatal ─────────────────────── +t="$TMP/findings"; mkdir -p "$t" +write_report "$t" core vulnerable +write_report "$t" vertx +log=$(write_log "$t" 2) +run_guard "$log" "$t" +assert "findings are reported, not fatal" 0 "$LAST_EC" +assert_contains " ...naming the advisory" "CVE-2026-54513" "$LAST_OUT" +assert_contains " ...and its score" "9.3" "$LAST_OUT" + +# ── 3. the real regression: 401 on every module ─────────────────────────────── +t="$TMP/unauth"; mkdir -p "$t" +write_empty_report "$t" core +write_empty_report "$t" vertx +log=$(write_log "$t" 2 failed) +run_guard "$log" "$t" +assert "401 on every module fails the check" 1 "$LAST_EC" +assert_contains " ...and says it is not a vulnerability finding" "not a vulnerability finding" "$LAST_OUT" + +# ── 4. structural leg ALONE - the log line reworded away ────────────────────── +# If Sonatype renames that warning, leg 1 silently stops matching. The empty `reports` map must +# still be enough on its own, or the guard rots into the very defect it was written against. +t="$TMP/reworded"; mkdir -p "$t" +write_empty_report "$t" core +write_empty_report "$t" vertx +log=$(write_log "$t" 2) +run_guard "$log" "$t" +assert "empty reports fail even with NO failure line in the log" 1 "$LAST_EC" +assert_contains " ...blaming the empty reports" "zero component reports" "$LAST_OUT" + +# ── 5. nothing exported at all ──────────────────────────────────────────────── +t="$TMP/nothing"; mkdir -p "$t" +log=$(write_log "$t" 0) +run_guard "$log" "$t" +assert "no exported report at all fails the check" 1 "$LAST_EC" + +# ── 6. coverage leg ALONE - a module silently produced nothing ──────────────── +t="$TMP/partial"; mkdir -p "$t" +write_report "$t" core +write_report "$t" vertx +log=$(write_log "$t" 3) +run_guard "$log" "$t" +assert "log says 3 modules checked but only 2 reported - fails" 1 "$LAST_EC" +assert_contains " ...naming the shortfall" "a module produced nothing" "$LAST_OUT" + +# ── 7. missing log ──────────────────────────────────────────────────────────── +t="$TMP/nolog"; mkdir -p "$t" +run_guard "$t/does-not-exist.log" "$t" +assert "missing Maven log fails rather than passing vacuously" 1 "$LAST_EC" + +echo +if [ "$failures" -eq 0 ]; then + echo "All bin/check-ossindex-audit.sh self-tests passed" + exit 0 +fi +echo "$failures self-test(s) FAILED" +exit 1 diff --git a/docs/inflight/ci-review-agent.md b/docs/inflight/ci-review-agent.md index 189aac057..576ff1261 100644 --- a/docs/inflight/ci-review-agent.md +++ b/docs/inflight/ci-review-agent.md @@ -11,10 +11,14 @@ question. Needs confirmation from the action's docs or maintainers about token scrubbing before it spawns Bash subprocesses. Until then: trusted authors only. (`pull-requests: write` may also be droppable back to `read` if the action posts via its own app token.) -- **Two grants still missing:** `actionlint`, so the reviewer cannot lint workflow PRs - it said so - itself on astubbs#102, and it ships on `ubuntu-latest` with `.github/actionlint.yaml` already present; and - `bin/todo-index.sh`, whose script merged with astubbs#103 while the grant did not follow. Land both in a - **non-workflow** PR, or the validation skip above means they are never exercised. +- **Four grants still missing:** `actionlint`, so the reviewer cannot lint workflow PRs - it said so + itself on astubbs#102, and it ships on `ubuntu-latest` with `.github/actionlint.yaml` already present; + `bin/todo-index.sh`, whose script merged with astubbs#103 while the grant did not follow; and + `bin/check-ossindex-audit.sh` + `bin/test-check-ossindex-audit.sh`, which arrived with the OSS Index + audit lane and were left ungranted for the same reason the others were - editing + `claude-code-review.yml` in the PR that adds a script forfeits that PR's own review. Land all four in + a **non-workflow** PR, or the validation skip above means they are never exercised. Grant **both + spellings** of each script path (`bin/x.sh:*` and `./bin/x.sh:*`), per `bin/AGENTS.md`. - **`bin/ci-integration-test.sh` is granted but unproven** against the 30-minute cap - Testcontainers on a 2-core hosted runner is slow, and an overrun looks like a timeout rather than a misconfiguration. Also unverified whether Docker works inside the action's sandbox at all. From 0244782f804e4a3b0f412dc2c29ecd09482f6240 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 11:35:58 +1200 Subject: [PATCH 02/29] ci(deps): fix two renderer bugs review found in the audit guard The guard could die in the one direction it exists to prevent: a scan that genuinely ran, reported as an unexplained red. - A coordinate can appear under `vulnerable` with an empty `vulnerabilities` list - an advisory OSS Index has not scored yet. That left the component's advisory map empty, and `max()` on it raised ValueError. Under `set -euo pipefail` that killed the whole guard with a traceback instead of rendering the findings it had just successfully collected. Now `top_of()`, which returns 0.0 for an unscored component. - Advisories were keyed on `id`, so two carrying no id both landed on "?" and the second silently overwrote the first - under-reporting real findings in both the per-component count and the run total. Now keyed positionally when no id is present, which still de-duplicates the same advisory across modules. Both are covered by new self-test cases, and both cases fail against the previous guard - verified with a control arm rather than assumed. Also from the same review pass: - Parse the parser's `key=value` facts in one whitelisted pass instead of six near-identical `sed` calls, and fail loudly when a fact is missing. A key renamed in the python block used to become a silently empty variable, with nothing tying the two halves together. - Document that leg 3 counts every `ossindex-report.json` under the tree with no run-identity check, so a stale one from an earlier or wider run is counted as this run's output. CI is immune (fresh checkout); a local re-run is not, so the documented local flow now clears them first. - The workflow header stated "off by default in the pom" as present fact. It is only true once astubbs/parallel-consumer#259 merges; until then the plugin still runs, 401ing, in every other Maven job. Qualified. - `docs/inflight/ci-review-agent.md` said four reviewer grants were missing. `bin/todo-index.sh` was already granted in bd021cb2 - both spellings. Three, and recorded so it is not re-added. Adds docs/inflight/ci-ossindex-lane-reassessment.md: GitHub's automatic Maven transitive dependency submission was enabled on this repo on 2026-08-11, which puts this lane's transitive-coverage justification on a timer. The note records the measured pre-flip overlap, the baseline, and the concrete re-measurement to run after the next push to master. Decision for now is to keep the scanner. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .github/workflows/dependency-audit.yml | 10 +++- bin/check-ossindex-audit.sh | 60 +++++++++++++++---- bin/test-check-ossindex-audit.sh | 46 ++++++++++++++ .../inflight/ci-ossindex-lane-reassessment.md | 58 ++++++++++++++++++ docs/inflight/ci-review-agent.md | 15 ++--- 5 files changed, 169 insertions(+), 20 deletions(-) create mode 100644 docs/inflight/ci-ossindex-lane-reassessment.md diff --git a/.github/workflows/dependency-audit.yml b/.github/workflows/dependency-audit.yml index 3b40d0fdf..ad4f3b4ec 100644 --- a/.github/workflows/dependency-audit.yml +++ b/.github/workflows/dependency-audit.yml @@ -15,8 +15,14 @@ # # ONE JOB, DELIBERATELY. `ossindex-maven-plugin` is bound to `validate`, which every Maven job in # this repo runs, so "enabled in CI" would mean six-plus scans per PR from a single account and a -# burnt rate limit for zero extra signal. It is off by default in the pom -# (`ossindex.skip`, astubbs/parallel-consumer#259) and switched on only here. +# burnt rate limit for zero extra signal. This job is the only place it is deliberately switched ON +# (`-Dossindex.skip=false`, below). +# +# Switching it OFF everywhere else needs an `ossindex.skip` property in the pom, which +# astubbs/parallel-consumer#259 adds and which has NOT merged yet. Until it does, the plugin still +# runs here on `validate` in every other Maven job - unauthenticated, 401ing, silently green. This +# job neither causes nor worsens that, and has no ordering dependency on that PR (see the Maven step +# below); it is simply the one place the scan is real. Drop this paragraph once #259 lands. # # A GREEN MAVEN RUN IS NOT EVIDENCE THE SCAN HAPPENED. The plugin's `fail` parameter covers # "vulnerable components were found", not "the request failed" - with an expired token it prints a diff --git a/bin/check-ossindex-audit.sh b/bin/check-ossindex-audit.sh index 08cb7b0b9..5c176e10d 100755 --- a/bin/check-ossindex-audit.sh +++ b/bin/check-ossindex-audit.sh @@ -54,8 +54,13 @@ # # bin/check-ossindex-audit.sh [tree-root] # -# The Maven run must export reports: +# The Maven run must export reports. Clear the previous run's first: leg 3 counts every +# `ossindex-report.json` under the tree with no run-identity check, so a stale one - from an earlier +# run, or from a wider one when you are now building a subset - is counted as this run's output. It +# can then either mask a module that genuinely produced nothing, or invent a shortfall that is not +# real. CI is immune (fresh checkout every time); a local re-run is not. # +# find . -type f -name ossindex-report.json -delete # ./mvnw --batch-mode test-compile \ # -Dossindex.skip=false -Dossindex.fail=false -Dossindex.authId=ossindex \ # -Dossindex.reportFile=target/ossindex-report.json | tee audit.log @@ -109,6 +114,19 @@ components = 0 # coordinates -> {"advisories": {id: score}, "modules": set()} found = {} + +def top_of(entry): + """Highest CVSS on a component, 0.0 when nothing on it carries a score. + + A coordinate can appear under `vulnerable` with an empty (or absent) `vulnerabilities` list - + an advisory OSS Index has not scored yet - which leaves `advisories` empty. Bare `max()` on + that raises ValueError, and since the caller runs under `set -euo pipefail` that would kill + the whole guard with a traceback: a scan that DID run, reported as an unexplained red. + """ + scores = entry["advisories"].values() + return max(scores) if scores else 0.0 + + for p in paths: try: with open(p) as fh: @@ -127,8 +145,13 @@ for p in paths: for coords, report in (doc.get("vulnerable") or {}).items(): entry = found.setdefault(coords, {"advisories": {}, "modules": set()}) entry["modules"].add(module) - for vuln in report.get("vulnerabilities") or []: - entry["advisories"][vuln.get("id", "?")] = vuln.get("cvssScore") or 0.0 + for i, vuln in enumerate(report.get("vulnerabilities") or []): + # Positional fallback rather than a single shared "?": OSS Index does not always carry + # an id, and collapsing every unidentified advisory onto one key silently overwrites + # scores and under-counts findings. Positional keeps the same advisory de-duplicating + # across modules, which is what the shared key was getting right by accident. + vuln_id = vuln.get("id") or ("unidentified-%d" % i) + entry["advisories"][vuln_id] = vuln.get("cvssScore") or 0.0 advisories = sum(len(e["advisories"]) for e in found.values()) scores = [s for e in found.values() for s in e["advisories"].values()] @@ -145,19 +168,33 @@ if found: print("| Component | Advisories | Top CVSS | Modules |") print("|---|---|---|---|") # Worst first, so the thing to act on is the first thing read. - for coords, e in sorted(found.items(), key=lambda kv: -max(kv[1]["advisories"].values())): + for coords, e in sorted(found.items(), key=lambda kv: -top_of(kv[1])): ids = ", ".join("`%s`" % i for i in sorted(e["advisories"])) print("| `%s` | %s | %s | %s |" % ( - coords, ids, max(e["advisories"].values()), ", ".join(sorted(e["modules"])))) + coords, ids, top_of(e), ", ".join(sorted(e["modules"])))) PY ) -empty_reports=$(sed -n 's/^empty_reports=//p' <<< "$facts") -report_files=$(sed -n 's/^report_files=//p' <<< "$facts") -components=$(sed -n 's/^components=//p' <<< "$facts") -vulnerable=$(sed -n 's/^vulnerable=//p' <<< "$facts") -advisories=$(sed -n 's/^advisories=//p' <<< "$facts") -top_score=$(sed -n 's/^top_score=//p' <<< "$facts") +# One pass over the `key=value` block rather than one `sed` per fact. The name whitelist keeps the +# parser's output from assigning arbitrary variables, and the emptiness check below turns a fact +# renamed above but not here into a loud failure - previously it just became an empty variable, with +# nothing tying the two halves together. +empty_reports='' report_files='' components='' vulnerable='' advisories='' top_score='' +while IFS='=' read -r key value; do + case "$key" in + ---) break ;; + empty_reports|report_files|components|vulnerable|advisories|top_score) + printf -v "$key" '%s' "$value" ;; + esac +done <<< "$facts" + +for fact in empty_reports report_files components vulnerable advisories top_score; do + if [ -z "${!fact}" ]; then + echo "FAIL: internal error - the audit parser emitted no '${fact}' fact." >&2 + exit 1 + fi +done + findings_table=$(sed -n '/^---$/,$p' <<< "$facts" | tail -n +2) problems=() @@ -189,6 +226,7 @@ if [ "${#problems[@]}" -gt 0 ]; then echo cat <<'REPRO' ``` +find . -type f -name ossindex-report.json -delete # stale reports are counted as this run's ./mvnw --batch-mode test-compile -Dossindex.skip=false -Dossindex.fail=false \ -Dossindex.authId=ossindex -Dossindex.reportFile=target/ossindex-report.json | tee audit.log bin/check-ossindex-audit.sh audit.log diff --git a/bin/test-check-ossindex-audit.sh b/bin/test-check-ossindex-audit.sh index 0b6c918e2..ac9c81f3a 100755 --- a/bin/test-check-ossindex-audit.sh +++ b/bin/test-check-ossindex-audit.sh @@ -21,6 +21,8 @@ # 5. no report exported at all -> FAIL (1) <- leg 1+2 # 6. log claims 3 modules checked, only 2 reported -> FAIL (1) <- leg 3 alone # 7. missing log file -> FAIL (1) +# 8. vulnerable component with NO scored advisories -> pass (0) <- renderer crash +# 9. two advisories carrying no id -> pass (0), both counted # # Cases 4 and 6 are the ones worth keeping. Case 4 is the future failure this guard is really for: # if Sonatype ever reworded the warning, leg 1 goes quiet and only the structural leg is left. Case 6 @@ -174,6 +176,50 @@ t="$TMP/nolog"; mkdir -p "$t" run_guard "$t/does-not-exist.log" "$t" assert "missing Maven log fails rather than passing vacuously" 1 "$LAST_EC" +# ── 8. a vulnerable component with nothing scored on it ─────────────────────── +# OSS Index can list a coordinate under `vulnerable` while its `vulnerabilities` array is empty - +# an advisory it has not scored yet. That leaves the component's advisory map empty, which a bare +# `max()` in the renderer cannot take: it raises ValueError and, under `set -euo pipefail`, kills +# the guard with a traceback. A scan that DID run would be reported as an unexplained red - the +# guard failing in exactly the direction it exists to prevent. +t="$TMP/unscored"; mkdir -p "$t/core/target" +cat > "$t/core/target/ossindex-report.json" <<'JSON' +{ + "reports": { "org.example:thing:jar:1.0:compile": { "coordinates": "pkg:maven/org.example/thing@1.0" } }, + "vulnerable": { + "org.example:thing:jar:1.0:compile": { + "coordinates": "pkg:maven/org.example/thing@1.0", + "vulnerabilities": [] + } + } +} +JSON +log=$(write_log "$t" 1) +run_guard "$log" "$t" +assert "a vulnerable component with no scored advisories does not crash the guard" 0 "$LAST_EC" +assert_contains " ...and still renders that component" "org.example:thing" "$LAST_OUT" + +# ── 9. advisories with no id must not collapse onto one key ─────────────────── +# Keyed on `id`, so two unidentified advisories both landed on "?" and the second overwrote the +# first - the component's advisory count, and the run total, silently under-reported real findings. +t="$TMP/unidentified"; mkdir -p "$t/core/target" +cat > "$t/core/target/ossindex-report.json" <<'JSON' +{ + "reports": { "org.example:thing:jar:1.0:compile": { "coordinates": "pkg:maven/org.example/thing@1.0" } }, + "vulnerable": { + "org.example:thing:jar:1.0:compile": { + "coordinates": "pkg:maven/org.example/thing@1.0", + "vulnerabilities": [ { "cvssScore": 4.1 }, { "cvssScore": 7.7 } ] + } + } +} +JSON +log=$(write_log "$t" 1) +run_guard "$log" "$t" +assert "two unidentified advisories are counted separately, not collapsed" 0 "$LAST_EC" +assert_contains " ...reporting both of them" "2 advisory(ies)" "$LAST_OUT" +assert_contains " ...and keeping the higher score" "7.7" "$LAST_OUT" + echo if [ "$failures" -eq 0 ]; then echo "All bin/check-ossindex-audit.sh self-tests passed" diff --git a/docs/inflight/ci-ossindex-lane-reassessment.md b/docs/inflight/ci-ossindex-lane-reassessment.md new file mode 100644 index 000000000..196671ee5 --- /dev/null +++ b/docs/inflight/ci-ossindex-lane-reassessment.md @@ -0,0 +1,58 @@ +# OSS Index lane - does it still earn its keep? + +The `deps: ossindex audit` lane (added by astubbs/parallel-consumer#279) was justified on Dependabot +missing **transitive** components. That premise has partly expired, so the lane needs one deliberate +re-look rather than an indefinite assumption. + +GitHub has shipped automatic Maven transitive dependency submission since +[2024-07-29](https://github.blog/changelog/2024-07-29-automatically-submit-your-maven-transitive-dependencies-to-the-dependency-graph/) +/ [2025-03-26](https://github.blog/changelog/2025-03-26-transitive-dependencies-are-now-available-for-maven/). +It was simply never switched on here. **Enabled 2026-08-11.** + +## What was measured before the flip + +Measured, not reasoned from the docs, via +`gh api repos/astubbs/parallel-consumer/dependency-graph/sbom`: + +| Component | In the dependency graph | Dependabot alerted | +|---|---|---| +| `jackson-databind` | yes | yes | +| `micrometer-core` | yes | yes | +| `lz4-java` | **no** | no | +| `HdrHistogram` | **no** | no | +| `netty-codec-http` | **no** | no | + +The overlap is exact: the graph held only what the poms declare, and the three "Dependabot blind +spots" are precisely the components absent from it. The transitive-closure gap was real. Note the +consequence was smaller than first claimed, though - only `lz4-java` had an advisory to alert on. + +**Baseline at the moment of the flip**, so the re-measurement means something: 121 packages in the +graph, zero occurrences of `lz4-java` / `HdrHistogram` / `netty-codec-http`, 6 open Dependabot +alerts. Automatic submission runs on a push to the default branch, so it had not repopulated yet. + +## The trigger + +**After the next push to `master`**, re-run the SBOM query and check two things: + +1. do `lz4-java`, `HdrHistogram` and `netty-codec-http` now appear in the graph; +2. does a Dependabot alert fire for `lz4-java` (GHSA-xx22-p4ch-683r exists for it). + +## What the answer would mean + +If the graph picks up the full closure and Dependabot starts covering it, the **transitive-coverage** +justification for this lane is gone, and only two remain: + +- **proving the scan actually ran** - Dependabot has no equivalent, and it is the entire reason + `bin/check-ossindex-audit.sh` exists; +- **a whole-tree scheduled scan that can fail a build.** + +That is a genuine question to ask at that point, not a foregone conclusion either way. The honest +answer might be that the lane collapses to just the scheduled scan. + +Weigh in the maintenance cost the dependency graph does not carry: the exclusion list in +astubbs/parallel-consumer#281 has to be kept honest as advisories are published, corrected and +retired, and OSS Index has already produced two false positives and one phantom CVE id in a single +scan. + +**Decision (2026-08-11): keep the scanner.** Nothing was removed or weakened; this note exists so the +reassessment actually happens instead of being rediscovered. diff --git a/docs/inflight/ci-review-agent.md b/docs/inflight/ci-review-agent.md index 576ff1261..57ddaec25 100644 --- a/docs/inflight/ci-review-agent.md +++ b/docs/inflight/ci-review-agent.md @@ -11,14 +11,15 @@ question. Needs confirmation from the action's docs or maintainers about token scrubbing before it spawns Bash subprocesses. Until then: trusted authors only. (`pull-requests: write` may also be droppable back to `read` if the action posts via its own app token.) -- **Four grants still missing:** `actionlint`, so the reviewer cannot lint workflow PRs - it said so +- **Three grants still missing:** `actionlint`, so the reviewer cannot lint workflow PRs - it said so itself on astubbs#102, and it ships on `ubuntu-latest` with `.github/actionlint.yaml` already present; - `bin/todo-index.sh`, whose script merged with astubbs#103 while the grant did not follow; and - `bin/check-ossindex-audit.sh` + `bin/test-check-ossindex-audit.sh`, which arrived with the OSS Index - audit lane and were left ungranted for the same reason the others were - editing - `claude-code-review.yml` in the PR that adds a script forfeits that PR's own review. Land all four in - a **non-workflow** PR, or the validation skip above means they are never exercised. Grant **both - spellings** of each script path (`bin/x.sh:*` and `./bin/x.sh:*`), per `bin/AGENTS.md`. + and `bin/check-ossindex-audit.sh` + `bin/test-check-ossindex-audit.sh`, which arrived with the OSS + Index audit lane and were left ungranted for the same reason - editing `claude-code-review.yml` in + the PR that adds a script forfeits that PR's own review. Land all three in a **non-workflow** PR, or + the validation skip above means they are never exercised. Grant **both spellings** of each script + path (`bin/x.sh:*` and `./bin/x.sh:*`), per `bin/AGENTS.md`. (`bin/todo-index.sh` was listed here as + a fourth until astubbs#279 - both spellings are in fact already granted, added in `bd021cb2`. + Recorded so it is not re-added.) - **`bin/ci-integration-test.sh` is granted but unproven** against the 30-minute cap - Testcontainers on a 2-core hosted runner is slow, and an overrun looks like a timeout rather than a misconfiguration. Also unverified whether Docker works inside the action's sandbox at all. From 1c2a6cc9cbf39f806034945b4cffb8277c54b262 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 11:43:16 +1200 Subject: [PATCH 03/29] ci(deps): qualify the issue reference in the audit workflow header A bare `#259` on an added line fails the PR Checklist issue-ref gate: the fork's numbering sits inside confluentinc's range, so a bare number is ambiguous. The paragraph already names the PR in full two lines above, so this one just drops the number rather than repeating it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .github/workflows/dependency-audit.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/dependency-audit.yml b/.github/workflows/dependency-audit.yml index ad4f3b4ec..a1be358bf 100644 --- a/.github/workflows/dependency-audit.yml +++ b/.github/workflows/dependency-audit.yml @@ -22,7 +22,7 @@ # astubbs/parallel-consumer#259 adds and which has NOT merged yet. Until it does, the plugin still # runs here on `validate` in every other Maven job - unauthenticated, 401ing, silently green. This # job neither causes nor worsens that, and has no ordering dependency on that PR (see the Maven step -# below); it is simply the one place the scan is real. Drop this paragraph once #259 lands. +# below); it is simply the one place the scan is real. Drop this paragraph once that PR lands. # # A GREEN MAVEN RUN IS NOT EVIDENCE THE SCAN HAPPENED. The plugin's `fail` parameter covers # "vulnerable components were found", not "the request failed" - with an expired token it prints a From 408c8a9eac7167b939430e2559059b33f186220d Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 11:54:20 +1200 Subject: [PATCH 04/29] docs(inflight): record that automatic dependency submission is failing The reassessment note tells a future reader to re-measure the dependency graph after the next push to master. That instruction was incomplete: GitHub's managed `Automatic Dependency Submission (Maven)` workflow has failed on every run since the feature was enabled, so the graph will not repopulate and the re-measurement would read as "no change" for entirely the wrong reason. The failure is repo-wide, not this branch's - identical on astubbs/parallel-consumer#259 and astubbs/parallel-consumer#278. Its `validate-project` step cannot resolve `parallel-consumer-core` (or its test-jar) on a clean runner, which is exactly the failure mode dependency-audit.yml already documents and works around by running `test-compile` instead of `validate`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .../inflight/ci-ossindex-lane-reassessment.md | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/docs/inflight/ci-ossindex-lane-reassessment.md b/docs/inflight/ci-ossindex-lane-reassessment.md index 196671ee5..0407e19ef 100644 --- a/docs/inflight/ci-ossindex-lane-reassessment.md +++ b/docs/inflight/ci-ossindex-lane-reassessment.md @@ -30,9 +30,33 @@ consequence was smaller than first claimed, though - only `lz4-java` had an advi graph, zero occurrences of `lz4-java` / `HdrHistogram` / `netty-codec-http`, 6 open Dependabot alerts. Automatic submission runs on a push to the default branch, so it had not repopulated yet. +## Blocker: automatic submission is currently failing + +**Fix this first, or the trigger below silently never fires.** Since the flip, GitHub's managed +`Automatic Dependency Submission (Maven)` workflow (`submit-maven`) has failed on every run - the +same failure on `ci/ossindex-audit-job`, `docs/agents-gh-base-repo` and `build/enforce-plugin-versions` +alike, so it is repo-wide and not any one PR's doing. + +Its `validate-project` step cannot resolve the reactor's own modules: + +``` +Failed to execute goal on project parallel-consumer-vertx: Could not resolve dependencies + bz.stub.parallelconsumer:parallel-consumer-core:jar:0.6.0.0-SNAPSHOT -> not in central + bz.stub.parallelconsumer:parallel-consumer-core:jar:tests:0.6.0.0-SNAPSHOT -> not in central +``` + +This is the failure mode `.github/workflows/dependency-audit.yml` already documents for its own +Maven step: a bare `validate` on a clean runner has no `parallel-consumer-core` (or its test-jar) +installed, so the second module dies. That job works around it by running `test-compile`; the +managed workflow has no such step and cannot be edited in-repo, so it needs configuring (or +replacing with a repo-owned submission workflow that builds the reactor first). + +**Until it succeeds at least once on `master`, the dependency graph will not repopulate**, and the +re-measurement below would read as "no change" for the wrong reason. + ## The trigger -**After the next push to `master`**, re-run the SBOM query and check two things: +**After `submit-maven` succeeds on `master`**, re-run the SBOM query and check two things: 1. do `lz4-java`, `HdrHistogram` and `netty-codec-http` now appear in the graph; 2. does a Dependabot alert fire for `lz4-java` (GHSA-xx22-p4ch-683r exists for it). From 949837dd3ee561cac9eeaa266d856b631d7d0fa4 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 13:03:15 +1200 Subject: [PATCH 05/29] docs(inflight): drop the grants entry instead of narrating that it closed The ledger tracks what is OPEN. Rewriting a closed item into an accurate account of how it closed is the failure mode docs/inflight/AGENTS.md names explicitly, and I did it: the "three grants still missing" bullet became a paragraph explaining that they are no longer missing. That history belongs in the commit that landed them, which already carries it. What survives is the only genuinely open part - the two audit scripts are granted but have never been exercised, since the reviewer cannot run them until the PR granting them merges. Folded into the existing "granted but unproven" bullet next to bin/ci-integration-test.sh rather than kept as its own entry, because it is the same open question about the same thing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- docs/inflight/ci-review-agent.md | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/docs/inflight/ci-review-agent.md b/docs/inflight/ci-review-agent.md index cf26b1178..e6fdd1d6c 100644 --- a/docs/inflight/ci-review-agent.md +++ b/docs/inflight/ci-review-agent.md @@ -11,15 +11,9 @@ question. Needs confirmation from the action's docs or maintainers about token scrubbing before it spawns Bash subprocesses. Until then: trusted authors only. (`pull-requests: write` may also be droppable back to `read` if the action posts via its own app token.) -- **The grants landed, but this PR's own review is the price.** `actionlint`, - `bin/check-ossindex-audit.sh` and `bin/test-check-ossindex-audit.sh` are now in the allowlist - (both spellings each, per `bin/AGENTS.md`). They were parked on a catch-22 - editing - `claude-code-review.yml` makes it differ from the default-branch copy, so the action skips and - **reports that skip as success**, forfeiting the review of whichever PR adds them. They landed in - astubbs/parallel-consumer#279 deliberately, *after* its review had completed and its threads were - closed, so the forfeited review was one nobody needed. **Consequence to remember: any further - review on astubbs/parallel-consumer#279 is a no-op, and its green review tick must not be read as - reviewed.** The grants themselves are unexercised until the first PR after this one merges. -- **`bin/ci-integration-test.sh` is granted but unproven** against the 30-minute cap - Testcontainers +- **Granted but unproven**, in two different ways. `bin/check-ossindex-audit.sh` and + `bin/test-check-ossindex-audit.sh` have never been exercised by the reviewer - the first PR + after the one that granted them is the first real test. And `bin/ci-integration-test.sh` is + unproven against the 30-minute cap - Testcontainers on a 2-core hosted runner is slow, and an overrun looks like a timeout rather than a misconfiguration. Also unverified whether Docker works inside the action's sandbox at all. From ebeee0d42e5874dcf250d8fa684c240cb60f2a71 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 13:11:29 +1200 Subject: [PATCH 06/29] docs(inflight): the managed dependency submission cannot work here Turned back off on 2026-08-11 rather than leave a permanently red `submit-maven` on every PR. The cause is not publishing and not repositories: GitHub's managed workflow probes with a bare `mvn validate` at the repo root, which builds no artifacts, so any reactor module depending on a sibling's jar cannot resolve it - from the reactor (nothing built) or remotely. That is a design limitation of the managed workflow for multi-module projects. Its own log says so, pointing at the submission action for "your own workflow". Records the snapshot theory explicitly as a dead end so nobody re-derives it: we do publish snapshots, the pom does not declare that repository, and declaring one would be the wrong fix - it changes the whole project's resolution to satisfy one auxiliary probe, lets a partial build silently resolve a stale published snapshot instead of failing, and still needs every future SNAPSHOT published and declared forever. Waiting for the v6 release fixes nothing for the same reason. Leaves the decision as: a repo-owned workflow that builds the reactor first, or nothing - because the transitive gap it was meant to close is already covered by the OSS Index lane. That is now a stronger argument for keeping the lane than when this note was written, since the obvious managed alternative does not work on a project of this shape. The re-measurement trigger is retargeted accordingly. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .../inflight/ci-ossindex-lane-reassessment.md | 61 ++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/docs/inflight/ci-ossindex-lane-reassessment.md b/docs/inflight/ci-ossindex-lane-reassessment.md index 0407e19ef..f44470e65 100644 --- a/docs/inflight/ci-ossindex-lane-reassessment.md +++ b/docs/inflight/ci-ossindex-lane-reassessment.md @@ -30,6 +30,34 @@ consequence was smaller than first claimed, though - only `lz4-java` had an advi graph, zero occurrences of `lz4-java` / `HdrHistogram` / `netty-codec-http`, 6 open Dependabot alerts. Automatic submission runs on a push to the default branch, so it had not repopulated yet. +## What dependency submission actually is + +Worth stating, because the name suggests more than it does and the question came up twice. + +The dependency graph normally builds itself by **statically parsing `pom.xml`** - so it holds what +you *declare*, not what Maven *resolves*. That is the whole reason the transitive components are +missing from it, and therefore why Dependabot never alerted on `lz4-java` despite +`GHSA-xx22-p4ch-683r` existing: **Dependabot alerts read from the graph**, and the graph never had +the package. + +Dependency submission closes that by running Maven and POSTing the *resolved* dependency list - +coordinates and versions - to the repository's own graph via GitHub's Dependency Submission API. It +does **not** send source anywhere and involves no code scanning; the payload is a list of library +coordinates the poms already imply, just resolved rather than declared. + +Two ways to drive it, and they submit to the **same API and the same graph** - the only difference +is who runs Maven: + +| | Managed (the repo setting) | Repo-owned workflow | +|---|---|---| +| Maven invocation | GitHub's fixed bare `mvn validate` probe | ours | +| Editable | no - the workflow is not in `.github/workflows` | yes | +| Submits to | Dependency Submission API | the same API | + +So a repo-owned workflow buys exactly one thing: the ability to insert a build step before +submitting. The action is not smarter - **the same `validate`-builds-nothing trap bites it too** if +it is pointed at the repo root without building the reactor first. + ## Blocker: automatic submission is currently failing **Fix this first, or the trigger below silently never fires.** Since the flip, GitHub's managed @@ -54,9 +82,40 @@ replacing with a repo-owned submission workflow that builds the reactor first). **Until it succeeds at least once on `master`, the dependency graph will not repopulate**, and the re-measurement below would read as "no change" for the wrong reason. +### Turned back OFF on 2026-08-11 - re-enable after the v6 release + +Rather than leave a permanently red check on every PR, the *Automatic dependency submission* setting +was switched off again. **Re-enable it after 0.6.0.0 ships**, then re-run the trigger below. + +Root cause, and it is neither "we have never published" nor anything about repositories: **GitHub's +managed submission workflow probes with a bare `mvn validate` at the repo root.** `validate` builds +no artifacts, so any reactor module depending on a sibling's **jar** cannot resolve it - from the +reactor (nothing built) or remotely. It is a design limitation of the managed workflow for +multi-module projects, not a misconfiguration here, and the job's own log points at the escape +hatch: "for submitting Maven dependencies from your own workflow, refer to the submission action". + +Snapshots are a red herring, recorded so nobody re-runs this reasoning. We *do* publish them - +`parallel-consumer-core:0.6.0.0-SNAPSHOT` is served with HTTP 200 from +`central.sonatype.com/repository/maven-snapshots` - and the root pom does not declare that +repository. **Do not fix it by declaring one.** That would change the whole project's dependency +resolution to satisfy one auxiliary probe, let a partial build (`-pl` without `-am`) silently +resolve a stale published snapshot instead of failing loudly, and still require every future +`-SNAPSHOT` to be published *and* declared in perpetuity. Waiting for the v6 release does not fix it +either, for the same reason. + +**Current decision: leave the managed submission OFF.** The two real options, if it is ever wanted: + +1. A repo-owned workflow running `advanced-security/maven-dependency-submission-action`, where the + Maven invocation is ours and can build the reactor first (`mvn install -DskipTests`, or + `test-compile` as `dependency-audit.yml` does for exactly this reason). +2. Nothing - because the coverage gap it was meant to close is the transitive one, and the OSS Index + lane in astubbs/parallel-consumer#279 already covers transitive components. That is the argument + *for* keeping the lane, and it is stronger now than when this note was written: the obvious + managed alternative does not work on a project of this shape. + ## The trigger -**After `submit-maven` succeeds on `master`**, re-run the SBOM query and check two things: +**If a repo-owned submission workflow is ever added and succeeds on `master`**, re-run the SBOM query and check two things: 1. do `lz4-java`, `HdrHistogram` and `netty-codec-http` now appear in the graph; 2. does a Dependabot alert fire for `lz4-java` (GHSA-xx22-p4ch-683r exists for it). From fe95c93ff932fd32e96351e82ce37a60458f9295 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 14:22:26 +1200 Subject: [PATCH 07/29] docs(inflight): the review forfeit is a blocking red, not a meaningless green The first bullet described the world before bin/check-review-posted.sh existed. That guard landed in a4db924a, and it converts exactly the silent success this entry warns about into a FAILING job - so the stated symptom, "a green review check that verified nothing", has not been reachable for some time. There is no green check to misread. This is not a pedantic correction. The stale entry is what this branch reasoned from when it decided to spend the review forfeit here: the argument was that the forfeit is free once a PR is done being reviewed, which holds if the cost is an ignorable green tick and does not hold when the cost is a required check that is red on every push and cannot be cleared by any re-run. Evidence on this PR: four runs red at four SHAs carrying the workflow edit (6ad8d16d, 949837dd, ebeee0d4, 28926628), against three green at the three SHAs before it (99f2fc68, 1c2a6cc9, 408c8a9e). The grants stay - this records the price accurately so the next decision is made against the real number. Checked, and deliberately not claimed as open: the guard's own documented limit, that it proves a comment citing this run exists rather than that a review happened. The action's progress comment does not carry a run URL (verified across every claude[bot] comment on this PR - only the completed "Claude finished" comments cite one), so a run that posts progress and then dies still fails the guard. Nothing to track there. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- docs/inflight/ci-review-agent.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/inflight/ci-review-agent.md b/docs/inflight/ci-review-agent.md index e6fdd1d6c..370368555 100644 --- a/docs/inflight/ci-review-agent.md +++ b/docs/inflight/ci-review-agent.md @@ -1,10 +1,15 @@ # Automated PR reviewer - gaps that affect what you can trust -- **A green `review` check can mean the reviewer never ran.** `claude-code-action` refuses to run when - the workflow file differs from the default-branch copy, and reports that skip as **success**. So any - PR editing `claude-code-review.yml` - or a workflow it validates - gets a green review check that - verified nothing. Correct behaviour on the action's part, invisible unless you read the job log. - **Do not read a green `review` check as "reviewed" on a workflow-touching PR.** +- **A PR that edits `claude-code-review.yml` can never go green on `claude-review`, and that is a + required check.** `claude-code-action` refuses to run when the file differs from the + default-branch copy and reports the skip as success; `bin/check-review-posted.sh` catches the + silent success and fails the job. The guard working as designed ([`docs/ci.md`](../ci.md)) - but + the consequence is that the reviewer's own allowlist cannot be extended through the normal gated + flow. Every run on such a PR is red, **no re-run can clear it** (the input is the workflow diff, + which is fixed per commit), and landing it takes an admin ruleset bypass. Budget for that before + adding a grant, and prefer spending the forfeit on a minimal PR over a feature branch. The cost is + a bypassed required check, not a green tick nobody should trust - **do not weaken the gate to + avoid it.** - **Credential exposure is unresolved, not cleared.** The review job runs PR-authored Maven/test code in the same job that holds `secrets.CLAUDE_CODE_OAUTH_TOKEN`, with `pull-requests: write`. Bounded by fork PRs not receiving secrets (same-repo, push-access only), which is not an answer to the actual From 7a0fa5447bd54dcb74b8f7c1171198a572658ccd Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 14:25:01 +1200 Subject: [PATCH 08/29] ci(deps): point the scheduled-build exception at docs/ci.md astubbs/parallel-consumer#272 moved "there is no scheduled build, deliberately" out of AGENTS.md into docs/ci.md. This header cited the rule by its old home, so the pointer went stale the moment master landed - the kind of reference that rots silently because nothing checks it. Swept the rest of this branch for the same breakage: the only other AGENTS.md citation is bin/check-ossindex-audit.sh's "bin/AGENTS.md -> Scripts that guard other scripts", and that file and section both still exist. No other stale anchors. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .github/workflows/dependency-audit.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/dependency-audit.yml b/.github/workflows/dependency-audit.yml index a1be358bf..386ccc63b 100644 --- a/.github/workflows/dependency-audit.yml +++ b/.github/workflows/dependency-audit.yml @@ -9,7 +9,7 @@ # is what the `schedule:` trigger below is for: an unchanged tree acquires new advisories, and # nothing else in this repo would ever notice. # -# This is the one exception to AGENTS.md's "there is no scheduled build, deliberately". That rule +# This is the one exception to docs/ci.md's "there is no scheduled build, deliberately". That rule # rejects cron lanes that RE-RUN suites the PR gate already covers. This lane covers something the # gate cannot: the passage of time. # From dbaede0a88d0e3ccfed65023cc0983a0e696a812 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 14:39:58 +1200 Subject: [PATCH 09/29] docs(inflight): the CVE triage is pushed and open, not unpushed work The note said the per-CVE triage was "not merged, and not pushed yet", so a reader was told there was nothing to follow. It is pushed: astubbs/parallel-consumer#281, open and carrying docs/inflight/deps-cve-backlog.md. Names the PR so the pointer resolves. Kept the "not merged" half, which is still true and is what the surrounding text depends on. Deliberately left two nearby claims alone rather than sweeping them up, because both remain true on master today and belong to astubbs/parallel-consumer#281 to retire at its own merge prep, per docs/inflight/AGENTS.md: "Why the audit still cannot simply be switched to `fail=true`" (the exclusions that would make it viable are on that PR, not here), and "ossindex.skip stays true for now ... until these are triaged". No "delete when #281 merges" marker left behind either - that doc forbids them, on the grounds that the merge is exactly when nobody is looking. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- docs/inflight/ci-ossindex-audit-dead.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/inflight/ci-ossindex-audit-dead.md b/docs/inflight/ci-ossindex-audit-dead.md index e93eefbc8..5623a01be 100644 --- a/docs/inflight/ci-ossindex-audit-dead.md +++ b/docs/inflight/ci-ossindex-audit-dead.md @@ -112,10 +112,10 @@ What the scanner reported (all ids are `CVE-2026-NNNNN`): | `at.yawk.lz4:lz4-java:1.10.1` | runtime | 59949 (6.5) | no | | `org.hdrhistogram:HdrHistogram:2.2.2` | runtime | 14683 (2.0), 14686 (2.0) | no | -The per-CVE triage is being written up separately as `docs/inflight/deps-cve-backlog.md`, in flight -on branch `security/cve-backlog-triage` - **not merged, and not pushed yet**, so that is a pointer to -work in progress rather than something you can follow today. What follows is only what the *scanner -comparison* turned out to be worth, because the first reading of it was wrong. +The per-CVE triage is written up separately as `docs/inflight/deps-cve-backlog.md`, on branch +`security/cve-backlog-triage` - open for review as astubbs/parallel-consumer#281 and **not merged**, +so it is a pointer to work you can read today, but not to anything on master. What follows is only +what the *scanner comparison* turned out to be worth, because the first reading of it was wrong. ### Correction: the Dependabot gap is one component, not three From 5e99246ce63e321c38b82f5444820838355b6a91 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 14:49:09 +1200 Subject: [PATCH 10/29] Revert "docs(inflight): the CVE triage is pushed and open, not unpushed work" This reverts dbaede0a, leaving docs/inflight/ci-ossindex-audit-dead.md untouched by this branch. astubbs/parallel-consumer#281 has since made the same correction, and it is the better home: on that branch docs/inflight/deps-cve-backlog.md actually exists, so the sentence calling it unfollowable was not merely stale there, it was false about a file that PR adds. It also retired "Why the audit still cannot simply be switched to `fail=true`", which I had deliberately left alone - correctly so, since configuring the `excludeVulnerabilityIds` exclusions that section names as the blocker is exactly what that PR does. Both edits belong to the PR that resolves the work, per docs/inflight/AGENTS.md. Two PRs editing the same regions of one ledger file is a conflict waiting to happen, and that PR merged first. The file returns to exactly its merge-base content, so this branch carries no delta on it and master's version wins outright. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- docs/inflight/ci-ossindex-audit-dead.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/inflight/ci-ossindex-audit-dead.md b/docs/inflight/ci-ossindex-audit-dead.md index 5623a01be..e93eefbc8 100644 --- a/docs/inflight/ci-ossindex-audit-dead.md +++ b/docs/inflight/ci-ossindex-audit-dead.md @@ -112,10 +112,10 @@ What the scanner reported (all ids are `CVE-2026-NNNNN`): | `at.yawk.lz4:lz4-java:1.10.1` | runtime | 59949 (6.5) | no | | `org.hdrhistogram:HdrHistogram:2.2.2` | runtime | 14683 (2.0), 14686 (2.0) | no | -The per-CVE triage is written up separately as `docs/inflight/deps-cve-backlog.md`, on branch -`security/cve-backlog-triage` - open for review as astubbs/parallel-consumer#281 and **not merged**, -so it is a pointer to work you can read today, but not to anything on master. What follows is only -what the *scanner comparison* turned out to be worth, because the first reading of it was wrong. +The per-CVE triage is being written up separately as `docs/inflight/deps-cve-backlog.md`, in flight +on branch `security/cve-backlog-triage` - **not merged, and not pushed yet**, so that is a pointer to +work in progress rather than something you can follow today. What follows is only what the *scanner +comparison* turned out to be worth, because the first reading of it was wrong. ### Correction: the Dependabot gap is one component, not three From 642d89aa010aa81a771db7ef3983abb7950d1155 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 14:59:42 +1200 Subject: [PATCH 11/29] ci(deps): astubbs/parallel-consumer#259 has landed, so drop the conditional caveat The header carried a paragraph saying the pom's `ossindex.skip` property "has NOT merged yet", so the plugin still ran unauthenticated in every other Maven job - and it ended with its own instruction, "Drop this paragraph once that PR lands." That PR landed, and this branch already contains it: pom.xml declares ossindex.skip / ossindex.fail and wires ${ossindex.skip} into the plugin config, pulled in here by the master merge (208ae5e2b is an ancestor of HEAD). Verified rather than assumed, along with the claim the paragraph was hedging: dependency-audit.yml is the only workflow that passes -Dossindex.skip=false. So "exactly one job scans" is unconditional now, not aspirational, and the caveat was telling readers the opposite of the truth. Found by the review routed through claude.yml - the first substantive finding on this branch that the reviewer's own allowlist grants could not have produced, since it needed only reading. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .github/workflows/dependency-audit.yml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/dependency-audit.yml b/.github/workflows/dependency-audit.yml index 386ccc63b..a577ef8d1 100644 --- a/.github/workflows/dependency-audit.yml +++ b/.github/workflows/dependency-audit.yml @@ -18,11 +18,9 @@ # burnt rate limit for zero extra signal. This job is the only place it is deliberately switched ON # (`-Dossindex.skip=false`, below). # -# Switching it OFF everywhere else needs an `ossindex.skip` property in the pom, which -# astubbs/parallel-consumer#259 adds and which has NOT merged yet. Until it does, the plugin still -# runs here on `validate` in every other Maven job - unauthenticated, 401ing, silently green. This -# job neither causes nor worsens that, and has no ordering dependency on that PR (see the Maven step -# below); it is simply the one place the scan is real. Drop this paragraph once that PR lands. +# Everywhere else it is switched OFF by the `ossindex.skip` property in the pom +# (astubbs/parallel-consumer#259, merged), so "exactly one job scans" is now unconditional rather +# than aspirational: this is the only workflow that passes `-Dossindex.skip=false`. # # A GREEN MAVEN RUN IS NOT EVIDENCE THE SCAN HAPPENED. The plugin's `fail` parameter covers # "vulnerable components were found", not "the request failed" - with an expired token it prints a From c6af27fb25f32e3da0e7869017221d6854f02406 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 15:21:02 +1200 Subject: [PATCH 12/29] ci(deps): the standing backlog is gone, so say why reporting-only remains Three places justified reporting-only with "the tree carries a standing backlog" of findings, so gating would be permanently red. astubbs/parallel-consumer#281 triaged that backlog and merged: the audit now reports zero findings, verified end to end before that PR was pushed. The premise is false. That matters more than wording, because the passage exists to inform the decision about whether to gate. Read as written it says "do not gate, you would be permanently red". The truth is closer to the opposite - the known debt is now excluded with documented retirement conditions, so a NEW finding is by construction something nobody has looked at, which is an argument FOR gating. Reporting-only is kept, but as a conservative default rather than a necessity: the exclusions are only as good as their retirement conditions, and this lane has not yet run clean over time. The comments now say that, and say the case for flipping is getting stronger rather than blocked. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .github/workflows/dependency-audit.yml | 6 ++++-- bin/check-ossindex-audit.sh | 13 +++++++++---- docs/ci.md | 5 +++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/.github/workflows/dependency-audit.yml b/.github/workflows/dependency-audit.yml index a577ef8d1..dc5fd0fdf 100644 --- a/.github/workflows/dependency-audit.yml +++ b/.github/workflows/dependency-audit.yml @@ -29,8 +29,10 @@ # anything here, and note its self-test runs FIRST, per the house pattern in repo-hygiene.yml. # # RED MEANS "THE CHECK BROKE", NOT "WE HAVE CVEs". Findings are written to the job summary and do -# not fail the job. The tree has a standing backlog of them; a job red on every PR for known debt is -# ignored within a week. See bin/check-ossindex-audit.sh for the full reasoning. +# not fail the job. That is now a conservative default rather than a necessity: since +# astubbs/parallel-consumer#281 triaged the backlog, the audit reports zero findings, so gating would +# not be permanently red. Left reporting-only until the clean run has held for a while - flipping it +# is one line. See bin/check-ossindex-audit.sh for the full reasoning. # # Required repo secrets: OSSINDEX_USERNAME (the account email), OSSINDEX_TOKEN (API token from # ossindex.sonatype.org -> account settings). Both already exist. diff --git a/bin/check-ossindex-audit.sh b/bin/check-ossindex-audit.sh index 5c176e10d..0e36100c9 100755 --- a/bin/check-ossindex-audit.sh +++ b/bin/check-ossindex-audit.sh @@ -31,10 +31,15 @@ # scan ran, found nothing -> exit 0 # scan ran, found problems -> exit 0, findings rendered (reporting only, deliberately) # -# Findings are not fatal today because the tree carries a standing backlog of them: a job that goes -# red on every PR for known debt is ignored inside a week, which costs more than it buys. Red is -# reserved for "this check stopped working", which is rare and therefore still means something. -# Making findings fatal is a one-line change here, once the backlog is triaged - not before. +# Findings are not fatal today. The original reason was a standing backlog - a job that goes red on +# every PR for known debt is ignored inside a week - but astubbs/parallel-consumer#281 triaged it, so +# the audit now reports zero findings and gating would NOT be permanently red. What remains is a +# weaker, deliberate caution: the exclusions carrying that backlog are only as good as their stated +# retirement conditions, and this lane has not yet run clean over time. Red stays reserved for "this +# check stopped working", which is rare and therefore still means something. Flipping findings to +# fatal is a one-line change here, and the argument for it is now stronger than the argument against +# - a NEW finding, on a tree whose known debt is already excluded with reasons, is by construction +# something nobody has looked at. # # HOW "DID IT ACTUALLY SCAN" IS DECIDED # diff --git a/docs/ci.md b/docs/ci.md index 62988b5c0..d41d359c7 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -81,8 +81,9 @@ stack traces (see [`docs/testing.md`](testing.md)). vulnerabilities` only reviews dependencies a PR *changes*, so nothing else notices an unchanged tree acquiring a new advisory. (The one deliberate exception to "there is no scheduled build" below; it re-runs no suite the gate already covers.) **Findings do not fail it** - they go to the - job summary - because the tree carries a standing backlog and a job red on every PR is ignored - within a week. It goes red only when `bin/check-ossindex-audit.sh` cannot prove the scan happened, + job summary. That was originally because the tree carried a standing backlog; since + astubbs/parallel-consumer#281 triaged it the audit reports zero findings, so this is now a + conservative default rather than a necessity, and the case for gating is getting stronger. It goes red only when `bin/check-ossindex-audit.sh` cannot prove the scan happened, and that guard is not optional: the plugin has **no setting that makes an unreachable scanner fatal**, so against an expired token it prints a WARNING and reports `BUILD SUCCESS` - the exact silent-green defect this repo has already shipped once. `bin/test-check-ossindex-audit.sh` runs From c8d69a2bd70389b25a405f4c961665f8f41a3519 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 15:45:21 +1200 Subject: [PATCH 13/29] ci(deps): gate PRs on new CVEs, and name the check for what it scans Three changes to the whole-tree OSS Index lane. **Renamed `deps: ossindex audit` -> `deps: whole-tree CVE scan`.** "ossindex" is vendor jargon and unfindable in a checks list, and the old name gave no way to tell it apart from `deps: vulnerabilities` (maven.yml), which reviews only the dependencies a PR *changes*. "whole-tree" is the entire difference. Job names are an API here - repo rulesets reference them - so this is free only while the check is neither required nor on master, which it is not yet. **Findings now fail the job.** astubbs/parallel-consumer#281 retired the standing backlog into `excludeVulnerabilityIds` entries carrying stated retirement conditions, so a finding that reaches the gate is by construction something nobody has looked at. PR-time rather than the schedule alone because a solo maintainer will not read a scheduled alert. Maven stays lenient (`-Dossindex.fail=false`) and the guard decides red. Flipping the plugin to `fail=true` would kill the step via `pipefail` BEFORE bin/check-ossindex-audit.sh runs, losing both the did-it-actually-scan proof and the rendered summary on exactly the runs that have something to say. The two reds stay distinguishable - different exit codes, headings and stderr lines: exit 1 the scan cannot be proven to have run (the CHECK is broken), exit 2 the scan ran and found something (the TREE needs triage). When both are true, exit 1 wins. All three fail-closed legs are unchanged, including the structural "every report must carry a non-empty `reports` map". **The guard now honours `excludeVulnerabilityIds`.** Found on a live authenticated run, not reasoned about: the plugin applies the exclusion list when choosing which components to call vulnerable, but the exported report's per-component `vulnerabilities` array is unfiltered, and repeats the excluded ids in a top-level `excludedVulnerabilities` block. Gating on the raw array went red on CVE-2026-54518 - already excluded in the root pom with a reason - and no pom edit could ever have cleared it. The suppression count is now printed on every run, green ones included, so the list stays reviewable. Self-tests go from nine cases to twelve, asserting exact exit codes rather than "non-zero" so a later refactor cannot quietly collapse the two reds. Also drops a stale comment describing astubbs/parallel-consumer#259 as possibly-unmerged and the pom as hardcoding `fail=false` with no `authId`. It merged; the pom wires all three to properties. The durable fact - these are plugin user properties, so passing them explicitly is robust regardless of the pom - is kept. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .github/workflows/dependency-audit.yml | 57 ++++++--- bin/check-ossindex-audit.sh | 119 +++++++++++++---- bin/test-check-ossindex-audit.sh | 121 ++++++++++++++++-- docs/ci.md | 41 ++++-- .../inflight/ci-ossindex-lane-reassessment.md | 2 +- 5 files changed, 276 insertions(+), 64 deletions(-) diff --git a/.github/workflows/dependency-audit.yml b/.github/workflows/dependency-audit.yml index dc5fd0fdf..88847e3a2 100644 --- a/.github/workflows/dependency-audit.yml +++ b/.github/workflows/dependency-audit.yml @@ -28,11 +28,29 @@ # once. `bin/check-ossindex-audit.sh` is what makes the job honest; read its header before changing # anything here, and note its self-test runs FIRST, per the house pattern in repo-hygiene.yml. # -# RED MEANS "THE CHECK BROKE", NOT "WE HAVE CVEs". Findings are written to the job summary and do -# not fail the job. That is now a conservative default rather than a necessity: since -# astubbs/parallel-consumer#281 triaged the backlog, the audit reports zero findings, so gating would -# not be permanently red. Left reporting-only until the clean run has held for a while - flipping it -# is one line. See bin/check-ossindex-audit.sh for the full reasoning. +# THIS JOB GOES RED TWO DIFFERENT WAYS, AND THEY ARE NOT THE SAME THING. +# +# exit 1 the scan cannot be proven to have run - the CHECK is broken, and nothing was learned +# about the tree. Go and look at the token, the `ossindex` server id, or the service. +# exit 2 the scan ran and found an advisory - the LANE worked, and there is something to triage. +# +# Different exit codes, different summary headings, different stderr lines. Do not collapse them: a +# single undifferentiated red would leave the reader unable to tell whether the scanner had anything +# to say. When both are true, exit 1 wins - findings from an unprovable scan are evidence of nothing. +# +# WHY FINDINGS GATE THE PR. astubbs/parallel-consumer#281 retired the standing backlog: each item is +# now an explicit `excludeVulnerabilityIds` entry in the root pom carrying a stated retirement +# condition. On a tree whose known debt is excluded *with reasons*, a finding is by construction +# something nobody has looked at - so it is worth stopping for, and the "red every PR for known debt" +# objection no longer applies. And PR-time rather than the schedule alone because this repo has one +# maintainer: a weekly digest nobody is obliged to read is mail, not a control. The schedule still +# earns its place for what no PR can see - time passing - but it is the second channel, not the +# first. +# +# The escape hatch for a false positive (and this scanner produces them - see +# docs/inflight/ci-ossindex-audit-dead.md) is the same one the backlog uses: add the id to +# `excludeVulnerabilityIds` with a reason and a retirement condition. The guard honours that list and +# states the suppression count on every run, including green ones. See bin/check-ossindex-audit.sh. # # Required repo secrets: OSSINDEX_USERNAME (the account email), OSSINDEX_TOKEN (API token from # ossindex.sonatype.org -> account settings). Both already exist. @@ -62,7 +80,12 @@ permissions: jobs: ossindex: - name: "deps: ossindex audit" + # Named for what it does, not for who supplies it. "ossindex" is vendor jargon and unfindable in + # a checks list; this also has to be told apart at a glance from `deps: vulnerabilities` + # (maven.yml), which reviews only the dependencies a PR CHANGES - "whole-tree" is the whole + # difference. Job names are an API here, referenced by repo rulesets (see repo-hygiene.yml's + # header): renaming is free only while this check is not yet required and not on master. + name: "deps: whole-tree CVE scan" # Fork PRs receive no secrets, so the audit would 401 and the guard would correctly go red on # every one of them - a permanent false alarm that teaches people to ignore the check. Skip # instead. Same test the self-hosted lanes use. @@ -117,12 +140,16 @@ jobs: # "Could not resolve dependencies". Verified locally against a repository with those snapshots # removed - the reactor has to have compiled them. # - # Every property is passed explicitly so this command behaves identically whether or not - # astubbs/parallel-consumer#259 has merged. `ossindex.skip`, `ossindex.fail` and - # `ossindex.authId` are all user properties of the plugin ITSELF, so they work against the - # pom as it stands today (which hardcodes `fail=false` and declares no `authId`) and against - # the one that PR leaves behind (which wires all three to properties, defaulting skip on). - # So this job has no ordering dependency on that PR - and must NOT copy its pom edits here. + # Every property is passed explicitly. `ossindex.skip`, `ossindex.fail` and `ossindex.authId` + # are user properties of the plugin ITSELF, so the command is robust to how the pom happens to + # be wired - it does not depend on the ``/``/`` elements the root pom + # declares today, and would keep working if they were reshuffled. + # + # `-Dossindex.fail=false` is the load-bearing one, and it is NOT laxity - it is what makes + # bin/check-ossindex-audit.sh the single place that decides red. The pom defaults + # `ossindex.fail` to TRUE, and letting Maven fail here would take the step down via `pipefail` + # BEFORE the guard runs: on exactly the runs with findings we would lose both the + # did-it-actually-scan proof and the rendered summary. Maven stays lenient; the guard gates. - name: Audit dependencies against OSS Index shell: bash env: @@ -138,9 +165,9 @@ jobs: -Dossindex.reportFile=target/ossindex-report.json \ | tee ossindex-audit.log - # The whole point of the job. Renders findings into the run summary, and goes RED only when it - # cannot prove the scan actually happened. - - name: Verify the scan ran, and report what it found + # The whole point of the job, and the only step that decides red. Renders findings into the run + # summary, exits 1 when it cannot prove the scan happened, and 2 when the scan found something. + - name: Verify the scan ran, and gate on what it found if: always() shell: bash run: | diff --git a/bin/check-ossindex-audit.sh b/bin/check-ossindex-audit.sh index 0e36100c9..5186ad475 100755 --- a/bin/check-ossindex-audit.sh +++ b/bin/check-ossindex-audit.sh @@ -25,21 +25,39 @@ # Ossindex; Linux: under $XDG_CACHE_HOME). A warm cache serves results even with a bogus base-URL, # so any experiment about reachability must clear it first, and CI must never cache that directory. # -# THE SPLIT THIS SCRIPT ENCODES +# TWO REDS, AND THEY MUST NOT BE CONFUSED # -# scan did not run -> exit 1 (red: the check itself is broken, and that IS actionable) +# scan did not run -> exit 1 (red: the CHECK is broken - an empty result means nothing) # scan ran, found nothing -> exit 0 -# scan ran, found problems -> exit 0, findings rendered (reporting only, deliberately) +# scan ran, found problems -> exit 2 (red: the TREE has an advisory nobody has looked at) # -# Findings are not fatal today. The original reason was a standing backlog - a job that goes red on -# every PR for known debt is ignored inside a week - but astubbs/parallel-consumer#281 triaged it, so -# the audit now reports zero findings and gating would NOT be permanently red. What remains is a -# weaker, deliberate caution: the exclusions carrying that backlog are only as good as their stated -# retirement conditions, and this lane has not yet run clean over time. Red stays reserved for "this -# check stopped working", which is rare and therefore still means something. Flipping findings to -# fatal is a one-line change here, and the argument for it is now stronger than the argument against -# - a NEW finding, on a tree whose known debt is already excluded with reasons, is by construction -# something nobody has looked at. +# Distinct exit codes, distinct headings and distinct stderr lines, because they demand opposite +# responses: exit 1 means go fix the credentials or the lane and learn nothing about the tree; exit 2 +# means the lane worked and there is a real advisory to triage. Collapsing them into one +# undifferentiated red would make the second unreadable - the reader could no longer tell whether the +# scanner had anything to say. When both are true at once, exit 1 wins: findings from a scan that +# cannot be proven to have happened are not evidence of anything, in either direction. +# +# WHY FINDINGS ARE FATAL +# +# They were not, once, and the reason was a standing backlog: a job that goes red on every PR for +# known debt is ignored inside a week. astubbs/parallel-consumer#281 retired that backlog - every +# item is now either fixed or an explicit `excludeVulnerabilityIds` entry in the root pom carrying a +# stated retirement condition. That inverts the argument. On a tree whose known debt is already +# excluded *with reasons*, a finding is by construction something nobody has looked at, and each one +# is either a real advisory or an exclusion that needs writing down. Nothing red is left standing. +# +# And PR-time rather than the schedule alone, because this repo has one maintainer: a weekly digest +# nobody is obliged to read is not a control, it is mail. The PR gate is the only channel that +# reliably has attention, so that is where the finding has to land. The schedule still matters for +# what a PR cannot see - an unchanged tree acquiring a new advisory - but it is the second channel, +# not the first. +# +# The cost is a false positive blocking unrelated work, and it is real: OSS Index produced two false +# positives and one CVE id with no public record in a single run against this repo +# (docs/inflight/ci-ossindex-audit-dead.md). The escape hatch is the same one the backlog used - +# add the id to `excludeVulnerabilityIds` in the root pom with a reason and a retirement condition - +# which is a deliberate, reviewable act rather than a flag flip. # # HOW "DID IT ACTUALLY SCAN" IS DECIDED # @@ -70,6 +88,12 @@ # -Dossindex.skip=false -Dossindex.fail=false -Dossindex.authId=ossindex \ # -Dossindex.reportFile=target/ossindex-report.json | tee audit.log # +# `-Dossindex.fail=false` is NOT laxity, and must stay - it is what makes this script the single +# place that decides red. The pom defaults `ossindex.fail` to `true`, and letting Maven fail on +# findings kills the pipeline (`set -o pipefail`) BEFORE the guard runs: on exactly the runs that +# have something to say, you would lose both the did-it-actually-scan verdict and the rendered +# findings table. Maven stays lenient; this script gates. +# # `test-compile` rather than `validate`: the audit mojo resolves TEST-scope dependencies, and the # modules depend on `parallel-consumer-core:jar:tests`, so on a machine without those snapshots # installed a bare `ossindex:audit` dies with "Could not resolve dependencies". Verified. @@ -118,6 +142,10 @@ empty = 0 components = 0 # coordinates -> {"advisories": {id: score}, "modules": set()} found = {} +# Advisory ids the root pom's `excludeVulnerabilityIds` suppressed, as the plugin itself resolved +# them. Collected across every module and reported as a count, so a growing exclusion list stays +# visible in the summary rather than quietly absorbing the whole point of the check. +excluded_ids = set() def top_of(entry): @@ -147,10 +175,30 @@ for p in paths: continue components += len(reports) module = p.rsplit("/target/", 1)[0].rsplit("/", 1)[-1] + + # `excludeVulnerabilityIds` in the root pom is the documented escape hatch for a finding that + # does not apply, is disputed, or has no fixed version. The plugin honours it when deciding + # which COMPONENTS land in `vulnerable`, but the per-component `vulnerabilities` array it + # exports is unfiltered - an excluded advisory is still listed there, and is also repeated in + # this top-level `excludedVulnerabilities` block. Verified on a live authenticated run: with + # CVE-2026-54518 excluded in the pom, the plugin logged it under "Excluded vulnerabilities:" + # and still wrote it into `vulnerable`. Gating on the raw array would therefore go red on + # exactly the debt that was deliberately triaged, and no pom edit could ever clear it. + module_excluded = { + v.get("id") for v in (doc.get("excludedVulnerabilities") or []) if v.get("id")} + excluded_ids |= module_excluded + for coords, report in (doc.get("vulnerable") or {}).items(): + listed = report.get("vulnerabilities") or [] + live = [v for v in listed if v.get("id") not in module_excluded] + # Every advisory on this component was excluded, with a reason, in the pom. The plugin + # would not normally list such a component at all; belt and braces so an upstream change + # to that behaviour cannot resurrect suppressed findings as a red gate. + if listed and not live: + continue entry = found.setdefault(coords, {"advisories": {}, "modules": set()}) entry["modules"].add(module) - for i, vuln in enumerate(report.get("vulnerabilities") or []): + for i, vuln in enumerate(live): # Positional fallback rather than a single shared "?": OSS Index does not always carry # an id, and collapsing every unidentified advisory onto one key silently overwrites # scores and under-counts findings. Positional keeps the same advisory de-duplicating @@ -166,6 +214,7 @@ print("empty_reports=%d" % empty) print("components=%d" % components) print("vulnerable=%d" % len(found)) print("advisories=%d" % advisories) +print("excluded=%d" % len(excluded_ids)) print("top_score=%s" % (max(scores) if scores else 0)) print("---") @@ -184,16 +233,16 @@ PY # parser's output from assigning arbitrary variables, and the emptiness check below turns a fact # renamed above but not here into a loud failure - previously it just became an empty variable, with # nothing tying the two halves together. -empty_reports='' report_files='' components='' vulnerable='' advisories='' top_score='' +empty_reports='' report_files='' components='' vulnerable='' advisories='' excluded='' top_score='' while IFS='=' read -r key value; do case "$key" in ---) break ;; - empty_reports|report_files|components|vulnerable|advisories|top_score) + empty_reports|report_files|components|vulnerable|advisories|excluded|top_score) printf -v "$key" '%s' "$value" ;; esac done <<< "$facts" -for fact in empty_reports report_files components vulnerable advisories top_score; do +for fact in empty_reports report_files components vulnerable advisories excluded top_score; do if [ -z "${!fact}" ]; then echo "FAIL: internal error - the audit parser emitted no '${fact}' fact." >&2 exit 1 @@ -241,21 +290,45 @@ REPRO exit 1 fi +# Past here the scan is PROVEN to have run, so anything red below is a statement about the tree, not +# about the lane. Kept visibly separate - different heading, different exit code, different stderr +# line - so the two reds can never be mistaken for each other. if [ "$vulnerable" -eq 0 ]; then echo "## :white_check_mark: OSS Index audit: no vulnerable components" else - echo "## :warning: OSS Index audit: ${vulnerable} vulnerable component(s), ${advisories} advisory(ies)" + echo "## :x: OSS Index audit found ${vulnerable} vulnerable component(s), ${advisories} advisory(ies)" fi echo echo "Scanned **${components}** resolved components across **${report_files}** module(s)." -if [ "$vulnerable" -gt 0 ]; then +# Never silent: an exclusion list is the one way a finding can legitimately disappear, so it is +# stated on every run - green ones included - or it stops being reviewable. +if [ "$excluded" -gt 0 ]; then echo - echo "Highest CVSS score seen: **${top_score}**." + echo "**${excluded}** advisory(ies) were suppressed by \`excludeVulnerabilityIds\` in the root pom." + echo "Each carries a reason and a retirement condition there; they are excluded from the verdict" + echo "above, not overlooked." +fi +if [ "$vulnerable" -eq 0 ]; then echo - printf '%s\n' "$findings_table" + echo "_The scan ran and the tree is clean. This check goes red two different ways: **the scan" + echo "could not be proven to have run** (broken lane, exit 1), or **the scan found something**" + echo "(exit 2) - see the header of \`bin/check-ossindex-audit.sh\`._" + echo "ok: OSS Index audit ran - ${report_files} module report(s), ${components} components, 0 vulnerable, ${excluded} excluded" >&2 + exit 0 fi + +echo +echo "**The scan ran cleanly - this is a real finding, not a broken check.** Known debt is already" +echo "carried as \`excludeVulnerabilityIds\` entries in the root pom and filtered out above, so" +echo "anything reaching this table is an advisory nobody has looked at." +echo +echo "Highest CVSS score seen: **${top_score}**." +echo +printf '%s\n' "$findings_table" echo -echo "_Findings are **reported, not blocking**. This check goes red only when the scan fails to" -echo "run - see the header of \`bin/check-ossindex-audit.sh\` for why._" +echo "Either fix it (upgrade, or drop the dependency), or - if it does not apply, is disputed, or has" +echo "no fixed version - add the id to \`excludeVulnerabilityIds\` in the root pom **with a reason and" +echo "a retirement condition**, the way astubbs/parallel-consumer#281 did for the original backlog." -echo "ok: OSS Index audit ran - ${report_files} module report(s), ${components} components, ${vulnerable} vulnerable" >&2 +echo "FAIL: OSS Index audit found ${vulnerable} vulnerable component(s), ${advisories} advisory(ies), top CVSS ${top_score} - the scan itself ran fine." >&2 +exit 2 diff --git a/bin/test-check-ossindex-audit.sh b/bin/test-check-ossindex-audit.sh index ac9c81f3a..023926b1b 100755 --- a/bin/test-check-ossindex-audit.sh +++ b/bin/test-check-ossindex-audit.sh @@ -13,21 +13,42 @@ # * a 401 leaves `{ }` in the exported report and prints "Failed to fetch component-reports" # * a successful audit writes {"reports": {...}} plus {"vulnerable": {...}} when it found things # +# THE TWO REDS ARE DIFFERENT EXIT CODES, and this suite is what holds them apart: +# +# exit 1 = the scan cannot be proven to have run (the CHECK is broken) +# exit 2 = the scan ran and found something (the TREE has an untriaged advisory) +# +# Asserting the exact code, not merely "non-zero", is the point. A refactor that made findings exit 1 +# would satisfy a non-zero assertion while destroying the distinction the job summary - and the +# maintainer's response to it - both depend on. +# # Cases: # 1. clean scan, no findings -> pass (0) -# 2. clean scan, findings present -> pass (0), findings rendered +# 2. clean scan, findings present -> FAIL (2) <- the gate # 3. 401: failure line in the log AND `{ }` reports -> FAIL (1) <- the real regression # 4. token expiry shape with the log line ABSENT -> FAIL (1) <- leg 2 alone # 5. no report exported at all -> FAIL (1) <- leg 1+2 # 6. log claims 3 modules checked, only 2 reported -> FAIL (1) <- leg 3 alone # 7. missing log file -> FAIL (1) -# 8. vulnerable component with NO scored advisories -> pass (0) <- renderer crash -# 9. two advisories carrying no id -> pass (0), both counted +# 8. vulnerable component with NO scored advisories -> FAIL (2) <- renderer crash +# 9. two advisories carrying no id -> FAIL (2), both counted +# 10. findings AND an unprovable scan at once -> FAIL (1) <- precedence +# 11. the only advisory is excluded in the pom -> pass (0) <- the escape hatch +# 12. one excluded advisory beside a live one -> FAIL (2), only the live one +# +# Cases 4 and 6 are the ones worth keeping among the fail-closed legs. Case 4 is the future failure +# this guard is really for: if Sonatype ever reworded the warning, leg 1 goes quiet and only the +# structural leg is left. Case 6 proves the coverage leg is load-bearing rather than decorative - it +# fires with no failure line in the log and every present report perfectly well-formed. +# +# Case 10 pins the precedence. Findings recovered from a scan that cannot be proven to have run are +# not evidence of anything, so "did not run" has to win - and must not be dressed up as a finding +# about the tree. # -# Cases 4 and 6 are the ones worth keeping. Case 4 is the future failure this guard is really for: -# if Sonatype ever reworded the warning, leg 1 goes quiet and only the structural leg is left. Case 6 -# proves the coverage leg is load-bearing rather than decorative - it fires with no failure line in -# the log and every present report perfectly well-formed. +# Cases 11 and 12 are the ones the gate itself depends on: `excludeVulnerabilityIds` in the root pom +# is the only way out of a permanent red on a disputed or unfixable finding, and the exported report +# does NOT pre-filter by it. See the comment on case 11 - this was found on a live run, not reasoned +# about. # # Run: bin/test-check-ossindex-audit.sh (CI runs it BEFORE the guard it protects) @@ -126,15 +147,24 @@ run_guard "$log" "$t" assert "clean scan with no findings passes" 0 "$LAST_EC" assert_contains " ...and says so" "no vulnerable components" "$LAST_OUT" -# ── 2. clean scan WITH findings - reported, never fatal ─────────────────────── +# ── 2. clean scan WITH findings - the gate ──────────────────────────────────── +# Exit 2, not 1: the scan worked, so this is a statement about the TREE. Asserting the exact code +# is what stops a later refactor from collapsing the two reds into one unreadable red. t="$TMP/findings"; mkdir -p "$t" write_report "$t" core vulnerable write_report "$t" vertx log=$(write_log "$t" 2) run_guard "$log" "$t" -assert "findings are reported, not fatal" 0 "$LAST_EC" +assert "findings on a proven scan fail the check" 2 "$LAST_EC" assert_contains " ...naming the advisory" "CVE-2026-54513" "$LAST_OUT" assert_contains " ...and its score" "9.3" "$LAST_OUT" +assert_contains " ...saying the check itself is fine" "not a broken check" "$LAST_OUT" +if grep -qF "did not run" <<< "$LAST_OUT"; then + echo "FAIL: ...and NOT blaming the scan (output claimed the scan did not run)" + failures=$((failures + 1)) +else + echo "ok: ...and NOT blaming the scan" +fi # ── 3. the real regression: 401 on every module ─────────────────────────────── t="$TMP/unauth"; mkdir -p "$t" @@ -196,7 +226,7 @@ cat > "$t/core/target/ossindex-report.json" <<'JSON' JSON log=$(write_log "$t" 1) run_guard "$log" "$t" -assert "a vulnerable component with no scored advisories does not crash the guard" 0 "$LAST_EC" +assert "a vulnerable component with no scored advisories does not crash the guard" 2 "$LAST_EC" assert_contains " ...and still renders that component" "org.example:thing" "$LAST_OUT" # ── 9. advisories with no id must not collapse onto one key ─────────────────── @@ -216,10 +246,79 @@ cat > "$t/core/target/ossindex-report.json" <<'JSON' JSON log=$(write_log "$t" 1) run_guard "$log" "$t" -assert "two unidentified advisories are counted separately, not collapsed" 0 "$LAST_EC" +assert "two unidentified advisories are counted separately, not collapsed" 2 "$LAST_EC" assert_contains " ...reporting both of them" "2 advisory(ies)" "$LAST_OUT" assert_contains " ...and keeping the higher score" "7.7" "$LAST_OUT" +# ── 10. both reds at once - "did not run" must win ──────────────────────────── +# Findings recovered from a scan that cannot be proven to have happened are not evidence of +# anything. If this ever reported exit 2, a broken lane would be triaged as a dependency problem and +# nobody would go and look at the token. +t="$TMP/both"; mkdir -p "$t" +write_report "$t" core vulnerable +write_empty_report "$t" vertx +log=$(write_log "$t" 2 failed) +run_guard "$log" "$t" +assert "findings AND an unprovable scan report the broken scan, not the findings" 1 "$LAST_EC" +assert_contains " ...and say so explicitly" "not a vulnerability finding" "$LAST_OUT" + +# ── 11. an excluded advisory must not gate ──────────────────────────────────── +# The pom's `excludeVulnerabilityIds` is the documented escape hatch, and it is the ONLY thing +# standing between this gate and a permanent red on a disputed or unfixable finding. The plugin +# honours it when choosing which components to call vulnerable, but still writes the excluded +# advisory into the exported `vulnerabilities` array AND into a top-level `excludedVulnerabilities` +# block. Observed on a live authenticated run: CVE-2026-54518 was excluded in the pom, logged under +# "Excluded vulnerabilities:", and present in `vulnerable` all the same. Gating on the raw array +# would go red on exactly the debt that was deliberately triaged, and no pom edit could clear it. +t="$TMP/excluded"; mkdir -p "$t/core/target" +cat > "$t/core/target/ossindex-report.json" <<'JSON' +{ + "reports": { "org.example:thing:jar:1.0:compile": { "coordinates": "pkg:maven/org.example/thing@1.0" } }, + "excludedVulnerabilities": [ { "id": "CVE-2026-14683", "cvssScore": 2.0 } ], + "vulnerable": { + "org.example:thing:jar:1.0:compile": { + "coordinates": "pkg:maven/org.example/thing@1.0", + "vulnerabilities": [ { "id": "CVE-2026-14683", "cvssScore": 2.0 } ] + } + } +} +JSON +log=$(write_log "$t" 1) +run_guard "$log" "$t" +assert "an advisory excluded in the pom does not fail the check" 0 "$LAST_EC" +assert_contains " ...and the suppression is stated, not silent" "suppressed by" "$LAST_OUT" + +# ── 12. exclusions must not swallow the advisory next to them ───────────────── +# The opposite error: filtering by component rather than by advisory id would drop a live finding +# because a *different* advisory on the same component happened to be excluded. +t="$TMP/partly-excluded"; mkdir -p "$t/core/target" +cat > "$t/core/target/ossindex-report.json" <<'JSON' +{ + "reports": { "org.example:thing:jar:1.0:compile": { "coordinates": "pkg:maven/org.example/thing@1.0" } }, + "excludedVulnerabilities": [ { "id": "CVE-2026-14683", "cvssScore": 2.0 } ], + "vulnerable": { + "org.example:thing:jar:1.0:compile": { + "coordinates": "pkg:maven/org.example/thing@1.0", + "vulnerabilities": [ + { "id": "CVE-2026-14683", "cvssScore": 2.0 }, + { "id": "CVE-2026-99999", "cvssScore": 8.1 } + ] + } + } +} +JSON +log=$(write_log "$t" 1) +run_guard "$log" "$t" +assert "a live advisory beside an excluded one still fails the check" 2 "$LAST_EC" +assert_contains " ...counting only the live one" "1 advisory(ies)" "$LAST_OUT" +assert_contains " ...and naming it" "CVE-2026-99999" "$LAST_OUT" +if grep -qF "CVE-2026-14683" <<< "$LAST_OUT"; then + echo "FAIL: ...and not re-listing the excluded one" + failures=$((failures + 1)) +else + echo "ok: ...and not re-listing the excluded one" +fi + echo if [ "$failures" -eq 0 ]; then echo "All bin/check-ossindex-audit.sh self-tests passed" diff --git a/docs/ci.md b/docs/ci.md index d41d359c7..97ec5630e 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -74,20 +74,33 @@ stack traces (see [`docs/testing.md`](testing.md)). See [`docs/testing.md`](testing.md). - **`cancel-closed-pr-runs.yml`** - cancels a PR's in-flight runs when it closes, so a withdrawn PR stops occupying runners. Housekeeping only; gates nothing. -- **`dependency-audit.yml`** - "Dependency Audit", job `deps: ossindex audit`. The **only** place - `ossindex-maven-plugin` is switched on (`-Dossindex.skip=false`); it binds to `validate`, so - enabling it globally would mean six-plus scans per PR from one account. Runs on every in-repo PR, - on dispatch, and **weekly on a schedule** - the schedule is the point, because `deps: - vulnerabilities` only reviews dependencies a PR *changes*, so nothing else notices an unchanged - tree acquiring a new advisory. (The one deliberate exception to "there is no scheduled build" - below; it re-runs no suite the gate already covers.) **Findings do not fail it** - they go to the - job summary. That was originally because the tree carried a standing backlog; since - astubbs/parallel-consumer#281 triaged it the audit reports zero findings, so this is now a - conservative default rather than a necessity, and the case for gating is getting stronger. It goes red only when `bin/check-ossindex-audit.sh` cannot prove the scan happened, - and that guard is not optional: the plugin has **no setting that makes an unreachable scanner - fatal**, so against an expired token it prints a WARNING and reports `BUILD SUCCESS` - the exact - silent-green defect this repo has already shipped once. `bin/test-check-ossindex-audit.sh` runs - first. Skipped for fork PRs, which receive no secrets and would 401 forever. +- **`dependency-audit.yml`** - "Dependency Audit", job `deps: whole-tree CVE scan`. Named against + `deps: vulnerabilities` (`maven.yml`), which reviews only the dependencies a PR *changes*; this one + scans the whole resolved tree. The **only** place `ossindex-maven-plugin` is switched on + (`-Dossindex.skip=false`); it binds to `validate`, so enabling it globally would mean six-plus + scans per PR from one account. Runs on every in-repo PR, on dispatch, and **weekly on a schedule** + - the schedule catches what no PR can, an unchanged tree acquiring a new advisory. (The one + deliberate exception to "there is no scheduled build" below; it re-runs no suite the gate already + covers.) Skipped for fork PRs, which receive no secrets and would 401 forever. + - **Findings fail it.** astubbs/parallel-consumer#281 retired the standing backlog into + `excludeVulnerabilityIds` entries in the root pom, each carrying a stated retirement condition, + so a finding that reaches the gate is by construction an advisory nobody has looked at. + PR-time rather than the schedule alone because a solo maintainer will not read a scheduled + alert - the PR gate is the only channel with reliable attention. + - **Two different reds, and they stay distinguishable.** Exit 1: the scan could not be proven to + have run, so the *check* is broken and nothing was learned about the tree. Exit 2: the scan ran + and found something, so the *tree* needs triage. Separate headings in the job summary. When both + are true at once, exit 1 wins. + - **False positive, disputed, or no fixed version?** Add the id to `excludeVulnerabilityIds` in + the root pom with a reason and a retirement condition. `bin/check-ossindex-audit.sh` honours + that list - the exported report does *not* pre-filter by it - and states the suppression count + on every run, green ones included, so the list stays reviewable. + - The did-it-actually-scan guard is not optional: the plugin has **no setting that makes an + unreachable scanner fatal**, so against an expired token it prints a WARNING and reports + `BUILD SUCCESS` - the exact silent-green defect this repo has already shipped once. Maven runs + with `-Dossindex.fail=false` deliberately, so a findings-bearing run still reaches the guard + instead of dying in the Maven step and taking the summary with it. + `bin/test-check-ossindex-audit.sh` runs first. - **`release.yml`** - the dispatch-triggered release. See [`docs/releasing.md`](releasing.md). - **`.semaphore/`** - legacy Confluent internal CI/release pipelines, retained but inactive on the fork. diff --git a/docs/inflight/ci-ossindex-lane-reassessment.md b/docs/inflight/ci-ossindex-lane-reassessment.md index f44470e65..0e5363eeb 100644 --- a/docs/inflight/ci-ossindex-lane-reassessment.md +++ b/docs/inflight/ci-ossindex-lane-reassessment.md @@ -1,6 +1,6 @@ # OSS Index lane - does it still earn its keep? -The `deps: ossindex audit` lane (added by astubbs/parallel-consumer#279) was justified on Dependabot +The `deps: whole-tree CVE scan` lane (added by astubbs/parallel-consumer#279) was justified on Dependabot missing **transitive** components. That premise has partly expired, so the lane needs one deliberate re-look rather than an indefinite assumption. From 477b298ead725c39098484190005a392979e772d Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 16:19:19 +1200 Subject: [PATCH 14/29] build(deps): park two unreleased-fix Jackson CVEs, and split the exclusion list in two A live authenticated scan on the merged tree found two new advisories against com.fasterxml.jackson.core:jackson-databind:2.18.9, both published after astubbs/parallel-consumer#281 triaged this component - CVE-2026-68497 (8.7, CWE-770, unbounded allocation in the javax.xml.datatype deserializers) and CVE-2026-19032 (6.3, CWE-470, unrestricted URI schemes in java.nio.file.Path deserialization). **A bump was tried first and does not exist.** Both fixes are already merged upstream - FasterXML/jackson-databind#6127 (2026-07-29) and #6129 (2026-08-06), milestone 2.18.10 - but 2.18.10 is not on Central, and moving lines does not help either: the newest release on every 2.x line predates both merges (2.18.9 on 2026-07-08, 2.21.5 and 2.22.1 on 2026-07-07). No released 2.x artifact anywhere carries either fix, so there is nothing to move to. Moving lines is separately blocked regardless - 2.21.0-2.21.3 would make the already-excluded CVE-2026-54518 genuinely apply, and a global pin breaks VertxTest through wiremock-jre8. **The examples-only precondition was checked, not assumed.** mvn dependency:tree puts jackson-databind at 2.18.9:runtime in parallel-consumer-example-streams ONLY, via kafka-streams:3.9.2. Every other copy in the reactor is test scope and a different version - 2.13.4.2 via WireMock in core/vertx/example-vertx/ example-reactor, 2.17.2 in example-metrics. No published module resolves jackson-databind at runtime at all. Had one, this would have been a bump or a stop rather than an exclusion. Neither advisory is reachable in the example either: StreamsApp is String-serde end to end, builds no ObjectMapper, and never names Path, Duration or XMLGregorianCalendar. The comment says plainly that this is a judgement about blast radius - the examples parent sets maven.deploy.skip / maven.install.skip / skipPublishing, so nothing shipped inherits it - and not a claim that an 8.7 is harmless. An example is still code people copy. **The exclusion list is now explicitly two lists.** The existing five entries are standing: retiring them needs someone else to act, on no timetable. These two are temporary, waiting on a specific merged-but-unreleased fix, and are dated so a reader can see when "temporary" started - a temporary exclusion that reads as permanent never gets revisited. Retiring them is one action: bump the jackson-bom import in example-streams and delete both ids. Neither id is in OSV, NVD or GHSA (all checked, all empty), so Sonatype is the only source carrying them and the fixed-version data comes from the merged PRs rather than any advisory database. That is recorded, because the next reader will otherwise search the databases and conclude the ids are bogus. Verified on the merged tree with a cold report cache: 214 components across 9 modules, 0 vulnerable, 7 suppressed (was 5), and bin/check-ossindex-audit.sh exits 0 - the point of the exercise, since c8d69a2b makes findings exit 2. Self tests, mvn validate with the copyright check on, and bin/check-issue-refs.sh all pass; dependency:tree is byte-identical before and after, so no Jackson coordinate moved and VertxTest is untouched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- docs/inflight/deps-cve-backlog.md | 59 ++++++++++++++++++++++++++++--- pom.xml | 56 ++++++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/docs/inflight/deps-cve-backlog.md b/docs/inflight/deps-cve-backlog.md index f00fad0c9..8db02c22b 100644 --- a/docs/inflight/deps-cve-backlog.md +++ b/docs/inflight/deps-cve-backlog.md @@ -7,12 +7,18 @@ authenticated run possible (`authId`, `ossindex.skip`, `fail=true`) landed on `m astubbs#259; only the triage below was outstanding. Two of the five components were fixable and are fixed. Three are not, and the Jackson bump leaves one -over-broad finding behind - **all four rows below** are carried as documented +over-broad finding behind - **all five rows below** are carried as documented `` entries in the root pom's `ossindex-maven-plugin` config. `fail` is already `true`; these exclusions are what lets the audit actually be *run* - by flipping `ossindex.skip`, or by the scheduled CI job - without going permanently red. The pom comments are the short form; this file is the long form. +**Two classes of exclusion, and do not confuse them.** Most rows below are *standing*: retiring them +needs someone else to act, on no timetable we control. The last row is *temporary* - the upstream fix +is already written and merged, and the entry exists only because no release carries it yet. A +temporary exclusion that is still here in a few months has failed at its one job, so it is dated in +both this file and the pom. Check the dated ones first whenever you touch the list. + Recorded here because **the scan result on its own is misleading**: some of the reported CVEs do not apply to the versions we ship, and the ones that do are mostly blocked by something *other* than what you would guess. Re-deriving that takes an afternoon of advisory reading; this is the result. @@ -48,10 +54,11 @@ not redundant and neither is a superset of the other: | Component | Where from | Verdict | Excluded id(s) | |---|---|---|---| -| `micrometer-core:1.13.15` (compile) | direct, `parallel-consumer-core` | real, **not reachable**, no free fix | CVE-2026-40984 | -| `netty-codec-http:4.1.136.Final` (compile) | `vertx-core:4.5.31` → `parallel-consumer-vertx` | **unverifiable - no CVE record exists** | CVE-2026-59903 | -| `HdrHistogram:2.2.2` (runtime) | `micrometer-core` → `parallel-consumer-core` | **both CVEs officially disputed**, no fixed release | CVE-2026-14683, CVE-2026-14686 | -| `jackson-databind:2.18.9` (runtime) | `kafka-streams:3.9.2` → **`example-streams` only** | **false positive** - out of the advisory's range | CVE-2026-54518 | +| `micrometer-core:1.13.15` (compile) | direct, `parallel-consumer-core` | standing: real, **not reachable**, no free fix | CVE-2026-40984 | +| `netty-codec-http:4.1.136.Final` (compile) | `vertx-core:4.5.31` → `parallel-consumer-vertx` | standing: **unverifiable - no CVE record exists** | CVE-2026-59903 | +| `HdrHistogram:2.2.2` (runtime) | `micrometer-core` → `parallel-consumer-core` | standing: **both CVEs officially disputed**, no fixed release | CVE-2026-14683, CVE-2026-14686 | +| `jackson-databind:2.18.9` (runtime) | `kafka-streams:3.9.2` → **`example-streams` only** | standing: **false positive** - out of the advisory's range | CVE-2026-54518 | +| `jackson-databind:2.18.9` (runtime) | `kafka-streams:3.9.2` → **`example-streams` only** | **TEMPORARY (2026-08-11)** - real, unreachable here, fix merged but unreleased | CVE-2026-68497, CVE-2026-19032 | Only `micrometer-core` is declared in a pom. The others are unmanaged transitives - there is no "held at" row for them and there never will be, which is why they are here and not in @@ -140,6 +147,48 @@ to be. That is why `jackson-bom` is in `.github/dependabot.yml`'s ignore list al `jackson-databind` pin: this line gets bumped by hand, and the exclusion list gets re-checked when it is. Going forward on the 2.21 line means `>= 2.21.5`, and dropping the exclusion. +### `jackson-databind` - two later advisories, fixed upstream but not released (TEMPORARY, 2026-08-11) + +Two more advisories landed against `2.18.9` **after** the triage above was written, so they are new +rather than missed: + +| Id | Score | What it is | Upstream fix | +|---|---|---|---| +| CVE-2026-68497 | **8.7**, CWE-770 | Unbounded allocation deserializing `javax.xml.datatype.Duration` / `XMLGregorianCalendar` - long digit strings behave like the known big-number DoS | [FasterXML/jackson-databind#6127](https://github.com/FasterXML/jackson-databind/pull/6127), merged 2026-07-29, applies `StreamReadConstraints`' number-length limit to `CoreXMLDeserializers` | +| CVE-2026-19032 | 6.3, CWE-470 | `java.nio.file.Path` deserialization accepts arbitrary URI schemes | [FasterXML/jackson-databind#6129](https://github.com/FasterXML/jackson-databind/pull/6129), merged 2026-08-06, restricts the accepted schemes | + +**A bump was the preferred answer and it is not available.** Both PRs are merged and milestoned +**2.18.10**, which does not exist on Central (`.../jackson-databind/2.18.10/` is a 404, and the 2.18 +line stops at 2.18.9 in `maven-metadata.xml`). Nor does moving lines help: the newest release on +*every* 2.x line predates both merges - 2.18.9 (2026-07-08), 2.21.5 (2026-07-07), 2.22.1 +(2026-07-07), against fixes merged 2026-07-29 and 2026-08-06. **There is no released 2.x artifact +anywhere that carries either fix.** Changing lines is separately blocked anyway: 2.21.0-2.21.3 would +make CVE-2026-54518 above genuinely apply, and a global pin breaks `VertxTest` via `wiremock-jre8`. + +Neither id is in OSV, NVD or GHSA (all checked 2026-08-11, all empty), so **Sonatype is the only +source that has them**, and the fixed-version data above comes from the merged PRs rather than from +any advisory database. Do not expect Dependabot to corroborate this one either. + +**Not reachable in this example.** `StreamsApp` is `Serdes.String()` end to end, constructs no +`ObjectMapper`, and never names `Path`, `Duration` or `XMLGregorianCalendar`. `jackson-databind` is on +the classpath only because `kafka-streams` uses it for its own internal metadata. The single +`ObjectMapper` in the repo is in `example-metrics`' *test* code, a different module on a different +version at test scope. + +**Why excluding is defensible here, stated honestly.** The examples parent sets `maven.deploy.skip`, +`maven.install.skip` and `skipPublishing`, so nothing that ships inherits this dependency - the blast +radius is a demo you run yourself. That is a claim about blast radius, **not** a claim that an 8.7 is +harmless, and an example is still code people copy. The precondition was checked rather than assumed: +`mvn dependency:tree` puts `jackson-databind` at `2.18.9:runtime` in `example-streams` **only**, via +`kafka-streams:3.9.2`. Every other copy in the reactor is `test` scope and a different version - +`2.13.4.2` (WireMock) in `core`, `vertx`, `example-vertx`, `example-reactor`, and `2.17.2` in +`example-metrics`. **No published module - core, vertx, reactor, mutiny - resolves `jackson-databind` +at runtime at all.** Had any of them, this would have been a bump or a stop, not an exclusion. + +**RETIRE WHEN:** `2.18.10` reaches Central. Then bump the `jackson-bom` import in +`parallel-consumer-example-streams/pom.xml` and *delete* both ids from the pom - do not carry them +forward into the standing list. + ## Fixed, and how Both are `git log`-able; recorded here only because the *reasoning* is not obvious from the diff. diff --git a/pom.xml b/pom.xml index 87a2b4c1d..72117ae42 100644 --- a/pom.xml +++ b/pom.xml @@ -617,7 +617,16 @@ docs/inflight/deps-cve-backlog.md. Each entry says what would retire it. Re-check them whenever this list is - touched: an exclusion that has quietly become fixable is the failure mode. --> + touched: an exclusion that has quietly become fixable is the failure mode. + + The list is in two parts, and the difference matters. The STANDING entries + below are open-ended - retiring them needs someone else to act (an upstream + release that does not exist, a disputed record being settled, a migration we + have deferred). The TEMPORARY entries at the end are waiting on a specific + upstream fix that is already written and merged, and are expected to be + deleted within weeks; they are dated so a reader can see when "temporary" + started. If a temporary entry is more than a release cycle old, that is a bug + in this list, not a fact about the world. --> CVE-2026-54518 + + + CVE-2026-68497 + CVE-2026-19032 From a8a301a878fc5d74a4af11fc1f1e1a6302f41086 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 16:42:57 +1200 Subject: [PATCH 15/29] build(deps): scope the Jackson ignores to the bump we are waiting for The jackson-bom ignore was blanket. It was added to stop an automated jump into 2.21.x, where CVE-2026-54518 stops being a false positive and becomes real - correct intent, wrong scope. Every danger it names is a LINE change, and a blanket ignore also silences patch releases inside the pinned line. That is not hypothetical here: the root pom's two TEMPORARY exclusions (CVE-2026-68497, CVE-2026-19032) retire when jackson-databind 2.18.10 reaches Central, which is a PATCH bump - so the ignore was suppressing the one PR that would have announced the retirement condition. Neither id is in OSV, NVD or GHSA, so no Dependabot alert can arrive instead. The suppression was covering the notification we were relying on, and nothing reports a PR never opened. Both Jackson entries now block minor/major and allow patches, matching the micrometer entry that already did this. jackson-databind gets the same treatment for the same reason and to keep the two siblings the same shape: its recorded hazard - a global pin dragging WireMock onto an incompatible Jackson and breaking VertxTest - is a line change too, while a patch touches one test-scoped dependency in one example module, which is exactly how a future advisory against it would be fixed. The whole-tree CVE scan resolves test scope, so it would report such an advisory and leave it red with no automated route to green. Swept the rest of the list. net.bytebuddy stays blanket, and is now marked as deliberate rather than looking like an oversight beside the two narrowed entries: it is a freeze until wiremock 3.x, so there is no update size we want. astubbs/* (actions) is the same shape. The seven deferred-major entries are already scoped to major only, and each one's block agrees with the pom exclusion that depends on it - micrometer minor stays blocked, which is what CVE-2026-40984's "retire when the prometheus rename is migrated" says, and the 2.21.x line stays blocked, which is what CVE-2026-54518 needs. Recorded the rule above the list so the next entry starts scoped: blanket is for a freeze, anything you expect to move again names the sizes it fears. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .github/dependabot.yml | 46 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index f442c8827..9bd49b402 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -25,12 +25,31 @@ updates: # comments, whose ignore conditions live invisibly in Dependabot's state and give # future maintainers no clue why a dependency silently stopped updating. # Full rationale for each: docs/inflight/deps-deferred-majors.md. + # + # SCOPE EVERY IGNORE TO THE UPDATE YOU ACTUALLY FEAR. A bare `dependency-name` with + # no `update-types` silences the dependency completely, including the patch release + # you are waiting for - and an ignore is invisible: nothing reports the PR that was + # never opened. Write a blanket ignore only when the dependency is FROZEN, meaning + # you want no movement of any size until a stated condition is met (byte-buddy + # below is the one honest example). If you expect it to move again, say which sizes + # are dangerous and let the rest through. ignore: # Module-local, test-scoped pin coupled to WireMock's Jackson. Bumping it # globally forces WireMock onto an incompatible Jackson and breaks VertxTest. - # Managed by hand in the curated versions-maven-plugin sweep instead. + # Line changes stay hand-managed, via the curated versions-maven-plugin sweep. # See parallel-consumer-example-metrics/pom.xml. + # + # Patches allowed, minor/major blocked - narrowed for the same reason as the + # jackson-bom entry below, and deliberately kept the same shape as it, since the + # two pins are siblings. The recorded hazard is a LINE change; a patch inside the + # pinned line touches one test-scoped dependency in one example module and is + # exactly how a future advisory against it would be fixed. The whole-tree CVE scan + # resolves test scope, so it WILL report such an advisory - and a blanket ignore + # would leave that red with no automated route to green. - dependency-name: "com.fasterxml.jackson.core:jackson-databind" + update-types: + - "version-update:semver-minor" + - "version-update:semver-major" # The second Jackson pin: the jackson-bom import in # parallel-consumer-example-streams, which holds that module at 2.18.9. It needs # the same hand management as the databind pin above, and it is a *different* @@ -41,15 +60,34 @@ updates: # introduced in 2.18.0 and fixed only in 2.18.9, and CVE-2026-54518 applies to # 2.21.0-2.21.3 only. The root pom greenlights CVE-2026-54518 as a false positive # *because this module resolves 2.18.9*; a bump into 2.21.0-2.21.3 would make that - # finding real while the exclusion silently hid it. Bump this by hand, and - # re-check the exclusion list when you do. - # See docs/inflight/deps-cve-backlog.md. + # finding real while the exclusion silently hid it. Leaving the LINE is therefore a + # hand-managed decision, and the exclusion list gets re-checked when it happens. + # + # Allow patches within the pinned line; block minor/major - the same shape as the + # micrometer entry below, but here the patch allowance is load-bearing rather than + # tidy. This started out blanket, which was wrong in a way worth recording: every + # danger named above is a LINE change (2.18 -> 2.21), while a patch inside 2.18 is + # the exact thing we are waiting for. The root pom's TEMPORARY exclusions for + # CVE-2026-68497 and CVE-2026-19032 retire when jackson-databind 2.18.10 reaches + # Central - a patch bump IS the retirement mechanism - and a blanket ignore + # suppressed the only PR that would have announced it. Neither id is in OSV, NVD or + # GHSA, so no Dependabot alert can arrive instead: the ignore was covering the one + # notification we were relying on. bin/check-cve-exclusions.sh is the backstop for + # when that happens anyway. See docs/inflight/deps-cve-backlog.md. - dependency-name: "com.fasterxml.jackson:jackson-bom" + update-types: + - "version-update:semver-minor" + - "version-update:semver-major" # Frozen workaround pin, NOT a dep to track upward: byte-buddy is held at # mockito's version because wiremock-jre8 2.35.2 drags in an ancient byte-buddy # that breaks mockito (MockitoInitializationException). A bump buys nothing and # risks re-breaking it. Remove this pin entirely when wiremock moves to 3.x # (which drops the stale transitive) - do not bump it in the meantime. + # + # Blanket ON PURPOSE, and the only entry here that should be: the desired state is + # frozen, so there is no update size we want. Not an oversight left behind when the + # two Jackson entries were scoped - if a byte-buddy advisory ever lands, the answer + # is the wiremock 3.x move, not a patch. - dependency-name: "net.bytebuddy:*" # micrometer 1.13+ renamed the Prometheus registry package -> breaks CoreApp. # Allow patches within the pinned line; block minor/major until CoreApp migrates. From 3ef9c8307bd15a0f344670fcf703a9f338c12587 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 11 Aug 2026 16:43:20 +1200 Subject: [PATCH 16/29] ci(deps): make temporary CVE exclusions expire instead of asking to be read The root pom carries two TEMPORARY exclusions - advisories whose upstream fix is merged but unreleased - and nothing was ever going to tell us when to retire them. Both channels were shut: neither id exists in OSV, NVD or GHSA, so no Dependabot alert can fire, and the jackson-bom ignore had suppressed the 2.18.10 patch bump that is the retirement trigger (fixed in the previous commit). What was left was a comment addressed to whoever happened to read it. bin/check-cve-exclusions.sh parses excludeVulnerabilityIds out of the pom and fails once a temporary entry outlives its window. Undated, unparseable and future-dated markers FAIL rather than reading as "not expired yet" - a date that cannot be read must never evaluate as fresh, which would reproduce inside the audit's own escape hatch the silent-green defect the audit guard exists to prevent. An id with no rationale comment fails too: unclassified silently means standing-forever, the same bug by omission. RECOGNITION: a machine-read marker in the comment that introduces the entry, `TEMPORARY-SINCE: YYYY-MM-DD`, enforced rather than hoped for. The pom stays the single source of truth because excludeVulnerabilityIds accepts nothing but ids - Maven rejects unknown elements in a typed plugin configuration - so a comment is the only carrier that lives with the thing it describes. A sidecar registry would parse more easily and start drifting on the first edit, which is the same defect one level up. WINDOW: 90 days, one quarter, as a named constant the self-test reads rather than restates. Anchored on the cadence that actually retires these entries - the upstream patch line, not our own release cycle, which is far sparser and unrelated to when a fix ships. jackson-databind 2.18.x ran ten releases from 2024-09-27 to 2026-07-08 with gaps of 31, 31, 92, 68, 174, 117, 61, 35 and 40 days, median ~65, so the typical patch lands well inside the window. Expiry is not a missed deadline and the red is always clearable in one reviewable line: retire the entry if the fix shipped, or re-check upstream and re-date or reclassify it. That matters because a red nobody can clear is the failure mode already named here - the audit stayed reporting-only for exactly as long as it had a standing backlog. OWNERSHIP: Repo Hygiene, not the audit job that otherwise reads this list. That job is skipped for fork PRs and dies early on a token expiry or a Sonatype outage, and its "unprovable scan wins" precedence would swallow this - so the list would rot unwatched for exactly as long as the lane was broken. The check needs no JDK, no secrets and no network, which is this workflow's charter. `deps: CVE exclusion expiry` is a NEW job name and is deliberately not added to the master ruleset here; making it required is a separate act. Exit 3, leaving 1 and 2 to keep the meanings check-ossindex-audit.sh gives them (broken lane, real finding). Stale bookkeeping is neither: the scan ran and the tree is clean, only the paperwork rotted. Verified: the self-test's control arm is two fixtures differing by a single day either side of the limit - the fresh one passes, the expired one fails - and the suite was watched to fail both ways by breaking the guard (disabling the undated branch: 2 red; widening the comparison to 10x: 4 red). Live cold -cache authenticated scan: 9 modules, 214 components, 0 vulnerable, 7 suppressed, guard exit 0. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .github/workflows/claude-code-review.yml | 2 +- .github/workflows/repo-hygiene.yml | 28 ++ bin/AGENTS.md | 16 + bin/check-cve-exclusions.sh | 364 +++++++++++++++++++++++ bin/check-ossindex-audit.sh | 10 +- bin/test-check-cve-exclusions.sh | 289 ++++++++++++++++++ docs/ci.md | 21 +- docs/inflight/deps-cve-backlog.md | 59 ++++ pom.xml | 24 +- 9 files changed, 805 insertions(+), 8 deletions(-) create mode 100755 bin/check-cve-exclusions.sh create mode 100755 bin/test-check-cve-exclusions.sh diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 93aa0416a..622c30bb7 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -136,7 +136,7 @@ jobs: # and it reads attacker-influencable text (diff, PR body, comments), so an enumerable # allowlist is the safety margin against injection-into-execution. Keep it a list. claude_args: | - --allowedTools "Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*),Bash(bin/ci-unit-test.sh:*),Bash(./bin/ci-unit-test.sh:*),Bash(bin/ci-integration-test.sh:*),Bash(./bin/ci-integration-test.sh:*),Bash(bin/check-copyright-headers.sh:*),Bash(./bin/check-copyright-headers.sh:*),Bash(bin/check-shell-sigpipe.sh:*),Bash(./bin/check-shell-sigpipe.sh:*),Bash(bin/test-check-shell-sigpipe.sh:*),Bash(./bin/test-check-shell-sigpipe.sh:*),Bash(bin/test-check-copyright-headers.sh:*),Bash(./bin/test-check-copyright-headers.sh:*),Bash(bin/test-check-review-posted.sh:*),Bash(./bin/test-check-review-posted.sh:*),Bash(bin/check-quarantine-registry.sh:*),Bash(./bin/check-quarantine-registry.sh:*),Bash(bin/check-quarantine-owners.sh:*),Bash(./bin/check-quarantine-owners.sh:*),Bash(bin/todo-index.sh:*),Bash(./bin/todo-index.sh:*),Bash(actionlint:*),Bash(bin/check-ossindex-audit.sh:*),Bash(./bin/check-ossindex-audit.sh:*),Bash(bin/test-check-ossindex-audit.sh:*),Bash(./bin/test-check-ossindex-audit.sh:*),Bash(node .github/scripts/issue-ref-gate.test.js:*),Bash(node .github/scripts/changelog-ref-gate.test.js:*),Bash(./mvnw:*),Bash(mvnw:*),mcp__github_inline_comment__create_inline_comment" + --allowedTools "Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*),Bash(bin/ci-unit-test.sh:*),Bash(./bin/ci-unit-test.sh:*),Bash(bin/ci-integration-test.sh:*),Bash(./bin/ci-integration-test.sh:*),Bash(bin/check-copyright-headers.sh:*),Bash(./bin/check-copyright-headers.sh:*),Bash(bin/check-shell-sigpipe.sh:*),Bash(./bin/check-shell-sigpipe.sh:*),Bash(bin/test-check-shell-sigpipe.sh:*),Bash(./bin/test-check-shell-sigpipe.sh:*),Bash(bin/test-check-copyright-headers.sh:*),Bash(./bin/test-check-copyright-headers.sh:*),Bash(bin/test-check-review-posted.sh:*),Bash(./bin/test-check-review-posted.sh:*),Bash(bin/check-quarantine-registry.sh:*),Bash(./bin/check-quarantine-registry.sh:*),Bash(bin/check-quarantine-owners.sh:*),Bash(./bin/check-quarantine-owners.sh:*),Bash(bin/todo-index.sh:*),Bash(./bin/todo-index.sh:*),Bash(actionlint:*),Bash(bin/check-ossindex-audit.sh:*),Bash(./bin/check-ossindex-audit.sh:*),Bash(bin/test-check-ossindex-audit.sh:*),Bash(./bin/test-check-ossindex-audit.sh:*),Bash(bin/check-cve-exclusions.sh:*),Bash(./bin/check-cve-exclusions.sh:*),Bash(bin/test-check-cve-exclusions.sh:*),Bash(./bin/test-check-cve-exclusions.sh:*),Bash(node .github/scripts/issue-ref-gate.test.js:*),Bash(node .github/scripts/changelog-ref-gate.test.js:*),Bash(./mvnw:*),Bash(mvnw:*),mcp__github_inline_comment__create_inline_comment" --append-system-prompt "You CAN run this repo's tests yourself, and should when the PR adds or changes tests, or when a claim about test results is load-bearing to your review. Use bin/ci-unit-test.sh (fast, no Docker) or bin/ci-integration-test.sh (needs Docker); do NOT run the chaos suite, it is far too slow for a review pass. You can also run the repo's fast verification scripts directly to check a claim rather than trusting the PR body - e.g. bin/check-copyright-headers.sh, bin/check-quarantine-registry.sh - invoke them by their bare path, they are executable. For a SINGLE test class, use ./mvnw directly - e.g. ./mvnw -o -pl parallel-consumer-core -Dtest=SomeTest surefire:test - which takes seconds where a full suite takes minutes. That makes the highest-value check affordable: when a PR claims a test fails without its fix, revert the fix (git stash push the source files), run just that test, and confirm it actually goes RED - a test that passes either way proves nothing, and this is the single most common defect in a regression-test PR. Two rules for scoped runs: never report a targeted -Dtest result as though the suite were green (it skips the -Dexcluded.groups the wrappers pin, and covers one class), and always restore anything you stashed. For whole-suite evidence, use the wrappers, not a hand-rolled ./mvnw line. This workflow triggers BEFORE the CI test jobs finish, so their results are NOT available to you - if you want test evidence, execute it rather than deferring to CI or to the author's summary. When a PR adds a test, verify the suite ACTUALLY COLLECTED it (surefire only picks up *Test.java, Test*.java, *Tests.java, *TestCase.java - a test named otherwise silently never runs) rather than trusting a green total. If you run out of time, or a suite fails for environmental reasons, say so explicitly instead of implying you verified it. REVIEW THE PR TITLE AND DESCRIPTION AS PART OF THE DIFF, not as decoration. They are what a future reader gets instead of the diff, and on a squash merge the title BECOMES the commit title, so an inaccurate one is permanent. Check three things. (1) COMPLETENESS: does the description cover every substantive change in the diff? Descriptions are written early and drift as commits land, so a whole fix or workstream missing from an otherwise good description is the common failure - call out what is in the diff but not described. (2) ACCURACY: do specifics match reality - counts, file and test names, flags, numbers of things granted or changed, and any claimed verification? Re-derive them from the diff rather than trusting the prose; a stale "622 tests" or "two scripts" when the diff says otherwise is a real defect. Treat a checklist item marked N/A as a claim to verify like any other, especially "docs updated - N/A" when the diff changes docs. (3) TERMINOLOGY: does it use words the codebase uses, and does any word mislead? Flag a term that means something specific in this repo being used loosely - for example calling cross-test state contamination a "leak" in a repo where "leak" means a memory leak. Report these as normal findings, non-blocking unless the description actively misrepresents what the code does. BLOCKING FINDINGS MUST BE INLINE REVIEW COMMENTS. This repo sets required_review_thread_resolution, so an unresolved review thread is the ONLY thing that actually gates a merge - a finding written only in your summary comment can be, and will be, merged straight past. So: if you find something that should block the merge (a correctness bug, data loss or corruption, a security hole, a broken public contract, or a test that cannot do what it claims), raise it with mcp__github_inline_comment__create_inline_comment on the offending line, one thread per finding, each stating plainly what breaks and what would fix it. Keep the summary comment for everything else - context, praise, nits and non-blocking observations - and do NOT raise those inline, or routine notes become merge gates and the signal is lost. If you are unsure whether a finding is blocking, put it in the summary and say why you were unsure, rather than blocking on a maybe." # See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md # or https://code.claude.com/docs/en/cli-reference for available options diff --git a/.github/workflows/repo-hygiene.yml b/.github/workflows/repo-hygiene.yml index e9e0d3529..176dff11c 100644 --- a/.github/workflows/repo-hygiene.yml +++ b/.github/workflows/repo-hygiene.yml @@ -61,3 +61,31 @@ jobs: # an action in one grouped PR. - name: One version per action run: bash bin/check-action-versions.sh + + cve-exclusions: + name: "deps: CVE exclusion expiry" + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v6 + + # Self-test first, as everywhere else here. This one has a control arm: two fixtures that + # differ only by a single day either side of the limit, so a pass on one and a fail on the + # other shows the expiry rule is what decides, and not something incidental. + - name: Self-test the exclusion-expiry guard + run: bash bin/test-check-cve-exclusions.sh + + # The root pom's `excludeVulnerabilityIds` is what keeps the whole-tree CVE scan from going + # permanently red on a finding nobody can act on, and some of those entries are TEMPORARY - + # parked only until an upstream release carries a fix that is already merged. Nothing was + # going to tell us when it landed: the two ids this was written for are in no advisory + # database, so no Dependabot alert can fire, and the retirement condition lived in a pom + # comment addressed to whoever happened to read it. After 90 days the entry stops being a note + # and becomes a red check that names itself. + # + # HERE rather than in dependency-audit.yml, which is where the exclusion list is otherwise + # read: that job is skipped for fork PRs and dies early on a token expiry or a Sonatype + # outage, so the list would rot unwatched for exactly as long as the lane was broken. This + # check needs no JDK, no secrets and no network - the charter of this workflow. + - name: Temporary CVE exclusions must expire + run: bash bin/check-cve-exclusions.sh diff --git a/bin/AGENTS.md b/bin/AGENTS.md index 3bd328342..c3f079389 100644 --- a/bin/AGENTS.md +++ b/bin/AGENTS.md @@ -75,6 +75,22 @@ caught it in review, and the same mistake had been seen before. The github-actions ecosystem was re-enabled in `.github/dependabot.yml` at the same time; it had been off since 2022, when the repo genuinely had no workflows. +**Temporary CVE exclusions expire.** `bin/check-cve-exclusions.sh` parses the root pom's +`excludeVulnerabilityIds` and fails once an entry marked `TEMPORARY-SINCE: YYYY-MM-DD` is more than +90 days old - also on an undated, unparseable or future-dated marker, and on an id with no rationale +comment above it. It runs in Repo Hygiene rather than in the audit job that reads the same list, +because that job is skipped for fork PRs and dies early on a token expiry, which is exactly when an +unwatched list rots. It exits **3**, leaving 1 and 2 to keep the meanings +`bin/check-ossindex-audit.sh` gives them. Same class as the rule above - **Dependabot cannot catch +it**: the ids it was written for are in no advisory database, so no alert exists to fire, and a +blanket `ignore` had silenced the patch bump that would have retired them. + +**Scope every `ignore` in `.github/dependabot.yml` to the update you actually fear.** A bare +`dependency-name` with no `update-types` silences the dependency completely, including the patch +release you are waiting for - and the failure is invisible, because nothing reports a PR that was +never opened. Blanket is right only for a genuine freeze (`net.bytebuddy`, held until wiremock 3.x). +Anything you expect to move again gets `update-types`. + **SHA pins are exempt**, and are not drift. The `astubbs/*` forks are pinned to a commit on a *branch* on purpose - a moving branch ref would be the unsafe choice - so each use site tracks a different ref by design. Dependabot ignores them for the same reason. diff --git a/bin/check-cve-exclusions.sh b/bin/check-cve-exclusions.sh new file mode 100755 index 000000000..a5a47a093 --- /dev/null +++ b/bin/check-cve-exclusions.sh @@ -0,0 +1,364 @@ +#!/usr/bin/env bash +# +# Copyright (C) 2026 Antony Stubbs and contributors +# + +# Fails when the root pom's `excludeVulnerabilityIds` bookkeeping has gone stale: a TEMPORARY +# exclusion that has outlived its window, one that is undated or misdated, or an id with no +# rationale comment at all. +# +# WHY THIS EXISTS +# +# `excludeVulnerabilityIds` in the root pom is the escape hatch that keeps the whole-tree CVE scan +# (.github/workflows/dependency-audit.yml) from going permanently red on a disputed, unreachable or +# unfixable finding. Entries come in two kinds and the difference is the whole point: +# +# STANDING retiring it needs someone else to act, on no timetable we control - an upstream +# release that does not exist, a disputed record being settled, a migration deferred. +# TEMPORARY the upstream fix is already written and merged, and the entry exists only because no +# release carries it yet. Expected to be deleted within weeks. +# +# A temporary entry has exactly one job, and it is to go away. Nothing in this repo was ever going +# to tell us when, for two independent reasons, both verified on the Jackson pair that prompted this +# script (CVE-2026-68497, CVE-2026-19032): +# +# 1. Dependabot ALERTS cannot fire. Neither id exists in OSV, NVD or GHSA - Sonatype is the only +# source that has them - so there is no advisory for GitHub to alert on and no "fixed in" +# version for it to chase. +# 2. Dependabot VERSION UPDATES were switched off for the dependency. The `jackson-bom` ignore in +# .github/dependabot.yml was added to stop an automated jump into 2.21.x (where CVE-2026-54518 +# becomes genuinely real) - correct intent, but it was written blanket, so it also suppressed +# the 2.18.10 patch bump that is the retirement trigger. That is now scoped to allow patches, +# which is half the fix; this script is the other half. +# +# So the retirement condition lived only in a pom comment addressed to a human who has to happen to +# read it. This makes the list police itself: after MAX_TEMPORARY_AGE_DAYS the entry stops being a +# note and becomes a red check that names itself. +# +# THIS IS NOT A "YOU FAILED" RED, AND IT IS ALWAYS CLEARABLE +# +# Expiry does not assert the upstream release is late. It asserts the CLASSIFICATION has expired: +# an entry called temporary for a quarter is not temporary any more, whatever the reason. Two +# deliberate, reviewable one-line acts clear it, and both are the right outcome: +# +# * the fix shipped -> bump, and DELETE the entry (what the comment already tells you to do); +# * it did not -> re-argue it. Either re-date `TEMPORARY-SINCE:` having re-checked the +# upstream state, or move it into the standing list with a standing reason. +# +# That matters because a red nobody can clear is the failure mode this repo has already named: the +# audit was reporting-only for exactly as long as it had a standing backlog, because a job that goes +# red on every PR for known debt is ignored inside a week. This one is clearable on every run. +# +# HOW A TEMPORARY ENTRY IS RECOGNISED +# +# By a machine-readable marker in the comment that introduces it: +# +# +# +# The pom is the single source of truth on purpose. `excludeVulnerabilityIds` takes nothing but ids - +# Maven rejects unknown elements in a typed plugin configuration - so a comment is the only carrier +# that lives WITH the thing it describes. A sidecar registry (the shape bin/check-quarantine- +# registry.sh uses) would be easier to parse and would immediately start drifting from the list it +# describes, which is the defect one level up from the one being fixed. +# +# Parsing prose is brittle, so the format is enforced rather than hoped for: any comment inside the +# block containing the bare word TEMPORARY **must** carry `TEMPORARY-SINCE: YYYY-MM-DD`, with a real +# calendar date that is not in the future. Undated, unparseable, and future-dated all FAIL. That is +# deliberate and is the point of the whole exercise - an unreadable date that quietly evaluates as +# "not expired yet" would reproduce, inside the guard, precisely the silent-green defect the guard +# next door exists to prevent. +# +# Every id must also sit under some comment. An id appended with no rationale has no classification, +# and no classification silently reads as "standing, forever" - the same defect wearing a different +# hat. The pom's own header already requires each entry to say what would retire it; this is the +# machine-checkable half of that rule. What it does NOT check is the prose: whether the reason is any +# good, or whether "RETIRE WHEN" is present, is a review question and stays one. +# +# THE WINDOW +# +# MAX_TEMPORARY_AGE_DAYS = 90, one calendar quarter. Anchored on the cadence that actually retires +# these entries - the upstream patch line, not our own release cycle, which is far sparser and has +# nothing to do with when a fix ships. Measured on jackson-databind 2.18.x, the line this list is +# waiting on: ten releases 2024-09-27 -> 2026-07-08, gaps of 31, 31, 92, 68, 174, 117, 61, 35 and 40 +# days - median ~65. So 90 days clears the typical patch comfortably, and the slow ones surface as a +# prompt to go and re-check rather than as a missed deadline. Long enough that re-dating is rare, +# short enough that "temporary" cannot quietly mean half a year. +# +# EXIT CODES - AND WHY 2 IS RESERVED +# +# 0 the list is in good order (green runs still print each temporary entry's days remaining) +# 1 the check could not run - no pom, no exclusion block, a bad `today` argument +# 3 the list is STALE - an expired, undated or unclassified entry +# +# 2 is deliberately unused. bin/check-ossindex-audit.sh, the sibling this runs alongside, already +# owns two reds with fixed meanings: 1 = the scan cannot be proven to have run, 2 = the scan found +# something. Stale bookkeeping is neither - the scan is fine and the tree is fine; what has rotted is +# the paperwork - so it takes a third code rather than borrowing a meaning that is already spoken +# for. Leaving 2 free keeps "2 means a finding" true across both scripts. +# +# WHY THIS IS ITS OWN CHECK, IN REPO HYGIENE +# +# The natural home looks like bin/check-ossindex-audit.sh - it is the script that already renders +# the exclusion list. It is the wrong home, for three reasons: +# +# * That guard runs in one job that is SKIPPED for fork PRs (no secrets -> guaranteed 401) and can +# fail before it gets there on a token expiry or a Sonatype outage. Its exit-1 precedence rule - +# "a scan that cannot be proven to have run wins" - is correct for scan results and would +# swallow this: the list rots for as long as the lane is broken, which is the worst possible +# time to stop reading it. +# * It needs a Maven log, a JDK, a full test-compile and a live authenticated scan. This needs a +# text file. Checks that need nothing are cheap enough to run everywhere, and should. +# * It is the same KIND of check as this file's neighbours in repo-hygiene.yml: the failure is +# invisible rather than loud, so it has to be gated or it will be skimmed past. +# +# Usage: bin/check-cve-exclusions.sh [pom-file] [today] +# +# pom-file defaults to the repo root pom. +# today YYYY-MM-DD, defaults to the real date. For the self-test, and for asking "what expires +# before the next release?" without waiting for the calendar. +# +# Markdown report goes to stdout, so CI can append it to $GITHUB_STEP_SUMMARY; the verdict goes to +# stderr. Run: bin/check-cve-exclusions.sh (CI runs it in the Repo Hygiene workflow, after its +# self-test bin/test-check-cve-exclusions.sh). + +set -euo pipefail + +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +POM="${1:-$ROOT/pom.xml}" +TODAY="${2:-}" + +# One calendar quarter. Reasoning in the header - it is anchored on jackson-databind's observed +# patch cadence, not picked round. Changing it is a policy change, so change it here and nowhere +# else: the self-test derives its fixtures from this value rather than hard-coding 90. +MAX_TEMPORARY_AGE_DAYS=90 + +facts=$(python3 - "$POM" "$TODAY" "$MAX_TEMPORARY_AGE_DAYS" <<'PY' +import datetime +import re +import sys + +pom_path, today_arg, max_age = sys.argv[1], sys.argv[2], int(sys.argv[3]) + +fatals = [] # the check could not run -> exit 1 +problems = [] # the list is stale -> exit 3 +rows = [] # (label, ids, detail) for the markdown table + +DATE_FORMAT = "%Y-%m-%d" + + +def parse_date(text): + """Strict YYYY-MM-DD. strptime, not date.fromisoformat, which on 3.11+ also accepts week dates + and other ISO spellings - a date nobody meant to write must not parse.""" + return datetime.datetime.strptime(text, DATE_FORMAT).date() + + +today = None +if today_arg: + try: + today = parse_date(today_arg) + except ValueError: + fatals.append("the `today` argument %r is not a YYYY-MM-DD date" % today_arg) +else: + today = datetime.date.today() + +text = "" +try: + with open(pom_path, encoding="utf-8") as fh: + text = fh.read() +except OSError as e: + fatals.append("cannot read %s: %s" % (pom_path, e)) + +# EVERY block, not the first. Taking `search` here would silently ignore a second +# `excludeVulnerabilityIds` - suppressions hiding behind suppressions, which is the exact shape of +# bug this script exists to make impossible. +blocks = list(re.finditer( + r"(?P.*?)", text, re.S)) +if text and not blocks: + fatals.append( + "no block found in %s - the exclusion list has moved or been " + "renamed, and this check cannot see it. It must not pass vacuously." % pom_path) + +# A comment introduces a section; every id after it belongs to that section until the next comment. +TOKEN = re.compile( + r"" + r"|\s*(?P[^<]*?)\s*", re.S) +# The bare word, so `TEMPORARY-SINCE:` matches too (the hyphen is a word boundary) and a lowercase +# "temporary" in ordinary prose does not. +TEMPORARY_WORD = re.compile(r"\bTEMPORARY\b") +TEMPORARY_SINCE = re.compile(r"TEMPORARY-SINCE:\s*(\S+)") + +sections = [] +total_ids = 0 + +for block in blocks: + current = None + for token in TOKEN.finditer(block.group("body")): + line = text.count("\n", 0, block.start("body") + token.start()) + 1 + comment = token.group("comment") + if comment is not None: + current = {"line": line, "ids": [], "temporary": False, + "since": None, "malformed": None} + if TEMPORARY_WORD.search(comment): + current["temporary"] = True + stamp = TEMPORARY_SINCE.search(comment) + if not stamp: + current["malformed"] = ( + "it says TEMPORARY but carries no `TEMPORARY-SINCE: YYYY-MM-DD`, so there " + "is no date to expire it against") + else: + try: + current["since"] = parse_date(stamp.group(1)) + except ValueError: + current["malformed"] = ( + "`TEMPORARY-SINCE: %s` is not a real YYYY-MM-DD date" + % stamp.group(1)) + sections.append(current) + continue + + vuln_id = token.group("id") + total_ids += 1 + if current is None: + problems.append( + "`%s` (pom.xml line %d) has no rationale comment above it. An unclassified id " + "reads as standing-forever, which is how a temporary exclusion becomes permanent " + "by accident." % (vuln_id, line)) + continue + current["ids"].append(vuln_id) + +temporary = [s for s in sections if s["temporary"]] + +for section in temporary: + ids = ", ".join("`%s`" % i for i in section["ids"]) or "_(no ids under this banner)_" + if section["malformed"]: + problems.append("the temporary banner at pom.xml line %d is malformed: %s. Fix the banner - " + "an undated temporary entry can never expire, which is the whole defect." + % (section["line"], section["malformed"])) + rows.append(("malformed", ids, section["malformed"])) + continue + + age = (today - section["since"]).days + if age < 0: + problems.append( + "the temporary banner at pom.xml line %d is dated %s, which is in the future. A " + "future start date never expires; it is a typo, not a grace period." + % (section["line"], section["since"].isoformat())) + rows.append(("misdated", ids, "dated %s - in the future" % section["since"].isoformat())) + elif age > max_age: + problems.append( + "%s: temporary since %s, %d days old - past the %d-day limit (pom.xml line %d)" + % (", ".join(section["ids"]) or "(no ids)", section["since"].isoformat(), + age, max_age, section["line"])) + rows.append(("EXPIRED", ids, + "temporary since %s - **%d days**, limit %d" + % (section["since"].isoformat(), age, max_age))) + else: + rows.append(("ok", ids, + "temporary since %s - %d days old, %d remaining" + % (section["since"].isoformat(), age, max_age - age))) + +for f in fatals: + print("fatal=%s" % f) +for p in problems: + print("problem=%s" % p) +print("total_ids=%d" % total_ids) +print("standing_sections=%d" % len([s for s in sections if not s["temporary"]])) +print("temporary_sections=%d" % len(temporary)) +print("---") + +if rows: + print("| State | Advisory id(s) | Age |") + print("|---|---|---|") + for label, ids, detail in rows: + print("| %s | %s | %s |" % (label, ids, detail)) +PY +) + +fatals=() +problems=() +total_ids='' standing_sections='' temporary_sections='' +while IFS= read -r line; do + case "$line" in + ---) break ;; + fatal=*) fatals+=("${line#fatal=}") ;; + problem=*) problems+=("${line#problem=}") ;; + total_ids=*) total_ids="${line#total_ids=}" ;; + standing_sections=*) standing_sections="${line#standing_sections=}" ;; + temporary_sections=*) temporary_sections="${line#temporary_sections=}" ;; + esac +done <<< "$facts" + +# Same belt-and-braces the audit guard uses: a fact renamed in the parser but not here would +# otherwise become an empty variable and quietly weaken every test below it. +for fact in total_ids standing_sections temporary_sections; do + if [ -z "${!fact}" ]; then + echo "FAIL: internal error - the exclusion parser emitted no '${fact}' fact." >&2 + exit 1 + fi +done + +table=$(sed -n '/^---$/,$p' <<< "$facts" | tail -n +2) + +if [ "${#fatals[@]}" -gt 0 ]; then + echo "## :x: CVE exclusion list could not be read" + echo + echo "**This is not a stale-exclusion finding.** The check could not run, so it learned nothing" + echo "about the list - do not read a passing neighbour as coverage." + echo + for f in "${fatals[@]}"; do + echo "- ${f}" + done + echo "FAIL: cannot read the CVE exclusion list - ${#fatals[@]} problem(s), see the report above." >&2 + exit 1 +fi + +if [ "${#problems[@]}" -gt 0 ]; then + echo "## :x: A temporary CVE exclusion has gone stale" + echo + echo "**Neither red the CVE scan produces.** The scan is fine and the tree is fine; what has" + echo "rotted is the bookkeeping in \`excludeVulnerabilityIds\` (root \`pom.xml\`)." + echo + for p in "${problems[@]}"; do + echo "- ${p}" + done + echo + printf '%s\n' "$table" + echo + echo "### What to do" + echo + echo "A temporary exclusion says an upstream fix is merged but unreleased. After" + echo "${MAX_TEMPORARY_AGE_DAYS} days that claim needs re-checking, not renewing by default." + echo + echo "1. **Has the fix shipped?** For the Jackson pair, that means asking Central whether" + echo " \`2.18.10\` exists yet:" + echo + cat <<'REPRO' +``` +curl -sI https://repo1.maven.org/maven2/com/fasterxml/jackson/core/jackson-databind/2.18.10/ | head -1 +``` +REPRO + echo + echo " If it is there: bump the \`jackson-bom\` import in" + echo " \`parallel-consumer-example-streams/pom.xml\` and **delete** the ids - do not carry them" + echo " into the standing list." + echo "2. **Still unreleased?** Re-check upstream, then either re-date \`TEMPORARY-SINCE:\` with" + echo " what you found, or move the entry into the standing list with a standing reason. Both" + echo " are one deliberate line, and both are reviewable - which is the point." + echo + echo "Background: \`docs/inflight/deps-cve-backlog.md\`." + echo "FAIL: ${#problems[@]} stale or malformed entry(ies) in the root pom's excludeVulnerabilityIds." >&2 + exit 3 +fi + +echo "## :white_check_mark: CVE exclusion list is in good order" +echo +echo "**${total_ids}** advisory id(s) excluded in the root \`pom.xml\`: ${standing_sections} standing" +echo "group(s), ${temporary_sections} temporary. Every id carries a rationale comment, and every" +echo "temporary one is dated and inside its ${MAX_TEMPORARY_AGE_DAYS}-day window." +if [ "$temporary_sections" -gt 0 ]; then + echo + printf '%s\n' "$table" + echo + echo "_A temporary entry that runs out of days fails this check (exit 3). That is a prompt to" + echo "re-check upstream and either retire the entry or re-argue it - not a deadline you missed._" +fi +echo "ok: ${total_ids} exclusion(s) - ${temporary_sections} temporary, all dated and within ${MAX_TEMPORARY_AGE_DAYS} days" >&2 diff --git a/bin/check-ossindex-audit.sh b/bin/check-ossindex-audit.sh index 5186ad475..a724f278d 100755 --- a/bin/check-ossindex-audit.sh +++ b/bin/check-ossindex-audit.sh @@ -301,12 +301,18 @@ fi echo echo "Scanned **${components}** resolved components across **${report_files}** module(s)." # Never silent: an exclusion list is the one way a finding can legitimately disappear, so it is -# stated on every run - green ones included - or it stops being reviewable. +# stated on every run - green ones included - or it stops being reviewable. The retirement +# conditions on that list are no longer only prose: bin/check-cve-exclusions.sh dates the temporary +# entries and fails once one outlives its window. That check lives in repo-hygiene.yml rather than +# here on purpose - this job is skipped for fork PRs and dies early on a broken lane, which is +# exactly when an unwatched list would rot. See its header. if [ "$excluded" -gt 0 ]; then echo echo "**${excluded}** advisory(ies) were suppressed by \`excludeVulnerabilityIds\` in the root pom." echo "Each carries a reason and a retirement condition there; they are excluded from the verdict" - echo "above, not overlooked." + echo "above, not overlooked. Temporary entries are dated and expire -" + echo "\`bin/check-cve-exclusions.sh\` (Repo Hygiene) fails once one outlives its window, so those" + echo "conditions are enforced rather than merely written down." fi if [ "$vulnerable" -eq 0 ]; then echo diff --git a/bin/test-check-cve-exclusions.sh b/bin/test-check-cve-exclusions.sh new file mode 100755 index 000000000..2843331bf --- /dev/null +++ b/bin/test-check-cve-exclusions.sh @@ -0,0 +1,289 @@ +#!/usr/bin/env bash +# +# Copyright (C) 2026 Antony Stubbs and contributors +# + +# Self-test for bin/check-cve-exclusions.sh - the "a temporary CVE exclusion must expire" guard. +# +# The guard exists because nothing else was ever going to tell us when to retire a temporary +# exclusion: the two ids it was written for are in no advisory database, so no Dependabot alert can +# fire, and the retirement condition lived only in a pom comment. A guard for that which has never +# been seen to fire is decoration - so every branch below has a negative control, and the expiry rule +# has a matched pair (cases 3 and 4) that differ by ONE DAY and nothing else. +# +# THREE EXIT CODES, AND THE SUITE IS WHAT HOLDS THEM APART: +# +# 0 the list is in good order +# 1 the check could not run (no pom, no exclusion block, bad `today`) +# 3 the list is STALE (expired, undated, misdated, or unclassified entry) +# +# 2 is deliberately never produced: bin/check-ossindex-audit.sh owns it for "the scan found +# something", and these two scripts are read side by side. Asserting exact codes, not merely +# non-zero, is what stops a later refactor from collapsing three distinct verdicts into one +# unreadable red - the reader could no longer tell a rotten list from a broken scanner. +# +# THE FIXTURES DERIVE THEIR DATES FROM THE GUARD'S OWN CONSTANT, read out of the script rather than +# written as 90 here. A policy change to the window is then a one-line edit that this suite follows, +# instead of a one-line edit plus a mysteriously red self-test. +# +# Cases: +# 1. the REAL root pom, judged today -> pass (0) <- integration arm +# 2. temporary, dated today -> pass (0) +# 3. temporary, exactly at the limit -> pass (0) <- control arm A +# 4. temporary, ONE DAY past the limit -> FAIL (3) <- control arm B +# 5. temporary with no TEMPORARY-SINCE at all -> FAIL (3) +# 6. TEMPORARY-SINCE with an impossible date -> FAIL (3) +# 7. TEMPORARY-SINCE in the future -> FAIL (3) +# 8. lowercase "temporary" in ordinary prose -> pass (0) <- no false positive +# 9. an id with no rationale comment above it -> FAIL (3) +# 10. standing entries only -> pass (0) +# 11. no block at all -> FAIL (1) +# 12. the pom file does not exist -> FAIL (1) +# 13. a `today` argument that is not a date -> FAIL (1) +# 14. an expired entry in a SECOND block -> FAIL (3) +# 15. one fresh and one expired banner together -> FAIL (3), naming only the expired +# +# Cases 3 and 4 are the pair that matters. They are byte-identical but for the date, so a pass on 3 +# and a fail on 4 shows the expiry rule - and not something incidental - is what decides. Case 5 is +# the defect class this repo spent a day naming: an undated temporary entry must FAIL, because a date +# that cannot be read must never evaluate as "not expired yet". Cases 6 and 7 are the same defect in +# its other two disguises. Case 8 is the false-positive control: without it, tightening the marker +# regex would be untestable and the guard would eventually be loosened to shut it up. +# +# Case 9 covers the quieter half of the rule. An id appended with no comment has no classification, +# and no classification silently means "standing, forever" - a temporary exclusion made permanent by +# omission rather than by decision. +# +# Case 11 is fail-closed placement: if the block is renamed or moved, the check must go red rather +# than report a clean list it never found. That is the same reasoning as the audit guard's structural +# leg - "the file exists" proves nothing, "the file has content" does. +# +# Run: bin/test-check-cve-exclusions.sh (CI runs it BEFORE the guard it protects) + +set -uo pipefail + +BIN="$(cd "$(dirname "$0")" && pwd)" +GUARD="$BIN/check-cve-exclusions.sh" +REAL_POM="$(cd "$BIN/.." && pwd)/pom.xml" +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + +failures=0 + +# Read the window out of the guard instead of restating it. A herestring, not a pipe into grep: +# `writer | grep` under pipefail turns a match into a failure (bin/AGENTS.md -> "Scripts that guard +# other scripts"), and this file is scanned by bin/check-shell-sigpipe.sh like any other. +guard_source="$(cat "$GUARD")" +MAX_DAYS="$(sed -n 's/^MAX_TEMPORARY_AGE_DAYS=\([0-9]*\).*/\1/p' <<< "$guard_source")" +if [ -z "$MAX_DAYS" ]; then + echo "FAIL: could not read MAX_TEMPORARY_AGE_DAYS out of $GUARD - the fixtures cannot be dated." + exit 1 +fi +echo "info: expiry window read from the guard: ${MAX_DAYS} days" + +TODAY="$(date -u +%Y-%m-%d)" + +# A date $1 days before TODAY. Negative goes forwards. python3 rather than `date -d`, which BSD date +# (macOS) does not support - this suite runs locally as well as on ubuntu-latest. +days_ago() { # + python3 -c 'import datetime,sys; print(datetime.date.fromisoformat(sys.argv[1]) - datetime.timedelta(days=int(sys.argv[2])))' "$TODAY" "$1" +} + +assert() { # + if [ "$2" = "$3" ]; then + echo "ok: $1" + else + echo "FAIL: $1 (expected exit '$2', got '$3')" + failures=$((failures + 1)) + fi +} + +assert_contains() { # + if grep -qF "$2" <<< "$3"; then + echo "ok: $1" + else + echo "FAIL: $1 (output did not contain '$2')" + failures=$((failures + 1)) + fi +} + +assert_absent() { # + if grep -qF "$2" <<< "$3"; then + echo "FAIL: $1 (output unexpectedly contained '$2')" + failures=$((failures + 1)) + else + echo "ok: $1" + fi +} + +# Writes a fixture pom whose exclusion block is exactly $2 (raw XML), and echoes its path. +write_pom() { # + local path="$TMP/$1.xml" + cat > "$path" < + + ossindex-maven-plugin + + +$2 + + + + +XML + echo "$path" +} + +# Sets LAST_EC and LAST_OUT. Not `ec=$(run_guard ...)`: a command substitution runs in a subshell, +# so anything it assigns is lost - the trap the ossindex self-test documents next door. +run_guard() { # [today] + LAST_EC=0 + LAST_OUT="$("$GUARD" "$1" "${2:-}" 2>/dev/null)" || LAST_EC=$? +} + +# ── 1. the real list, judged today - the integration arm ────────────────────── +# Every other case is a fixture. This one asserts the guard and the pom it actually polices agree +# right now, so a pom edit that breaks the format is caught by the self-test too, not only by CI. +run_guard "$REAL_POM" "$TODAY" +assert "the repo's own exclusion list passes as written" 0 "$LAST_EC" +assert_contains " ...and reports the temporary entry's remaining days" "remaining" "$LAST_OUT" + +# ── 2. a temporary entry dated today ────────────────────────────────────────── +fresh_block=" + CVE-2026-00001" +pom=$(write_pom fresh "$fresh_block") +run_guard "$pom" "$TODAY" +assert "a temporary entry dated today passes" 0 "$LAST_EC" + +# ── 3 & 4. THE CONTROL ARM: identical fixtures, one day apart ───────────────── +# The only difference between these two poms is the date in the banner. If 3 passed and 4 also +# passed, the guard would be inert; if both failed, something other than expiry would be deciding. +at_limit=$(days_ago "$MAX_DAYS") +one_over=$(days_ago $((MAX_DAYS + 1))) + +pom=$(write_pom at-limit " + CVE-2026-00002") +run_guard "$pom" "$TODAY" +assert "a temporary entry exactly at the ${MAX_DAYS}-day limit still passes" 0 "$LAST_EC" + +pom=$(write_pom one-over " + CVE-2026-00002") +run_guard "$pom" "$TODAY" +assert "the SAME entry one day past the limit fails" 3 "$LAST_EC" +assert_contains " ...naming the stale id" "CVE-2026-00002" "$LAST_OUT" +assert_contains " ...and its age" "$((MAX_DAYS + 1)) days" "$LAST_OUT" +assert_contains " ...and what to go and check" "2.18.10" "$LAST_OUT" +assert_absent " ...and NOT claiming the scan is broken" "could not be read" "$LAST_OUT" + +# ── 5. TEMPORARY with no date - must fail, not be skipped ───────────────────── +# The defect class: an entry whose date cannot be read must never evaluate as "not expired yet". +pom=$(write_pom undated " + CVE-2026-00003") +run_guard "$pom" "$TODAY" +assert "an undated TEMPORARY entry fails rather than passing silently" 3 "$LAST_EC" +assert_contains " ...saying the banner is malformed" "no \`TEMPORARY-SINCE" "$LAST_OUT" + +# ── 6. a date that matches the shape but is not a date ──────────────────────── +pom=$(write_pom impossible " + CVE-2026-00004") +run_guard "$pom" "$TODAY" +assert "an impossible TEMPORARY-SINCE date fails" 3 "$LAST_EC" +assert_contains " ...naming the unparseable value" "2026-13-45" "$LAST_OUT" + +# ── 7. a future date never expires, so it must be rejected outright ─────────── +tomorrow=$(days_ago -1) +pom=$(write_pom future " + CVE-2026-00005") +run_guard "$pom" "$TODAY" +assert "a future-dated TEMPORARY entry fails" 3 "$LAST_EC" +assert_contains " ...calling it a typo, not a grace period" "in the future" "$LAST_OUT" + +# ── 8. false-positive control: prose, not a marker ──────────────────────────── +# Only the bare uppercase word means anything. Without this case, tightening the regex would be +# untestable, and the first false alarm would be "fixed" by loosening the guard. +pom=$(write_pom prose " + CVE-2026-00006") +run_guard "$pom" "$TODAY" +assert "lowercase \"temporary\" in prose is not treated as a marker" 0 "$LAST_EC" + +# ── 9. an id with no rationale comment at all ───────────────────────────────── +pom=$(write_pom orphan " CVE-2026-00007") +run_guard "$pom" "$TODAY" +assert "an id with no rationale comment fails" 3 "$LAST_EC" +assert_contains " ...explaining that unclassified reads as standing-forever" "standing-forever" "$LAST_OUT" + +# ── 10. a list with nothing temporary in it ─────────────────────────────────── +pom=$(write_pom standing " + CVE-2026-00008") +run_guard "$pom" "$TODAY" +assert "a purely standing list passes" 0 "$LAST_EC" +assert_contains " ...and counts it as standing" "1 standing" "$LAST_OUT" + +# ── 11. the block is gone - fail closed, do not report a clean list ─────────── +cat > "$TMP/noblock.xml" <<'XML' + + ossindex-maven-plugin + +XML +run_guard "$TMP/noblock.xml" "$TODAY" +assert "a pom with no exclusion block fails rather than passing vacuously" 1 "$LAST_EC" +assert_contains " ...and says the check could not run" "could not be read" "$LAST_OUT" +assert_absent " ...not that the list is stale" "gone stale" "$LAST_OUT" + +# ── 12. no pom at all ───────────────────────────────────────────────────────── +run_guard "$TMP/does-not-exist.xml" "$TODAY" +assert "a missing pom fails" 1 "$LAST_EC" + +# ── 13. a `today` argument that is not a date ───────────────────────────────── +# The override exists for this suite and for "what expires before the next release?". If it silently +# fell back to the real date, a typo would produce a confident answer to a question nobody asked. +run_guard "$REAL_POM" "not-a-date" +assert "a malformed \`today\` argument fails instead of falling back" 1 "$LAST_EC" + +# ── 14. a second block must not hide behind the first ───────────────────────── +# Suppressions hiding behind suppressions. A `search` for the first block would report the clean one +# and never see this. +cat > "$TMP/twoblocks.xml" < + + + + CVE-2026-00009 + + + + + + CVE-2026-00010 + + + +XML +run_guard "$TMP/twoblocks.xml" "$TODAY" +assert "an expired entry in a second exclusion block is still caught" 3 "$LAST_EC" +assert_contains " ...naming it" "CVE-2026-00010" "$LAST_OUT" + +# ── 15. a fresh banner must not launder the expired one beside it ───────────── +pom=$(write_pom mixed " + CVE-2026-00011 + + + CVE-2026-00012") +run_guard "$pom" "$TODAY" +assert "one fresh and one expired banner fails on the expired one" 3 "$LAST_EC" +assert_contains " ...naming the expired id" "CVE-2026-00012" "$LAST_OUT" +if grep -qF "CVE-2026-00011: temporary since" <<< "$LAST_OUT"; then + echo "FAIL: ...and not accusing the fresh one" + failures=$((failures + 1)) +else + echo "ok: ...and not accusing the fresh one" +fi + +echo +if [ "$failures" -eq 0 ]; then + echo "All bin/check-cve-exclusions.sh self-tests passed" + exit 0 +fi +echo "$failures self-test(s) FAILED" +exit 1 diff --git a/docs/ci.md b/docs/ci.md index 97ec5630e..c06b7e6fb 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -60,6 +60,23 @@ stack traces (see [`docs/testing.md`](testing.md)). required status checks** (`shell: sigpipe`, `workflows: action versions`) - which is exactly why the job names are an API. They exist because the failures they catch are invisible rather than loud, and they gate precisely so those failures cannot be skimmed past. + - `cve-exclusions` runs `bin/check-cve-exclusions.sh`, which **expires temporary CVE + exclusions**. Entries in the root pom's `excludeVulnerabilityIds` come in two kinds: *standing* + (retiring them needs someone else to act, on no timetable we control) and *temporary* (the + upstream fix is merged, and the entry exists only because no release carries it yet). A + temporary entry is marked `TEMPORARY-SINCE: YYYY-MM-DD` in the comment above it, and the check + fails once it is more than **90 days** old - one quarter, anchored on jackson-databind 2.18.x's + observed patch cadence (median gap ~65 days), since the upstream patch line, not our own + release cycle, is what actually retires these entries. Undated, unparseable and future-dated + markers **fail** rather than reading as "not expired yet", and so does an id with no rationale + comment, because an unclassified entry silently means standing-forever. **Exit 3**, deliberately + not 1 or 2: those belong to `bin/check-ossindex-audit.sh` (broken lane / real finding) and this + is neither - the scan and the tree are fine, the bookkeeping has rotted. The red is always + clearable in one reviewable line: retire the entry if the fix shipped, or re-date / reclassify + it having re-checked upstream. It lives here rather than in the audit job because that job is + skipped for fork PRs and dies early on a token expiry, so the list would go unwatched exactly + when it matters most. **`deps: CVE exclusion expiry` is a new job name and is NOT yet a required + status check** - adding it to the master ruleset is a separate, deliberate act. - **`claude-code-review.yml`** - automated PR review. The job ends with a gate, `bin/check-review-posted.sh` (self-tested by `bin/test-check-review-posted.sh`, which runs first), asserting that a review from *this* run actually landed on the PR. Without it the check @@ -94,7 +111,9 @@ stack traces (see [`docs/testing.md`](testing.md)). - **False positive, disputed, or no fixed version?** Add the id to `excludeVulnerabilityIds` in the root pom with a reason and a retirement condition. `bin/check-ossindex-audit.sh` honours that list - the exported report does *not* pre-filter by it - and states the suppression count - on every run, green ones included, so the list stays reviewable. + on every run, green ones included, so the list stays reviewable. If the fix is already merged + upstream and you are only waiting for a release, mark it `TEMPORARY-SINCE: YYYY-MM-DD` so it + expires (see `repo-hygiene.yml` above) instead of quietly joining the standing list. - The did-it-actually-scan guard is not optional: the plugin has **no setting that makes an unreachable scanner fatal**, so against an expired token it prints a WARNING and reports `BUILD SUCCESS` - the exact silent-green defect this repo has already shipped once. Maven runs diff --git a/docs/inflight/deps-cve-backlog.md b/docs/inflight/deps-cve-backlog.md index 8db02c22b..1e5cd0eb3 100644 --- a/docs/inflight/deps-cve-backlog.md +++ b/docs/inflight/deps-cve-backlog.md @@ -19,6 +19,58 @@ is already written and merged, and the entry exists only because no release carr temporary exclusion that is still here in a few months has failed at its one job, so it is dated in both this file and the pom. Check the dated ones first whenever you touch the list. +## The list polices itself now - do not trust the comments alone + +The dating used to be a note to a human, and nothing was ever going to make anyone read it. Both +notification channels were closed, independently: + +- **No Dependabot alert can fire.** Neither temporary id is in OSV, NVD or GHSA (see below), so there + is no advisory for GitHub to alert on and no "fixed in" version for it to chase. +- **No Dependabot version-update PR could arrive either.** The `jackson-bom` ignore in + `.github/dependabot.yml` was written blanket, to stop an automated jump into 2.21.x where + CVE-2026-54518 becomes real. Correct intent, wrong scope: it also suppressed the **2.18.10 patch + bump that is the retirement trigger**. The suppression was covering the notification the exclusion + depended on. That entry now blocks minor/major and allows patches, so the bump will be proposed + when it lands; `jackson-databind` was narrowed the same way, and `net.bytebuddy` was left blanket + on purpose - it is a deliberate freeze, not a deferral. + +So the retirement condition is enforced rather than merely written down. `bin/check-cve-exclusions.sh` +runs in Repo Hygiene on every PR - no JDK, no secrets, no network - and parses this list out of the +root pom: + +- any comment in the block containing the bare word `TEMPORARY` **must** carry + `TEMPORARY-SINCE: YYYY-MM-DD`. Undated, unparseable and future-dated all **fail**. A date that + cannot be read must never evaluate as "not expired yet" - that would reproduce, inside the audit's + own escape hatch, the silent-green defect the audit guard exists to prevent; +- every id must sit under a rationale comment, because an unclassified id reads as standing-forever: + temporary made permanent by omission rather than by decision; +- a temporary entry more than **90 days** old fails, with **exit 3**. Not 1 (the audit lane is + broken) and not 2 (the scan found something) - here the scan and the tree are both fine, and only + the paperwork has rotted. + +**90 days is one quarter, anchored on the cadence that actually retires these entries** - the +upstream patch line, not our own release cycle, which is far sparser and has nothing to do with when +a fix ships. jackson-databind 2.18.x ran ten releases from 2024-09-27 to 2026-07-08, with gaps of 31, +31, 92, 68, 174, 117, 61, 35 and 40 days: median ~65. The typical patch lands well inside the window, +and the slow ones surface as a prompt to go and look. + +**Expiring is not a missed deadline, and the red is always clearable.** It says the *classification* +has expired: an entry called temporary for a quarter is not temporary any more, whatever the reason. +Either the fix shipped - bump, and delete the entry - or it did not, in which case re-check upstream +and either re-date `TEMPORARY-SINCE:` with what you found, or move the row into the standing list +with a standing reason. Both are one deliberate, reviewable line. That matters, because a red nobody +can clear is the failure mode this repo has already named: the audit stayed reporting-only for +exactly as long as it had a standing backlog, since a job that goes red on every PR for known debt is +ignored inside a week. + +The marker lives in the pom rather than in a sidecar registry deliberately. `excludeVulnerabilityIds` +accepts nothing but ids - Maven rejects unknown elements in a typed plugin configuration - so a +comment is the only carrier that sits *with* the thing it describes. A registry file would be easier +to parse and would start drifting from the list it describes on the first edit, which is the same +defect one level up. Parsing prose is brittle, so the format is enforced rather than hoped for, and +`bin/test-check-cve-exclusions.sh` pins it - including a control arm of two fixtures that differ only +by a single day either side of the limit. + Recorded here because **the scan result on its own is misleading**: some of the reported CVEs do not apply to the versions we ship, and the ones that do are mostly blocked by something *other* than what you would guess. Re-deriving that takes an afternoon of advisory reading; this is the result. @@ -189,6 +241,13 @@ at runtime at all.** Had any of them, this would have been a bump or a stop, not `parallel-consumer-example-streams/pom.xml` and *delete* both ids from the pom - do not carry them forward into the standing list. +Two things now chase this rather than one comment. Dependabot will propose the patch bump once +2.18.10 exists, the `jackson-bom` ignore having been scoped to allow it; and if that never arrives, +`bin/check-cve-exclusions.sh` fails the Repo Hygiene check on **2026-11-10**, 90 days after the +`TEMPORARY-SINCE: 2026-08-11` marker in the pom. Check Central first when it does: +`curl -sI https://repo1.maven.org/maven2/com/fasterxml/jackson/core/jackson-databind/2.18.10/` +(404 as of 2026-08-11). + ## Fixed, and how Both are `git log`-able; recorded here only because the *reasoning* is not obvious from the diff. diff --git a/pom.xml b/pom.xml index 72117ae42..e68c4969b 100644 --- a/pom.xml +++ b/pom.xml @@ -624,9 +624,21 @@ release that does not exist, a disputed record being settled, a migration we have deferred). The TEMPORARY entries at the end are waiting on a specific upstream fix that is already written and merged, and are expected to be - deleted within weeks; they are dated so a reader can see when "temporary" - started. If a temporary entry is more than a release cycle old, that is a bug - in this list, not a fact about the world. --> + deleted within weeks. If a temporary entry is more than a release cycle old, + that is a bug in this list, not a fact about the world. + + THE FORMAT BELOW IS ENFORCED, not a convention. bin/check-cve-exclusions.sh + (Repo Hygiene) parses this block on every PR: + + * every id must sit under a comment - an unclassified id reads as + standing-forever, which is how temporary becomes permanent by accident; + * any comment saying TEMPORARY must carry `TEMPORARY-SINCE: YYYY-MM-DD`, + and undated / unparseable / future-dated all FAIL rather than passing as + "not expired yet"; + * a temporary entry older than 90 days fails the check, naming itself. + + Nothing else was ever going to tell us: these ids are not in OSV, NVD or + GHSA, so no Dependabot alert exists to chase them. --> CVE-2026-54518 - ` above an excluded id opened a section, so the id passed and the run reported "every id carries a rationale" - the guard's own invariant satisfied by a comment that says nothing. That is the unreviewed-suppression-becomes-permanent defect the script exists to prevent, wearing the guard's own uniform. A comment now has to carry at least three words to open a section. Below that floor it opens none, and the ids under it are reported unclassified - fail closed, the same way a missing comment already was. A TEMPORARY banner is never dropped for lacking words: it is still parsed for its date and reported as malformed, so the expiry clock cannot be lost by writing less. The floor is not a quality judgement and does not pretend to be. It separates a rationale from a separator rule, an empty comment and a bare placeholder. An unrelated but wordy comment still passes, and that stays a review question - the machine half of the rule is deliberately the half a machine can decide. Three self-test cases, all confirmed red against the old code: an empty comment, a separator rule, and a dated TEMPORARY banner with no prose. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- bin/check-cve-exclusions.sh | 56 +++++++++++++++++++++++++------- bin/test-check-cve-exclusions.sh | 35 ++++++++++++++++++++ 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/bin/check-cve-exclusions.sh b/bin/check-cve-exclusions.sh index f66edefc6..e3f42c29f 100755 --- a/bin/check-cve-exclusions.sh +++ b/bin/check-cve-exclusions.sh @@ -71,11 +71,19 @@ # check does when it CANNOT decide, and make that state distinguishable from pass - applied here to # the audit's own escape hatch, which is the one place the rule had not yet been applied. # -# Every id must also sit under some comment. An id appended with no rationale has no classification, -# and no classification silently reads as "standing, forever" - the same defect wearing a different -# hat. The pom's own header already requires each entry to say what would retire it; this is the -# machine-checkable half of that rule. What it does NOT check is the prose: whether the reason is any -# good, or whether "RETIRE WHEN" is present, is a review question and stays one. +# Every id must also sit under a comment that CARRIES PROSE - at least MIN_RATIONALE_WORDS words. An +# id appended with no rationale has no classification, and no classification silently reads as +# "standing, forever" - the same defect wearing a different hat. "Some comment" was the original +# rule and it was too weak to hold: `` above an id opened a section and the run reported +# "every id carries a rationale", so the invariant was satisfiable by a comment that said nothing. +# A comment below the floor now opens no section at all, and the ids under it are reported +# unclassified - fail closed, the same way a missing comment already was. +# +# What it does NOT check is the prose itself: whether the reason is any good, whether it is even +# ABOUT the id beneath it, or whether "RETIRE WHEN" is present. The floor rejects a separator rule, +# an empty comment and a bare placeholder; an unrelated but wordy comment still passes. That is a +# review question and stays one - the machine half of the rule is deliberately the half a machine +# can actually decide. # # THE WINDOW # @@ -189,20 +197,37 @@ TOKEN = re.compile( # "temporary" in ordinary prose does not. TEMPORARY_WORD = re.compile(r"\bTEMPORARY\b") TEMPORARY_SINCE = re.compile(r"TEMPORARY-SINCE:\s*(\S+)") +# A comment has to carry PROSE to count as a rationale. Without this, `` above an id opened a +# section and the run reported "every id carries a rationale" - the guard's own invariant satisfied +# by a comment that says nothing, which is precisely the unreviewed-suppression-becomes-permanent +# defect it exists to prevent. The floor is deliberately low and is not a quality judgement: it +# separates a rationale from a separator rule, an empty comment or a bare placeholder, and nothing +# more. An unrelated but wordy comment still passes - see the header; that stays a review question. +RATIONALE_WORD = re.compile(r"[A-Za-z]{2,}") +MIN_RATIONALE_WORDS = 3 sections = [] total_ids = 0 for block in blocks: current = None + contentless_comment_line = None for token in TOKEN.finditer(block.group("body")): line = text.count("\n", 0, block.start("body") + token.start()) + 1 comment = token.group("comment") if comment is not None: - current = {"line": line, "ids": [], "temporary": False, + temporary = bool(TEMPORARY_WORD.search(comment)) + has_prose = len(RATIONALE_WORD.findall(comment)) >= MIN_RATIONALE_WORDS + if not temporary and not has_prose: + # Fail closed. Do NOT open a section: the ids below are left unclassified and + # reported as such, rather than inheriting a rationale that was never written. + current = None + contentless_comment_line = line + continue + current = {"line": line, "ids": [], "temporary": temporary, "since": None, "malformed": None} - if TEMPORARY_WORD.search(comment): - current["temporary"] = True + contentless_comment_line = None + if temporary: stamp = TEMPORARY_SINCE.search(comment) if not stamp: current["malformed"] = ( @@ -215,16 +240,25 @@ for block in blocks: current["malformed"] = ( "`TEMPORARY-SINCE: %s` is not a real YYYY-MM-DD date" % stamp.group(1)) + # A dated banner with no prose is still unexplained. Reported through the same + # channel so a TEMPORARY marker is never silently dropped for lacking words. + if current["malformed"] is None and not has_prose: + current["malformed"] = ( + "it is dated but carries no rationale prose, so nothing says what the " + "entry is for or what would retire it") sections.append(current) continue vuln_id = token.group("id") total_ids += 1 if current is None: + where = ("has no rationale comment above it" + if contentless_comment_line is None else + "sits under the comment at pom.xml line %d, which carries no rationale prose" + % contentless_comment_line) problems.append( - "`%s` (pom.xml line %d) has no rationale comment above it. An unclassified id " - "reads as standing-forever, which is how a temporary exclusion becomes permanent " - "by accident." % (vuln_id, line)) + "`%s` (pom.xml line %d) %s. An unclassified id reads as standing-forever, which is " + "how a temporary exclusion becomes permanent by accident." % (vuln_id, line, where)) continue current["ids"].append(vuln_id) diff --git a/bin/test-check-cve-exclusions.sh b/bin/test-check-cve-exclusions.sh index 2843331bf..d42b11f53 100755 --- a/bin/test-check-cve-exclusions.sh +++ b/bin/test-check-cve-exclusions.sh @@ -36,6 +36,9 @@ # 7. TEMPORARY-SINCE in the future -> FAIL (3) # 8. lowercase "temporary" in ordinary prose -> pass (0) <- no false positive # 9. an id with no rationale comment above it -> FAIL (3) +# 9a. an id under an EMPTY comment `` -> FAIL (3) <- content floor +# 9b. an id under a separator rule `` -> FAIL (3) <- content floor +# 9c. a TEMPORARY banner, dated, carrying no prose -> FAIL (3) <- content floor # 10. standing entries only -> pass (0) # 11. no block at all -> FAIL (1) # 12. the pom file does not exist -> FAIL (1) @@ -54,6 +57,13 @@ # and no classification silently means "standing, forever" - a temporary exclusion made permanent by # omission rather than by decision. # +# Cases 9a-9c are the same rule against the cheapest way to defeat it, and they exist because it WAS +# defeatable: "sits under some comment" was satisfied by ``, so an empty comment above an id +# produced exit 0 and a report claiming every id carried a rationale. 9a and 9b are the two shapes a +# content-free comment actually takes here; 9c is the temporary variant, where the banner is dated +# (so the expiry clock works) but says nothing about what the entry is for. All three must be red for +# the same reason case 9 is - an id whose rationale nobody wrote is an unreviewed suppression. +# # Case 11 is fail-closed placement: if the block is renamed or moved, the check must go red rather # than report a clean list it never found. That is the same reasoning as the audit guard's structural # leg - "the file exists" proves nothing, "the file has content" does. @@ -213,6 +223,31 @@ run_guard "$pom" "$TODAY" assert "an id with no rationale comment fails" 3 "$LAST_EC" assert_contains " ...explaining that unclassified reads as standing-forever" "standing-forever" "$LAST_OUT" +# ── 9a. an EMPTY comment is not a rationale ─────────────────────────────────── +# The exact bypass this floor was added for: `` used to open a section, so the id below it +# passed and the summary claimed every id carried a rationale. +pom=$(write_pom emptycomment " + CVE-2026-00013") +run_guard "$pom" "$TODAY" +assert "an empty comment does not count as a rationale" 3 "$LAST_EC" +assert_contains " ...naming the id" "CVE-2026-00013" "$LAST_OUT" +assert_contains " ...and pointing at the comment that says nothing" "no rationale prose" "$LAST_OUT" + +# ── 9b. a separator rule is not a rationale either ──────────────────────────── +pom=$(write_pom separator " + CVE-2026-00014") +run_guard "$pom" "$TODAY" +assert "a separator rule does not count as a rationale" 3 "$LAST_EC" + +# ── 9c. a dated TEMPORARY banner with no prose ──────────────────────────────── +# The marker must never be silently dropped for lacking words, so this reports as a malformed banner +# rather than vanishing: the entry is expirable but unexplained, and both halves are required. +pom=$(write_pom wordlessbanner " + CVE-2026-00015") +run_guard "$pom" "$TODAY" +assert "a dated TEMPORARY banner carrying no prose fails" 3 "$LAST_EC" +assert_contains " ...saying what is missing" "no rationale prose" "$LAST_OUT" + # ── 10. a list with nothing temporary in it ─────────────────────────────────── pom=$(write_pom standing " CVE-2026-00008") From ef9d2dd7a048e94ea63085d14758182e54b25ef9 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 12 Aug 2026 13:41:26 +1200 Subject: [PATCH 26/29] ci(deps): skip the whole-tree audit on Dependabot PRs, not fail it The fork skip does not cover them. A Dependabot branch lives in this repository, so `head.repo.full_name == github.repository` passes it straight through - but a Dependabot-triggered run resolves `secrets.*` against the Dependabot secret store, not the Actions one, so OSSINDEX_USERNAME and OSSINDEX_TOKEN come back empty and the credential preflight fails deterministically. Every dependency-update PR would have carried a red whole-tree audit, on the PRs least likely to be read closely. The preflight turning a missing secret into a red check is right when the secret is missing by accident. Here it is missing by design, every time, which is the permanent false alarm that teaches people to ignore a check. Gated on `github.actor`, not the PR author, because the actor is what selects the secret store: when a human pushes to a Dependabot branch the run is theirs, the Actions secrets are present, and the scan should run normally. Mirroring the secrets into the Dependabot store was the other option and is worse: it would put the OSS Index token in a second store, reachable by a job that runs `test-compile` over a dependency set an automated process just changed. The scheduled whole-tree scan already covers what those PRs merge, one channel later and without handing the credential around. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .github/workflows/dependency-audit.yml | 27 ++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/.github/workflows/dependency-audit.yml b/.github/workflows/dependency-audit.yml index 88847e3a2..e49ce0bc1 100644 --- a/.github/workflows/dependency-audit.yml +++ b/.github/workflows/dependency-audit.yml @@ -86,10 +86,29 @@ jobs: # difference. Job names are an API here, referenced by repo rulesets (see repo-hygiene.yml's # header): renaming is free only while this check is not yet required and not on master. name: "deps: whole-tree CVE scan" - # Fork PRs receive no secrets, so the audit would 401 and the guard would correctly go red on - # every one of them - a permanent false alarm that teaches people to ignore the check. Skip - # instead. Same test the self-hosted lanes use. - if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository + # TWO WAYS A PR ARRIVES WITHOUT THE OSS INDEX CREDENTIALS, AND BOTH MUST SKIP RATHER THAN GO RED. + # The preflight below turns a missing secret into a red check on purpose - it is the right answer + # when the secret is missing by accident. On these two it would be missing by design, every time, + # which is a permanent false alarm that teaches people to ignore the check. + # + # 1. FORK PRs receive no secrets at all. Same test the self-hosted lanes use. + # 2. DEPENDABOT PRs. The branch is in THIS repository, so the fork test above passes it - but + # GitHub swaps the secret store for a Dependabot-triggered run: `secrets.*` resolves against + # Dependabot secrets, not Actions secrets, so OSSINDEX_USERNAME/OSSINDEX_TOKEN come back + # empty and the preflight fails deterministically. Every dependency-update PR would carry a + # red whole-tree audit, on the PRs least likely to be read line by line. + # + # Gated on `github.actor` rather than the PR author because the actor is what actually selects + # the secret store: when a human pushes to a Dependabot branch the run is theirs, Actions secrets + # ARE present, and the scan should run normally. + # + # The alternative - mirroring the two secrets into the Dependabot secret store - was rejected. It + # would put the OSS Index token in a second store, reachable by a job that runs `test-compile` + # over a dependency set an automated process just changed. The scheduled whole-tree scan covers + # what those PRs merge anyway, one channel later and without handing the credential around. + if: >- + (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) + && github.actor != 'dependabot[bot]' runs-on: ubuntu-latest timeout-minutes: 20 steps: From 8d92d4fe2df01200d5f66b92414cc396924385e7 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 12 Aug 2026 13:41:47 +1200 Subject: [PATCH 27/29] docs(ci): naming a bin/ script is now the grant, so say so where it is done This branch's glob grants falsified bin/AGENTS.md, which still told agents the allowlist "is not a glob" and that a new script stays invisible until someone adds it by hand - binding guidance that would have had the next contributor undo the convention as an oversight. Shrinks bin/AGENTS.md rather than correcting it in place. That file is read by every agent touching the directory, so it should carry only what someone can get wrong by an action taken THERE: naming a script `check-*` or `test-check-*` grants it to the reviewer automatically, so a script that writes, publishes or deploys must not be named that way. Everything else about the allowlist now points at docs/ci.md. Net effect on the file is shorter than master, not longer. The rest moves to docs/ci.md -> "Editing the reviewer", canonical for the review system since astubbs/parallel-consumer#287, whose audience is someone editing the reviewer - rare, deliberate, and already in that file: - the boundary the allowlist actually is. The fork refusal settles whose code runs beside the credential and the review/refresh-gate split settles what the token can reach; neither touches what that code can talk the reviewer into RUNNING, which is the one the allowlist holds. - the naming convention as the new form of that boundary, and why the pattern stops at two prefixes. - what still needs a manual grant: both spellings, both files, and an earlier PR than the script. Drops the bin/test-check-review-posted.sh incident narrative. It was the evidence for a rule that no longer needs arguing - the naming convention now enforces it - and a file every bin/ agent reads should not pay context rent for a settled argument. It survives in git history. Swept the repo for the same staleness by the claim rather than the wording: docs/inflight/ci-review-agent.md, .github/workflows/README.md and both workflows' comments were already correct, and docs/ci.md's pattern bullet already was too. bin/AGENTS.md was the only hit. Also grepped every bold label removed from it - none was cited anywhere; "Scripts that guard other scripts", which is cited twice, is untouched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- bin/AGENTS.md | 56 ++++++++++++--------------------------------------- docs/ci.md | 41 +++++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 45 deletions(-) diff --git a/bin/AGENTS.md b/bin/AGENTS.md index 9de1d77ad..e605b619b 100644 --- a/bin/AGENTS.md +++ b/bin/AGENTS.md @@ -2,49 +2,19 @@ Repo scripts. Two conventions live here because nothing else enforces them. -## Adding a verification script? Consider granting it to the reviewer - -`.github/workflows/claude-code-review-dispatch.yml` gives the PR reviewer an **enumerated -allowlist** of commands (`--allowedTools`). It is not a glob, and there is no automated check that -it stays in step with this directory - so a new script is invisible to the reviewer until someone -adds it by hand. (It lived in `claude-code-review.yml` until the reviewer moved to on demand; that -file is now just the gate.) - -That gap is not theoretical. `bin/test-check-review-posted.sh` went ungranted while its sibling -`bin/test-check-copyright-headers.sh` was granted, so when PR astubbs#210 changed the review gate, the -reviewer could not run the gate's own self-test and had to reason about the fix statically instead - -on a PR whose whole subject was that gate misreporting. - -**Grant a script when it is read-only and lets the reviewer check a claim rather than infer it** - -`check-*.sh`, `test-check-*.sh`, the `ci-*-test.sh` wrappers. A reviewer that can re-run what a PR -asserts catches a false claim; one that cannot is guessing. - -**Do not grant** anything that writes, publishes, or reaches the network beyond `gh` reads. Two -different boundaries meet at that allowlist, and mistaking one for the other is how it gets widened: - -- **Whose code the job runs** is settled by the reviewer's own fork guard - its "Validate inputs and - refuse fork heads" step rejects any PR whose head is not in this repo, because - `workflow_dispatch` holds `CLAUDE_CODE_OAUTH_TOKEN` while checking out `refs/pull//head`. The - old `pull_request` trigger got that for free, since GitHub withholds secrets from fork PRs; on a - dispatch it is explicit. The allowlist has nothing to do with this one. -- **What that code can talk the reviewer into running** is what the allowlist is for, and the fork - guard does nothing for it. An in-repo head does not make the diff, the PR body and the comments - trustworthy - they are still attacker-influencable text being fed to a model that can call Bash. - Enumerating the commands rather than granting `Bash(*)` is the margin against - injection-into-execution. - -Widening it to `bin/*` hands away the second margin for the convenience of not editing one line, and -the first guard will not notice. - -**Grant BOTH spellings.** These are prefix matches, not globs: `Bash(bin/foo.sh:*)` does **not** -match `./bin/foo.sh`. Every entry is listed twice for that reason, and a half-added grant is worse -than none - the reviewer's invocation fails in a way that reads like the script is broken. - -**Editing the reviewer does not cost you the review on that PR, but your new grant does not apply -to it either.** The action's workflow-validation guard skips itself (exiting 0) whenever the -workflow file invoking it differs from the default branch - but the reviewer is dispatched -`--ref master`, so it runs master's copy of the workflow and reviews normally, without the new -grant. Expect the reviewer to say it lacks it. The grant goes live on merge. +## Naming a script here can grant it to the PR reviewer + +`bin/check-*.sh` and `bin/test-check-*.sh` are granted to the review agent **by pattern**, so a +script matching either prefix becomes runnable by the reviewer the moment it is on the default +branch. Nobody approves it; the name is the grant. + +**So do not give that prefix to a script that writes, publishes, deploys, or reaches the network +beyond `gh` reads.** The two prefixes were chosen to keep `deploy.sh`, `chaos-test.sh`, +`soak-test.sh` and friends outside the grant, and a misnamed script defeats that silently. + +Everything else about the allowlist - the two boundaries it sits between, what still needs a manual +grant, and why a grant must land before the pull request that needs it - is in +[`docs/ci.md`](../docs/ci.md) -> "Editing the reviewer". ## Scripts that guard other scripts diff --git a/docs/ci.md b/docs/ci.md index 163fe2997..91d41fe02 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -504,11 +504,48 @@ default branch's copy, so a PR cannot rewrite its own reviewer. Two consequences - **So new `bin/` guards are granted by pattern, not one at a time.** Both allowlists carry `Bash(bin/check-*.sh:*)` and `Bash(bin/test-check-*.sh:*)` (each in the bare and `./` spelling), which means **a new `bin/check-*.sh` needs no allowlist change at all** - it is covered the - moment it exists on the default branch. Adding a guard under some other name still needs its - grant landed in an earlier PR than the script. The reasoning, and what the pattern deliberately + moment it exists on the default branch. The reasoning, and what the pattern deliberately accepts, is in [`claude-code-review-dispatch.yml`](../.github/workflows/claude-code-review-dispatch.yml). +#### What the allowlist is for, and what it is not for + +Two boundaries meet here, and mistaking one for the other is how the list gets widened. *Whose* code +runs beside the credential is settled by the fork refusal; what the job's **token** can reach is +settled by splitting `review` from `refresh-gate`. Neither touches the third question: **what that +code can talk the reviewer into running.** An in-repo head does not make the diff, the PR body and +the comments trustworthy - they are still attacker-influencable text being fed to a model that can +call Bash. Keeping the grants to this repo's own read-only scripts, rather than `Bash(*)` or +`Bash(bin/*.sh:*)`, is the margin against injection-into-execution. + +So: **grant a script when it is read-only and lets the reviewer check a claim rather than infer it. +Do not grant anything that writes, publishes, or reaches the network beyond `gh` reads.** + +**The pattern grants turn that rule into a naming convention, and the convention is now the +boundary.** Nobody approves a new `bin/check-foo.sh`; naming it is what grants it. The two prefixes +were chosen to keep `deploy.sh`, `chaos-test.sh`, `soak-test.sh`, `performance-test.sh`, +`quarantined-test.sh` and `quarantine-lane-report.sh` outside the grant, so a script that writes must +not be named `check-*` or `test-check-*` - a misnamed one defeats the scoping silently and nothing +will flag it. ([`bin/AGENTS.md`](../bin/AGENTS.md) carries that one rule at the point someone would +break it.) `bin/test-check-docs-data.sh` is the single granted script that writes: a considered +exception, argued in the workflow comment, not a precedent. + +#### Adding a grant that no pattern covers + +For anything outside those two prefixes - the `ci-*-test.sh` wrappers, `./mvnw`, `actionlint`, +`bin/todo-index.sh`, the `node .github/scripts/*.test.js` entries - all three of these apply: + +- **Both spellings.** Rules match the command as written, so `Bash(bin/foo.sh:*)` does **not** match + `./bin/foo.sh`. A half-added grant is worse than none: the invocation fails in a way that reads + like the script is broken, which is how the review on astubbs/parallel-consumer#108 logged ten + permission denials, gave up, and still reported success. +- **Both files.** `claude-code-review-dispatch.yml` and `claude.yml` carry the same list and nothing + mechanical keeps them in step. +- **An earlier PR than the script.** Per the bullet above, a grant never applies to the PR that adds + it. That discipline is the one nobody keeps - four check scripts arrived with + astubbs/parallel-consumer#279, all four granted in that same PR, and its reviewer could run none of + them - which is exactly why the two `bin/` families became patterns instead. + ## Self-hosted lanes Setup and operation: [`docs/self-hosted-runner.md`](self-hosted-runner.md). None of these gate From 6242ae17451a3558caf0493c7439d319cf7b9949 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 12 Aug 2026 13:49:56 +1200 Subject: [PATCH 28/29] docs(solutions): findings are fatal now, so stop telling readers they exit 0 The durable guide still documented the audit guard as reporting-only - `scan ran, found problems -> exit 0, findings rendered (reporting only, deliberately)` - which this branch inverted to exit 2. The scripts cite that guide by name in their headers, so a maintainer following it would have applied the opposite gating semantics to the thing it is the record for. Corrected to the two-reds split, and the reason findings became fatal is kept rather than overwritten: they were NOT fatal originally, deliberately, because a standing backlog would have made the check red on every PR. astubbs/parallel-consumer#281 retired that backlog, which inverts the argument rather than repealing it. The general rule - a red that fires on known debt means nothing - survives intact; what changed is the tree. Retires docs/inflight/ci-ossindex-audit-dead.md. The gap it tracked is what this PR closes, and the inflight convention is to delete the file in the PR that resolves it rather than rewrite it into a DONE narrative - it would otherwise sit on master asserting that #279 is unmerged and that no scheduled whole-tree scan exists, and be inherited as live work by the next agent scanning in-flight state. Its durable half was not deleted with it. The scanner's error profile is the load- bearing part, cited from pom.xml, the guard, and the workflow, so it moves to docs/solutions/security-issues/ with the local-reproduction recipe: one real Dependabot miss, two scanner false positives, one phantom CVE id and two disputed findings, in a single run. Two lessons generalise out of it - an over-claim in your own favour outlives one against you, and two scanners disagreeing is not evidence either is right. The two client traps already had a home in the sibling record and are linked, not copied. All six inbound references re-pointed, and the workflow header's own version of the withdrawn three-component claim corrected with it - leaving it would have recreated the contradiction this commit exists to remove. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- .github/workflows/dependency-audit.yml | 23 ++- bin/check-ossindex-audit.sh | 2 +- docs/inflight/ci-ossindex-audit-dead.md | 188 ------------------ docs/inflight/deps-cve-backlog.md | 6 +- ...s-need-reading-before-acting-2026-08-12.md | 116 +++++++++++ ...that-reports-success-without-having-run.md | 53 +++-- ...-an-instrument-that-could-have-said-yes.md | 2 +- pom.xml | 6 +- 8 files changed, 175 insertions(+), 221 deletions(-) delete mode 100644 docs/inflight/ci-ossindex-audit-dead.md create mode 100644 docs/solutions/security-issues/oss-index-reports-need-reading-before-acting-2026-08-12.md diff --git a/.github/workflows/dependency-audit.yml b/.github/workflows/dependency-audit.yml index e49ce0bc1..b4c77eaae 100644 --- a/.github/workflows/dependency-audit.yml +++ b/.github/workflows/dependency-audit.yml @@ -3,11 +3,14 @@ # WHAT THIS COVERS THAT NOTHING ELSE DOES # # `deps: vulnerabilities` (maven.yml, actions/dependency-review-action) gates PRs, but only on the -# dependencies a PR *changes*. Dependabot alerts cover the standing tree, but miss transitive -# components entirely - `netty-codec-http`, `lz4-java` and `HdrHistogram` all carry advisories here -# and have no Dependabot alert in any state. The gap is a **whole-tree scan, on a schedule**, which -# is what the `schedule:` trigger below is for: an unchanged tree acquires new advisories, and -# nothing else in this repo would ever notice. +# dependencies a PR *changes*. Dependabot alerts cover the standing tree, but read from GitHub's +# dependency graph, which holds what the poms DECLARE rather than what Maven resolves - so a purely +# transitive component can carry a published advisory and never produce an alert. `lz4-java` is the +# measured case here: GHSA-xx22-p4ch-683r exists and nothing fired. (One confirmed miss, not three - +# the wider claim was checked and withdrawn; see +# docs/solutions/security-issues/oss-index-reports-need-reading-before-acting-2026-08-12.md.) The +# gap is a **whole-tree scan, on a schedule**, which is what the `schedule:` trigger below is for: +# an unchanged tree acquires new advisories, and nothing else in this repo would ever notice. # # This is the one exception to docs/ci.md's "there is no scheduled build, deliberately". That rule # rejects cron lanes that RE-RUN suites the PR gate already covers. This lane covers something the @@ -47,10 +50,12 @@ # earns its place for what no PR can see - time passing - but it is the second channel, not the # first. # -# The escape hatch for a false positive (and this scanner produces them - see -# docs/inflight/ci-ossindex-audit-dead.md) is the same one the backlog uses: add the id to -# `excludeVulnerabilityIds` with a reason and a retirement condition. The guard honours that list and -# states the suppression count on every run, including green ones. See bin/check-ossindex-audit.sh. +# The escape hatch for a false positive - and this scanner produces them, measured: two false +# positives and one phantom CVE id in a single run +# (docs/solutions/security-issues/oss-index-reports-need-reading-before-acting-2026-08-12.md) - is +# the same one the backlog uses: add the id to `excludeVulnerabilityIds` with a reason and a +# retirement condition. The guard honours that list and states the suppression count on every run, +# including green ones. See bin/check-ossindex-audit.sh. # # Required repo secrets: OSSINDEX_USERNAME (the account email), OSSINDEX_TOKEN (API token from # ossindex.sonatype.org -> account settings). Both already exist. diff --git a/bin/check-ossindex-audit.sh b/bin/check-ossindex-audit.sh index a724f278d..ae1810cb8 100755 --- a/bin/check-ossindex-audit.sh +++ b/bin/check-ossindex-audit.sh @@ -55,7 +55,7 @@ # # The cost is a false positive blocking unrelated work, and it is real: OSS Index produced two false # positives and one CVE id with no public record in a single run against this repo -# (docs/inflight/ci-ossindex-audit-dead.md). The escape hatch is the same one the backlog used - +# (docs/solutions/security-issues/oss-index-reports-need-reading-before-acting-2026-08-12.md). The escape hatch is the same one the backlog used - # add the id to `excludeVulnerabilityIds` in the root pom with a reason and a retirement condition - # which is a deliberate, reviewable act rather than a flag flip. # diff --git a/docs/inflight/ci-ossindex-audit-dead.md b/docs/inflight/ci-ossindex-audit-dead.md deleted file mode 100644 index 7f3ef596b..000000000 --- a/docs/inflight/ci-ossindex-audit-dead.md +++ /dev/null @@ -1,188 +0,0 @@ -# OSS Index audit: a green run is not evidence it scanned - -`ossindex-maven-plugin` gets `HTTP/1.1 401 Unauthorized` from `ossindex.sonatype.org` - the API no -longer serves unauthenticated component-report requests. It was bound to `validate` with -`fail=false`, so every build ran it, got 401 on every module, and stayed green: a security check -reporting success by not running. - -**Wired but not switched on** (astubbs/parallel-consumer#259): `authId` now names an `ossindex` -`` in `settings.xml`, `fail` is `true`, and `ossindex.skip` defaults to `true` so the audit -is off *and visibly off* instead of running and silently failing. Flip it with -`-Dossindex.skip=false`. - -Note that none of that wiring is a *prerequisite* for running a scan: `authId` is a plugin user -property, so `-Dossindex.authId=ossindex -Dossindex.skip=false` behaves identically with or without -the `` element in the pom. The element earns its place as a declarative default, not as an -enabler - nobody was ever blocked on it. - -`OSSINDEX_USERNAME` and `OSSINDEX_TOKEN` exist as repo secrets. **A CI job now exists on branch -`ci/ossindex-audit-job`** (astubbs/parallel-consumer#279, open, not merged) with a three-legged -did-it-actually-scan guard. The constraints it had to satisfy, kept here because they are the -reasoning rather than the code: - -- **One job only, not "on in CI".** The audit is bound to `validate`, which every Maven job runs, so - enabling it globally means six-plus scans per PR from one account and a burnt rate limit for no - extra signal. -- **The job needs its own did-it-actually-scan check**, or it recreates the original bug somewhere - more expensive: a green check that scanned nothing. Failing the step on the - `Failed to fetch component-reports` log line is *a* leg, never the only one - a log-string guard - dies silently the day the vendor rewords its message, which is the same failure it exists to catch. - Pair it with structural evidence from the exported report, per - [`../solutions/workflow-issues/a-check-that-reports-success-without-having-run.md`](../solutions/workflow-issues/a-check-that-reports-success-without-having-run.md). -- **Give it a `schedule:` too.** That is what closes the whole-tree gap below, and is nearly free - once the job exists. -- Credentials reach Maven via `setup-java` (`server-id: ossindex`, `server-username`, - `server-password`), the same pattern `publish.yml` already uses for Central. -- **Never cache the OSS Index report directory** in the job - see the second trap below. - -## Two traps to know about before turning it on - -Both produce the same false signal - a scan that looks like it worked while never reaching the -service - which is the whole subject of this note. - -### 1. `fail=true` does not make an unreachable scanner fatal - -It covers "vulnerable components were found", not "the request failed". Verified directly: forced on -with no credentials and `fail=true`, the 401 is still only a `[WARNING]` and the build reports -SUCCESS. The plugin exposes no setting that changes this. - -So this cannot be made self-policing from config alone. Whoever adds the token must confirm the run -actually returned findings - and be aware that a later token expiry, revocation or outage degrades -silently back to green-but-not-scanning, with nothing to catch it. If it goes into a gating lane, -the lane needs its own check that the scan produced output. - -### 2. The client caches reports on disk, and a warm cache fakes a successful scan - -`ossindex-maven-plugin` keeps an on-disk report cache - on macOS -`~/Library/Application Support/Sonatype/Ossindex`. **A warm cache returns full, correct-looking -results even against a bogus base URL and an expired token.** That is not a theoretical risk: it -silently invalidated the first two negative controls run against this plugin before anyone noticed -the results were too good. - -Two consequences, both easy to get wrong: - -- **Any reachability or credential experiment must clear that directory first**, or a "yes, my token - works" result means nothing. This applies to `-Dossindex.baseUrl=...` negative controls too. -- **CI must never cache it.** A cached report directory would let the job report findings - and pass - its did-it-actually-scan guard - on a run that never contacted the service at all, rebuilding the - original defect inside the very guard meant to prevent it. - -## Running it locally (optional - nobody needs an account to build) - -Add an `ossindex` server to `~/.m2/settings.xml`, matching `authId` in the root pom: - -```xml - - ossindex - you@example.com - your-api-token - -``` - -Then run it with **`test-compile`**, not `validate`: - -```bash -rm -rf ~/Library/Application\ Support/Sonatype/Ossindex # or the cache lies to you - trap 2 -mvn test-compile -Dossindex.skip=false -``` - -Two things that will otherwise waste an afternoon: - -- **`mvn validate` alone cannot work on a cold local repo**, despite the audit being *bound* to - `validate`. The audit mojo declares `requiresDependencyResolution=test`, so Maven resolves the - full test scope before it runs - which needs `parallel-consumer-core:jar:tests`. Without installed - snapshots that artifact does not exist yet and the build dies at the second module. `test-compile` - builds it on the way through. (The `runtime` in the plugin config only filters what - gets *audited*; it does not reduce what Maven has to resolve first.) -- **Clear the report cache before any run you intend to draw a conclusion from.** Without the server - entry the command 401s and, per trap 1, still *succeeds* - and with a warm cache it will not even - 401, it will hand back the previous run's findings. A clean run is not evidence the credentials - work. Check the output says it scanned. - -## What it found the moment it was switched on - -The token works, and the first authenticated run failed the build on **5 vulnerable components, 12 -CVEs**, top severity **9.3**. This is why `ossindex.skip` stays `true` for now: turning it on by -default breaks every build until these are triaged. - -What the scanner reported (all ids are `CVE-2026-NNNNN`): - -| Component | Scope | CVEs (severity) | Dependabot | -|---|---|---|---| -| `com.fasterxml.jackson.core:jackson-databind:2.16.2` | runtime | 54512 (9.2), 54513 (9.3), 54514 (6.9), 54515 (6.9), 54518 (6.9), 59888 (6.3), 59889 (6.3) | 5 of 7 | -| `io.micrometer:micrometer-core:1.13.15` | compile | 40984 (8.7) | yes | -| `io.netty:netty-codec-http:4.1.136.Final` | compile | 59903 (6.5) | no | -| `at.yawk.lz4:lz4-java:1.10.1` | runtime | 59949 (6.5) | no | -| `org.hdrhistogram:HdrHistogram:2.2.2` | runtime | 14683 (2.0), 14686 (2.0) | no | - -The per-CVE triage - which finding is real, which is a false positive, and what each one is blocked -on - lives in `docs/inflight/deps-cve-backlog.md`. What follows here is only what the *scanner -comparison* turned out to be worth, because the first reading of it was wrong. - -### Correction: the Dependabot gap is one component, not three - -An earlier version of this note claimed **three of the five components have no Dependabot alert in -any state**, and inferred that GitHub's Maven dependency graph must be failing to resolve the full -transitive closure. Checked against OSV, GHSA and MITRE, that over-claimed in the scanner's favour, -and the mechanism was guesswork: - -- **`at.yawk.lz4:lz4-java` is the one real gap.** GHSA-xx22-p4ch-683r exists and no Dependabot alert - fired. A single genuine miss still carries the point that the two tools are not redundant - but it - is one, not three, and one miss does not evidence a systematic transitive-closure failure. -- **The jackson "5 of 7" was OSS Index being wrong, not Dependabot.** `CVE-2026-54518` was - introduced in 2.21.0 (GHSA-rcqc-6cw3-h962) and `CVE-2026-59889` in 2.18.0 (GHSA-5gvw-p9qm-jgwh); - both live in `UnwrappedPropertyHandler` code that does not exist in 2.16. Neither applies to - 2.16.2. Dependabot's five was the correct number; the scanner's seven was inflated by two false - positives. -- **`CVE-2026-59903` (netty-codec-http) has no public record at all.** Absent from OSV and GHSA, and - `cveawg.mitre.org` returns `CVE_RECORD_DNE`. Its neighbours -59898, -59899 and -59901 are the real - Netty August batch and are all fixed in 4.1.136.Final - the version already in use. Unverifiable, - nothing to do, and not a Dependabot blind spot. -- **HdrHistogram's two are self-disputed.** Both carry `isDisputed: true` in their own records, are - VulDB-assigned and local-only, and say it is "still unclear if this vulnerability genuinely - exists". - -So the honest scoreboard for that first authenticated run is: one Dependabot miss, two scanner false -positives, one phantom id, and two disputed local-only findings. **The false-positive rate is itself -a finding about this scanner**, and belongs in any decision about gating on it. The comparison was -worth running - just not only in the direction it was first read. - -**Only one of the two highest findings is a deliberately-held-back pin.** `micrometer-core` is: -the root pom holds `micrometer-core.version` at 1.13.15 to keep the family aligned with -`micrometer-registry-prometheus`, which example-metrics pins to 1.12.x until the -`io.micrometer.prometheus` -> `io.micrometer.prometheusmetrics` rename is migrated. Pins parked for a -compatibility reason are where CVEs accumulate, precisely because nobody upgrades them, and triage -needs to weigh that rather than just bump. - -`jackson-databind` is **not** that, though an earlier version of this note said it was. There are -three distinct copies in the reactor and only one of them is the scanned artifact -(`mvn dependency:tree -Dincludes=com.fasterxml.jackson.core:jackson-databind`): - -| Where | Version | Scope | Origin | -|---|---|---|---| -| `parallel-consumer-example-streams` | **2.16.2** | runtime | transitive, unpinned - **this is the flagged one** | -| `parallel-consumer-example-metrics` | 2.17.2 | test | explicit module-local pin | -| core, vertx, example-vertx, example-reactor | 2.13.4.2 | test | transitive via WireMock | - -The "kept module-local because pinning it globally breaks `VertxTest` through `wiremock-jre8`" -reason belongs to the 2.17.2 test pin and the 2.13.4.2 WireMock copy. It is a real constraint on -adding a *global* `dependencyManagement` entry, but it is not why the flagged 2.16.2 sits where it -does - that one is simply an unpinned transitive runtime dependency of an example module that nobody -has ever set. So triaging it is a different job from unblocking a parked pin, and WireMock is a -reason not to fix it with a global pin rather than a reason it cannot be fixed. - -## What is genuinely covered meanwhile - -- **`deps: vulnerabilities`** (`maven.yml`, `actions/dependency-review-action@v4`, - `fail-on-severity: high`) really does gate PRs - but only for dependencies a PR *changes*. -- **Dependabot alerts** cover the existing tree against newly published advisories. - -The residual gap is therefore narrower than "no CVE scanning": there is no **whole-tree, scheduled** -scan that fails a build. Anyone reading only the 401 would over-correct here and bolt on a third -scanner duplicating the two that already work. - -## If the token turns out not to be worth it - -OWASP dependency-check is the usual alternative for the whole-tree gap, and is **not** the -credential-free option it is assumed to be: unkeyed NVD updates are rate-limited into uselessness, -so it wants an NVD API key too, plus a checked-in suppression file for its CPE false positives. diff --git a/docs/inflight/deps-cve-backlog.md b/docs/inflight/deps-cve-backlog.md index 1e5cd0eb3..4ed66ac74 100644 --- a/docs/inflight/deps-cve-backlog.md +++ b/docs/inflight/deps-cve-backlog.md @@ -2,7 +2,8 @@ The first *authenticated* `ossindex-maven-plugin` run failed the build on 5 components / 12 CVEs. Before that it had been returning HTTP 401 on every build while configured `fail=false`, so it -reported success without ever scanning - see `ci-ossindex-audit-dead.md`. The wiring that made an +reported success without ever scanning - see +`../solutions/workflow-issues/a-check-that-reports-success-without-having-run.md`. The wiring that made an authenticated run possible (`authId`, `ossindex.skip`, `fail=true`) landed on `master` in astubbs#259; only the triage below was outstanding. @@ -86,7 +87,8 @@ mvn test-compile -Dossindex.skip=false `requiresDependencyResolution=test`, so on a cold local repo `validate` alone dies resolving `parallel-consumer-core:jar:tests`. The `authId` in the pom names an `ossindex` `` in `~/.m2/settings.xml`; without those credentials the plugin 401s and still reports SUCCESS, so confirm -the output shows it scanned. Both traps in full: `ci-ossindex-audit-dead.md`. Re-derive the +the output shows it scanned. Both traps in full, and how to reproduce a run locally: +`../solutions/security-issues/oss-index-reports-need-reading-before-acting-2026-08-12.md`. Re-derive the dependency paths with `mvn dependency:tree`. ## Do not assume Dependabot covers this diff --git a/docs/solutions/security-issues/oss-index-reports-need-reading-before-acting-2026-08-12.md b/docs/solutions/security-issues/oss-index-reports-need-reading-before-acting-2026-08-12.md new file mode 100644 index 000000000..37040e0db --- /dev/null +++ b/docs/solutions/security-issues/oss-index-reports-need-reading-before-acting-2026-08-12.md @@ -0,0 +1,116 @@ +# OSS Index finds real advisories and invents some - read a run before acting on it + +**The rule: never bump, pin or panic straight from an OSS Index finding.** Check the id against OSV, +GHSA and MITRE first, and check the affected range against the version actually resolved. This +scanner's output is worth having and is not self-certifying, and the exclusion list in the root pom +exists because some of what it reports cannot be acted on at all. + +This is the scanner behind `deps: whole-tree CVE scan` +([`.github/workflows/dependency-audit.yml`](../../../.github/workflows/dependency-audit.yml)), whose +findings gate PRs. That gate is only as good as the reading behind each exclusion, which is why this +record exists. + +## The evidence: the first authenticated run, checked line by line + +Before this, `ossindex-maven-plugin` had been returning HTTP 401 on every build while configured +`fail=false` - reporting success without ever scanning. That defect, and the guard built for it, are +in +[`../workflow-issues/a-check-that-reports-success-without-having-run.md`](../workflow-issues/a-check-that-reports-success-without-having-run.md); +what follows is what the scan said once it could actually run. + +It failed the build on **5 components, 12 CVEs**, top severity 9.3: + +| Component | Scope | CVEs (severity) | Dependabot | +|---|---|---|---| +| `com.fasterxml.jackson.core:jackson-databind:2.16.2` | runtime | 54512 (9.2), 54513 (9.3), 54514 (6.9), 54515 (6.9), 54518 (6.9), 59888 (6.3), 59889 (6.3) | 5 of 7 | +| `io.micrometer:micrometer-core:1.13.15` | compile | 40984 (8.7) | yes | +| `io.netty:netty-codec-http:4.1.136.Final` | compile | 59903 (6.5) | no | +| `at.yawk.lz4:lz4-java:1.10.1` | runtime | 59949 (6.5) | no | +| `org.hdrhistogram:HdrHistogram:2.2.2` | runtime | 14683 (2.0), 14686 (2.0) | no | + +Read as a scoreboard against Dependabot, that first looked like *three components Dependabot misses +entirely*, and therefore like evidence of a systematic transitive-closure failure in GitHub's +dependency graph. Checked against the advisory databases, it was not: + +- **`at.yawk.lz4:lz4-java` is the one real Dependabot gap.** GHSA-xx22-p4ch-683r exists and no alert + fired. A single genuine miss still carries the point that the two tools are not redundant - but it + is one, not three, and one miss is not a systematic failure. +- **The jackson "5 of 7" was OSS Index being wrong, not Dependabot.** `CVE-2026-54518` was introduced + in 2.21.0 (GHSA-rcqc-6cw3-h962) and `CVE-2026-59889` in 2.18.0 (GHSA-5gvw-p9qm-jgwh); both live in + `UnwrappedPropertyHandler` code that does not exist in 2.16. Neither applies to 2.16.2. Dependabot's + five was the correct number; the scanner's seven was inflated by two false positives. +- **`CVE-2026-59903` (netty-codec-http) has no public record at all.** Absent from OSV and GHSA, and + `cveawg.mitre.org` returns `CVE_RECORD_DNE`. Its neighbours -59898, -59899 and -59901 are the real + Netty August batch, all fixed in 4.1.136.Final - the version already in use. Unverifiable, nothing + to do, and not a Dependabot blind spot. +- **HdrHistogram's two are self-disputed.** Both carry `isDisputed: true` in their own records, are + VulDB-assigned and local-access-only, and say it is "still unclear if this vulnerability genuinely + exists". + +**Honest scoreboard for that run: one Dependabot miss, two scanner false positives, one phantom id, +and two disputed local-only findings.** The comparison was worth running - just not only in the +direction it was first read. The false-positive rate is itself a finding about the scanner, and it +belongs in any decision about gating on it. + +Two lessons generalise past this tool: + +- **An over-claim in your own favour survives longer than one against you.** The three-component + reading confirmed the thesis the scan was run to test, so nobody re-derived it. The correction cost + one pass over three advisory databases and should have happened first. +- **A scanner disagreeing with another scanner is not evidence either is right.** Both directions of + the disagreement have to be checked against the primary record, or you have swapped one unverified + source for two. + +## What that means for gating + +Findings are fatal (`exit 2`), so a false positive can block unrelated work. The escape hatch is +deliberate and reviewable rather than a flag flip: add the id to `excludeVulnerabilityIds` in the root +pom **with a reason and a retirement condition**. `bin/check-cve-exclusions.sh` then polices that +list, so a suppression written in a hurry cannot quietly become permanent. + +The per-CVE triage - which finding is real, which is a false positive, and what each is blocked on - +lives in [`../../inflight/deps-cve-backlog.md`](../../inflight/deps-cve-backlog.md) while any of it is +still open. + +## Reproducing a run locally + +Nobody needs an account to build this repo; this is for when you are triaging a finding. + +Add an `ossindex` server to `~/.m2/settings.xml`, matching `authId` in the root pom: + +```xml + + ossindex + you@example.com + your-api-token + +``` + +Then run it with **`test-compile`**, not `validate`: + +```bash +rm -rf ~/Library/Application\ Support/Sonatype/Ossindex # or the cache answers for you +mvn test-compile -Dossindex.skip=false +``` + +Two things that otherwise waste an afternoon, both consequences of traps documented in full in +[the sibling record](../workflow-issues/a-check-that-reports-success-without-having-run.md): + +- **`mvn validate` alone cannot work on a cold local repo**, despite the audit being *bound* to + `validate`. The audit mojo declares `requiresDependencyResolution=test`, so Maven resolves the full + test scope before it runs - which needs `parallel-consumer-core:jar:tests`. Without installed + snapshots that artifact does not exist and the build dies at the second module. `test-compile` + builds it on the way through. (The `runtime` in the plugin config filters what gets + *audited*; it does not reduce what Maven has to resolve first.) +- **Clear the report cache before any run you intend to draw a conclusion from.** Without the server + entry the command 401s and still *succeeds*; with a warm cache it will not even 401, it will hand + back the previous run's findings. A clean-looking run is not evidence the credentials work - check + the output says it scanned. + +## Was a third scanner ever the answer? + +No, and the alternative is worth recording so nobody re-derives it. OWASP dependency-check is the +usual suggestion for the whole-tree gap and is **not** the credential-free option it is assumed to be: +unkeyed NVD updates are rate-limited into uselessness, so it wants an NVD API key too, plus a +checked-in suppression file for its own CPE false positives. Whether this lane still earns its keep at +all is tracked in [`../../inflight/ci-ossindex-lane-reassessment.md`](../../inflight/ci-ossindex-lane-reassessment.md). diff --git a/docs/solutions/workflow-issues/a-check-that-reports-success-without-having-run.md b/docs/solutions/workflow-issues/a-check-that-reports-success-without-having-run.md index 4ce1e5b7f..d845cc436 100644 --- a/docs/solutions/workflow-issues/a-check-that-reports-success-without-having-run.md +++ b/docs/solutions/workflow-issues/a-check-that-reports-success-without-having-run.md @@ -53,7 +53,7 @@ the prompt to add a working one. | Check | What it did when it could not run | Where it stands | |---|---|---| -| `ossindex-maven-plugin` on `validate` | HTTP 401 on every module - the API stopped serving anonymous component-report requests - logged as `[WARNING]`, build green, scan never happened | External guard, `bin/check-ossindex-audit.sh` (unmerged, see below) | +| `ossindex-maven-plugin` on `validate` | HTTP 401 on every module - the API stopped serving anonymous component-report requests - logged as `[WARNING]`, build green, scan never happened | External guard, `bin/check-ossindex-audit.sh` (see below) | | The same plugin with `fail=true` | Still green. `fail` covers *"vulnerable components were found"*, not *"the request failed"* | No plugin setting exists; the external guard is the only answer | | `claude-code-action` posting no review | Ran green for months without posting a single review, because the plugin stops before commenting unless `--comment` is passed | Gated by `bin/check-review-posted.sh` | | `claude-code-action` workflow-validation skip | Refuses to run when the workflow file differs from the default-branch copy; logged *"Exiting due to workflow validation skip"*, exited 0, PR sat mergeable with a green check and no review (astubbs/parallel-consumer#124) | Same gate - and it deliberately fails such PRs | @@ -107,25 +107,40 @@ itself, evidence that it ran. ### 2. When the tool cannot fail, put the decision outside the tool -Both guards here are the same move - one merged and running, one written and reviewed on a branch that -has not landed yet: a small script, run after the tool, that answers *did this actually happen* using -evidence the tool leaves behind. +Both guards here are the same move: a small script, run after the tool, that answers *did this +actually happen* using evidence the tool leaves behind. - `bin/check-review-posted.sh` matches **this run's** id in a posted comment's `[View job]` link, so a comment citing this run is proof this run posted something. It states its own limit: it proves a comment exists, not that its contents are a good review. -- `bin/check-ossindex-audit.sh` (on branch `ci/ossindex-audit-job`, astubbs/parallel-consumer#279, not - yet merged) reads the Maven log and the exported reports, with a split the plugin cannot express: +- `bin/check-ossindex-audit.sh` (added by astubbs/parallel-consumer#279) reads the Maven log and the + exported reports, with a split the plugin cannot express: ``` -scan did not run -> exit 1 (red: the check itself is broken, and that IS actionable) +scan did not run -> exit 1 (red: the CHECK is broken - nothing was learned about the tree) scan ran, found nothing -> exit 0 -scan ran, found problems -> exit 0, findings rendered (reporting only, deliberately) +scan ran, found problems -> exit 2 (red: the TREE has an advisory nobody has looked at) ``` -Note which state is red. Findings are not fatal, because the tree carries a standing backlog of them -and a job that goes red on every PR for known debt is ignored inside a week. Red is reserved for -*"this check stopped working"*, which is rare and therefore still means something. +**Two different reds, and holding them apart is the point.** They demand opposite responses - exit 1 +sends you to the token or the lane and tells you nothing about the dependencies; exit 2 says the lane +worked and there is something to triage. Collapsed into one undifferentiated red, the reader can no +longer tell whether the scanner had anything to say. When both are true at once, exit 1 wins: findings +from a scan that cannot be proven to have happened are not evidence, in either direction. + +**Findings were NOT fatal in the first version, and why that changed is the more useful half.** They +were rendered and exited 0, because the tree carried a standing backlog and a job that goes red on +every PR for known debt is ignored inside a week - which would have cost the exit-1 leg its audience +too. astubbs/parallel-consumer#281 retired that backlog: every item is now either fixed or an explicit +`excludeVulnerabilityIds` entry in the root pom carrying a stated retirement condition. That inverts +the argument rather than overruling it. On a tree whose known debt is already excluded *with reasons*, +a finding is by construction something nobody has looked at, so there is no standing red left for +people to learn to ignore. The general rule survives intact - **a red that fires on known debt means +nothing** - and what changed is the tree, not the rule. + +The escape hatch for a false positive, and this scanner produces them, is that same exclusion list. +`bin/check-cve-exclusions.sh` polices it so a suppression added in a hurry cannot quietly become +permanent: it is the identical move applied one level up, to the audit's own escape hatch. ### 3. Prefer structural evidence from the artifact over string evidence from the log @@ -153,7 +168,7 @@ string" - it is **never let a log string be the only leg.** ### 4. Test the guard's silent case explicitly -`bin/test-check-ossindex-audit.sh` (alongside the guard, same unmerged branch) includes a case +`bin/test-check-ossindex-audit.sh` (alongside the guard) includes a case asserting that empty reports fail **with no failure line in the log at all**. That test is what makes leg 2 load-bearing: remove leg 2 and only that case goes green, and that case is precisely the shape the vendor's next reword will produce. @@ -278,9 +293,11 @@ A guard for this class has to say *which* kind of red it is. - [`../build-errors/maven-multi-module-plugin-and-resolution-traps.md`](../build-errors/maven-multi-module-plugin-and-resolution-traps.md) - the Maven mechanics behind two rows of the table: the profile-scoped plugin version that only warns, and the `validate`-builds-nothing trap that breaks the managed dependency submission. -- `docs/inflight/ci-ossindex-audit-dead.md`, and `docs/inflight/ci-ossindex-lane-reassessment.md` on - branch `ci/ossindex-audit-job` - why the audit ships off-and-visibly-off, and whether the lane still - earns its keep. -- astubbs/parallel-consumer#259 (merged) - `requirePluginVersions`, and the audit turned - off-and-visible rather than on-and-hoping. -- astubbs/parallel-consumer#279 (open) - the CI job and `bin/check-ossindex-audit.sh`. +- `docs/inflight/ci-ossindex-lane-reassessment.md` - whether the lane still earns its keep now that + GitHub can submit Maven transitive dependencies itself. +- astubbs/parallel-consumer#259 - `requirePluginVersions`, and the audit turned off-and-visible rather + than on-and-hoping. +- astubbs/parallel-consumer#279 - the CI job, `bin/check-ossindex-audit.sh`, and the exclusion-expiry + guard beside it. +- astubbs/parallel-consumer#281 - retiring the standing backlog, which is what let findings become + fatal. diff --git a/docs/solutions/workflow-issues/negative-results-need-an-instrument-that-could-have-said-yes.md b/docs/solutions/workflow-issues/negative-results-need-an-instrument-that-could-have-said-yes.md index d29ee8247..39941035b 100644 --- a/docs/solutions/workflow-issues/negative-results-need-an-instrument-that-could-have-said-yes.md +++ b/docs/solutions/workflow-issues/negative-results-need-an-instrument-that-could-have-said-yes.md @@ -72,7 +72,7 @@ findings even against a bogus base URL and an expired token. The experiment was configuration cannot reach the service"; the run came back with a complete report; and the obvious reading - "so it *can* reach it" - was wrong twice before anyone noticed. The paths, the `rm`, and why CI must never cache that directory are in the sibling doc and in -`docs/inflight/ci-ossindex-audit-dead.md`. +[`../security-issues/oss-index-reports-need-reading-before-acting-2026-08-12.md`](../security-issues/oss-index-reports-need-reading-before-acting-2026-08-12.md). The transferable part is the shape. **Before running any experiment whose expected outcome is failure, ask what is stored between your command and the thing you are testing.** A cache does not diff --git a/pom.xml b/pom.xml index e68c4969b..db8a3295c 100644 --- a/pom.xml +++ b/pom.xml @@ -604,8 +604,10 @@ actually returned findings, because nothing else will. Clear the client's on-disk report cache first, or a warm cache will answer for it. - Why `fail` cannot police this, both traps in full, and what the first - authenticated run found: docs/inflight/ci-ossindex-audit-dead.md --> + Why `fail` cannot police this, and both traps in full: + docs/solutions/workflow-issues/a-check-that-reports-success-without-having-run.md + What the first authenticated run found, and this scanner's false-positive + rate: docs/solutions/security-issues/oss-index-reports-need-reading-before-acting-2026-08-12.md --> ossindex ${ossindex.skip} ${ossindex.fail} From 641281faf7525272f5ea255a3e07c60f690c5eb3 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 12 Aug 2026 13:50:11 +1200 Subject: [PATCH 29/29] docs(inflight): do not re-enable a workflow this note proves cannot work The reassessment note told a future reader to re-enable managed dependency submission after 0.6.0.0 ships, then two paragraphs later established that the managed workflow probes with a bare `mvn validate`, that this cannot resolve reactor siblings whatever is published, and that "waiting for the v6 release does not fix it either, for the same reason" - concluding the setting should stay off. Following the instruction would have restored a permanently red check on every PR and learned nothing. The instruction was written before the root cause was found and never revisited; the trigger at the foot of the note, which requires a working repo-owned submission workflow first, is the version that survived the analysis. Kept as a correction rather than a silent deletion, so the next reader can see the conclusion moved and does not re-derive the abandoned one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STG7ygSMtmFuu36BiqdA1m --- docs/inflight/ci-ossindex-lane-reassessment.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/inflight/ci-ossindex-lane-reassessment.md b/docs/inflight/ci-ossindex-lane-reassessment.md index 0e5363eeb..efefac7e8 100644 --- a/docs/inflight/ci-ossindex-lane-reassessment.md +++ b/docs/inflight/ci-ossindex-lane-reassessment.md @@ -82,10 +82,14 @@ replacing with a repo-owned submission workflow that builds the reactor first). **Until it succeeds at least once on `master`, the dependency graph will not repopulate**, and the re-measurement below would read as "no change" for the wrong reason. -### Turned back OFF on 2026-08-11 - re-enable after the v6 release +### Turned back OFF on 2026-08-11 - and it stays off Rather than leave a permanently red check on every PR, the *Automatic dependency submission* setting -was switched off again. **Re-enable it after 0.6.0.0 ships**, then re-run the trigger below. +was switched off again. **Do not simply switch it back on**, at the v6 release or any other point: +the root cause below is a property of the managed workflow, not of anything a release changes, so +re-enabling it restores a permanently failing check and nothing else. The first draft of this note +did say "re-enable after 0.6.0.0 ships", which was written before the root cause was understood and +is contradicted by the next two paragraphs; the trigger at the bottom is the version that survived. Root cause, and it is neither "we have never published" nor anything about repositories: **GitHub's managed submission workflow probes with a bare `mvn validate` at the repo root.** `validate` builds