ci: audit main and develop daily, accept RUSTSEC-2024-0449, and open an issue when the scheduled audit fails

.cargo/audit.toml

@@ -0,0 +1,9 @@
+# cargo-audit's ignore list. cargo-deny reads deny.toml; cargo-audit reads
+# this file, relative to the directory it runs in, and release.yml runs it
+# from the repository root with --deny warnings. The reason for each entry
+# is written in deny.toml. tests/audit_ignore_lists.rs keeps the two equal.
+
+[advisories]
+ignore = [
+    "RUSTSEC-2024-0449",
+]

.github/workflows/audit.yml

@@ -1,32 +1,43 @@
 name: Security audit
 
-# Two triggers answering two different questions, and the split is the whole
-# point rather than a convenience.
+# Three triggers, three questions, and the split is the whole point.
 #
-# The weekly cron audits the WHOLE tree against an advisory database that moves
-# on somebody else's schedule. A finding there belongs to a third party: RustSec
-# published something today about a crate nobody here touched. That must not be
-# able to block a pull request, which is why this workflow was cron-only, and
-# `standards/testing` says so in as many words: "a finding that belongs to a
-# third party must NOT fail the run, or someone else's disclosure timing blocks
-# every pull request in the organisation".
+# The daily schedule audits the WHOLE tree on both branches against an
+# advisory database that moves on somebody else's schedule. A finding
+# there belongs to a third party: RustSec published something today
+# about a crate nobody here touched. That must not be able to block a
+# pull request, which is why this workflow kept pull requests off the
+# third-party-timing lane, and `standards/testing` says so in as many
+# words: "a finding that belongs to a third party must NOT fail the run,
+# or someone else's disclosure timing blocks every pull request in the
+# organisation".
 #
-# The pull-request trigger asks the other question: does THIS change introduce a
-# dependency with a known advisory? That finding belongs to the pull request, and
-# blocking is the right answer. Scoping it to the manifest and the lockfile is
-# what keeps the two apart: a pull request that touches neither cannot introduce
-# a dependency, so upstream's timing reaches it through the cron and not through
-# a red check it did not cause.
+# The pull-request trigger asks the other question: does THIS change
+# introduce a dependency with a known advisory? That finding belongs to
+# the pull request, and blocking is the right answer. Scoping it to the
+# manifest, the lockfile and the two ignore lists is what keeps the two
+# apart: a pull request that touches none of them cannot introduce a
+# dependency, so upstream's timing reaches it through the cron and not
+# through a red check it did not cause.
 #
-# The residue, stated rather than discovered: a pull request that does change the
-# lockfile on the day an unrelated advisory lands is blocked by that advisory.
-# That is the narrow case where the two questions overlap, and it lands on the
-# pull request that is changing dependencies, which is where someone is already
-# looking at them.
+# The residue, stated rather than discovered: a pull request that does
+# change the lockfile on the day an unrelated advisory lands is blocked
+# by that advisory. That is the narrow case where the two questions
+# overlap, and it lands on the pull request that is changing
+# dependencies, which is where someone is already looking at them.
+#
+# The schedule audits main and starts a run of this workflow on develop;
+# the report job below opens an issue when either does not succeed
+# (#2034).
 on:
   schedule:
-    - cron: "0 6 * * 1"  # Every Monday at 06:00 UTC
+    - cron: "0 6 * * *"  # Daily at 06:00 UTC
   workflow_dispatch:
+    inputs:
+      parent-run:
+        description: Set by the scheduled run on main to its own run id when it starts the develop audit; empty for a manual dispatch
+        type: string
+        default: ""
   pull_request:
     branches:
       - main
@@ -35,12 +46,126 @@ on:
       - "Cargo.toml"
       - "Cargo.lock"
       - "deny.toml"
+      - ".cargo/audit.toml"
       - ".github/workflows/audit.yml"
       - ".github/workflows/reusable-rust-audit.yml"
 
 permissions:
   contents: read
 
+# On a schedule or pull_request event `inputs.parent-run` is empty and
+# the run is named after the workflow. On a workflow_dispatch started by
+# the schedule the input carries the parent's run id and the run is
+# named so the parent can find its own child by displayTitle.
+run-name: ${{ inputs.parent-run && format('Security audit of develop for run {0}', inputs.parent-run) || github.workflow }}
+
 jobs:
+  # Kept as audit so its check names stay the same.
   audit:
     uses: ./.github/workflows/reusable-rust-audit.yml
