Route enclave attestation nonces through the checked entropy helper

[kwsantiago]

Summary

Four attestation-nonce sites in the enclave commands drew from a thread RNG rather than the core entropy helper. This finishes what #922 started for the hardware signing path.

The nonce at the fourth site is the one later passed to attestation verification as the anti-replay binding, so it is the most load-bearing of the four. As with #922, the defect is not weak randomness, since a thread RNG is a CSPRNG. It is that these draws bypassed the entropy health check that fails closed on a degraded source.

Also removes a claim from the entropy module's own documentation stating that all production callers had been migrated to the checked helper. That was not true while these four existed, and a doc comment asserting a completed migration is worse than no comment: it invites the next reader to skip exactly the audit that turned these up. Its replacement says only what the function is for.

All four enclosing commands already returned a fallible type, so the change is a direct substitution with no signature changes. The now-unused rand::Rng import is dropped; rand remains a live dependency of the crate.

After this there are no non-health-checked randomness draws left in keep-cli outside tests. The one remaining reference to the old call is a comment explaining why the helper is used instead.

Test plan

cargo build, cargo clippy --all-targets -- -D warnings, and cargo fmt --check clean across keep-cli and keep-core. cargo test -p keep-cli passes: 136 unit and 41 integration tests.

Verified by grep that no rand::rng() or thread_rng() call sites remain in keep-cli production code.

No behavioral test is added, for the same reason as #922: each site produces 32 random bytes before and after, and the only observable difference is whether a degraded RNG surfaces as an error rather than passing silently. Asserting that needs a seam for injecting a failing entropy source, which the helper does not currently provide.

Follow-up

Nothing mechanically enforces this policy, so the next direct RNG call will need to be caught by hand again. A clippy disallowed-methods entry would close that, but it would need to distinguish production paths from tests, fuzz targets, and the FROST library's own OsRng usage, so it is filed separately rather than bundled here.

Part of #788.

[coderabbitai]

Warning

Review limit reached

@kwsantiago, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8986c82c-4f72-452a-b31c-09e555f5521d

📥 Commits

Reviewing files that changed from the base of the PR and between b8bef42 and cb70422.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (3)

• keep-cli/Cargo.toml
• keep-cli/src/commands/enclave.rs
• keep-core/src/entropy.rs

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kwsantiago]

Folded in the two review items that belonged here.

The rand dependency is dropped. This PR removed the last reference to it in the crate, so leaving it declared would keep supply-chain surface for nothing, and nothing in the toolchain warns about an orphaned dependency: no compiler warning, and no unused-dependency check in CI, so it would have sat there indefinitely.

Nonces are now drawn before the client is constructed at all four sites, matching the ordering the hardware signing path uses. The review confirmed nothing actually leaks on the old ordering, since the vsock client is a value constructor and the mock's database lock releases on drop, so this is about the two paths agreeing rather than a fix. The comment explaining why is carried over so the ordering does not get undone later as a cosmetic tidy-up.

Not folding in the remaining finding: eight production callers of the panicking random_bytes still exist across other crates, including the bunker transport secret key and connect secret, which are the two that matter most. That is real and is tracked, but it spans four crates and one of them needs a signature change, so it is not something to bury in a PR about the enclave commands.

Also confirmed and deliberately unchanged: the FROST round-one commit in the CLI passes OsRng directly, and should keep doing so. That is the library's own API, the RFC derives the nonce by hashing the random input together with the signing share rather than trusting it alone, and every other commit site in the workspace does the same. Changing only this one would buy inconsistency and no security.