Repository navigation
chore: bump rand to 0.8.6 - #243
Conversation
Fixes GHSA-cq8v-f236-94qc (low): rand's ThreadRng could be unsound when a custom logger accessed rand::rng() during a reseed, casting a raw pointer to an aliased mutable reference (undefined behavior under Stacked Borrows). rand is only reached transitively, at compile time, through the phf_macros proc macro used to build swc's ecma AST tables; it is not part of the wasm build's runtime dependency graph.
|
|
avocet-bot
left a comment
There was a problem hiding this comment.
Review: #243 — "chore: bump rand to 0.8.6"
Reviewed head SHA: de2c7acf90ee44c58267d4486bd3887fd59c6f9e (confirmed equal to headRefOid at review time — no drift)
Author: Bert Belder (piscisaureus) · Branch: deps/security-bumps → main
Verdict: APPROVE — merge once CI (build-javascript) is green and the CLA is satisfied.
Background: what this project is and what the PR touches
eszip is Deno's library/CLI for serializing a JavaScript/TypeScript module graph — every module a program transitively imports — into a single flat, streamable archive so it can be loaded without re-resolving imports. To do this it depends on swc, the Rust-based ECMAScript parser/transformer. Crucially for this review, the crate ships a WebAssembly build; what matters is which dependencies end up in the runtime graph versus which are only used at compile time.
This PR is a lockfile-only dependency bump. The entire change is a single Cargo.lock hunk (2 additions / 2 deletions):
name = "rand"
-version = "0.8.5" +version = "0.8.6"
-checksum = "34af8d1a…49d09" +checksum = "5ca0ecfa931c29007047d1bc58e623ab12e5590e8c7cc53200d5202b69266d8a"
dependencies = [ "rand_core" ]
There is no Cargo.toml change. rand is not a direct dependency of eszip; it is reached transitively at compile time through the phf_macros proc-macro that swc uses to build its ECMAScript AST lookup tables. As the PR body notes, it is therefore not part of the crate's wasm runtime output. The dependencies list for the rand entry is unchanged (still just rand_core), so the transitive dependency graph shape does not change.
Why the bump is being made
The bump targets GHSA-cq8v-f236-94qc (severity: low): rand's ThreadRng could be unsound when a custom logger accessed rand::rng() during a reseed, casting a raw pointer to an aliased mutable reference (undefined behavior under Stacked Borrows). The advisory's vulnerable range is >= 0.7.0, < 0.8.6, and its first patched version is exactly 0.8.6 — so this bump lands on the minimal fix for the 0.8.x line. Since eszip only pulls rand in at compile time via a proc-macro, the practical exposure to this UB in eszip itself is negligible, but keeping the lockfile off the advisory range is still correct hygiene.
Verification performed
The single failure mode that could make a lockfile-only bump un-mergeable is a version that does not exist or whose recorded checksum does not match the registry — cargo would then reject the lockfile. Both were checked directly against the crates.io sparse index (https://index.crates.io/ra/nd/rand):
- rand 0.8.6 exists and is not yanked. The index lists
0.8.6withyanked=false. (This was the a-priori risk, since 0.8.5 was for a long time the latest 0.8.x — but 0.8.6/0.8.7/0.8.8 now exist.) - Checksum matches byte-for-byte. The index
cksumfor 0.8.6 is5ca0ecfa931c29007047d1bc58e623ab12e5590e8c7cc53200d5202b69266d8a— identical to the value written intoCargo.lock. The lock entry is valid and reproducible. - Semver-compatible patch bump. 0.8.5 → 0.8.6 is a patch release within the same 0.8.x minor line; no breaking API changes, and any upstream
^0.8/0.8constraint permits it without a manifest edit. - No transitive graph change. The
randlock entry still depends only onrand_core, unchanged.
Findings
No correctness, regression, security, concurrency, or edge-case issues found in the diff. The change is minimal, valid, and reproducible.
- Missing tests: Not applicable — a lockfile checksum/version bump has no behavior to test; correctness is established by registry verification (done above) and CI compilation.
- CI status (informational, non-blocking): at review time
build-javascripthad one prior passing run and two pending runs;license/clawas pending ("Contributor License Agreement is not signed yet"). Neither indicates a defect in the diff — the CLA is a process gate (author is a Deno core member) and CI simply had not finished. Recommend lettingbuild-javascriptgo green before merge, but there is no code reason to expect failure.
Conclusion
Clean, minimal, valid security patch bump. rand 0.8.6 exists, is unyanked, and its published checksum matches the lockfile exactly; the referenced advisory is genuine and 0.8.6 is its designated fix. Safe to approve; merge once CI is green and the CLA is signed.
Review skill evidence
- Invoked the
pr-review-toolkit:code-reviewerClaude Code subagent (via the Task tool) as the primary review pass; it independently gathered PR metadata, the diff, CI status, and verified the advisory and crates.io checksum. - Independently re-verified rand 0.8.6's existence, unyanked status, and checksum against the crates.io sparse index outside the subagent, and confirmed the live
headRefOidequals the required head SHAde2c7acf90ee44c58267d4486bd3887fd59c6f9eimmediately before saving/posting.
rand0.8.5 -> 0.8.6 -- fixes unsoundThreadRngbehavior whena custom logger accesses
rand::rng()during a reseed, casting araw pointer to an aliased mutable reference
(GHSA-cq8v-f236-94qc, low)
randis only reached transitively, at compile time, through thephf_macrosproc macro used to build swc's ecma AST tables; it isnot part of this crate's wasm build output.