+
+  # A schedule only runs on the default branch. Checking develop out
+  # inside this run would execute develop's tree in main's context,
+  # where any step can write the cache every branch reads (CodeQL,
+  # #2036). So the schedule starts a separate run of this workflow on
+  # develop, waits for it, and fails when that run does not succeed or
+  # never appears at all: a "queued forever" dispatch and a dispatch
+  # that returned a run id but exited non-zero both leave this job
+  # non-success, and the report job below then opens an issue for the
+  # develop branch instead of declaring it green by omission.
+  audit-develop:
+    name: start the develop audit
+    if: github.event_name == 'schedule'
+    runs-on: ubuntu-latest
+    permissions:
+      actions: write
+    # Bound: the dispatched audit installs its tools and runs both
+    # checks; forty minutes covers that plus a queue, and no run has
+    # been measured yet.
+    timeout-minutes: 40
+    env:
+      GH_TOKEN: ${{ github.token }}
+      GH_REPO: ${{ github.repository }}
+    steps:
+      - name: Dispatch audit.yml on develop and wait for it
+        run: |
+          set -euo pipefail
+          gh workflow run audit.yml --ref develop -f parent-run="$GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT"
+          ID=
+          for _ in $(seq 1 10); do
+            ID=$(gh run list --workflow audit.yml --branch develop --event workflow_dispatch --limit 20 --json databaseId,displayTitle --jq "map(select(.displayTitle == \"Security audit of develop for run $GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT\")) | .[0].databaseId // empty")
+            if [ -n "$ID" ]; then
+              break
+            fi
+            sleep 15
+          done
+          if [ -z "$ID" ]; then
+            echo "::error::the develop audit was dispatched but its run never appeared"
+            exit 1
+          fi
+          if ! gh run watch "$ID" --exit-status --interval 30; then
+            echo "::error::the develop audit run $ID did not succeed; see $GITHUB_SERVER_URL/$GH_REPO/actions/runs/$ID"
+            exit 1
+          fi
+          echo "Develop audit run: $GITHUB_SERVER_URL/$GH_REPO/actions/runs/$ID"
+
+  # Runs when this run's audit did not succeed (failure, cancellation or
+  # timeout). On the schedule it reports both branches, because the job
+  # above fails when develop's run fails or never appears. A develop run
+  # started by the schedule passes parent-run and does not report itself;
+  # a manual dispatch reports its own branch. issues: write is granted
+  # to this job alone; nothing here checks out code or installs tools,
+  # and every value reaches the script through env.
+  #
+  # Two reporters running at once can open two issues; that only happens
+  # when a manual dispatch fails at the same moment as the schedule, and
+  # a duplicate issue is cheaper than a lost report, which is what a
+  # concurrency group would risk (GitHub keeps one pending job and
+  # cancels the rest).
+  report:
+    name: open an issue when the scheduled audit fails
+    runs-on: ubuntu-latest
+    needs: [audit, audit-develop]
+    if: always() && ((github.event_name == 'schedule' && (needs.audit.result != 'success' || needs.audit-develop.result != 'success')) || (github.event_name == 'workflow_dispatch' && !inputs.parent-run && needs.audit.result != 'success'))
+    permissions:
+      issues: write
+    # Bound: no runs measured yet, so the ten-minute floor every job
+    # here uses.
+    timeout-minutes: 10
+    env:
+      GH_TOKEN: ${{ github.token }}
+      GH_REPO: ${{ github.repository }}
+      REF_NAME: ${{ github.ref_name }}
+      AUDIT_RESULT: ${{ needs.audit.result }}
+      DISPATCH_RESULT: ${{ needs.audit-develop.result }}
+      RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
+      ISSUE_TITLE: Scheduled dependency audit failed
+    steps:
+      - name: Open a single tracking issue and update it on each failure
+        run: |
+          set -euo pipefail
+          NUMBER=$(gh issue list --state open --search "in:title \"$ISSUE_TITLE\"" --json number,title \
+            --jq 'map(select(.title == env.ISSUE_TITLE)) | .[0].number // empty')
+          DATE=$(date -u +%Y-%m-%dT%H:%M:%SZ)
+          BRANCHES=""
+          if [ "$AUDIT_RESULT" != "success" ]; then
+            BRANCHES="${BRANCHES}${REF_NAME} (${AUDIT_RESULT}), "
+          fi
+          if [ "$DISPATCH_RESULT" != "success" ] && [ "$DISPATCH_RESULT" != "skipped" ]; then
+            BRANCHES="${BRANCHES}develop (${DISPATCH_RESULT}; see the start the develop audit job for the develop run), "
+          fi
+          BRANCHES="${BRANCHES%, }"
+          if [ -n "$NUMBER" ]; then
+            gh issue comment "$NUMBER" --body "Failed again on ${DATE} on: ${BRANCHES}. Run: ${RUN_URL}"
+            echo "Commented on #${NUMBER}."
+            exit 0
+          fi
+          gh issue create \
+            --title "$ISSUE_TITLE" \
+            --label "type:security" --label "prio:P1" --label "area:ci" --label "status:triage" \
+            --body "The scheduled dependency audit failed on ${DATE} on: ${BRANCHES}.
+
+          Run: ${RUN_URL}
+
+          Close this once the scheduled audit is green on both branches. While this stays open, each new failure adds a comment here instead of opening another issue."
+          echo "Opened new issue."

deny.toml

@@ -10,7 +10,15 @@ all-features = true
 [advisories]
 version = 2
 yanked = "deny"
-ignore = []
+# RUSTSEC-2024-0449: unsafe-libyaml 0.2.11 is unmaintained. It is not a
+# known vulnerability. It comes in through serde_yaml_ng 0.10.0, the latest
+# release, so no bump clears it. What it parses is the compose file of the
+# person running podup. Moving off it is #2035, and that pull request
+# removes this entry and the one in .cargo/audit.toml.
+# tests/audit_ignore_lists.rs keeps the two lists equal.
+ignore = [
+    { id = "RUSTSEC-2024-0449", reason = "unmaintained, not a known vulnerability; pulled by serde_yaml_ng 0.10.0, the latest release; see #2035" },
+]
 
 [licenses]
 version = 2

tests/audit_ignore_lists.rs

@@ -0,0 +1,160 @@
+//! `cargo audit` and `cargo deny` read two different files but must agree
+//! on which RUSTSEC ids are ignored. `deny.toml` carries the cargo-deny
+//! list and `.cargo/audit.toml` carries the cargo-audit one. The release
+//! job (`release.yml`) runs `cargo audit --deny warnings` from the
+//! repository root, so an ignore that lives only in deny.toml does not
+//! help the release gate. The two lists must be equal. This test only
+//! accepts one canonical layout per file and refuses the rest.
+use std::fs;
+use std::path::Path;
+fn read_repo(rel: &str) -> String {
+	fs::read_to_string(Path::new(env!("CARGO_MANIFEST_DIR")).join(rel))
+		.unwrap_or_else(|e| panic!("{rel} readable: {e}"))
+}
+fn is_rustsec(s: &str) -> Option<&str> {
+	let b = s.as_bytes();
+	let d = |r: std::ops::Range<usize>| b[r].iter().all(u8::is_ascii_digit);
+	(b.len() == 17 && b.starts_with(b"RUSTSEC-") && b[12] == b'-' && d(8..12) && d(13..17))
+		.then_some(s)
+}
+const DENY_SHAPE: &str = "`{ id = \"RUSTSEC-YYYY-NNNN\", reason = \"...\" },`";
+fn parse_entry(file: &str, l: &str) -> Option<String> {
+	let s = l.strip_suffix(',')?.trim();
+	if file.ends_with("/audit.toml") {
+		is_rustsec(s.strip_prefix('"')?.strip_suffix('"')?).map(String::from)
+	} else {
+		let inner = s.strip_prefix('{')?.strip_suffix('}')?.trim();
+		let (left, right) = inner.split_once(',')?;
+		let id = left
+			.trim()
+			.strip_prefix("id = ")?
+			.strip_prefix('"')?
+			.strip_suffix('"')?;
+		let rv = right
+			.trim()
+			.strip_prefix("reason = ")?
+			.strip_prefix('"')?
+			.strip_suffix('"')?;
+		(!rv.contains('"') && !rv.contains('\\')).then_some(())?;
+		is_rustsec(id).map(String::from)
+	}
+}
+fn ignored(file: &str, body: &str, shape: &str) -> Vec<String> {
+	macro_rules! bad { ($l:expr) => { panic!("{file}: write the ignore list in the canonical form, one entry per line: {shape}; found: {}", $l) } }
+	let dup = || panic!("{file}: expected exactly one ignore key");
+	let mut in_adv = false;
+	let mut in_arr = false;
+	let mut seen = false;
+	let mut out = Vec::new();
+	for line in body.lines() {
+		let l = line.trim();
+		in_adv = in_adv || l == "[advisories]";
+		if !in_adv {
+			continue;
+		}
+		let ig = l.split_once('=').is_some_and(|(k, _)| k.trim() == "ignore");
+		match (in_arr, ig, l) {
+			(false, _, l) if l.is_empty() || l.starts_with('#') => continue,
+			(false, _, l) if l.starts_with('[') && l.ends_with(']') && l != "[advisories]" => break,
+			(false, false, _) => continue,
+			(false, true, l) => {
+				if seen {
+					dup();
+				}
+				seen = true;
+				match l {
+					"ignore = []" => {}
+					"ignore = [" => in_arr = true,
+					_ => bad!(l),
+				}
+			}
+			(true, true, _) => dup(),
+			(true, false, "]") => in_arr = false,
+			(true, false, l) => match parse_entry(file, l) {
+				Some(id) => out.push(id),
+				None => bad!(l),
+			},
+		}
+	}
+	if !seen {
+		dup();
+	}
+	if in_arr {
+		panic!("{file}: missing closing `]` for ignore array");
+	}
+	out
+}
+fn cargo_ids(body: &str) -> Vec<String> {
+	ignored(".cargo/audit.toml", body, "`\"RUSTSEC-YYYY-NNNN\",`")
+}
+fn deny_ids(body: &str) -> Vec<String> {
+	ignored("deny.toml", body, DENY_SHAPE)
+}
+#[test]
+fn the_two_ignore_lists_agree_on_advisory_ids() {
+	let mut deny = deny_ids(&read_repo("deny.toml"));
+	let mut audit = cargo_ids(&read_repo(".cargo/audit.toml"));
+	deny.sort();
+	audit.sort();
+	assert_eq!(
+		deny, audit,
+		"deny.toml and .cargo/audit.toml disagree. deny: {deny:?}, audit: {audit:?}"
+	);
+}
+#[test]
+fn empty_ignore_array_yields_empty() {
+	assert!(cargo_ids("[advisories]\nignore = []\n").is_empty());
+}
+#[test]
+fn canonical_two_entries_read_in_order() {
+	assert_eq!(
+		cargo_ids(
+			"[advisories]\nignore = [\n    \"RUSTSEC-2024-0001\",\n    \"RUSTSEC-2024-0002\",\n]\n"
+		),
+		vec![
+			"RUSTSEC-2024-0001".to_string(),
+			"RUSTSEC-2024-0002".to_string()
+		]
+	);
+}
+#[test]
+#[should_panic(expected = "write the ignore list in the canonical form")]
+fn commented_out_entry_panics() {
+	let _ = cargo_ids(
+		"[advisories]\nignore = [\n    # \"RUSTSEC-2024-0001\",\n    \"RUSTSEC-2024-0002\",\n]\n",
+	);
+}
+#[test]
+#[should_panic(expected = "write the ignore list in the canonical form")]
+fn one_line_array_panics() {
+	let _ = cargo_ids("[advisories]\nignore = [\"RUSTSEC-0000-0000\"]\n");
+}
+#[test]
+#[should_panic(expected = "expected exactly one ignore key")]
+fn second_ignore_key_panics() {
+	let _ = cargo_ids("[advisories]\nignore = [\nignore = [],\n]\n");
+}
+#[test]
+fn deny_entry_with_reason_mentioning_id_yields_only_id() {
+	assert_eq!(
+		deny_ids("[advisories]\nignore = [\n    { id = \"RUSTSEC-2024-0001\", reason = \"see RUSTSEC-9999-9999\" },\n]\n"),
+		vec!["RUSTSEC-2024-0001".to_string()]
+	);
+}
+#[test]
+#[should_panic(expected = "expected exactly one ignore key")]
+fn second_ignore_key_after_the_array_panics() {
+	let _ = cargo_ids("[advisories]\nignore = []\nignore = []\n");
+}
+#[test]
+#[should_panic(expected = "write the ignore list in the canonical form")]
+fn ignore_key_with_other_spacing_panics() {
+	let _ = cargo_ids("[advisories]\nignore  = [\n]\n");
+}
+#[test]
+#[should_panic(expected = "write the ignore list in the canonical form")]
+fn deny_reason_with_a_backslash_panics() {
+	let _ = deny_ids(
+		"[advisories]\nignore = [\n    { id = \"RUSTSEC-2024-0001\", reason = \"C:\\work\" },\n]\n",
+	);
+}