Migrate rand 0.8 to 0.10, and test the two values it generates

[mattpodwysocki]

Ported from mapbox-cli-private#139, which can no longer merge there now that oss/ is a submodule (#132). CI was green there; re-verified here.

It exists because Dependabot tried this by moving the lockfile alone, which cannot work: Cargo.toml declared rand = "0.8", so a lock pinning 0.10.2 fails every --locked invocation with cannot update the lock file because --locked was passed. The private repo's dependabot.yml names this case exactly — "a semver-major step — clap 5, rand 0.9 — is a person's decision made in its own PR".

It removes six crates and adds none

The best argument for the migration, and I didn't expect it. The lock already carried rand 0.10.2, because quinn-proto needs it through reqwest — so rand = "0.8" was forcing a second, older copy alongside it. Migrating deduplicates onto the version already in the graph and drops 0.8's exclusive subtree:

Removed	rand@0.8.8, rand_chacha@0.3.1, rand_core@0.6.4, ppv-lite86@0.2.21, zerocopy@0.8.57, zerocopy-derive@0.8.57
Added	nothing
Packages	207 → 201

(.gitattributes marks *.lock -diff, so the lockfile shows as binary in review. I compared the two as name@version sets rather than reading the diff — worth knowing, because comm on the raw name lists gives a wrong answer here: rand appears twice in the old lock, and dedup makes the naive diff look like the dependency was deleted outright.)

There was no urgency behind it: the advisories check passes on 0.8.8, so nothing shipped is vulnerable. The 0.10.1 soundness fix and 0.10.2 memory-safety fix don't apply to the version we were on.

The code change is two renames

thread_rng() → rng(), and Rng::gen → random(). rand::random is a free function, so use rand::Rng goes away rather than becoming RngExt.

The randomness guarantee is unchanged, and now written down. Both values — the PKCE verifier and the OAuth state — draw from ThreadRng, which rand 0.10 declares TryCryptoRng: the same CSPRNG guarantee 0.8's thread_rng gave. I checked that in rand's source rather than assuming it, because it is the one property these two strings live or die by, and the comment now says so next to each.

Nothing tested either of them before

generate_pkce had no test at all, so the suite had nothing to say about migrating the crate that produces it. Four now:

• RFC 7636 §4.1 and §4.2 — 43 characters, and the challenge really is BASE64URL(SHA256(ASCII(verifier))) rather than a hash of the raw bytes behind the encoding.
• The unreserved alphabet only — both strings go into a query string unescaped, so a +, / or = would arrive meaning something else.
• Successive pairs differ, for both verifier and challenge.
• The state parameter — same generator; a predictable one is a CSRF hole rather than a cosmetic flaw.

I checked the important one fails for the right reason rather than trusting a green tick: replacing the generator with [7u8; 32] leaves the length and alphabet tests passing — a constant is perfectly well-formed and perfectly URL-safe — and only every_pkce_pair_is_different goes red. That is the botched migration a reviewer would otherwise have to catch by eye, and the reason the other three are not sufficient alone.

Verified

cargo clippy --locked --all-targets clean — the check the Dependabot attempt fails. 543 tests, fmt clean, and the 46 existing auth:: tests still pass.

[zmofei]

Small, well-scoped diff. Verified the Cargo.lock change is a genuine dedup (quinn-proto already pulled rand 0.10.2 via reqwest) removing 6 crates with nothing added. The two call-site renames preserve the CSPRNG guarantee (checked against rand's TryCryptoRng docs, not assumed). New tests cover generate_pkce and the OAuth state, previously untested — and the author validated the distinctness test actually catches a broken generator (verified with a fixed [7u8; 32] substitution). CI green. No issues found.