Commit Cargo.lock; constant-time API-key lookup - #1
Merged
Conversation
Two hardening fixes surfaced during a code review: - Track Cargo.lock. This crate ships binaries (stateset-icp-handler, icp-conformance, icp-mcp-stdio), so the lockfile belongs in version control for reproducible CI/Docker builds. The pinned stateset-embedded rev fixed direct deps, but transitive deps floated -- contradicting the reproducibility note in Cargo.toml. - Replace HashMap::get in ApiKeyStore::lookup with a no-early-exit constant-time comparison over all stored keys, so request latency is not a comparison oracle for the bearer token. Mirrors the existing webhook HMAC check (webhook::signing::constant_time_eq) for defense-in-depth. Build, clippy (-D warnings), auth_enforcement, and integration suites pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two hardening fixes from a code review of the handler.
1. Track
Cargo.lockThis crate ships binaries (
stateset-icp-handler,icp-conformance,icp-mcp-stdio), so the lockfile belongs in version control for reproducible CI/Docker builds. The pinnedstateset-embeddedrev fixed direct deps, but transitive deps floated — which contradicted the "keeps CI/Docker builds reproducible" note inCargo.toml. RemovedCargo.lockfrom.gitignoreand committed the resolved tree.2. Constant-time API-key lookup
ApiKeyStore::lookupusedHashMap::get, making request latency a (weak) comparison oracle for the bearer token. Replaced it with a no-early-exit constant-time comparison over all stored keys, mirroring the existing webhook HMAC check (webhook::signing::constant_time_eq) for defense-in-depth. Keys are high-entropy, so impact was low, but this removes the oracle entirely and keeps the auth path consistent with the webhook path.Verification
cargo build✅cargo clippy -D warnings✅ (clean)cargo test --test auth_enforcement✅ (5 passed)cargo test --test integration✅ (31 passed)🤖 Generated with Claude Code