From 1286d0f98b4a513c67d835e9e2e5b46b0c718686 Mon Sep 17 00:00:00 2001 From: Suppaseth Charoenkarnka Date: Sun, 12 Jul 2026 23:09:57 +0700 Subject: [PATCH] fix(token): day_string epoch fallback + rollup scan returns error not panic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-merge Copilot round-3 findings on v3.4.12: - token_gain::day_string still fell back to Utc::now() for an out-of-range ts — the exact behavior the new rollup utc_or_epoch avoids. Match it (Unix EPOCH) so both agree; a corrupt ts buckets under 1970, not today. - rollup::scan used .expect() on a malformed key → process crash on an externally corrupted redb. Return a new RollupError::MalformedKey instead (the fn already returns Result). Add a test. --- .../onebrain-cli/src/commands/token_gain.rs | 7 +++-- crates/onebrain-token/src/gain/rollup.rs | 27 ++++++++++++++++++- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/crates/onebrain-cli/src/commands/token_gain.rs b/crates/onebrain-cli/src/commands/token_gain.rs index ca0951a..2883eb7 100644 --- a/crates/onebrain-cli/src/commands/token_gain.rs +++ b/crates/onebrain-cli/src/commands/token_gain.rs @@ -72,8 +72,11 @@ fn write_reset_marker(gdir: &Path, marker: &ResetMarker) -> Result<()> { } fn day_string(ts: i64) -> String { - let dt = - chrono::DateTime::::from_timestamp(ts, 0).unwrap_or_else(chrono::Utc::now); + // Fall back to the Unix EPOCH (not the current time) for an out-of-range + // `ts`, matching `onebrain_token`'s rollup `utc_or_epoch`: a corrupt value + // buckets under 1970 (self-flagging) instead of silently under today. + let dt = chrono::DateTime::::from_timestamp(ts, 0) + .unwrap_or(chrono::DateTime::::UNIX_EPOCH); format!("{:04}-{:02}-{:02}", dt.year(), dt.month(), dt.day()) } diff --git a/crates/onebrain-token/src/gain/rollup.rs b/crates/onebrain-token/src/gain/rollup.rs index f63fcfe..13b4a3a 100644 --- a/crates/onebrain-token/src/gain/rollup.rs +++ b/crates/onebrain-token/src/gain/rollup.rs @@ -47,6 +47,8 @@ pub enum RollupError { Io(#[from] std::io::Error), #[error("serde error decoding a rollup value: {0}")] Serde(#[from] serde_json::Error), + #[error("malformed rollup key (external corruption?): {0}")] + MalformedKey(String), } /// Aggregated counters for one `(period, surface, transform, level, @@ -252,7 +254,11 @@ pub fn scan( let mut out = Vec::new(); for row in t.iter()? { let (k, v) = row?; - let key = parse_key(k.value()).expect("this module only ever writes well-formed keys"); + // This module only ever writes well-formed keys, but an externally + // corrupted redb shouldn't crash the process — surface it as an error + // the caller can handle (the fn already returns `Result`). + let key = + parse_key(k.value()).ok_or_else(|| RollupError::MalformedKey(k.value().to_string()))?; let value: RollupValue = serde_json::from_str(v.value())?; out.push((key, value)); } @@ -276,6 +282,25 @@ mod tests { assert_eq!(year_key(i64::MIN), "1970"); } + #[test] + fn scan_returns_malformed_key_error_not_panic_on_corrupt_key() { + // An externally corrupted redb (a key that isn't a 5-segment composite) + // must surface as a RollupError, never a process-crashing panic. + let dir = tempdir().unwrap(); + let db = Database::create(dir.path().join("token.redb")).unwrap(); + ensure_tables(&db).unwrap(); + { + let w = db.begin_write().unwrap(); + { + let mut t = w.open_table(GAIN_DAILY).unwrap(); + t.insert("corrupt-key-no-colons", "{}").unwrap(); + } + w.commit().unwrap(); + } + let err = scan(&db, GAIN_DAILY).unwrap_err(); + assert!(matches!(err, RollupError::MalformedKey(_)), "got {err:?}"); + } + fn event(ts: i64, surface: Surface, transform: &str, before: u64, after: u64) -> GainEvent { GainEvent { ts,