Files
ruvnet--RuView/v2/crates/homecore/src/bus.rs
T
rUv bf1dfe79fd fix(homecore core): TOCTOU race dropped/reordered state_changed events under concurrent writers (~93k→0) + 2 fail-closed hardenings (#1087)
* fix(homecore): atomic state set — close TOCTOU lost/reordered state_changed events

StateMachine::set did get() (release shard lock) → compute next + no-op
decision → insert() (re-acquire lock) → send(). The read-modify-write was
not atomic w.r.t. a concurrent writer on the same entity: a writer that
read a stale `old` could mis-classify a real transition as a no-op and drop
its state_changed event (a missed automation trigger) or fire an event whose
new_state duplicated the previously delivered one (a spurious trigger for any
automation keyed on old_state != new_state). ADR-127 §2.1 promises "writer
atomically replaces the map entry"; the implementation did not.

Fix: hold the DashMap shard write-lock across the whole read→decide→insert→
fire sequence via entry()/insert_entry(). tx.send is non-blocking, non-async,
and never re-enters the map, so firing under the shard lock cannot deadlock
and keeps global event order in lock-step with global commit order.

Pinned by concurrent_set_fires_no_duplicate_adjacent_events: 4 writers
toggling one entity A/B; asserts no two consecutive fired events carry the
same new_state (impossible under correct serialisation). Fails reliably on
the old code (~365-476 duplicate-adjacent events on the first trial), passes
on the fix across repeated runs.

Co-Authored-By: claude-flow <ruv@ruv.net>

* harden(homecore): bound entity_id length — close memory-DoS at the REST boundary

homecore-api/src/rest.rs parses untrusted path segments straight through
EntityId::parse (get/delete/set_state). With no length cap, an otherwise-valid
id like "a." + many MB of [a-z0-9_] was accepted; a POST /api/states/<giant>
would persist it into the DashMap state store, permanently growing memory
(amplification across distinct ids).

Fix: reject ids longer than MAX_ENTITY_ID_LEN (255, HA-compatible) up front in
parse(), before any per-char scan, with a new EntityIdError::TooLong. Fails
closed at the boundary type so every caller (REST, registry deserialize,
automation) is protected.

Pinned by entity_id_length_boundary: exactly-MAX accepted, MAX+1 rejected,
4 MiB id rejected as TooLong. Fails on old code (oversized parses Ok).

Co-Authored-By: claude-flow <ruv@ruv.net>

* harden(homecore): isolate panicking service handlers (catch_unwind)

ServiceRegistry::call already ran handlers outside the registry lock (the
Arc<dyn ServiceHandler> is cloned out of the read guard first), so a panic
could never poison the RwLock or block other callers — good. But a panicking
handler unwound through call() into the caller's task; the task driving the
engine (e.g. an axum request handler invoking a service) could be aborted by
one buggy integration.

Fix: wrap the handler future in AssertUnwindSafe + FutureExt::catch_unwind and
convert a panic into ServiceError::HandlerPanicked. Mirrors HA isolating
service-handler exceptions. The registry stays fully usable afterwards.

Pinned by panicking_handler_is_isolated_and_registry_survives: the panicking
call returns HandlerPanicked (not an unwind), a sibling healthy service still
returns its value, and the bad service remains registered. Fails on old code
(the await point panics instead of returning Err).

Co-Authored-By: claude-flow <ruv@ruv.net>

* test(homecore): pin event-bus lag safety (bounded broadcast, no DoS)

Documents-with-evidence that the core EventBus does NOT have the homecore-api
WS broadcast-lag failure: with EVENT_CHANNEL_CAPACITY=4096, firing 3x capacity
while a subscriber never drains keeps fire_* non-blocking (publisher never
waits on slow receivers), gives the slow receiver a recoverable Lagged(n)
(drop-oldest + re-sync) rather than a closed channel, and leaves the bus live
for a fresh fast subscriber. No code change — pins the clean dimension.

Co-Authored-By: claude-flow <ruv@ruv.net>

* docs(homecore): record ADR-127 §9 security+concurrency review + CHANGELOG

Documents the three pinned fixes (HC-RACE-01 state-set TOCTOU, HC-EID-LEN-01
entity_id memory-DoS, HC-SVC-PANIC-01 service-handler isolation) and the
clean dimensions (bounded event-bus lag handling, lock discipline / no
lock-across-await, no panic-on-input) with their evidence.

Co-Authored-By: claude-flow <ruv@ruv.net>
2026-06-14 22:28:05 -04:00

151 lines
5.3 KiB
Rust
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
//! Event bus — typed system events + untyped domain events.
//!
//! ADR-127 §2.2: HA's single dict-typed event channel becomes two:
//! - typed `SystemEvent` channel for known shapes (recorder, automation)
//! - untyped `DomainEvent` channel for arbitrary integration events
//!
//! Capacity 4,096 on both. Lagged receivers must re-sync (recorder
//! re-reads current state; automation re-evaluates triggers).
use std::sync::Arc;
use tokio::sync::broadcast;
use crate::event::{DomainEvent, SystemEvent};
pub const EVENT_CHANNEL_CAPACITY: usize = 4096;
#[derive(Clone)]
pub struct EventBus {
inner: Arc<EventBusInner>,
}
struct EventBusInner {
system_tx: broadcast::Sender<SystemEvent>,
domain_tx: broadcast::Sender<DomainEvent>,
}
impl EventBus {
pub fn new() -> Self {
let (system_tx, _) = broadcast::channel(EVENT_CHANNEL_CAPACITY);
let (domain_tx, _) = broadcast::channel(EVENT_CHANNEL_CAPACITY);
Self {
inner: Arc::new(EventBusInner { system_tx, domain_tx }),
}
}
pub fn subscribe_system(&self) -> broadcast::Receiver<SystemEvent> {
self.inner.system_tx.subscribe()
}
pub fn subscribe_domain(&self) -> broadcast::Receiver<DomainEvent> {
self.inner.domain_tx.subscribe()
}
/// Fire a typed system event. Returns the number of active
/// receivers (zero is fine).
pub fn fire_system(&self, event: SystemEvent) -> usize {
self.inner.system_tx.send(event).unwrap_or(0)
}
/// Fire an untyped domain event. Mirrors `hass.bus.async_fire`.
pub fn fire_domain(&self, event: DomainEvent) -> usize {
self.inner.domain_tx.send(event).unwrap_or(0)
}
}
impl Default for EventBus {
fn default() -> Self {
Self::new()
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::event::Context;
#[tokio::test]
async fn fire_system_reaches_subscriber() {
let bus = EventBus::new();
let mut rx = bus.subscribe_system();
bus.fire_system(SystemEvent::HomeCoreStarted);
let event = rx.recv().await.unwrap();
assert!(matches!(event, SystemEvent::HomeCoreStarted));
}
#[tokio::test]
async fn fire_domain_reaches_subscriber() {
let bus = EventBus::new();
let mut rx = bus.subscribe_domain();
bus.fire_domain(DomainEvent::new(
"ruview_csi_frame",
serde_json::json!({"frame_id": 42}),
Context::new(),
));
let event = rx.recv().await.unwrap();
assert_eq!(event.event_type, "ruview_csi_frame");
assert_eq!(event.event_data["frame_id"], 42);
}
/// Bus-lag safety (same failure class as the homecore-api WS
/// broadcast-lag DoS, here on the core bus): a subscriber that never
/// drains must NOT block the publisher, must NOT make the channel grow
/// without bound, and must NOT take down a healthy fast subscriber. The
/// bounded `tokio::sync::broadcast` gives the slow receiver a recoverable
/// `Lagged(n)` (drop-oldest, re-sync) while `fire_*` stays non-blocking.
///
/// Evidence: with EVENT_CHANNEL_CAPACITY = 4096 we fire 3× capacity
/// while a slow subscriber sits idle. Every `fire_domain` returns
/// promptly (publisher never blocked); the slow receiver observes
/// `Lagged` then re-syncs to live events; the fast receiver — created
/// after the flood and kept drained — receives all subsequent events
/// with no loss. The bus stays live throughout.
#[tokio::test]
async fn slow_subscriber_does_not_block_publisher_or_kill_the_bus() {
use tokio::sync::broadcast::error::TryRecvError;
let bus = EventBus::new();
// Slow subscriber: subscribes, then never drains during the flood.
let mut slow = bus.subscribe_domain();
// Publisher fires 3× capacity. None of these may block.
let total = EVENT_CHANNEL_CAPACITY * 3;
for i in 0..total {
// Returns the receiver count (>=1 here); the point is it
// returns AT ALL without awaiting the slow receiver.
let _ = bus.fire_domain(DomainEvent::new(
"flood",
serde_json::json!({ "i": i }),
Context::new(),
));
}
// The slow receiver is forced past capacity → recoverable Lagged,
// NOT a closed channel and NOT a hang.
let mut saw_lagged = false;
loop {
match slow.try_recv() {
Ok(_) => {}
Err(TryRecvError::Lagged(n)) => {
assert!(n > 0);
saw_lagged = true;
}
Err(TryRecvError::Empty) => break,
Err(TryRecvError::Closed) => panic!("bus closed — must stay live"),
}
}
assert!(saw_lagged, "slow subscriber should have lagged, not blocked the bus");
// The bus is still live: a fresh fast subscriber receives new events.
let mut fast = bus.subscribe_domain();
bus.fire_domain(DomainEvent::new("live", serde_json::json!({"ok": true}), Context::new()));
let evt = fast.recv().await.unwrap();
assert_eq!(evt.event_type, "live");
// And the lagged subscriber recovers (re-syncs) to live events too.
let evt2 = slow.recv().await.unwrap();
assert_eq!(evt2.event_type, "live");
}
}