Repository navigation
Judge mobile policy time windows at the stored time floor too - #1118
Conversation
The allowed-hours and allowed-days rules read Utc::now() directly, bypassing the clock every other validity decision is judged at. They now take the caller's PolicyClock and require both the device reading and the floored reading to fall inside the window, so a clock set back behind a later time the app has already seen is refused. A spend recorded for the daily and weekly limits is now dated at the floored time rather than the device clock. Dated by a clock set back, it left the limit window early once the clock was put right, and the limit could be spent again. The rule docs now say what these rules are worth against whoever holds the device. The floor never rises past the clock and does not follow real time across a reboot, and a clock set forward is not caught at all, so the time rules are a convenience until there is a time source the holder cannot set.
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Evaluate time windows keep-mobile/src/policy.rs |
Hour and day restrictions check both the supplied clock and the effective floored time. Unrepresentable readings deny evaluation. Tests cover window boundaries, rollback, and overnight ranges. |
Record spends with explicit timestamps keep-mobile/src/lib.rs, keep-mobile/src/policy.rs, keep-mobile/src/velocity.rs |
KeepMobile::record_policy_transaction selects the spend timestamp and returns InvalidPolicy if the floor is unusable while a policy is configured. Policy recording passes that timestamp to VelocityTracker::record_at. Tests cover floor-based timestamps and refusal to record when the floor is unusable under a policy. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Merge Risk: 
Minimal · up to dfd9a
-
No identified issue prevents merging after normal checks.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed -
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
I’m a rabbit, and I check the clock,
Two readings keep each window in view.
A floored timestamp marks each spend,
The tracker saves the moment true.
I twitch my nose and hop away.
Comment @coderabbitai help to get the list of available commands.
What
PolicyClockthe caller establishes, notUtc::now(). Both the device reading and the floored reading have to fall inside the window.PolicyRulesnow say what the time rules are worth against whoever holds the device.Why
The time-window rules were the one place in
evaluatethat read the wall clock directly, so they saw a different time from the expiry check and ignored the stored floor entirely. A clock set back behind the floor is now refused.Velocity spends were stamped by the device clock. A spend made with the clock set back was dated early and left the 24-hour window early once the clock was put right, so the daily limit could be spent twice in a short time. Totals are still measured on the device clock, which is the reading that can only count more.
Limits
Without a time source the holder cannot set, these rules are a convenience against mistakes rather than a control against the holder, and the docs now say so. Setting the clock forward is not detected. The floor's own limits are documented on
allowed_hours.Tests
record_policy_transaction, and an unusable floor under a policy refuses the recording.cargo test -p keep-mobile(548 tests) and workspace clippy with the CI features pass.Summary by CodeRabbit