Repository navigation
test: flare-vault: cheap Argon2 KDF params under test (8-25s per vault test today) - #814
Conversation
… Argon2 cost flare-vault's KdfParams::default() ran full Argon2id (3/64MiB/4 lanes) in every test that touched a vault, costing 8-25s per test on CI. Test builds (cfg(test), or the new insecure-fast-kdf feature enabled from the root crate's dev-dependencies) now derive keys with the cheapest Argon2 accepts. The feature can't reach a release build: lib.rs raises a compile_error! if it's enabled without debug_assertions. Production defaults are pinned by a test so a future change is deliberate. gateway_secrets.rs's separate PBKDF2-HMAC-SHA256 KDF in auth_crypt.rs (600k iterations) was the other hidden cost behind the slow gateway_secrets/migration tests; it now also drops to 1 iteration under cfg(test), with the production iteration count pinned the same way. Agentflare-Agent: claude-code Agentflare-Branch: task/652-flare-vault-cheap-argon2-kdf-params-unde Agentflare-Item: 652 Agentflare-Session: 3cb04fb0-5821-4d74-a3fe-f33d6e2a718c
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 42 minutes. View limit detailsLimit details: You’ve used all 3 included reviews currently available. Your 46 included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Repository: getappz/agentflare/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (5)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: getappz/agentflare/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (5)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. 📝 WalkthroughWalkthroughTest builds can use lower KDF costs for vault and gateway-secret tests. Production KDF settings remain explicit, and the vault fast-KDF feature is rejected in release builds. ChangesTest KDF parameters
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other Merge Risk: 🔵 Low · up to Production release builds keep full Argon2 costs. However, some developer builds of the CLI can silently create a vault with weak key-derivation costs. Because vault files do not record their costs, a later production build cannot open such a vault. This is a bounded developer-workflow hazard and is reasonable to address before or soon after merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58dd97a9be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[cfg(all(feature = "insecure-fast-kdf", not(debug_assertions)))] | ||
| compile_error!( | ||
| "flare-vault feature `insecure-fast-kdf` is test-only and must not be enabled in a release build" | ||
| ); |
There was a problem hiding this comment.
Running cargo test --release -p agentflare now fails before executing any tests: the root package's dev-dependency enables insecure-fast-kdf, while Cargo's --release option “Build[s] artifacts in release mode,” disabling debug_assertions and triggering this compile_error!. This prevents optimized test runs that can expose release-only behavior; the guard or test-feature plumbing should distinguish a release test artifact from a distributable release binary.
Useful? React with 👍 / 👎.
💡 Codex Reviewagentflare/crates/flare-vault/src/lib.rs Lines 6 to 9 in 58dd97a Allow release-mode tests to use the test KDF
Running ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Hakari verified clean. Final status:
Done. Committed as
68a4acfontask/652-flare-vault-cheap-argon2-kdf-params-unde.What changed:
flare-vault:KdfParams::default()now uses cheap Argon2 params (1/16KiB/1) undercfg(test)or the newinsecure-fast-kdffeature; production stays3/65536/4, pinned by a test.lib.rshas acompile_error!if the feature is on withoutdebug_assertions, so it can't reach a release build. Root crate'sCargo.tomlenables the feature only in[dev-dependencies].src/auth_crypt.rs(used bygateway_secrets.rs, and transitively byvault.rsmigration tests) runs its own PBKDF2-HMAC-SHA256 KDF at 600,000 iterations, entirely separate from flare-vault's Argon2. This — not flare-vault — was whygateway_secrets::tests::*(12-14.6s) andvault::tests::migrates_legacy_gateway_secret_on_first_unlock/migration_does_not_overwrite_an_existing_vault_secretstayed slow even after the flare-vault fix. Now drops to 1 iteration undercfg(test)(same-crate, so no feature-flag plumbing needed), production count pinned by a test.dead_code/assertions_on_constantsclippy break the prior round'sFAST_KDFconst introduced (removed it, simplified the test that used it).Verified (foreground):
cargo fmt --check— cleancargo clippy -p flare-vault --all-targets --all-features -D warnings -A unsafe_code -A clippy::pedantic— cleancargo clippy -p agentflare --all-targets --all-featuressame flags — clean except one pre-existing, unrelated dead-code error insrc/agent_launch_tests.rs(from master commit7731882, not touched by this diff — full-workspace clippy will still fail until that's fixed separately)cargo hakari generate --diff/verify— no diff neededcargo nextest run -p flare-vault: 25/25 pass, all <0.4s (was ~8.3-8.6s)cargo nextest run --bin agentflare vault:: gateway_secrets:: auth_crypt::: 14/14 pass, all <1s, worst case 0.974s (concurrency test, not KDF-bound) — was up to 25.5sConcerns:
C:drive hit 0 bytes free mid-task from accumulated worktreetarget/dirs (other worktrees at 5.3/4.9/3.2 GB). I freed space bycargo clean-ing only this worktree's own target and clearing the regenerable~/.cargo/registry/srccache — didn't touch other worktrees/agents' data. Worth a broader disk-hygiene pass across.worktrees/*separately.cargo clippy --workspace --all-targets --all-featuresstill fails on the pre-existingfast_stalldead-code issue insrc/agent_launch_tests.rs; out of this task's scope but blocks the literal gate command as written.Opened by
claude-codeon flared:c997d745ae66 for item #652 via agentflare.Summary by CodeRabbit