Stop the rate-limit tests racing the derivation they trigger

[kwsantiago]

Summary

build (macos-latest) failed on main with test_rate_limiting_outer asserting that an unlock attempt was refused when it was not. The job does not run on pull requests, so this only appears after merge.

Not a regression from the commit it failed on, which touched only keep-mobile while this test lives in keep-core. It is a timing race the test has always had.

The backoff is measured from the last recorded failure and doubles past the free allowance, so one, two, four seconds and so on. Each attempt costs an Argon2 derivation, and that cost is constant. The test spent its five free attempts, then made three rapid ones and asserted the third was refused, at which point the required wait was four seconds. On a runner where a single derivation takes longer than that, the window has already elapsed by the time the next attempt arrives, every attempt is allowed, and the assertion fails. The existing comment anticipated exactly this and picked a fixed number of attempts, which cannot be enough for an arbitrarily slow machine.

Both affected tests now keep attempting until one is refused, bounded so a real regression fails rather than loops. This is not a weaker assertion: the delay grows exponentially while the derivation cost does not, so the backoff overtakes it within a couple of attempts on any machine. It removes the dependency on the runner being fast rather than papering over the failure.

The combined-unlock test in the same module shares the shape but asserts differently and was already passing; it is left alone rather than changed speculatively.

Test plan

All three rate-limit tests pass locally, and the full cargo test -p keep-core --lib suite is clean at 386 tests. cargo clippy --all-targets -- -D warnings and cargo fmt --check clean.

Falsified rather than assumed: with the rate-limit check stubbed to always allow, all three fail. So the loop still catches the defect the test exists for; it just no longer races the derivation.

The original failure cannot be reproduced on this machine, because reproducing it requires an Argon2 derivation slower than the four-second window. The mechanism is established from the backoff computation and the failing assertion rather than from a local repro, and the fix removes the dependency on that timing entirely rather than tuning it.

Summary by CodeRabbit

• Tests
  • Improved rate-limiting tests to avoid timing-dependent failures.
  • Added bounded repeated-attempt checks for both outer and hidden storage scenarios.
  • Clarified failure messages when rate limiting does not activate.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1243c6d5-f5ed-42a8-b712-384b66deee78

📥 Commits

Reviewing files that changed from the base of the PR and between 88dd3bc and 56fdf87.

📒 Files selected for processing (1)

• keep-core/src/hidden/volume.rs

---

Walkthrough

Rate-limiting tests for outer and hidden volumes now use bounded retry loops and assert that at least one unlock attempt returns KeepError::RateLimited, removing fixed timing-dependent attempt expectations.

Changes

Rate-limit test stabilization

Layer / File(s)	Summary
Bounded unlock assertions keep-core/src/hidden/volume.rs	Outer-volume and hidden-volume tests repeatedly attempt unlocks on a single storage instance and verify that rate limiting engages.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

• privkeyio/keep#13: Adds related unlock-flow rate limiting and test coverage in the same module.
• privkeyio/keep#538: Covers related HiddenStorage rate-limiting behavior and audit flushing.

Poem

A bunny retries by moonlit light,
Until rate limits do their right.
No ticking clock, no fixed decree,
Just bounded hops prove refusal can be.
“Robust tests!” the rabbit sings with glee.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly captures the main change: making rate-limit tests avoid racing the derivation they trigger.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-rate-limit-test-flake

---

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.}