Skip to content

fix(cli): avoid panic on oversized --older-than durations - #1705

Merged
newhoggy merged 1 commit into
mainfrom
issue-1691-parse-since-panic
Sep 17, 2026
Merged

newhoggy merged 1 commit into
mainfrom
issue-1691-parse-since-panic

Conversation

@newhoggy

Copy link
Copy Markdown
Contributor

Summary

  • parse_since (src/cli/log/query.rs) used the panicking chrono::Duration::seconds/minutes/hours/days/weeks constructors and Utc::now() - dur, so an oversized but i64-valid --older-than crashed the process instead of returning the documented "invalid --older-than" error.
  • Switches all five units to their try_* fallible constructors and Utc::now() - dur to checked_sub_signed, mapping any overflow to an anyhow context error. Widened beyond the two reproduced shapes (d/w) to all units, since s/m/h can also overflow internally (e.g. Duration::minutes(i64::MAX) multiplies by 60).
  • Affects both omni-dev log prune and omni-dev drive lease prune, which both call this shared helper.

Fixes #1691

Test plan

  • cargo test --lib cli::log::query — 25 passed, including new since_rejects_oversized_duration covering both overflow shapes from the issue
  • cargo clippy --all-targets -- -D warnings — clean
  • cargo fmt --check — clean
  • No CLI surface change, so no snapshot update needed

@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown

Coverage

Total: 97.28% ⚪ 0 pp vs main

Comparing f1fd72e..7f6c226 (merge-base → PR head)

No per-file coverage changes vs main.

🔇 5 ignored region(s), 0 tolerated region(s)

ignore removes the lines from both reports; tolerate keeps them in the reported percentage but scores them against the baseline, so a cross-run flip cannot move a delta. Regions are read from each revision's own source.

File Kind Lines Rev Reason
src/cli/drive/lease.rs ignore 408-412 both a sheet restore's fresh backup is always a DriveCopy — restore.rs's own recheck refuses unless the target is still a spreadsheet, and acquire() only ever takes a Bytes backup of a non-native target; this arm exists solely for exhaustiveness over the shared LeaseBackup enum
src/cli/drive/lease.rs ignore 882-885 both guards this test helper against misuse; every call site below passes an acquire subcommand
src/cli/drive/lease.rs ignore 896-899 both guards this test helper against misuse; every call site below passes a prune subcommand
src/drive/lease/ledger.rs ignore 666-670 both mutate_locked refuses before ever calling the closure, so its body never runs — a hit here is a regression, not a coverage gap
src/drive/lease/restore.rs ignore 977-981 both every test using this double refuses before authenticating; a hit here is a regression, not a coverage gap

Patch coverage

Patch: 100% (13/13 new lines covered)

File Patch Uncovered new lines
src/cli/log/query.rs 100% (13/13)

📦 Full per-file coverage summary · run summary

parse_since used the panicking chrono::Duration constructors and a
plain DateTime subtraction, so an i64-valid but oversized --older-than
(e.g. 100000000d or 9999999999999999w) crashed the process instead of
returning the "invalid --older-than" error both `omni-dev log prune`
and `omni-dev drive lease prune` already wrap around it. Switch to the
checked try_* constructors and checked_sub_signed, and cover both
overflow shapes (construction and subtraction) with a test.

Fixes #1691
@newhoggy
newhoggy force-pushed the issue-1691-parse-since-panic branch from b40d9cf to 7f6c226 Compare September 17, 2026 15:49
@newhoggy
newhoggy merged commit 31587d8 into main Sep 17, 2026
18 checks passed
@newhoggy
newhoggy deleted the issue-1691-parse-since-panic branch September 17, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(cli): parse_since panics on oversized --older-than in log prune and drive lease prune

1 participant