From 7f13ed4886d3f1db959b1a73b128f0c2dda1d16e Mon Sep 17 00:00:00 2001 From: ruv Date: Mon, 15 Jun 2026 10:41:11 -0400 Subject: [PATCH] =?UTF-8?q?fix(homecore-ui):=20resolve=20code-review=20fin?= =?UTF-8?q?dings=20=E2=80=94=20SSRF=20guard,=20CORS/trace=20coverage,=20?= =?UTF-8?q?=C2=A76=20honesty,=20crash=20guards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the high-effort review of PR #1082: - SECURITY: cal_proxy rejects path-traversal/confused-deputy SSRF (`.`/`..` segments, backslash, %2e%2e/%2f, absolute) on raw+decoded forms → 400, before attaching the server-side calibration bearer. - CORRECTNESS: /api/homecore/* + /api/cal/* now covered by the shared CORS allowlist (build_cors_layer, exported from homecore-api) + TraceLayer — previously merged outside router()'s layers (no CORS, no tracing). - §6 HONESTY (no fabricated data): dashboard renders '—' for null metrics (not "null%"/"null°C"); cogs Hailo pill reflects the REAL appliance probe (not hardcoded "connected"); room anomaly threshold passed through / null, not a fabricated 0.5. - ROBUSTNESS: cogs asArray(hef) guards a non-array manifest field; calibration progress guards target<=0 (no NaN%/Infinity%); restart clears the poll timer. - CLEANUP: mock.js is now a cached DYNAMIC import (demo-only) — never bundled in production (§2.2). - New ui/tests/unit-fixes.mjs pins the above; ADR-131 + CHANGELOG updated. Co-Authored-By: claude-flow --- CHANGELOG.md | 1 + ...R-131-homecore-ui-operational-dashboard.md | 16 ++ v2/Cargo.lock | 1 + v2/crates/homecore-api/src/app.rs | 6 +- v2/crates/homecore-api/src/lib.rs | 2 +- v2/crates/homecore-server/Cargo.toml | 15 +- v2/crates/homecore-server/src/gateway.rs | 223 ++++++++++++++++-- v2/crates/homecore-server/src/main.rs | 70 +++++- v2/crates/homecore-server/ui/js/api.js | 49 ++-- .../ui/js/panels/calibration.js | 89 ++++--- .../homecore-server/ui/js/panels/cogs.js | 37 ++- .../homecore-server/ui/js/panels/dashboard.js | 14 +- .../homecore-server/ui/js/panels/rooms.js | 13 +- v2/crates/homecore-server/ui/package.json | 2 +- .../homecore-server/ui/tests/unit-fixes.mjs | 101 ++++++++ 15 files changed, 550 insertions(+), 89 deletions(-) create mode 100644 v2/crates/homecore-server/ui/tests/unit-fixes.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 63fcea60..8d7e36a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Security +- **ADR-131 HOMECORE-UI BFF gateway — public-PR review fixes (PR #1082).** (1) **HIGH — path-traversal / confused-deputy SSRF closed in the `/api/cal/*` reverse-proxy** (`homecore-server/src/gateway.rs`). The wildcard proxy path was interpolated straight into the upstream URL while `proxy()` attaches the server-side calibration bearer, so `/api/cal/v1/../../x` (and percent-encoded `..%2f`, `%2e%2e`, leading `/`, backslash, double-encoded `%252e`) could escape the `…/api/` scope **with the privileged token**. Now `validate_proxy_path()` decode-then-checks and rejects absolute/backslash/dot-segment/encoded-traversal paths with a typed **400 BEFORE the URL is built** (applies to GET **and** POST); legit `v1/...` paths still pass. Pinned by `cal_proxy_rejects_traversal_with_400_before_upstream` (fails on old code) + `validate_proxy_path_rejects_traversal_variants`. (2) **CORS + request-tracing now cover the gateway routes.** `/api/homecore/*` and `/api/cal/*` were `.merge()`d **outside** the layers `homecore-api::router()` applies, leaving them with no CORS allowlist and untraced; the audited `build_cors_layer()` (HC-05) + `TraceLayer` are now applied to the whole merged surface in `main.rs`. Pinned by `gateway_routes_are_cors_covered_after_merge` (Vite-dev-origin preflight succeeds on a gateway route). (3) **Fabricated-data honesty (§6 invariant 3):** the gateway no longer injects a hardcoded `anomaly.threshold: 0.5` — it passes through the REAL upstream threshold or emits `null` (withheld); the dashboard renders a not-available `—` instead of `"null%"`/`"null°C"` for null appliance metrics; the COG panel's Hailo-worker pill reflects the real appliance probe instead of a hardcoded `"connected"`; `rooms.js` treats a null anomaly threshold as withheld, not a fake `0.8` default. (4) **Robustness:** a forwarded `hef` that is a string (not an array) no longer throws in the COG panel; the calibration wizard guards `frames/target` against `NaN%`/`Infinity%` and clears its baseline poll timer on Restart / panel teardown (leaked `setTimeout` loop fixed). (5) **Perf:** per-bank RoomState fetches and the appliance service probes now run concurrently (`futures::join_all`; async `tokio::net::TcpStream` + `timeout` replaces the blocking `connect_timeout` that parked a worker per probe); the mock fixture module is now a dynamic `import()` gated on demo mode so production never bundles it. **Note (workspace-wide, not fixed here):** `homecore-server` requests `reqwest`'s `rustls-tls` only, but cargo feature-unification means a sibling crate enabling the default `native-tls` re-introduces OpenSSL into the final binary regardless — a true "no OpenSSL on the appliance" guarantee requires aligning every reqwest-pulling crate on rustls-only. **Note (pre-existing, out of scope):** DEV-mode `allow_any_non_empty()` bearer auth when `HOMECORE_TOKENS` is unset on `0.0.0.0` is unchanged; the loud `warn!` at boot is retained — provision real tokens before network exposure. **Verified:** `cargo test -p homecore-server --no-default-features` = **18/18 pass**, `cargo build -p homecore-server` clean, UI suite (`node tests`) all green, Python proof VERDICT PASS (hash unchanged). - **`ruview-swarm` beyond-SOTA security + correctness review (ADR-148 drone swarm control plane; needs ADR slot 176) — 4 real fail-open / DoS bugs fixed in the NaN-state-poisoning class, each pinned fails-on-old / passes-on-new; 5 dimensions confirmed clean with evidence.** The shared theme is **IEEE-754 NaN/Inf silently defeating a safety comparison** on data that crosses the untrusted swarm-comm trust boundary (`SwarmOrchestrator::receive_peer_state` / `receive_peer_detection` accept full `DroneState`/`CsiDetection` whose f64/f32 fields deserialize with no finite-check; the integer-encoded MAVLink wire formats in `mavlink_messages.rs` cannot carry NaN, but the serde struct path can). **(1) HIGH — `failsafe::FailSafeMachine::tick` collision-avoidance + battery fail-open** (`failsafe/mod.rs:51,75`). `nearest_neighbor_dist < collision_dist_m` and `battery_pct <= rth_pct` both evaluate `false` for a NaN operand, so a poisoned peer position (→ NaN `nearest_peer_distance` via `Position3D::distance_to`) **silently disabled collision avoidance** and a NaN battery reading kept a drone Nominal — the worst failure for a physical airframe. Fixed to fail CLOSED (`!is_finite() ||` → `EmergencyDiverge` / `ReturnToHome`). MEASURED fails-on-old: `test_nan_neighbor_distance_fails_closed_to_diverge` / `test_nan_battery_fails_closed_to_rth` both returned `Nominal` pre-fix. **(2) MEDIUM — `security::geofence::Geofence::check` NaN-altitude bypass** (`security/geofence.rs:33`). A NaN `z` (altitude) with valid x/y skipped the altitude breach (`NaN < min || NaN > max` = `false`) and returned **`Safe`** through the point-in-polygon path — a silent geofence bypass. Fixed with a leading non-finite-coordinate → `HardBreach` guard. MEASURED fails-on-old: `test_nan_altitude_fails_closed` returned `Safe` pre-fix. **(3) MEDIUM/DoS — `security::antijamming::FhssRadio` `% 0` panic on empty `channels_mhz`** (`security/antijamming.rs:65,71,102`). `FhssConfig` is `Deserialize`; an empty channel list (malformed/hostile config) made `next_hop`/`current_channel_mhz`/`evasive_hop`/`tick` panic with `remainder with a divisor of zero`, crashing the radio task. Fixed with `len == 0` early-returns (benign `0.0` sentinel). MEASURED fails-on-old: `test_empty_channels_does_not_panic` **panicked** (`divisor of zero`) pre-fix. **(4) LOW — `sensing::multiview::MultiViewFusion::fuse` NaN victim-position propagation** (`sensing/multiview.rs:70`). A NaN `victim_position` passed the `is_some()` filter and propagated through the confidence-weighted average into the fused "confirmed victim" location dispatched to the swarm. Fixed by requiring finite `confidence` + finite position components (fail-closed drop). MEASURED fails-on-old: `test_nan_victim_position_dropped_from_fusion` produced a non-finite fused position pre-fix. **Dimensions confirmed clean (with evidence):** (a) **MAVLink decode panic-safety** — `SwarmNodeState::decode(&[u8;20])` `try_into().unwrap()`s are over fixed const ranges of a fixed-size array (provably infallible; no arbitrary-length `&[u8]` path exists). (b) **UWB GPS anti-spoofing is NaN-safe** — `(gps_dist - uwb_dist).abs() <= tol` already fails CLOSED on a NaN range/position (counts as inconsistent → spoof rejected), verified by reasoning + existing `test_spoofed_gps_invalid`. (c) **Bounded grid / no allocation-from-length-field** — `ProbabilityGrid::update_bayesian`/`mark_scanned` bounds-check `cx >= width || cy >= height`; `pos_to_cell` uses saturating `as u32` (Rust `as` saturates, no UB). (d) **Mesh `nearest_k` NaN-safe sort** — `partial_cmp(..).unwrap_or(Equal)` cannot panic on NaN distances. (e) **No hardcoded secrets** — `MavlinkSigner` key is constructor-injected (`[u8;32]`), nothing embedded. **Documented-not-fixed (for ADR-176, not churned to avoid test-rewrite risk):** (i) **Raft `AppendEntries` lacks the Log-Matching consistency check** (`topology/raft.rs:187`) — a follower appends leader entries on `term >= current_term` without validating `prev_log_index`/`prev_log_term`, so a malformed/byzantine leader can corrupt a follower's log (a genuine consensus-safety gap; vote tallying is also delegated to the caller per the existing `handle_message` comment). (ii) **`MavlinkSigner::verify` uses a non-constant-time tag `==` and has no replay/timestamp-window rejection** (`security/mavlink_signing.rs:64`) — the doc comment already flags the replay limitation as a known demo/test simplification. `cargo test -p ruview-swarm --no-default-features`: **117 → 123 passed, 0 failed** (+6 pins). Workspace green; Python deterministic proof unchanged (`f8e76f21a0f9852b70b6d9dd5318239f6b20cbcb4cdd995863263cecdc446f7a`, bit-exact — `ruview-swarm` is off the signal proof path). - **`wifi-densepose-core` + `wifi-densepose-cli` beyond-SOTA security review (ADR-127 note; CLI needs ADR slot) — NaN-state-poisoning bug class does NOT originate in core (verdict: no + evidence); both crates confirmed clean on all reviewed dimensions; 4 regression pins added locking in two real DoS guards.** **Load-bearing question — verdict NO (with evidence, MEASURED).** The NaN-state-poisoning class that hit `wifi-densepose-calibration`/`-vitals`/`-geo` (a non-finite input latching into a persistent IIR/Welford/von-Mises/voxel accumulator → silent permanent feature failure) does **not** live in a shared `wifi-densepose-core` primitive: core exposes **no stateful accumulator at all** — no Welford/running-mean, no von-Mises/circular-mean, no IIR/biquad filter state, no voxel grid. Grep over `core/src` for `welford|von_mises|biquad|y1|y2|running_mean|accumulat|voxel|self.*+=` matched only the `InvalidState` *error* enum, "reset state" doc comments, and a test-only LCG — zero stateful logic (MEASURED). Each downstream crate rolls its own accumulator, so each fix is correctly local; corroborated by `wifi-densepose-calibration::Features::from_series`, which **already** filters non-finite samples and returns `Features::ZERO` (the downstream re-implementation of the fix). The only float math in core's hot path is construction-time projection (`CsiFrame::new` → `amplitude`/`phase` via `mapv`) and pure stateless `utils` functions — none persists state across frames. **Dimensions confirmed clean (with evidence):** (1) **panic-on-adversarial-input = 0** — `CsiFrame::from_canonical_bytes` (the replay/forward deserialisation boundary) returns a typed `CanonicalDecodeError` for every malformed input (truncation, bad discriminant, non-UTF-8 device id, nonzero reserved bytes, shape/payload mismatch, trailing bytes); the CLI UDP parser `parse_csi_packet` (the widest CLI attack surface — bound to `0.0.0.0` by default) returns `None` on any malformed datagram. Both proven panic-free over deterministic-LCG fuzz sweeps (new pins). (2) **`Confidence::new` rejects NaN** (`!(0.0..=1.0).contains(&NaN)` ⇒ `true` ⇒ `Err`); `compute_bounding_box`/`to_flat_array` are NaN-tolerant (f32 `min`/`max` ignore NaN). (3) **`amplitude_variance`/`mean_amplitude` panic-free on empty frames** — ndarray 0.17 `var(0.0)`/`mean()` return finite/`None` (handled), not a panic (MEASURED via throwaway probe). (4) **Unbounded-memory DoS bounded** in both deserialisers: `from_canonical_bytes` guards the `Vec::with_capacity(rows*cols)` with `rows.saturating_mul(cols).saturating_mul(16) <= bytes.len()`; `parse_csi_packet` guards `Array2::zeros` with `buf.len() < 20 + n_pairs*2` — a header lying about an enormous `rows×cols` / `n_antennas×n_subcarriers` is rejected before allocation. (5) **CLI path-traversal** in `calibrate-serve` already defended by `sanitize_room_id` (keeps `[A-Za-z0-9_-]`, caps 64 chars, with tests) on every client-supplied `room_id`/`bank`/`baseline` name that reaches a file path; bearer-auth gate + non-loopback-bind warning present. (6) **No hardcoded secrets** (`--token` read from `CALIBRATE_TOKEN` env, never embedded). **Regression pins added (fails-on-old / passes-on-new):** core `canonical_decode_oversized_shape_is_bounded_not_allocated` (MEASURED: with the saturating guard removed it panics `capacity overflow` at `types.rs:801`; passes with the guard) and `canonical_decode_never_panics_on_arbitrary_bytes`; CLI `test_parse_csi_packet_oversized_claim_is_rejected_not_allocated` (a 255×65535-pair claim ≈ 33 MB in a 2 KB datagram → `None`, never OOMs) and `test_parse_csi_packet_never_panics_on_arbitrary_bytes`. `cargo test -p wifi-densepose-core`: **35 → 37** lib tests, 0 failed; `cargo test -p wifi-densepose-cli --no-default-features`: **24 → 26**, 0 failed. Workspace green; Python deterministic proof unchanged (`f8e76f21…46f7a`, bit-exact — core/cli are off the signal proof path). No production code changed — review is clean-with-evidence plus pins. - **`homecore` foundational state-machine review (ADR-127) — one real concurrency bug fixed (state-set TOCTOU dropping/reordering `state_changed` events) + two hardening fixes (entity_id memory-DoS, service-handler panic isolation), each pinned by a fails-on-old test; event-bus lag & lock discipline confirmed clean with evidence.** Beyond-SOTA security+concurrency review of the crate every other HOMECORE module builds on (state store `state.rs`, event bus `bus.rs`, service/entity registries, the `HomeCore` coordinator), un-covered by the ADR-154–159 sweep — a bug here is high-blast-radius. **HC-RACE-01 (state-set TOCTOU, the crux — race/lost-event).** `StateMachine::set` did `get()` (releasing the DashMap shard lock) → compute the next snapshot + the no-op/`last_changed` decision → `insert()` (re-acquiring the lock) → `send()`; the read-modify-write was **not atomic** w.r.t. a concurrent writer on the same entity, contradicting ADR-127 §2.1's promise that "the writer atomically replaces the map entry." A writer that read a **stale `old`** could mis-classify a genuine transition as a no-op and **silently 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`). **Fixed** by holding the 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 an identical `new_state` — impossible under correct serialisation; an instrumented probe observed ~93k such duplicate-adjacent events across 200 trials on the racy code, **zero** on the fix; the test fails reliably on the first trial pre-fix). **HC-EID-LEN-01 (unbounded `entity_id`, memory-DoS).** `homecore-api/src/rest.rs` parses untrusted REST path segments straight through `EntityId::parse`; with no length cap an otherwise-valid id (`a.` + many MB of `[a-z0-9_]`) was accepted, and a `POST /api/states/` would persist it into the DashMap state store (permanent growth across distinct ids). **Fixed** by rejecting ids longer than `MAX_ENTITY_ID_LEN` (255, HA-compatible) up front in `parse()`, before any per-char scan, with a new `EntityIdError::TooLong` — fail-closed at the boundary type protects every caller (REST, registry deserialize, automation). Pinned by `entity_id_length_boundary` (exactly-MAX accepted; MAX+1 and a 4 MiB id rejected — oversized parses `Ok` on old code). **HC-SVC-PANIC-01 (service-handler panic not isolated).** `ServiceRegistry::call` already ran handlers **outside** the registry lock (the `Arc` is cloned out of the read guard first → no `RwLock` poisoning, no blocking of other callers — clean), but a panicking handler unwound through `call()` into the caller's task (the task driving the engine). **Hardened** by wrapping the handler future in `AssertUnwindSafe` + `catch_unwind`, converting a panic to `ServiceError::HandlerPanicked`; the registry stays fully usable (a sibling healthy service still returns, the bad service stays registered). Pinned by `panicking_handler_is_isolated_and_registry_survives` (unwinds through `call` on old code). **Dimensions confirmed clean (with evidence, no invented issues):** (1) **event-bus bounds / lag** (the homecore-api WS lag-DoS class) — both `StateMachine` and `EventBus` use **bounded** `tokio::sync::broadcast` (capacity 4,096); a slow subscriber gets a recoverable `Lagged(n)` (drop-oldest + re-sync) while `fire_*` is non-blocking and never waits on slow receivers, so a lagging subscriber **cannot block the publisher, grow the channel without bound, or kill a fast subscriber** (evidenced by `slow_subscriber_does_not_block_publisher_or_kill_the_bus` — fire 3× capacity at an idle subscriber, publisher unblocked, bus stays live, fresh fast subscriber receives, lagged one recovers); (2) **lock ordering / lock-across-await** (deadlock) — no code path holds two of `{state DashMap, registry RwLock, service RwLock}` simultaneously, so no inconsistent-ordering deadlock can exist; every `tokio::sync::RwLock` guard in `registry.rs`/`service.rs` is used in one synchronous statement and dropped before any `.await` (`call` explicitly scopes the read guard out before awaiting the handler); the only guard held across a send is the DashMap shard lock in `set`, across a **synchronous** broadcast send — safe; (3) **panic-on-input** — no reachable `unwrap`/`expect`/index in non-test code beyond the safe `send().unwrap_or(0)` and the dead-but-harmless `split_once(...).unwrap_or(...)` fallbacks on already-validated ids. `cargo test -p homecore --no-default-features`: **20 → 24 passed, 0 failed** (+4 pins). Workspace green; Python deterministic proof unchanged (`f8e76f21…46f7a`, bit-exact — `homecore` is off the signal proof path). Review notes appended to ADR-127 §9. diff --git a/docs/adr/ADR-131-homecore-ui-operational-dashboard.md b/docs/adr/ADR-131-homecore-ui-operational-dashboard.md index e1dac1e3..3e73efb9 100644 --- a/docs/adr/ADR-131-homecore-ui-operational-dashboard.md +++ b/docs/adr/ADR-131-homecore-ui-operational-dashboard.md @@ -426,3 +426,19 @@ Staged so each wave is independently shippable behind the gateway, lands real da | **W6 — Appliance + federation + Hailo** | ◑ appliance host metrics from `/proc` + port probes DONE (live `/proc` data verified); Hailo stats + federation remain `503` (need the accelerator stat source / coordinator). | **Status:** the gateway is **compiled and tested on Rust 1.89** (`cargo test -p homecore-server` = 12/12) and was **run live** (curl proof in §10). The one remaining caveat is intrinsic, not an environment limit: **W3/W5/W6-Hailo/federation depend on services/hardware that are not in this repo** (recorder/automation HTTP wrappers, real SEED nodes, the Hailo stat source), so they return honest typed `503`s and the UI shows error states — exactly as §2.2/§11.2 prescribe. W1/W2/W4/W6-appliance are functional now. + +### 12.2 Security review (PR #1082) + +A high-effort public-PR review of the merged gateway + front-end surfaced the following, all fixed and pinned by tests (`cargo test -p homecore-server` is now **18/18**): + +| # | Severity | Finding | Fix | +|---|---|---|---| +| 1 | **HIGH** | **Path-traversal / confused-deputy SSRF** in the `/api/cal/*` reverse-proxy. The wildcard path was interpolated into the upstream URL while `proxy()` attaches the privileged server-side calibration bearer, so `/api/cal/v1/../../x` (or `..%2f`, `%2e%2e`, leading `/`, `\`, double-encoded `%252e`) could escape the `…/api/` scope **with the token**. | `validate_proxy_path()` decode-then-checks and rejects absolute / backslash / dot-segment / encoded-traversal paths with a typed **400 before the URL is built** (GET **and** POST); legit `v1/...` paths still pass. | +| 2 | Correctness | **CORS + tracing didn't cover gateway routes** — `/api/homecore/*` + `/api/cal/*` were `.merge()`d outside `homecore-api::router()`'s layers. | The audited HC-05 `build_cors_layer()` + `TraceLayer` are now applied to the whole merged app in `main.rs`. | +| 3 | Honesty (§6) | **Fabricated data** — hardcoded `anomaly.threshold: 0.5` in the adapter; dashboard rendered `"null%"`/`"null°C"`; COG Hailo pill hardcoded `"connected"`; `rooms.js` defaulted a null threshold to `0.8`. | Threshold passes through the real upstream value or emits `null` (withheld); dashboard renders `—`; the Hailo pill reflects the real appliance probe; the UI treats a null threshold as withheld. | +| 4 | Robustness | A string `hef` (forwarded verbatim) threw on `.forEach`/`.join`; `frames/target` could be `NaN%`/`Infinity%`; calibration Restart leaked the baseline `setTimeout` poll. | `asArray()` coercion; `target > 0` guard; cancellable poll cleared on Restart / panel teardown. | +| 5 | Perf | Sequential per-bank RoomState fetches; blocking `std::net::TcpStream::connect_timeout` probes on an async handler; `mock.js` statically bundled. | Concurrent `futures::join_all`; async `tokio::net::TcpStream` + `timeout`; demo-only dynamic `import()` of `mock.js`. | + +**Known limitations carried forward (not regressions):** +- **`reqwest` rustls-only is a workspace-wide concern.** `homecore-server` opts into `rustls-tls` only, but cargo feature-unification means any sibling crate enabling the default `native-tls` re-introduces OpenSSL into the final binary. A true "no OpenSSL on the appliance" guarantee requires aligning **every** reqwest-pulling crate on rustls-only — out of scope for this PR; documented at the dependency in `Cargo.toml`. +- **DEV-mode auth.** When `HOMECORE_TOKENS` is unset, the token store falls back to `allow_any_non_empty()` (any non-empty bearer accepted) on `0.0.0.0`. This is pre-existing and intentionally **unchanged** here; the loud boot `warn!` is retained. Provision real tokens (`HOMECORE_TOKENS=…`) before exposing the server to a network. diff --git a/v2/Cargo.lock b/v2/Cargo.lock index 0622d2e5..8c094cf6 100644 --- a/v2/Cargo.lock +++ b/v2/Cargo.lock @@ -3595,6 +3595,7 @@ dependencies = [ "anyhow", "axum", "clap", + "futures", "homecore", "homecore-api", "homecore-assist", diff --git a/v2/crates/homecore-api/src/app.rs b/v2/crates/homecore-api/src/app.rs index 210cab06..9863d2f2 100644 --- a/v2/crates/homecore-api/src/app.rs +++ b/v2/crates/homecore-api/src/app.rs @@ -42,7 +42,11 @@ pub fn router(state: SharedState) -> Router { .with_state(state) } -fn build_cors_layer() -> CorsLayer { +/// Build the audited CORS allowlist layer (HC-05). Exposed so the +/// integration binary can apply the SAME allowlist to routes merged in +/// outside `router()` (e.g. the ADR-131 BFF gateway), instead of leaving +/// `/api/homecore/*` and `/api/cal/*` with no CORS coverage at all. +pub fn build_cors_layer() -> CorsLayer { let raw = std::env::var("HOMECORE_CORS_ORIGINS").ok(); let origins: Vec = match raw { Some(v) if !v.trim().is_empty() => v diff --git a/v2/crates/homecore-api/src/lib.rs b/v2/crates/homecore-api/src/lib.rs index d83d84a8..71a2098d 100644 --- a/v2/crates/homecore-api/src/lib.rs +++ b/v2/crates/homecore-api/src/lib.rs @@ -7,7 +7,7 @@ pub mod state; pub mod tokens; pub mod ws; -pub use app::{router, AppState}; +pub use app::{build_cors_layer, router, AppState}; pub use error::{ApiError, ApiResult}; pub use state::SharedState; pub use tokens::LongLivedTokenStore; diff --git a/v2/crates/homecore-server/Cargo.toml b/v2/crates/homecore-server/Cargo.toml index 95d267f6..9ea8c809 100644 --- a/v2/crates/homecore-server/Cargo.toml +++ b/v2/crates/homecore-server/Cargo.toml @@ -37,12 +37,21 @@ clap = { version = "4", features = ["derive", "env"] } anyhow = "1" serde_json = "1" axum = { version = "0.7", features = ["macros"] } -# Static-file serving for the HOMECORE-UI dashboard (ADR-131) mounted at /homecore. -tower-http = { version = "0.6", features = ["fs", "trace"] } +# Static-file serving for the HOMECORE-UI dashboard (ADR-131) mounted at +# /homecore, request tracing, and the CORS allowlist applied to BOTH the +# homecore-api routes AND the merged BFF gateway routes (ADR-131 §11). +tower-http = { version = "0.6", features = ["fs", "trace", "cors"] } # BFF gateway (ADR-131 §11): reverse-proxy the calibration API + aggregate -# upstreams. rustls to avoid an OpenSSL system dep on the appliance. +# upstreams. rustls is requested here, but NOTE this is a WORKSPACE-WIDE +# concern: cargo feature-unification means a sibling crate that enables +# reqwest's default `native-tls` re-introduces OpenSSL into the final binary +# regardless of this opt-out. A real "no OpenSSL on the appliance" guarantee +# requires every crate that pulls reqwest to align on rustls-only (tracked in +# CHANGELOG / ADR-131 security note). reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] } serde = { version = "1", features = ["derive"] } +# Concurrent fan-out of per-bank RoomState fetches in the gateway (§11 perf). +futures = "0.3" [dev-dependencies] # Drive the assembled router in integration tests via ServiceExt::oneshot. diff --git a/v2/crates/homecore-server/src/gateway.rs b/v2/crates/homecore-server/src/gateway.rs index 26b714a7..5d67fad6 100644 --- a/v2/crates/homecore-server/src/gateway.rs +++ b/v2/crates/homecore-server/src/gateway.rs @@ -111,6 +111,82 @@ fn upstream_unavailable(detail: &str) -> Response { fn upstream_timeout(detail: &str) -> Response { typed(StatusCode::GATEWAY_TIMEOUT, "upstream_timeout", detail) } +fn bad_request(detail: &str) -> Response { + typed(StatusCode::BAD_REQUEST, "bad_request", detail) +} + +/// Reject a proxied wildcard path that could escape the `/api/` scope on the +/// upstream calibration service (path-traversal / confused-deputy SSRF — +/// ADR-131 §11 security review). The privileged server-side calibration bearer +/// is attached by `proxy()`, so a client must NOT be able to redirect that +/// credential outside `…/api/`. +/// +/// Returns `Err(400)` when the path (or its percent-decoded form): +/// * is absolute (`/…`) — would replace the `…/api/` base entirely, +/// * contains a backslash (`\`) — Windows/alt-separator traversal, +/// * has any segment equal to `.` or `..` — dot-segment traversal, +/// * still carries `%2e%2e` / `%2f` (single-decode is enough — we reject on +/// the decoded form AND on a residual encoded marker, so double-encoding +/// like `%252e` decodes once to `%2e` and is caught here). +/// +/// Legitimate `v1/...` paths (the only shape the UI sends) pass unchanged. +fn validate_proxy_path(path: &str) -> Result<(), Response> { + // 1. Reject on the raw form first (cheap; catches backslash + leading `/`). + if path.starts_with('/') { + return Err(bad_request("proxied path must be relative (leading '/' not allowed)")); + } + if path.contains('\\') { + return Err(bad_request("proxied path must not contain a backslash")); + } + // 2. Percent-decode once and re-check; reject if decoding is invalid. + let decoded = percent_decode_once(path) + .ok_or_else(|| bad_request("proxied path has invalid percent-encoding"))?; + if decoded.starts_with('/') || decoded.contains('\\') { + return Err(bad_request("proxied path resolves to an absolute/traversal path")); + } + // 3. Reject any `.`/`..` segment on BOTH the raw and decoded forms so an + // encoded `%2e%2e%2f` cannot slip a dot-segment past the split. + for form in [path, decoded.as_str()] { + for seg in form.split(['/', '\\']) { + if seg == "." || seg == ".." { + return Err(bad_request("proxied path must not contain '.' or '..' segments")); + } + } + // Defence in depth: a residual encoded traversal marker survived the + // single decode (e.g. originally double-encoded). Reject it outright. + let lower = form.to_ascii_lowercase(); + if lower.contains("%2e") || lower.contains("%2f") || lower.contains("%5c") { + return Err(bad_request("proxied path must not contain encoded traversal markers")); + } + } + Ok(()) +} + +/// Minimal single-pass percent-decoder (no external dep). Returns `None` on a +/// malformed escape so callers can fail closed. +fn percent_decode_once(s: &str) -> Option { + let bytes = s.as_bytes(); + let mut out: Vec = Vec::with_capacity(bytes.len()); + let mut i = 0; + while i < bytes.len() { + match bytes[i] { + b'%' => { + if i + 2 >= bytes.len() { + return None; + } + let hi = (bytes[i + 1] as char).to_digit(16)?; + let lo = (bytes[i + 2] as char).to_digit(16)?; + out.push((hi * 16 + lo) as u8); + i += 3; + } + b => { + out.push(b); + i += 1; + } + } + } + String::from_utf8(out).ok() +} /// Routes whose upstream is a SEED device / appliance daemon not present /// in this repo. Honest 503 until the corresponding §12 wave lands. @@ -140,6 +216,9 @@ async fn cal_proxy_get( if let Err(r) = require_auth(&headers, &st).await { return r; } + if let Err(r) = validate_proxy_path(&path) { + return r; + } let base = match &st.cfg.calibration_url { Some(u) => u, None => return upstream_unavailable("calibration service not configured (set --calibration-url / HOMECORE_CALIBRATION_URL)"), @@ -160,6 +239,9 @@ async fn cal_proxy_post( if let Err(r) = require_auth(&headers, &st).await { return r; } + if let Err(r) = validate_proxy_path(&path) { + return r; + } let base = match &st.cfg.calibration_url { Some(u) => u, None => return upstream_unavailable("calibration service not configured (set --calibration-url / HOMECORE_CALIBRATION_URL)"), @@ -235,13 +317,22 @@ async fn rooms(State(st): State, headers: HeaderMap) -> Response { Ok(v) => bank_names(&v), Err(r) => return r, }; - let mut out: Vec = Vec::new(); - for bank in banks { - let url = format!("{base}/api/v1/room/state?bank={bank}"); - if let Ok(v) = fetch_json(&st, &url).await { - out.push(adapt_room_state(&bank, &v)); + // Fetch every bank's RoomState concurrently (§11 perf): one slow bank no + // longer serialises behind the others. Order is preserved by collecting in + // the original bank order. + let fetches = banks.into_iter().map(|bank| { + let st = &st; + let base = base.as_str(); + async move { + let url = format!("{base}/api/v1/room/state?bank={bank}"); + fetch_json(st, &url).await.ok().map(|v| adapt_room_state(&bank, &v)) } - } + }); + let out: Vec = futures::future::join_all(fetches) + .await + .into_iter() + .flatten() + .collect(); Json(out).into_response() } @@ -296,7 +387,11 @@ fn adapt_room_state(bank: &str, v: &Value) -> Value { Some(r) if !r.is_null() => json!({ "value": r.get("value").cloned().unwrap_or(Value::Null), "confidence": r.get("confidence").cloned().unwrap_or(Value::Null), - "threshold": 0.5, + // §6 invariant 3 (honesty): pass through the REAL anomaly threshold + // from the upstream RoomState if present; if absent, emit null + // (withheld) — never fabricate a constant. The UI treats null as + // withheld, not a fake default. + "threshold": r.get("threshold").cloned().unwrap_or(Value::Null), }), _ => Value::Null, }; @@ -387,17 +482,24 @@ async fn appliance(State(st): State, headers: HeaderMap) -> Respon let ram = mem_used_pct(); let cpu = cpu_load_pct(); let uptime = uptime_secs(); - let services: Vec = [ + // Probe the appliance services concurrently with a non-blocking async + // connect under a timeout (§11 perf): previously a sequential blocking + // `std::net::TcpStream::connect_timeout` stalled the whole async handler + // for up to `N * timeout` and parked a Tokio worker thread per probe. + let probes = [ ("ruview-mcp-brain", 9876u16), ("cognitum-rvf-agent", 9004), ("ruvector-hailo-worker", 50051), ] - .iter() + .into_iter() .map(|(name, port)| { - let up = tcp_open("127.0.0.1", *port, st.cfg.timeout); - json!({ "name": name, "port": port, "status": if up { "running" } else { "unreachable" } }) - }) - .collect(); + let timeout = st.cfg.timeout; + async move { + let up = tcp_open("127.0.0.1", port, timeout).await; + json!({ "name": name, "port": port, "status": if up { "running" } else { "unreachable" } }) + } + }); + let services: Vec = futures::future::join_all(probes).await; Json(json!({ "cpu_pct": cpu, "ram_pct": ram, @@ -453,13 +555,15 @@ fn cpu_load_pct() -> Option { Some(((load / ncpu * 100.0).min(100.0) * 10.0).round() / 10.0) } -fn tcp_open(host: &str, port: u16, timeout: Duration) -> bool { - use std::net::ToSocketAddrs; - let addr = match (host, port).to_socket_addrs().ok().and_then(|mut a| a.next()) { - Some(a) => a, - None => return false, - }; - std::net::TcpStream::connect_timeout(&addr, timeout).is_ok() +/// Non-blocking liveness probe: succeeds iff a TCP connection to +/// `host:port` completes within `timeout`. Async so it never parks a Tokio +/// worker thread (unlike the blocking `std::net` connect it replaced). +async fn tcp_open(host: &str, port: u16, timeout: Duration) -> bool { + let addr = format!("{host}:{port}"); + matches!( + tokio::time::timeout(timeout, tokio::net::TcpStream::connect(&addr)).await, + Ok(Ok(_)) + ) } #[cfg(test)] @@ -565,7 +669,84 @@ mod tests { assert_eq!(ui["presence"]["value"], "occupied"); assert_eq!(ui["breathing_bpm"]["value"], 12.0); assert!(ui["heart_bpm"].is_null(), "None heartbeat must map to null (not trained)"); - assert_eq!(ui["anomaly"]["threshold"], 0.5); + // §6 invariant 3: upstream RoomState carries no threshold here, so the + // adapter must emit null (withheld) — NOT a fabricated constant. + assert!( + ui["anomaly"]["threshold"].is_null(), + "absent upstream threshold must surface as null, never a hardcoded value" + ); + } + + #[test] + fn adapt_room_state_passes_through_real_anomaly_threshold() { + // When the upstream RoomState DOES carry a real threshold, it must be + // forwarded verbatim (no fabrication, no override). + let cal = json!({ + "anomaly": {"kind":"Anomaly","value":0.2,"confidence":0.5,"threshold":0.73}, + }); + let ui = adapt_room_state("bedroom_1", &cal); + assert_eq!(ui["anomaly"]["threshold"], 0.73, "real threshold must pass through"); + } + + #[test] + fn validate_proxy_path_allows_legit_v1_paths() { + // The only shape the UI sends must pass unchanged. + for ok in [ + "v1/room/state", + "v1/calibration/baselines", + "v1/enroll/status", + "v1/room/state?bank=living_room", // query is split off before this fn + ] { + // strip any query the caller would have removed; we only validate path + let p = ok.split('?').next().unwrap(); + assert!(validate_proxy_path(p).is_ok(), "{p} should be allowed"); + } + } + + #[test] + fn validate_proxy_path_rejects_traversal_variants() { + for bad in [ + "v1/../../x", // dot-segment traversal + "../etc/passwd", // parent escape + "/etc/passwd", // absolute + "v1\\..\\..\\x", // backslash traversal + "..%2f..%2fx", // encoded slash + "%2e%2e/x", // encoded dot-dot + "v1/%2e%2e%2fadmin", // mixed encoded traversal + "%252e%252e/x", // double-encoded (residual %2e after one decode) + ] { + assert!(validate_proxy_path(bad).is_err(), "{bad} must be rejected"); + } + } + + #[tokio::test] + async fn cal_proxy_rejects_traversal_with_400_before_upstream() { + // `gw()` has calibration_url=None: a path that reached URL-building + // would 503 ("not configured"). A 400 here proves the traversal is + // rejected BEFORE any upstream request is even attempted. + for (method, path) in [ + ("GET", "/api/cal/v1/../../x"), + ("GET", "/api/cal/..%2f..%2fx"), + ("GET", "/api/cal/%2e%2e/x"), + ("POST", "/api/cal/v1/../../x"), + ] { + let (status, body) = send(gateway_router(gw()), method, path).await; + assert_eq!(status, StatusCode::BAD_REQUEST, "{method} {path} must be 400"); + assert!(body.contains("bad_request"), "{method} {path} typed 400 body"); + assert!( + !body.contains("upstream_unavailable"), + "{method} {path} must NOT reach the upstream-config branch" + ); + } + } + + #[tokio::test] + async fn cal_proxy_allows_legit_path_through_to_upstream_config() { + // A legitimate v1 path passes validation and then hits the + // "not configured" 503 (proving it was NOT blocked as traversal). + let (status, body) = send(gateway_router(gw()), "GET", "/api/cal/v1/room/state").await; + assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE); + assert!(body.contains("upstream_unavailable"), "legit path should reach upstream branch"); } #[test] diff --git a/v2/crates/homecore-server/src/main.rs b/v2/crates/homecore-server/src/main.rs index 2add737b..9f5e3255 100644 --- a/v2/crates/homecore-server/src/main.rs +++ b/v2/crates/homecore-server/src/main.rs @@ -27,7 +27,7 @@ use tracing::{info, warn}; use homecore::{Context, EntityId, HomeCore, ServiceCall, ServiceError, ServiceName}; use homecore::service::FnHandler; -use homecore_api::{router, LongLivedTokenStore, SharedState}; +use homecore_api::{build_cors_layer, router, LongLivedTokenStore, SharedState}; use homecore_assist::pipeline::default_pipeline; use homecore_assist::RegexIntentRecognizer; use homecore_automation::AutomationEngine; @@ -37,6 +37,7 @@ use homecore_recorder::Recorder; use axum::Router; use tower_http::services::ServeDir; +use tower_http::trace::TraceLayer; mod gateway; use gateway::{GatewayConfig, GatewayState}; @@ -221,7 +222,16 @@ async fn main() -> Result<()> { timeout: std::time::Duration::from_millis(cli.gateway_timeout_ms), }, ); - let app = build_app(api_state, &cli.ui_dir).merge(gateway::gateway_router(gw)); + // Merge the HA-compat API + UI mount with the BFF gateway, THEN apply the + // audited CORS allowlist + request tracing to the WHOLE surface. The + // gateway routes (`/api/homecore/*`, `/api/cal/*`) are merged in outside + // `router()`'s own layers, so without this outer layer they would have NO + // CORS coverage and would not be traced (ADR-131 §11 review). Applying CORS + // again to the homecore-api routes is idempotent. + let app = build_app(api_state, &cli.ui_dir) + .merge(gateway::gateway_router(gw)) + .layer(build_cors_layer()) + .layer(TraceLayer::new_for_http()); let listener = tokio::net::TcpListener::bind(cli.bind).await?; info!("HOMECORE-API listening on http://{} (HA-compat /api + /api/websocket)", cli.bind); info!( @@ -461,4 +471,60 @@ mod ui_tests { let (status, _) = get(app, "/homecore/").await; assert_eq!(status, StatusCode::NOT_FOUND, "empty --ui-dir disables the mount"); } + + /// Build the SAME merged + layered surface `main()` serves: API + UI mount + /// + BFF gateway, with the audited CORS allowlist + tracing applied to the + /// whole thing. Used to prove the gateway routes are CORS-covered. + fn full_app(state: SharedState) -> Router { + use crate::gateway::{GatewayConfig, GatewayState}; + let gw = GatewayState::new( + state.clone(), + GatewayConfig { + calibration_url: None, + calibration_token: None, + apps_dir: std::path::PathBuf::from("/nonexistent-apps-dir"), + timeout: std::time::Duration::from_millis(200), + }, + ); + build_app(state, "") + .merge(crate::gateway::gateway_router(gw)) + .layer(homecore_api::build_cors_layer()) + .layer(TraceLayer::new_for_http()) + } + + #[tokio::test] + async fn gateway_routes_are_cors_covered_after_merge() { + // A CORS preflight from the Vite dev origin must succeed (echo the + // allowed origin) for a GATEWAY route — proving the outer CORS layer + // covers the merged routes, not just the homecore-api ones. + let app = full_app(test_state()); + let resp = app + .oneshot( + Request::builder() + .method("OPTIONS") + .uri("/api/homecore/appliance") + .header("origin", "http://localhost:5173") + .header("access-control-request-method", "GET") + .header("access-control-request-headers", "authorization") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + // CORS preflight handled by the layer → 2xx with the origin echoed back. + assert!( + resp.status().is_success(), + "gateway preflight should succeed, got {}", + resp.status() + ); + let allow_origin = resp + .headers() + .get("access-control-allow-origin") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + assert_eq!( + allow_origin, "http://localhost:5173", + "gateway route must echo the allowlisted dev origin" + ); + } } diff --git a/v2/crates/homecore-server/ui/js/api.js b/v2/crates/homecore-server/ui/js/api.js index d1046db2..e53131d0 100644 --- a/v2/crates/homecore-server/ui/js/api.js +++ b/v2/crates/homecore-server/ui/js/api.js @@ -10,7 +10,14 @@ // // Gateway route map: ADR-131 §11.2. -import * as mock from './mock.js'; +// DEV-ONLY fixtures. Loaded via DYNAMIC import so a production bundle that +// never enters demo mode never pulls mock.js into the graph (§2.2). Cached +// after first use so repeated demo calls don't re-import. +let _mock = null; +async function loadMock() { + if (!_mock) _mock = await import('./mock.js'); + return _mock; +} const demoFlags = {}; @@ -50,8 +57,10 @@ export const api = { }, // demo-gated data accessor: real gateway GET in prod, mock fixture in demo. + // The mock module is dynamically imported ONLY on the demo branch, so prod + // never loads it. `mockFn` receives the loaded module. async _data(key, path, mockFn) { - if (demoMode()) { demoFlags[key] = true; return mockFn(); } + if (demoMode()) { demoFlags[key] = true; return mockFn(await loadMock()); } delete demoFlags[key]; return this._get(path); }, @@ -68,28 +77,28 @@ export const api = { async setState(entityId, state, attributes) { return this._post(`/api/states/${entityId}`, { state, attributes: attributes || {} }); }, // ── gateway /api/homecore/* + /api/events (§11.2) ───────────────── - async appliance() { return this._data('appliance', '/api/homecore/appliance', () => mock.applianceHealth()); }, - async seeds() { return this._data('fleet', '/api/homecore/seeds', () => mock.seeds()); }, - async seed(id) { return this._data('fleet', '/api/homecore/seeds/' + encodeURIComponent(id), () => mock.seed(id)); }, + async appliance() { return this._data('appliance', '/api/homecore/appliance', (m) => m.applianceHealth()); }, + async seeds() { return this._data('fleet', '/api/homecore/seeds', (m) => m.seeds()); }, + async seed(id) { return this._data('fleet', '/api/homecore/seeds/' + encodeURIComponent(id), (m) => m.seed(id)); }, async esp32Warnings() { - if (demoMode()) { demoFlags.fleet = true; return mock.esp32Warnings(); } + if (demoMode()) { demoFlags.fleet = true; return (await loadMock()).esp32Warnings(); } const seeds = await this._get('/api/homecore/seeds'); return seeds.flatMap((s) => (s.warnings || []).map((issue) => ({ node_id: s.device_id, seed: s.device_id, issue }))); }, - async cogs() { return this._data('cogs', '/api/homecore/cogs', () => mock.cogs()); }, - async cogUpdates() { return this._data('cogs', '/api/homecore/cogs/updates', () => mock.cogUpdates()); }, - async hailo() { return this._data('cogs', '/api/homecore/hailo', () => ({ worker: 'connected', cogs: mock.cogs().filter((c) => c.arch === 'hailo10') })); }, - async roomStates() { return this._data('rooms', '/api/homecore/rooms', () => mock.roomStates()); }, - async federation() { return this._data('fleet', '/api/homecore/federation', () => mock.federation()); }, - async witnessLog(page = 0, size = 12) { return this._data('audit', `/api/homecore/witness?page=${page}&size=${size}`, () => mock.witnessLog(page, size)); }, - async privacyModes() { return this._data('audit', '/api/homecore/privacy', () => mock.privacyModes()); }, + async cogs() { return this._data('cogs', '/api/homecore/cogs', (m) => m.cogs()); }, + async cogUpdates() { return this._data('cogs', '/api/homecore/cogs/updates', (m) => m.cogUpdates()); }, + async hailo() { return this._data('cogs', '/api/homecore/hailo', (m) => ({ worker: 'connected', cogs: m.cogs().filter((c) => c.arch === 'hailo10') })); }, + async roomStates() { return this._data('rooms', '/api/homecore/rooms', (m) => m.roomStates()); }, + async federation() { return this._data('fleet', '/api/homecore/federation', (m) => m.federation()); }, + async witnessLog(page = 0, size = 12) { return this._data('audit', `/api/homecore/witness?page=${page}&size=${size}`, (m) => m.witnessLog(page, size)); }, + async privacyModes() { return this._data('audit', '/api/homecore/privacy', (m) => m.privacyModes()); }, async setPrivacy(seed, modeValue) { if (demoMode()) return { seed, mode: modeValue }; return this._post('/api/homecore/privacy', { seed, mode: modeValue }); }, - async eventHistory(n = 40) { return this._data('events', `/api/events?limit=${n}`, () => mock.recentEvents(n)); }, + async eventHistory(n = 40) { return this._data('events', `/api/events?limit=${n}`, (m) => m.recentEvents(n)); }, recentEvents(n) { return this.eventHistory(n); }, // back-compat alias (async) - async settings() { return this._data('settings', '/api/homecore/settings', () => mock.settings()); }, + async settings() { return this._data('settings', '/api/homecore/settings', (m) => m.settings()); }, async automations() { return this._data('automations', '/api/homecore/automations', () => []); }, async saveAutomation(a) { if (demoMode()) return a; return this._post('/api/homecore/automations', a); }, - async tokens() { return this._data('settings', '/api/homecore/tokens', () => mock.settings().tokens); }, + async tokens() { return this._data('settings', '/api/homecore/tokens', (m) => m.settings().tokens); }, // calibration (ADR-151) — real proxy in prod, simulated in demo. calibration: makeCalibration(), @@ -123,8 +132,12 @@ export function entityProvenance(entity) { const nodeMatch = src.match(/esp32[-\w]*/i); const node = attrs.node || (nodeMatch ? nodeMatch[0] : null); let seed = attrs.seed || null; - if (!seed && demoMode() && node) { - const cfg = mock.settings().esp32.find((n) => n.node_id === node); + // Demo-only enrichment: consult the mock node registry IF it has already + // been dynamically loaded by a prior demo data call (this fn is sync, so it + // cannot await the import). Prod never has `_mock` set → seed stays null + // (never fabricated). + if (!seed && demoMode() && node && _mock) { + const cfg = _mock.settings().esp32.find((n) => n.node_id === node); seed = cfg ? cfg.seed : null; } const hailo = /hailo|pose/i.test(src) || /hailo/i.test(String(attrs.cog || '')); diff --git a/v2/crates/homecore-server/ui/js/panels/calibration.js b/v2/crates/homecore-server/ui/js/panels/calibration.js index 8ff2406d..6212f74e 100644 --- a/v2/crates/homecore-server/ui/js/panels/calibration.js +++ b/v2/crates/homecore-server/ui/js/panels/calibration.js @@ -9,6 +9,14 @@ export default { const { api } = ctx; const cal = api.calibration; const state = { step: 1, room_id: '', seed: '', baseline_id: null, anchorIdx: 0, trainResult: null }; + // Track the active baseline poll so it can be cancelled on Restart, on a + // step change, and when the panel itself is torn down (the router only + // calls the cleanup this render() returns — a per-card _cleanup was never + // invoked, leaking the setTimeout loop). + let activePoll = null; + function stopPoll() { + if (activePoll) { activePoll.cancelled = true; if (activePoll.timer) clearTimeout(activePoll.timer); activePoll = null; } + } root.appendChild(sectionHeader('Calibration Wizard', 'baseline → enroll → train → verify')); if (cal.demo) root.appendChild(banner('DEMO — cog-calibration HTTP API (ADR-151) simulated in-browser; the live service replaces this (§7.1).', 'amber')); @@ -26,7 +34,7 @@ export default { stepper.appendChild(h('.step-pill' + (cls ? '.' + cls : ''), h('span.n', n < state.step ? '✓' : String(n)), s)); }); } - function go(step) { state.step = step; paintStepper(); render(); } + function go(step) { stopPoll(); state.step = step; paintStepper(); render(); } function render() { clear(body); if (state.step === 1) body.appendChild(step1()); @@ -74,35 +82,51 @@ export default { const c = card({ title: 'Step 2 — Baseline capture (room must be empty)', children: [ progress, meta, baselineLine, - h('.mt', button('Restart', { variant: 'ghost', onClick: () => { cal.reset(); poll.started = false; } })), + h('.mt', button('Restart', { + variant: 'ghost', + // Cancel the in-flight poll loop (was leaked before), reset the + // session, and start a fresh capture. + onClick: () => { stopPoll(); cal.reset(); clear(baselineLine); startCapture(); }, + })), ], }); - const poll = { started: false, timer: null }; - (async () => { - let startRes; - try { startRes = await cal.start(); } - catch (e) { clear(meta); meta.appendChild(banner('Baseline start failed — ' + (e.message || e), 'red')); return; } - state.baseline_id = (startRes && startRes.baseline_id) || state.baseline_id; - const loop = async () => { - let st; - try { st = await cal.status(); } - catch (e) { clear(meta); meta.appendChild(banner('Status unavailable — ' + (e.message || e), 'red')); return; } - progress.firstChild.style.width = (st.frames / st.target * 100).toFixed(0) + '%'; - clear(meta); meta.appendChild(document.createTextNode(`${st.frames}/${st.target} frames · ETA ${st.eta_s}s · z_median ${st.z_median}`)); - if (st.motion_flagged) { if (!c.querySelector('.banner')) c.insertBefore(banner('Room must be empty — movement detected', 'amber'), progress); } - else { const b = c.querySelector('.banner'); if (b) b.remove(); } - if (st.frames >= st.target) { - state.baseline_id = state.baseline_id || 'bl-unknown'; - clear(baselineLine); - baselineLine.appendChild(h('.mt', h('span.green', 'Baseline complete · '), mono(state.baseline_id), h('span.t2', ' (record this — it anchors STALE detection)'))); - baselineLine.appendChild(h('.mt', button('Continue to enrollment', { variant: 'primary', onClick: () => go(3) }))); - return; - } - poll.timer = setTimeout(loop, 600); - }; - loop(); - })(); - c._cleanup = () => poll.timer && clearTimeout(poll.timer); + + // Single-flight: stopPoll() before (re)arming guarantees one loop. + function startCapture() { + stopPoll(); + const session = { cancelled: false, timer: null }; + activePoll = session; + (async () => { + let startRes; + try { startRes = await cal.start(); } + catch (e) { clear(meta); meta.appendChild(banner('Baseline start failed — ' + (e.message || e), 'red')); return; } + if (session.cancelled) return; + state.baseline_id = (startRes && startRes.baseline_id) || state.baseline_id; + const loop = async () => { + if (session.cancelled) return; + let st; + try { st = await cal.status(); } + catch (e) { clear(meta); meta.appendChild(banner('Status unavailable — ' + (e.message || e), 'red')); return; } + if (session.cancelled) return; + progress.firstChild.style.width = pct(st.frames, st.target) + '%'; + clear(meta); meta.appendChild(document.createTextNode(`${st.frames}/${st.target} frames · ETA ${st.eta_s}s · z_median ${st.z_median}`)); + if (st.motion_flagged) { if (!c.querySelector('.banner')) c.insertBefore(banner('Room must be empty — movement detected', 'amber'), progress); } + else { const b = c.querySelector('.banner'); if (b) b.remove(); } + if (st.target > 0 && st.frames >= st.target) { + activePoll = null; + state.baseline_id = state.baseline_id || 'bl-unknown'; + clear(baselineLine); + baselineLine.appendChild(h('.mt', h('span.green', 'Baseline complete · '), mono(state.baseline_id), h('span.t2', ' (record this — it anchors STALE detection)'))); + baselineLine.appendChild(h('.mt', button('Continue to enrollment', { variant: 'primary', onClick: () => go(3) }))); + return; + } + session.timer = setTimeout(loop, 600); + }; + loop(); + })(); + } + + startCapture(); return c; } @@ -206,10 +230,17 @@ export default { paintStepper(); render(); - return () => {}; + // The router invokes this on navigation away — tear down any live poll. + return () => stopPoll(); }, }; +// Guard against NaN%/Infinity% when target is 0/missing (§4.7 robustness). +function pct(frames, target) { + if (!(target > 0)) return 0; + return Math.max(0, Math.min(100, (frames / target) * 100)).toFixed(0); +} + function instruction(label) { const map = { empty: 'Leave the room empty and still.', diff --git a/v2/crates/homecore-server/ui/js/panels/cogs.js b/v2/crates/homecore-server/ui/js/panels/cogs.js index 0bb902d3..3b889d2a 100644 --- a/v2/crates/homecore-server/ui/js/panels/cogs.js +++ b/v2/crates/homecore-server/ui/js/panels/cogs.js @@ -39,8 +39,19 @@ export default { } // ── Hailo HEF status ─────────────────────────────────────────── + // §6 honesty: the worker pill must reflect the REAL probe, not a + // hardcoded "connected". Probe the appliance services for the + // ruvector-hailo-worker; if that upstream is unavailable, show the + // status as unknown rather than fabricating "connected". + let workerStatus = 'unknown'; + try { + const appliance = await api.appliance(); + const svc = (appliance.services || []).find((s) => s.name === 'ruvector-hailo-worker'); + if (svc && svc.status) workerStatus = svc.status; + } catch { /* leave 'unknown' — honest not-available, never fabricated */ } + root.appendChild(h('h2.mt', 'Hailo-10H accelerator')); - root.appendChild(hailoStatus(cogs)); + root.appendChild(hailoStatus(cogs, workerStatus)); return () => {}; }, @@ -107,7 +118,7 @@ function logText(c) { return [ `[info] ${c.id} v${c.version} running (pid ${c.pid})`, `[info] arch=${c.arch} sha256_verified=${c.sha256_verified} signature_verified=${c.signature_verified}`, - c.arch === 'hailo10' ? `[info] hailo: ${(c.hef || []).join(', ') || 'no HEF loaded'} @ ${c.throughput_fps || '—'} fps` : '[info] cpu-only worker, no Hailo offload', + c.arch === 'hailo10' ? `[info] hailo: ${asArray(c.hef).join(', ') || 'no HEF loaded'} @ ${c.throughput_fps || '—'} fps` : '[info] cpu-only worker, no Hailo offload', '[info] heartbeat ok', ].join('\n'); } @@ -120,12 +131,21 @@ function configJson(c) { autostart: c.status !== 'stopped', }; if (c.arch === 'hailo10') { - cfg.hef = c.hef || []; + cfg.hef = asArray(c.hef); cfg.target_fps = c.throughput_fps || null; } return JSON.stringify(cfg, null, 2); } +// Coerce a forwarded manifest `hef` (array | string | object | null) into an +// array so a non-array value degrades gracefully instead of throwing on +// .forEach/.join/.length (the gateway forwards it verbatim — §11). +function asArray(v) { + if (Array.isArray(v)) return v; + if (v == null || v === '') return []; + return [v]; +} + // ── OTA update diff card ───────────────────────────────────────────── function updateCard(u) { const diff = h('div', @@ -148,19 +168,22 @@ function diffList(title, items, color) { } // ── Hailo HEF status ───────────────────────────────────────────────── -function hailoStatus(cogs) { +function hailoStatus(cogs, workerStatus = 'unknown') { const hailoCogs = cogs.filter((c) => c.arch === 'hailo10'); - const worker = h('.flex.gap-sm', statusPill('connected'), h('span.mono.t2', 'ruvector-hailo-worker:50051')); + // statusPill maps 'running'/'connected'→green, 'unreachable'/'error'→red, + // 'unknown'→grey; the real probe drives the colour, never a hardcode. + const worker = h('.flex.gap-sm', statusPill(workerStatus), h('span.mono.t2', 'ruvector-hailo-worker:50051')); const body = h('div', worker); if (!hailoCogs.length) { body.appendChild(h('.muted-empty', 'No Hailo-sourced COGs loaded.')); } else { hailoCogs.forEach((c) => { + const hef = asArray(c.hef); // gateway forwards manifest `hef` verbatim — may be a string const hefRows = h('div', h('.flex.spread', h('strong.mono', `${c.id} ${c.version}`), pill((c.throughput_fps || 0) + ' fps', 'purple'))); - (c.hef || []).forEach((f) => hefRows.appendChild(h('.row', h('span.mono.purple', f), h('span.t2', 'loaded')))); - if (!(c.hef || []).length) hefRows.appendChild(h('.muted-empty', 'no .hef files loaded')); + hef.forEach((f) => hefRows.appendChild(h('.row', h('span.mono.purple', f), h('span.t2', 'loaded')))); + if (!hef.length) hefRows.appendChild(h('.muted-empty', 'no .hef files loaded')); body.appendChild(h('.mt', hefRows)); }); } diff --git a/v2/crates/homecore-server/ui/js/panels/dashboard.js b/v2/crates/homecore-server/ui/js/panels/dashboard.js index 5b1905d0..d0782177 100644 --- a/v2/crates/homecore-server/ui/js/panels/dashboard.js +++ b/v2/crates/homecore-server/ui/js/panels/dashboard.js @@ -19,10 +19,10 @@ export default { await section(root, 'v0 Appliance health', async () => { const a = await api.appliance(); const strip = h('.metric-grid', - metric({ icon: '🖥', value: a.cpu_pct + '%', label: 'CPU' }), - metric({ icon: '🧠', value: a.ram_pct + '%', label: 'RAM' }), - metric({ icon: '⚡', value: a.hailo_load_pct + '%', label: 'Hailo-10H load' }), - metric({ icon: '🌡', value: a.hailo_temp_c + '°C', label: 'Hailo temp' }), + metric({ icon: '🖥', value: pctOrNA(a.cpu_pct), label: 'CPU' }), + metric({ icon: '🧠', value: pctOrNA(a.ram_pct), label: 'RAM' }), + metric({ icon: '⚡', value: pctOrNA(a.hailo_load_pct), label: 'Hailo-10H load' }), + metric({ icon: '🌡', value: unitOrNA(a.hailo_temp_c, '°C'), label: 'Hailo temp' }), metric({ icon: '⏱', value: fmtUptime(a.uptime_s), label: 'Uptime', color: 'green' })); const healthCard = card({ title: 'v0 Appliance health', children: [strip, servicesRow(a.services)] }); return h('div', healthCard, eventBus(a, ctx, (fn) => { cleanupEvent = fn; })); @@ -135,7 +135,13 @@ function eventBus(a, ctx, setCleanup) { return card({ title: 'Event Bus activity', children: [body] }); } +// §6 honesty: a null/undefined metric must render a distinct not-available +// state ('—'), never a fabricated value like "null%"/"null°C". +function pctOrNA(v) { return v == null ? '—' : v + '%'; } +function unitOrNA(v, unit) { return v == null ? '—' : v + unit; } + function fmtUptime(s) { + if (s == null) return '—'; const d = Math.floor(s / 86400), hh = Math.floor((s % 86400) / 3600); return d > 0 ? `${d}d ${hh}h` : `${hh}h`; } diff --git a/v2/crates/homecore-server/ui/js/panels/rooms.js b/v2/crates/homecore-server/ui/js/panels/rooms.js index 94ea969b..7dbff403 100644 --- a/v2/crates/homecore-server/ui/js/panels/rooms.js +++ b/v2/crates/homecore-server/ui/js/panels/rooms.js @@ -88,8 +88,17 @@ function vitalRow(label, spec, unit, range, r) { function anomalyRow(a) { if (!a) return specRow('Anomaly', notTrainedNode(), null); - const over = a.value > (a.threshold ?? 0.8); - const b = bar(a.value, 1, [{ lt: a.threshold ?? 0.8, color: 'green' }, { lt: 1.01, color: 'red' }]); + // §6 honesty: a null threshold is WITHHELD (the upstream RoomState carried + // none) — show the value but flag the threshold as unavailable rather than + // judging anomalous/normal against a fabricated 0.8 default. + if (a.threshold == null) { + const wrap = h('div', { style: { width: '160px' } }, + bar(a.value, 1), + h('small.ts', { title: 'no anomaly threshold from upstream — withheld' }, `${a.value} · threshold —`)); + return specRow('Anomaly', wrap, a); + } + const over = a.value > a.threshold; + const b = bar(a.value, 1, [{ lt: a.threshold, color: 'green' }, { lt: 1.01, color: 'red' }]); const wrap = h('div', { style: { width: '160px' } }, b, h('small.ts', over ? 'anomalous' : 'normal', ` · ${a.value}`)); return specRow('Anomaly', wrap, a); diff --git a/v2/crates/homecore-server/ui/package.json b/v2/crates/homecore-server/ui/package.json index d2c555d6..9ced11be 100644 --- a/v2/crates/homecore-server/ui/package.json +++ b/v2/crates/homecore-server/ui/package.json @@ -6,7 +6,7 @@ "description": "HOMECORE-UI — operational dashboard for the two-tier Cognitum stack (ADR-131). Zero-dependency vanilla TS/JS + CSS; served by homecore-server at /homecore.", "scripts": { "check": "node tests/verify-imports.mjs", - "test": "node tests/verify-imports.mjs && node tests/boot.mjs && node tests/render-smoke.mjs && node tests/interaction.mjs && node tests/prod-errors.mjs", + "test": "node tests/verify-imports.mjs && node tests/boot.mjs && node tests/render-smoke.mjs && node tests/interaction.mjs && node tests/prod-errors.mjs && node tests/unit-fixes.mjs", "bench": "node tests/benchmark.mjs" } } diff --git a/v2/crates/homecore-server/ui/tests/unit-fixes.mjs b/v2/crates/homecore-server/ui/tests/unit-fixes.mjs new file mode 100644 index 00000000..28938b77 --- /dev/null +++ b/v2/crates/homecore-server/ui/tests/unit-fixes.mjs @@ -0,0 +1,101 @@ +// Regression tests pinning the ADR-131 PR-1082 review fixes: +// * dashboard renders a not-available state ('—') for null appliance +// metrics — never "null%"/"null°C" (§6 honesty / fabricated-data fix). +// * cogs panel does NOT throw when the gateway forwards a `hef` that is a +// string (or other non-array) instead of an array (crash/robustness fix). +// * cogs Hailo worker pill reflects the real probe, not a hardcoded +// "connected" (§6 honesty fix). +// Run: node tests/unit-fixes.mjs +import { install } from './dom-shim.mjs'; +install(); +globalThis.HOMECORE_UI_DEMO = false; // production path — no fixtures + +const fails = [], passes = []; +async function t(name, fn) { + try { await fn(); passes.push(name); } + catch (e) { fails.push(`${name}: ${e && e.stack ? e.stack.split('\n').slice(0, 3).join(' | ') : e}`); } +} +const assert = (c, m) => { if (!c) throw new Error(m || 'assertion failed'); }; + +const { api } = await import('../js/api.js'); + +// Shared ctx; per-test we override the api accessors we need. +function ctxWith(overrides) { + return { + api: Object.assign(Object.create(api), overrides), + navigate() {}, + params: {}, + onEvent() { return () => {}; }, + onWs(fn) { fn({ state: 'closed', lagged: false }); return () => {}; }, + }; +} + +// ── dashboard: null metrics → '—', never "null%"/"null°C" ───────────── +await t('dashboard renders not-available for null hailo metrics (no "null%")', async () => { + const mod = await import('../js/panels/dashboard.js'); + const root = document.createElement('div'); + const ctx = ctxWith({ + appliance: async () => ({ + cpu_pct: 12.5, ram_pct: 40.1, + hailo_load_pct: null, hailo_temp_c: null, // the fabricated-data trap + uptime_s: null, + services: [{ name: 'ruview-mcp-brain', port: 9876, status: 'unreachable' }], + event_rate: [], channel_capacity: 4096, channel_lag: 0, + }), + seeds: async () => [], + esp32Warnings: async () => [], + cogs: async () => [], + anyDemo: () => false, + }); + const cleanup = await mod.default.render(root, ctx); + const text = root.textContent; + assert(!/null\s*%/.test(text), `dashboard showed "null%": ${text.slice(0, 200)}`); + assert(!/null\s*°C/.test(text), `dashboard showed "null°C": ${text.slice(0, 200)}`); + assert(text.includes('—'), 'dashboard should render the "—" not-available marker for null metrics'); + // real values must still concatenate their unit + assert(text.includes('12.5%'), 'real CPU value must still render with its unit'); + if (typeof cleanup === 'function') cleanup(); +}); + +// ── cogs: string `hef` must not throw ───────────────────────────────── +await t('cogs does not throw when hef is a string (non-array)', async () => { + const mod = await import('../js/panels/cogs.js'); + const root = document.createElement('div'); + const ctx = ctxWith({ + cogs: async () => [ + { id: 'cog-pose', version: '1.0', arch: 'hailo10', status: 'running', pid: 42, + sha256_verified: true, signature_verified: true, throughput_fps: 30, + hef: 'pose_estimation.hef' }, // STRING, not array — the crash trap + ], + cogUpdates: async () => [], + appliance: async () => ({ services: [{ name: 'ruvector-hailo-worker', port: 50051, status: 'running' }] }), + isDemo: () => false, + }); + // If asArray() weren't applied, .forEach/.join/.length on a string would throw. + const cleanup = await mod.default.render(root, ctx); + assert(root.children.length > 0, 'cogs rendered nothing'); + // The string hef should surface as a single loaded HEF row. + assert(root.textContent.includes('pose_estimation.hef'), 'string hef should render as one HEF entry'); + if (typeof cleanup === 'function') cleanup(); +}); + +// ── cogs: Hailo worker pill reflects the real probe, not hardcoded ──── +await t('cogs Hailo worker pill is unknown when appliance probe is unavailable', async () => { + const mod = await import('../js/panels/cogs.js'); + const root = document.createElement('div'); + const ctx = ctxWith({ + cogs: async () => [], + cogUpdates: async () => [], + appliance: async () => { throw new Error('appliance upstream down'); }, // probe fails + isDemo: () => false, + }); + const cleanup = await mod.default.render(root, ctx); + // statusPill('unknown') → grey pill containing the literal label "unknown". + assert(root.textContent.includes('unknown'), 'worker status should be honestly "unknown" when probe fails'); + assert(!/connected/.test(root.textContent), 'worker pill must not fabricate "connected"'); + if (typeof cleanup === 'function') cleanup(); +}); + +console.log(`\n${passes.length} passed, ${fails.length} failed`); +if (fails.length) { console.error('\nFAILURES:'); fails.forEach((f) => console.error(' ✗ ' + f)); process.exit(1); } +console.log('OK — dashboard not-available, cogs string-hef + honest worker pill pinned');