fix(security): use OsRng for all security-critical key and token generation

[gabehamilton]

Summary

Switches all security-critical key, token, and secret generation from rand::thread_rng() (userspace CSPRNG) to direct OS entropy via rand::rngs::OsRng.

While thread_rng() uses a ChaCha12 CSPRNG seeded from OsRng, using OsRng directly for key material is industry best practice: it eliminates the PRNG intermediary, removes seed state that could theoretically be extracted from memory, and makes the entropy source explicit in the code.

Files changed

File	What changed
src/secrets/keychain.rs	Master encryption key generation
src/secrets/crypto.rs	Per-secret HKDF salt generation; removed shadowing inner import
src/orchestrator/auth.rs	Per-job bearer token generation
src/channels/web/mod.rs	Gateway auth token fallback generation
src/cli/oauth_defaults.rs	PKCE verifier + CSRF state for CLI OAuth
src/tools/mcp/auth.rs	MCP OAuth PKCE verifier
src/extensions/manager.rs	Auto-generated extension/channel secrets
src/setup/channels.rs	Webhook secret generation (bonus find)
src/pairing/store.rs	Pairing auth code generation (missed in initial pass, fixed per review)

Intentionally unchanged

• src/llm/retry.rs — thread_rng() for jitter calculation (not security-critical)
• src/pairing/store.rs human-readable suffix fallback was originally left, now fixed per reviewer feedback

Token format note

The gateway fallback token (generated when GATEWAY_AUTH_TOKEN is not set) changed from a 32-character alphanumeric string (~190 bits of entropy from Alphanumeric distribution) to a 64-character lowercase hex string (256 bits of raw entropy). This is strictly better. Operators with external tooling that validates the token format or assumes a 32-character length should regenerate their token after updating.

Tests

• Added 3 regression tests for generate_salt(): correct length, non-zero output, uniqueness
• All 1,853 existing tests pass, zero clippy warnings

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[Copilot]

Pull request overview

This PR strengthens security-critical randomness by switching key/token/secret generation from rand::thread_rng() (userspace CSPRNG) to direct OS entropy via rand::rngs::OsRng across the codebase’s auth, secrets, and setup flows.

Changes:

• Use OsRng for PKCE verifiers + CSRF state generation in CLI OAuth flows and MCP OAuth.
• Use OsRng for cryptographic salt/key/token generation in secrets + orchestrator auth.
• Use OsRng for auto-generated extension secrets and webhook secret generation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/secrets/keychain.rs	Use OsRng for master key generation
src/secrets/crypto.rs	Use OS entropy for per-secret salt generation
src/orchestrator/auth.rs	Use OsRng for per-job bearer token bytes
src/channels/web/mod.rs	Use OsRng for gateway auth token fallback generation
src/cli/oauth_defaults.rs	Use OsRng for PKCE verifier + CSRF state
src/tools/mcp/auth.rs	Use OsRng for MCP PKCE verifier generation
src/extensions/manager.rs	Use OsRng for auto-generated extension/channel secrets
src/setup/channels.rs	Use OsRng for webhook secret generation helper

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In this module OsRng is already imported from aes_gcm::aead at the top of the file, but generate_salt() introduces a second OsRng via use rand::rngs::OsRng; that shadows the existing one. This makes it hard to tell which RNG type is being used in different methods (and can become problematic if rand_core versions diverge). Prefer using the already-imported aes_gcm::aead::OsRng for salt generation as well, or alias one of the imports (e.g., RandOsRng) to avoid shadowing and keep the RNG choice explicit.

[ilblackdragon]

Good mechanical fix — switching to OsRng across all security-critical callsites is the right move.

Two notes:

1. Missing callsite in src/pairing/store.rs: Lines 147 and 157 still use rand::thread_rng() for pairing code generation. These are security-relevant (channel authentication codes) and should also use OsRng.

2. src/llm/retry.rs:60 uses thread_rng() for jitter calculation — that's fine, it's not security-critical.

Otherwise looks ready to merge once the pairing store callsites are added.

[zmanian]

Review

The security reasoning is sound. While thread_rng() is a ChaCha12 CSPRNG seeded from OS entropy (not broken), using OsRng directly for key material is industry best practice — eliminates the PRNG intermediary and leaves no seed state to extract from memory. The scope is well-chosen: security-critical paths changed, non-security uses (jitter, human-readable codes) correctly left alone.

Required before merge

1. OsRng import shadowing in src/secrets/crypto.rs (Low)

The file already imports OsRng at the module level via aes_gcm::aead::OsRng. The PR adds a scoped use rand::rngs::OsRng inside generate_salt(). Both are re-exports of the same rand_core type today, but they come from different crate paths and could diverge. Remove the inner import and use the already-in-scope OsRng:

pub fn generate_salt() -> Vec<u8> {
    let mut salt = vec![0u8; SALT_SIZE];
    rand::RngCore::fill_bytes(&mut OsRng, &mut salt); // uses module-level import
    salt
}

Should fix

2. Acknowledge token format change — Gateway fallback token changed from 32-char alphanumeric (~190 bits) to 64-char hex (256 bits). This is strictly better entropy but changes the format. Worth noting in the PR description in case any tooling validates token format/length.

3. Regression tests — Per CLAUDE.md policy, the changed functions should have tests verifying output is non-zero and correct length (would catch a broken RNG). Or use [skip-regression-check] with justification.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Already fixed