mirror of
https://github.com/ruvnet/RuView
synced 2026-07-21 17:13:19 +00:00
bf1dfe79fd
* 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>
292 lines
10 KiB
Rust
292 lines
10 KiB
Rust
//! Entity ID newtype + immutable state snapshot type.
|
|
//!
|
|
//! Mirrors `homeassistant/core.py` `State` and the `entity_id` string
|
|
//! validation that every public HA call performs.
|
|
//!
|
|
//! ## EntityId validation (ADR-127 §2.1 + Q1)
|
|
//!
|
|
//! HA accepts unicode entity IDs since 2024.3. HOMECORE P1 accepts the
|
|
//! ASCII subset `[a-z0-9_]+\.[a-z0-9_]+` and rejects everything else
|
|
//! with a clear error. Unicode acceptance is deferred to P2 once the
|
|
//! Q1 strictness decision is made (see ADR-127 §8).
|
|
|
|
use std::fmt;
|
|
use std::sync::Arc;
|
|
|
|
use chrono::{DateTime, Utc};
|
|
use serde::{Deserialize, Deserializer, Serialize, Serializer};
|
|
use thiserror::Error;
|
|
|
|
use crate::event::Context;
|
|
|
|
/// Validated `domain.name` entity identifier.
|
|
///
|
|
/// Construct via [`EntityId::parse`] or [`EntityId::new`]; both validate
|
|
/// against the format `[a-z0-9_]+\.[a-z0-9_]+`. Custom `Serialize` /
|
|
/// `Deserialize` round-trips as a plain JSON string (matching HA's wire
|
|
/// format) and re-validates on deserialize so invalid IDs from disk
|
|
/// fail at load time rather than at first use.
|
|
#[derive(Clone, Eq, PartialEq, Hash)]
|
|
pub struct EntityId(Arc<str>);
|
|
|
|
impl Serialize for EntityId {
|
|
fn serialize<S: Serializer>(&self, ser: S) -> Result<S::Ok, S::Error> {
|
|
ser.serialize_str(&self.0)
|
|
}
|
|
}
|
|
|
|
impl<'de> Deserialize<'de> for EntityId {
|
|
fn deserialize<D: Deserializer<'de>>(de: D) -> Result<Self, D::Error> {
|
|
let s = String::deserialize(de)?;
|
|
EntityId::parse(s).map_err(serde::de::Error::custom)
|
|
}
|
|
}
|
|
|
|
/// Maximum accepted `entity_id` length in bytes. Mirrors Home Assistant's
|
|
/// practical cap (`MAX_LENGTH_STATE_*` family — 255). The state machine and
|
|
/// entity/registry maps are keyed on `EntityId`, and the REST layer
|
|
/// (`homecore-api`) parses untrusted path segments straight through
|
|
/// [`EntityId::parse`]; an unbounded id would let a single `POST
|
|
/// /api/states/<giant>` permanently grow the state map (memory DoS). We
|
|
/// fail closed at the boundary instead.
|
|
pub const MAX_ENTITY_ID_LEN: usize = 255;
|
|
|
|
impl EntityId {
|
|
/// Validates and constructs an `EntityId`. Returns
|
|
/// [`EntityIdError`] if the input is not `domain.name` shape with
|
|
/// ASCII lowercase / digits / underscore in each segment, or if it
|
|
/// exceeds [`MAX_ENTITY_ID_LEN`] bytes.
|
|
pub fn parse(s: impl Into<String>) -> Result<Self, EntityIdError> {
|
|
let s: String = s.into();
|
|
// Bound the length BEFORE any further work so an oversized input is
|
|
// cheap to reject (no per-char scan of megabytes).
|
|
if s.len() > MAX_ENTITY_ID_LEN {
|
|
return Err(EntityIdError::TooLong {
|
|
len: s.len(),
|
|
max: MAX_ENTITY_ID_LEN,
|
|
});
|
|
}
|
|
let (domain, name) = s
|
|
.split_once('.')
|
|
.ok_or_else(|| EntityIdError::MissingDot(s.clone()))?;
|
|
if domain.is_empty() {
|
|
return Err(EntityIdError::EmptyDomain(s));
|
|
}
|
|
if name.is_empty() {
|
|
return Err(EntityIdError::EmptyName(s));
|
|
}
|
|
for ch in domain.chars().chain(name.chars()) {
|
|
if !(ch.is_ascii_lowercase() || ch.is_ascii_digit() || ch == '_') {
|
|
return Err(EntityIdError::InvalidChar { entity_id: s, ch });
|
|
}
|
|
}
|
|
Ok(Self(Arc::from(s)))
|
|
}
|
|
|
|
/// Same as [`Self::parse`] but takes a `&str` and returns
|
|
/// `Result<&'static EntityId, ...>` for constant entity IDs known
|
|
/// at compile time. Used by ADR-128 plugins to register fixed-name
|
|
/// services like `homeassistant.restart`.
|
|
pub fn new(s: &str) -> Result<Self, EntityIdError> {
|
|
Self::parse(s.to_owned())
|
|
}
|
|
|
|
/// Returns the `domain` part (everything before the first `.`).
|
|
pub fn domain(&self) -> &str {
|
|
self.0.split_once('.').map(|(d, _)| d).unwrap_or(&self.0)
|
|
}
|
|
|
|
/// Returns the `name` part (everything after the first `.`).
|
|
pub fn name(&self) -> &str {
|
|
self.0.split_once('.').map(|(_, n)| n).unwrap_or("")
|
|
}
|
|
|
|
/// Underlying string view.
|
|
pub fn as_str(&self) -> &str {
|
|
&self.0
|
|
}
|
|
}
|
|
|
|
impl fmt::Debug for EntityId {
|
|
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
|
write!(f, "EntityId({})", self.0)
|
|
}
|
|
}
|
|
|
|
impl fmt::Display for EntityId {
|
|
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
|
f.write_str(&self.0)
|
|
}
|
|
}
|
|
|
|
#[derive(Error, Debug, Clone, Eq, PartialEq)]
|
|
pub enum EntityIdError {
|
|
#[error("entity_id {0:?} is missing the required '.' between domain and name")]
|
|
MissingDot(String),
|
|
#[error("entity_id {0:?} has an empty domain segment")]
|
|
EmptyDomain(String),
|
|
#[error("entity_id {0:?} has an empty name segment")]
|
|
EmptyName(String),
|
|
#[error("entity_id {entity_id:?} contains invalid character {ch:?} — only [a-z0-9_] allowed (HA-compat ASCII subset; see ADR-127 §Q1)")]
|
|
InvalidChar { entity_id: String, ch: char },
|
|
#[error("entity_id is {len} bytes, exceeding the {max}-byte limit")]
|
|
TooLong { len: usize, max: usize },
|
|
}
|
|
|
|
/// Immutable state snapshot for one entity at one moment in time.
|
|
///
|
|
/// Mirrors `homeassistant.core.State`. Reader-cloneable via `Arc<State>`;
|
|
/// writers atomically replace the entry in the `DashMap` so observers
|
|
/// never see a partial mutation.
|
|
#[derive(Clone, Debug, Serialize, Deserialize)]
|
|
pub struct State {
|
|
pub entity_id: EntityId,
|
|
pub state: String,
|
|
/// Attribute bag — accepts whatever JSON the integration emits.
|
|
/// Mirrors HA's `Dict[str, Any]` attribute model.
|
|
pub attributes: serde_json::Value,
|
|
/// When the `state` field last changed value. Only bumped if the
|
|
/// new state string differs from the old; attribute-only updates
|
|
/// preserve this timestamp.
|
|
pub last_changed: DateTime<Utc>,
|
|
/// When this snapshot was written. Bumped on every `set` call,
|
|
/// including attribute-only updates.
|
|
pub last_updated: DateTime<Utc>,
|
|
/// Causality context — links state changes to the user / automation
|
|
/// / service call that originated them. Mirrors HA's `Context`.
|
|
pub context: Context,
|
|
}
|
|
|
|
impl State {
|
|
/// Construct a fresh state snapshot at `now`.
|
|
pub fn new(
|
|
entity_id: EntityId,
|
|
state: impl Into<String>,
|
|
attributes: serde_json::Value,
|
|
context: Context,
|
|
) -> Self {
|
|
let now = Utc::now();
|
|
Self {
|
|
entity_id,
|
|
state: state.into(),
|
|
attributes,
|
|
last_changed: now,
|
|
last_updated: now,
|
|
context,
|
|
}
|
|
}
|
|
|
|
/// Construct the next state snapshot. If the new `state` string
|
|
/// equals the prior `state`, `last_changed` is preserved.
|
|
pub fn next(
|
|
&self,
|
|
new_state: impl Into<String>,
|
|
new_attributes: serde_json::Value,
|
|
context: Context,
|
|
) -> Self {
|
|
let new_state = new_state.into();
|
|
let now = Utc::now();
|
|
let last_changed = if new_state == self.state {
|
|
self.last_changed
|
|
} else {
|
|
now
|
|
};
|
|
Self {
|
|
entity_id: self.entity_id.clone(),
|
|
state: new_state,
|
|
attributes: new_attributes,
|
|
last_changed,
|
|
last_updated: now,
|
|
context,
|
|
}
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
#[test]
|
|
fn entity_id_parses_valid() {
|
|
let e = EntityId::parse("light.living_room").unwrap();
|
|
assert_eq!(e.domain(), "light");
|
|
assert_eq!(e.name(), "living_room");
|
|
assert_eq!(e.as_str(), "light.living_room");
|
|
}
|
|
|
|
#[test]
|
|
fn entity_id_rejects_missing_dot() {
|
|
assert!(matches!(
|
|
EntityId::parse("light_living_room"),
|
|
Err(EntityIdError::MissingDot(_))
|
|
));
|
|
}
|
|
|
|
#[test]
|
|
fn entity_id_rejects_uppercase() {
|
|
let err = EntityId::parse("light.LivingRoom").unwrap_err();
|
|
match err {
|
|
EntityIdError::InvalidChar { ch, .. } => assert_eq!(ch, 'L'),
|
|
other => panic!("expected InvalidChar, got {other:?}"),
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn entity_id_rejects_unicode() {
|
|
// ADR-127 §Q1 — P1 is strict ASCII. Unicode acceptance deferred.
|
|
assert!(EntityId::parse("light.küche").is_err());
|
|
}
|
|
|
|
#[test]
|
|
fn entity_id_length_boundary() {
|
|
// The REST layer parses untrusted path segments straight through
|
|
// `parse`; an unbounded id is a memory-DoS vector (a `POST
|
|
// /api/states/<giant>` permanently grows the state map). Cap at
|
|
// MAX_ENTITY_ID_LEN, fail closed above it.
|
|
//
|
|
// Construct "sensor." (7 bytes) + N name bytes == exactly MAX.
|
|
let prefix = "sensor.";
|
|
let name_len = MAX_ENTITY_ID_LEN - prefix.len();
|
|
let at_max = format!("{prefix}{}", "a".repeat(name_len));
|
|
assert_eq!(at_max.len(), MAX_ENTITY_ID_LEN);
|
|
assert!(
|
|
EntityId::parse(at_max.clone()).is_ok(),
|
|
"an id of exactly MAX_ENTITY_ID_LEN bytes must be accepted"
|
|
);
|
|
|
|
let over = format!("{at_max}a"); // MAX + 1
|
|
assert!(matches!(
|
|
EntityId::parse(over),
|
|
Err(EntityIdError::TooLong { .. })
|
|
));
|
|
|
|
// A multi-megabyte, otherwise-valid id is rejected cheaply rather
|
|
// than persisted.
|
|
let huge = format!("sensor.{}", "a".repeat(4 * 1024 * 1024));
|
|
assert!(matches!(
|
|
EntityId::parse(huge),
|
|
Err(EntityIdError::TooLong { len, max })
|
|
if max == MAX_ENTITY_ID_LEN && len > MAX_ENTITY_ID_LEN
|
|
));
|
|
}
|
|
|
|
#[test]
|
|
fn state_next_preserves_last_changed_when_state_unchanged() {
|
|
let id = EntityId::parse("sensor.temp").unwrap();
|
|
let s1 = State::new(id.clone(), "20.0", serde_json::json!({}), Context::default());
|
|
std::thread::sleep(std::time::Duration::from_millis(2));
|
|
let s2 = s1.next("20.0", serde_json::json!({"updated": true}), Context::default());
|
|
assert_eq!(s1.last_changed, s2.last_changed);
|
|
assert!(s2.last_updated > s1.last_updated);
|
|
}
|
|
|
|
#[test]
|
|
fn state_next_bumps_last_changed_when_state_changes() {
|
|
let id = EntityId::parse("sensor.temp").unwrap();
|
|
let s1 = State::new(id, "20.0", serde_json::json!({}), Context::default());
|
|
std::thread::sleep(std::time::Duration::from_millis(2));
|
|
let s2 = s1.next("21.0", serde_json::json!({}), Context::default());
|
|
assert!(s2.last_changed > s1.last_changed);
|
|
}
|
|
}
|