Skip to content

fix: validate shared breaker records - #58

Closed
albertovincenzi wants to merge 1 commit into
fix/saturating-eta-arithmeticfrom
fix/validate-breaker-records
Closed

fix: validate shared breaker records#58
albertovincenzi wants to merge 1 commit into
fix/saturating-eta-arithmeticfrom
fix/validate-breaker-records

Conversation

@albertovincenzi

Copy link
Copy Markdown
Collaborator

Problem

Breaker records live in the shared broker namespace and were deserialized without validating the invariants Gate applies when it creates them. A malformed or future record could carry a negative or enormous retryAfterSeconds value. Three read paths then computed at + retryAfterSeconds * 1000 with ordinary signed arithmetic: debug builds can panic, release builds can wrap to a false deadline, and the console may report corrupt state as a live breaker.

Fix

  • Accept shared breaker records only when at is non-negative and retryAfterSeconds remains inside Gate's 1..=3600 contract.
  • Centralize deadline calculation in Record::until_ms using saturating multiply and add.
  • Use that calculation in the trip response, graph detail, recent-breaches view, and ETA explanation.
  • Ignore malformed shared records instead of allowing broker data to crash or poison read-only endpoints.

Verification

Unit tests cover a deadline at the i64 boundary and reject zero, oversized, and negative-timestamp records.

  • cargo test --workspace
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo fmt --all -- --check

Review / dependency

This PR is stacked on #44 because it consolidates the ETA overflow guard introduced there into the Record method. Review commit a5d5f9e relative to fix/saturating-eta-arithmetic. Merge order: #44, then this PR.

@alice-viola

Copy link
Copy Markdown
Contributor

Landed on master via #67 (merge commit 944ee9b) as part of the 62-PR integration — this PR's head commit a5d5f9e is an ancestor of master. GitHub could not mark it merged automatically because its base is fix/saturating-eta-arithmetic, not master. Closing as landed.

@alice-viola alice-viola closed this Sep 7, 2026
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.

2 participants