Repository navigation
deps: pin past the yanked chacha20 0.10.1 - #151
Conversation
`cargo deny check advisories` fails on `main`: the tree carries chacha20 0.10.1, which its author yanked. It comes in through tokio-tungstenite 0.30 -> tungstenite -> rand 0.10.2, which depends on chacha20 directly, so nothing here picked the version and no Cargo.toml edit avoids it. The lockfile moves to the republished 0.10.2. A yank is not a disclosed vulnerability, but the policy denies it because a withdrawn version has no upgrade path and no advisory coverage, and this one is the RNG under the WebSocket stack. The check is a required status, so the failure also blocks every unrelated pull request. Lockfile only, `cargo update -p chacha20`. The same resolution drops the digest/generic-array chain 0.10.1 pulled in and collapses the duplicate cpufeatures and block-buffer versions. `cargo deny check` is clean on all four sets; build and the full test suite pass.
There was a problem hiding this comment.
🟢 Approval recommended
The changes are limited to a lockfile update plus changelog documentation, and the updated lockfile correctly removes the yanked chacha20 0.10.1 from the resolved dependency graph.
Pull request overview
This PR resolves failing cargo deny check advisories on main by updating the dependency lockfile past the yanked chacha20 0.10.1 (pulled in via tungstenite -> rand 0.10.2 -> chacha20), restoring green required security-audit checks across all PRs.
Changes:
- Update
Cargo.lockto usechacha20 0.10.2(and related dependency graph simplifications/removals that result from the new resolution). - Document the incident and remediation in the Unreleased changelog under a new “Security” entry.
File summaries
| File | Description |
|---|---|
| CHANGELOG.md | Adds an Unreleased “Security” note explaining the yanked-crate audit failure and the lockfile-only fix. |
| Cargo.lock | Advances chacha20 from 0.10.1 (yanked) to 0.10.2 and updates the resolved transitive dependency set accordingly. |
Review details
- Files reviewed: 1/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
This is more urgent than the yanked-crate advisory it was opened for: it is also the fix for #155.
This PR's lock regeneration fixes exactly that. Verified at |
juan-malbeclabs
left a comment
There was a problem hiding this comment.
Verified, and it should go in as-is — but it is doing more than the title says.
The lock is correct (checked at head 91e8ec5)
- Every dependency reference resolves: 0 ambiguous refs, 0 orphan entries, walking the graph from all three workspace roots (
doublezero-edge-connect,shred-proxy,doublezero-edge). chacha20 0.10.1 → 0.10.2is the only version change in the entire lock. Everything else in the diff is the removal of six entries reachable from nothing plus the de-qualification of names they had made ambiguous.- The chain the changelog names checks out:
tokio-tungstenite 0.30 → tungstenite 0.30 → rand 0.10.2 → chacha20 0.10.2.chacha20is not a direct dependency and is not reachable any other way. cargo metadata --lockedandcargo build --workspace --release --lockedboth succeed on this lock; both fail onmain.
The one thing I would change: the changelog undersells it
This PR is also the fix for #155. main's lock has been unusable with --locked since the six Dependabot merges on 2026-09-07 — rand 0.9.4 and rand_chacha 0.9.0 name a bare "rand_core" while both rand_core 0.9.5 and 0.10.1 are locked — and the Dockerfile is the only place --locked is enforced. Every GHCR publish has failed since 2026-09-08: doublezero-edge-connect:mainnet-beta is still the 2026-09-07 build on the 0.38.0 client base, having missed doublezero v0.39.0 and the mainnet-beta-0.39.0-1 base image published on 09-10.
The six orphan removals in this diff are precisely what repairs that. The changelog entry is what ships, and "pin past a yanked crate" does not explain to a later reader why the image sat four days behind a release — worth a sentence, with the --locked repair framed as an effect rather than incidental diff noise.
Merge order
This one first. #157 (--locked in CI) is correctly red until it lands, and cargo-deny is red on every other open PR until it lands.
thijsvanemmerik
left a comment
There was a problem hiding this comment.
Approving. The lock file change is right, and it also fixes #155.
I checked it rather than read it. On main (e258968), cargo metadata --locked fails. On this branch (91e8ec5), it passes. I took a clean copy of main, ran cargo update -p chacha20, and got this branch's Cargo.lock byte for byte. So the file is what the resolver produces. Nobody hand edited it. The workspace builds, the tests pass, and cargo deny check goes from failed to ok. I also checked all 220 locked crates for yanks. This was the only one.
Two things in the write-up are wrong. Neither should hold up the merge. Fix them in #157, which already edits CHANGELOG.md. That way this branch keeps its SHA and the PRs stacked on it stay put.
1. chacha20 did not pull in the six crates this removes.
The Scope paragraph and the changelog both say the update drops the digest and generic-array chain that 0.10.1 pulled in. It did not. chacha20 0.10.1 depends on three things: cfg-if, cpufeatures 0.3.0 and rand_core 0.10.1. That is all.
The six crates that go away here (block-buffer 0.10.4, cpufeatures 0.2.17, crypto-common 0.1.7, digest 0.10.7, generic-array 0.14.7, version_check 0.9.5) were left behind by #85, which removed the last thing using them. Any re-resolve drops them. Leave chacha20 alone, re-resolve, and the same six still go. Of the 80 lines in this diff, the chacha20 bump is two.
Suggested wording: "the re-resolve also removes six orphan entries and two ambiguous bare rand_core refs left by the 2026-09-07 dependency merges, which is what makes the lock usable with --locked again (#155)."
2. 0.10.2 is not a republish.
It is a new patch release from 2026-08-27. It removes an SSE4.1 instruction from the SSE2 random number backend (RustCrypto/stream-ciphers#580). Upstream shipped it first, then yanked 0.10.0 and 0.10.1 about 40 minutes later. "the fixed 0.10.2" says it correctly.
One thing to watch after merging. This is not about the diff.
The :mainnet-beta and :testnet images running today were built from d67a4da. That is three commits behind main. So this merge does not rebuild the same image. It ships everything merged since then, all at once, to testnet, mainnet-beta and devnet in one run. That is 35 crate changes, including ed25519-dalek 2.2.0 to 3.0.0 and curve25519-dalek 4.1.3 to 5.0.0. src/shred/verify.rs uses that code to check shred signatures. It is also the first build on the doublezero 0.39.0 base.
None of that comes from this PR. This is just the first push that gets through. No CI job ever starts the container, so it is worth watching all three publish legs go green and booting the new image once.
The regeneration this PR makes for the yanked chacha20 also repairs a Cargo.lock that Cargo refuses with --locked, which had been failing every GHCR publish since 2026-09-08 and left :mainnet-beta on the 0.38.0 client base across the doublezero v0.39.0 release. The changelog entry is what ships, and "pin past a yanked crate" does not explain that to a later reader.
|
Pushed It says what actually went wrong — six Dependabot PRs merged unrebased, each lock correct against its own base, git merging them line by line into one Cargo refuses — and what it cost: every GHCR publish failing from 2026-09-08, with Nothing else moved; the lock is untouched. |
Every path that publishes a variant writes the same moving tag -- the push/tag/manual matrix in release.docker.edge-connect.yml, the doublezero-base-published dispatch, and the daily base poll -- so the tag ends up wherever the last push landed rather than at the newest commit. On 2026-09-11 two pushes to main three minutes apart inverted exactly that way: #151's testnet build finished at 14:03:20, ten minutes after with its own org.opencontainers.image.revision label reporting it. Only the dispatch workflow declared a group, and a workflow-level group serializes that workflow against itself -- never a dispatch against a poll or a publish. Its own NOTE said as much and called the gap benign because all three build the default branch's HEAD; two consecutive main pushes are the case that assumption does not cover. The lane moves to the reusable builder's `build` job, the one job that pushes, so all four callers share one lane per variant. The dispatch's group is removed rather than kept alongside: a run holding a group at workflow level while a job inside that run requests the same group would queue behind itself. Two builds get a lane of their own, because a newly queued run cancels the previously pending run of its group whatever cancel-in-progress says. A build that pushes nothing (the PR smoke test) takes a per-run lane, so it neither waits behind a publish nor evicts one from the queue; a release build takes a per-version lane, so a queued release cannot be cancelled by a later push to main, which would silently skip its pinned :<env>-X.Y.Z and :latest tags.
* ci(docker): serialize a variant's publishes in the job that pushes Every path that publishes a variant writes the same moving tag -- the push/tag/manual matrix in release.docker.edge-connect.yml, the doublezero-base-published dispatch, and the daily base poll -- so the tag ends up wherever the last push landed rather than at the newest commit. On 2026-09-11 two pushes to main three minutes apart inverted exactly that way: #151's testnet build finished at 14:03:20, ten minutes after with its own org.opencontainers.image.revision label reporting it. Only the dispatch workflow declared a group, and a workflow-level group serializes that workflow against itself -- never a dispatch against a poll or a publish. Its own NOTE said as much and called the gap benign because all three build the default branch's HEAD; two consecutive main pushes are the case that assumption does not cover. The lane moves to the reusable builder's `build` job, the one job that pushes, so all four callers share one lane per variant. The dispatch's group is removed rather than kept alongside: a run holding a group at workflow level while a job inside that run requests the same group would queue behind itself. Two builds get a lane of their own, because a newly queued run cancels the previously pending run of its group whatever cancel-in-progress says. A build that pushes nothing (the PR smoke test) takes a per-run lane, so it neither waits behind a publish nor evicts one from the queue; a release build takes a per-version lane, so a queued release cannot be cancelled by a later push to main, which would silently skip its pinned :<env>-X.Y.Z and :latest tags. * ci(docker): one publish lane per variant, with queue: max holding the releases Review caught the hole: giving a release tag its own lane (edge-connect-release-<env>-<version>) means it does not share a lane with a push to main — and a release build writes the moving tag beside its pinned :<env>-X.Y.Z, so the two can run together and leave :testnet wherever the slower one finished. That is the bug this change exists to remove, reintroduced one lane over. The comment's excuse for it was wrong too: a release tag is not necessarily main's HEAD, since main can advance after the tag is cut. So every build that pushes now shares one lane per variant, and `queue: max` makes that safe. The release lane existed because a group holds one run in progress and one pending, and a newly queued run cancels that pending one whatever cancel-in-progress says: a release publish waiting behind a build could be evicted by a later push to main and never publish its pinned tags. queue: max (GitHub, May 2026) lets up to 100 runs wait, FIFO by arrival at the lane, so nothing is dropped and the moving tag ends where the last arrival points. It cannot be combined with cancel-in-progress: true, which is not wanted here. Still not promised, and stated where the lane is: FIFO orders by arrival, not by commit, so a release tag cut on a commit that is not main's HEAD publishes the moving tag for that older commit. Ordering by commit means reading the tag's current org.opencontainers.image.revision before overwriting it, and that is worth writing the day this repo tags anything but HEAD. actionlint v1.7.12 is the newest release and predates the key, so .github/actionlint.yaml ignores that one message by its exact text. Verified that the ignore is that narrow: another unexpected key under concurrency still fails.
What
cargo deny check advisorieshas been failing onmain, and therefore on every open pull request:rand 0.10.2depends onchacha20directly, so nothing in this crate selected0.10.1and noCargo.tomlchange avoids it. The lockfile moves forward to the republished0.10.2.Why it is denied
A yank is not a disclosed vulnerability, and
deny.tomlsays so in its own words —yanked = "deny"sits under[advisories]deliberately. What a yank costs is the two things an audit exists to provide: a version its author withdrew has no upgrade path and no advisory coverage. This one is the RNG under the WebSocket stack, which is both the ingest backstop's client and the sink's server.The practical cost is separate and immediate:
cargo-denyis a required status check that runs unconditionally onpull_request(see the comment insecurity-audit.ymlexplaining why it is notpaths-filtered), so a red audit onmainis red on every unrelated PR.Scope
Lockfile only —
cargo update -p chacha20. The same resolution drops thedigest/generic-array/crypto-commonchain that0.10.1pulled in and collapses the duplicatecpufeaturesandblock-bufferversions, which is why the diff removes more than it adds.Verification
cargo deny check→advisories ok, bans ok, licenses ok, sources ok(wasadvisories FAILED).cargo build --all-targetsclean.cargo testgreen: 863 lib tests plus every integration target.