Skip to content

Fix the weekly cron job and move workflows off Node 20 - #328

Open
sarsko wants to merge 3 commits into
mainfrom
fix-weekly-cron-and-workflow-permissions
Open

Fix the weekly cron job and move workflows off Node 20#328
sarsko wants to merge 3 commits into
mainfrom
fix-weekly-cron-and-workflow-permissions

Conversation

@sarsko

@sarsko sarsko commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

The Cron jobs workflow has failed on every scheduled run since at least mid-June (latest, a month earlier). There were two independent causes, and one hid the other.

1. cargo audit fails on an unreachable advisory

The audit job exits 1 on RUSTSEC-2022-0040 (owning_ref 0.4.1, multiple soundness issues, no fixed upgrade available).

The advisory is not reachable in anything we build. owning_ref enters Cargo.lock only as an optional dependency of lock_api, which is reached three ways: transitively via parking_lot 0.12 and dashmap 6, and directly by shuttle-parking_lot-impl. In every case pulling it in requires the non-default owning_ref feature, which nothing in this workspace enables. cargo tree -i owning_ref --workspace reports no match, confirming it is absent from the resolved build graph. cargo audit flags it regardless because it scans the lockfile, which records optional dependencies whether or not their feature is activated.

Since there is no fixed version to upgrade to, this adds .cargo/audit.toml ignoring the advisory, with the reachability analysis and the conditions for removing the entry recorded next to it.

2. Nobody was told, because the notifier was broken too

The workflow set a top-level permissions: contents: write. Naming any scope implicitly sets every unnamed scope to none, which left issues: none, so the notify step could not open an issue and failed with Resource not accessible by integration.

This drops to contents: read at the top level, which is all either job needs, and grants issues: write on the jobs that notify.

The notify step was also only wired to the beta job, so the audit job — the one actually failing — had no notification path at all. It now has one, under its own label: the action reuses an open issue carrying the configured label, so a shared label would file an audit failure as a comment on a beta toolchain issue.

3. Node 20 action runtime

Every run carried Node.js 20 is deprecated. The following actions target Node.js 20 but are being forced to run on Node.js 24: actions/checkout@v2, jayqi/failed-build-issue-action@v1. Node 20 is removed from the runners on 2026-09-16, at which point these stop working.

  • actions/checkout v2/v3 → v5, the first major declaring using: node24. Deliberately not v7: v6 changed checkout to persist git credentials to a separate file, and bench.yml uses boa-dev/criterion-compare-action@v3, which last shipped in 2022 and runs its own git operations against the base branch. v5 clears the deadline without taking on that behavioural change.
  • The notifier tracked jayqi/failed-build-issue-action@v1, which resolved to v1.2.0 on node20. Upstream has since released v1.3.0 on node24 and v1 now points at it. The action also moved from jayqi/ to drivendataorg/; the old path still redirects, but this references the canonical owner rather than relying on that.

Unrelated fixes to workflows touched anyway

  • release.yml had no name: and no permissions: block at any level, so it displayed by file path and inherited the repository default token scope.
  • The clippy job in tests.yml ran rustup component add rustfmt, so it never installed the component it is named for. It only worked because the runner image ships clippy already.

Testing

cargo audit reproduced locally against cargo-audit 0.22.2: exit 1 before, exit 0 after, with output matching CI run 31987845860 exactly. All four workflows re-parsed and permissions verified per job.

Note that crons.yml only triggers on schedule and on pushes to main touching Cargo.toml, so CI on this PR will not exercise the audit job. It is worth a manual gh workflow run (or waiting for the Monday run) after merge to confirm green.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

sarsko added 3 commits August 21, 2026 18:46
The `Cron jobs` workflow has failed on every scheduled run since at least
mid-June. Two independent bugs, one of which hid the other.

1. The `audit` job fails because `cargo audit` exits 1 on RUSTSEC-2022-0040
   (`owning_ref` 0.4.1, multiple soundness issues, no fixed upgrade
   available). The advisory is not reachable in anything we build:
   `owning_ref` only enters Cargo.lock as an *optional* dependency of
   `lock_api`, reached via `parking_lot` 0.12, `dashmap` 6, and
   `shuttle-parking_lot-impl`. Pulling it in requires the non-default
   `owning_ref` feature, which nothing in this workspace enables, and
   `cargo tree -i owning_ref --workspace` confirms it is absent from the
   resolved build graph. `cargo audit` flags it regardless because it scans
   the lockfile, which records optional dependencies whether or not their
   feature is activated.

   Since there is no fixed version to upgrade to, add `.cargo/audit.toml`
   ignoring the advisory, with the reachability analysis and the conditions
   for removing the entry recorded alongside it.

2. Nobody was notified, because the notifier was broken too. The workflow
   set a top-level `permissions: contents: write`, and naming any scope
   implicitly sets every unnamed scope to `none`. That left `issues: none`,
   so `jayqi/failed-build-issue-action` could not open an issue and failed
   with "Resource not accessible by integration".

   Drop to `contents: read` at the top level, which is all either job
   actually needs, and grant `contents: read` + `issues: write` on the
   `beta` job so the notify step works.

Also bump `actions/checkout` v2 -> v5 here; see the following commit.

Verified by reproducing `cargo audit` locally against cargo-audit 0.22.2:
exit 1 before this change, exit 0 after, with output matching CI run
31987845860 exactly.
Every workflow run currently carries the annotation "Node.js 20 is
deprecated. The following actions target Node.js 20 but are being forced to
run on Node.js 24". Node 20 is removed from the runners on 2026-09-16, at
which point these actions stop working entirely.

Bump `actions/checkout` v3 -> v5 across the remaining workflows (v2 -> v5 in
crons.yml in the previous commit). v5 is the first major that declares
`using: node24`.

Deliberately stopping at v5 rather than the current v7: v6 changed checkout
to persist git credentials to a separate file, and `bench.yml` uses
`boa-dev/criterion-compare-action@v3`, which last shipped in 2022 and runs
its own git operations against the base branch. v5 clears the Node 20
deadline without taking on that behavioural change.

Two unrelated fixes to workflows touched anyway:

- `release.yml` had no `name:` and no `permissions:` block at any level, so
  it was displayed by file path and inherited the repository default token
  scope. Add both.
- The `clippy` job in `tests.yml` ran `rustup component add rustfmt`, so it
  never installed the component it is named for. It only worked because the
  runner image ships clippy already.
Two gaps in how the cron reports its own failures.

The notify step was only wired to the `beta` job, so the `audit` job -- the
one that has actually been failing every week -- had no notification path at
all. Add the same step there, with its own `issues: write` grant.

It uses a distinct label and title rather than sharing the default with
`beta`. The action reuses an open issue carrying the configured label, so a
shared label would file an audit failure as a comment on a beta toolchain
issue. Those are unrelated failures with unrelated fixes and should not share
a thread.

Separately, the action itself referenced `jayqi/failed-build-issue-action@v1`,
which resolved to v1.2.0 and declares `runs.using: node20`. Node 20 is
removed from the runners on 2026-09-16, so the step that tells us the cron
broke would itself have broken. Upstream released v1.3.0 on the node24
runtime, and `v1` now points at it, so tracking `v1` is enough.

The action also moved from jayqi/ to drivendataorg/. The old path still
redirects, but reference the canonical owner rather than rely on that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant