Skip to content

Prune/TTL maintenance (delivery.prune_completed, store_forward.prune_expired, outbound.drain_expired) is never invoked in production #244

Description

@forkwright

Finding

The retention/TTL maintenance methods that bound the delivery, store-forward, and outbound structures are dead code in every production path — they are called only from their own modules' unit tests. The router flush loop (run_router_flush) calls only process_timeouts() and next_to_send(), neither of which invokes any of them. As a result:

  • DeliveryTracker.records is never pruned, so Acknowledged/Failed/Expired records are never released; worse, prune_completed unconditionally retains all Queued/Sent records regardless of age, so records for lost packets (never ACK'd, never caller-expired) also accumulate forever.
  • StoreForward.queues messages live until drain_for is called; if a destination never returns, messages persist in memory and the per-destination cap stays saturated, rejecting new inserts. TTL is therefore not the governing policy.
  • OutboundQueue expired entries are never drained.

Evidence

crates/kerykeion/src/delivery.rs:199 — the only release path for completed records, never called outside tests, and active records are kept unconditionally:

pub fn prune_completed(&mut self, max_age: std::time::Duration) {
    let now = Instant::now();
    self.records.retain(|_, record| {
        match &record.status {
            DeliveryStatus::Acknowledged { .. }
            | DeliveryStatus::Failed { .. }
            | DeliveryStatus::Expired => now.duration_since(record.created) < max_age,
            // Keep active records.
            DeliveryStatus::Queued | DeliveryStatus::Sent { .. } => true,
        }
    });
}

crates/kerykeion/src/store_forward.rs:103 — TTL enforcement that no production path calls:

pub fn prune_expired(&mut self, now_ms: u64) {
    for queue in self.queues.values_mut() {
        queue.messages.retain(|msg| {

crates/kerykeion/src/outbound.rs:215pub fn drain_expired defined; the only callers are store_forward.rs:261 (test) and outbound.rs:403 (test). No call site in collector.rs, router.rs, or run_router_flush.

Why this matters

On a long-lived collector on a busy mesh, every one of these maps grows monotonically — a steady-state memory leak even with no adversary. Under adversarial pressure each becomes an unbounded sink the attacker can drive at the rate they choose. The store-forward effect is doubly harmful: because TTL never fires, a long-offline (or spoofed-unreachable) destination keeps its queue saturated and silently rejects further legitimate messages, degrading delivery during precisely the conditions the store-forward path exists to handle.

Desired correction

Invoke all three maintenance passes from the router flush tick (run_router_flush), which already runs periodically:

  • Call delivery.prune_completed(max_age) each tick with a configured max_age, and add a TTL transition so a Sent record without ACK moves to Expired after max_age instead of remaining active forever; consider an LRU capacity backstop.
  • Call store_forward.prune_expired(now_ms) each tick with current wall-clock milliseconds.
  • Call outbound.drain_expired() on the same tick for consistency.

Done when: run_router_flush calls prune_completed, prune_expired, and drain_expired at least once per flush interval in production code; Sent records older than the configured TTL transition to Expired; and integration tests confirm none of the three maps grows unboundedly over many tracked/stored packets and that elapsed-TTL messages are removed without requiring drain_for.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions