From 0ca903b49789700be2aa2b9c5484f7aea49876d7 Mon Sep 17 00:00:00 2001 From: ruv Date: Fri, 12 Jun 2026 01:33:52 -0400 Subject: [PATCH 1/3] feat(homecore-plugins): enforce plugin signature + capability isolation (ADR-162 P4/P5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-161 honestly relabelled the manifest's wasm_module_hash / wasm_module_sig / publisher_key as "(P4 — not yet enforced)" and the homecore_permissions claims as deferred P5 authority isolation. This makes both real and tested. P4 (signature/integrity verification, SECURITY): - New `verify` module: SHA-256 module-hash check + Ed25519 signature verification over the digest against publisher_key, with a PluginPolicy trust allowlist and an explicit AllowUnsigned dev escape hatch (loud warn). Secure default rejects unsigned / unknown-publisher / tampered modules. - Reuses the in-repo cog-ha-matter::witness_signing Ed25519 pattern; sha2 is a workspace dep, ed25519-dalek/hex/base64 already in the lock — no new external dep tree (only new edges in homecore-plugins). - WasmtimeRuntime::load_plugin verifies before instantiation; legacy load_wasm retained for trusted/test modules. P5 (authority/capability isolation, SECURITY): - New `permissions` module: PermissionSet distilled from homecore_permissions (state:write: or bare entity glob). hc_state_set now consults it and returns a typed -3 to the guest on an undeclared write (no host panic). Tests (fail on old code, which had no load_plugin/verify and an unchecked hc_state_set): tampered module rejected; valid sig from trusted key loads; valid sig from untrusted key rejected; unsigned rejected by default and loads only under AllowUnsigned; light.* plugin writes light.kitchen but is denied lock.front_door; no-permission plugin can write nothing. Real deterministic keypair signs real bytes. Manifest doc updated: P4/P5 now ENFORCED (was "not yet enforced"). homecore-plugins --features wasmtime: 32 passed (lib 23, integration 9), 0 failed. Co-Authored-By: claude-flow --- v2/Cargo.lock | 4 + v2/crates/homecore-plugins/Cargo.toml | 9 + v2/crates/homecore-plugins/src/error.rs | 12 + v2/crates/homecore-plugins/src/lib.rs | 16 +- v2/crates/homecore-plugins/src/manifest.rs | 24 +- v2/crates/homecore-plugins/src/permissions.rs | 168 ++++++++ v2/crates/homecore-plugins/src/verify.rs | 397 ++++++++++++++++++ .../homecore-plugins/src/wasmtime_runtime.rs | 77 +++- .../homecore-plugins/tests/integration.rs | 255 +++++++++++ 9 files changed, 947 insertions(+), 15 deletions(-) create mode 100644 v2/crates/homecore-plugins/src/permissions.rs create mode 100644 v2/crates/homecore-plugins/src/verify.rs diff --git a/v2/Cargo.lock b/v2/Cargo.lock index c31100fd..2d8d9ecc 100644 --- a/v2/Cargo.lock +++ b/v2/Cargo.lock @@ -3554,9 +3554,13 @@ name = "homecore-plugins" version = "0.1.0-alpha.0" dependencies = [ "async-trait", + "base64 0.22.1", + "ed25519-dalek", + "hex", "homecore", "serde", "serde_json", + "sha2", "thiserror 1.0.69", "tokio", "uuid", diff --git a/v2/crates/homecore-plugins/Cargo.toml b/v2/crates/homecore-plugins/Cargo.toml index 3b7325fa..7c4b23a1 100644 --- a/v2/crates/homecore-plugins/Cargo.toml +++ b/v2/crates/homecore-plugins/Cargo.toml @@ -50,6 +50,15 @@ serde_json = "1" # UUIDs for config entry IDs in host_abi.rs. uuid = { version = "1", features = ["v4"] } +# ── ADR-162 P4: plugin signature + integrity verification ────────────────── +# Reuses the same in-repo crypto stack as cog-ha-matter (witness_signing.rs): +# Ed25519 over a SHA-256 module digest. All four are already in the workspace +# Cargo.lock (cog-ha-matter / bfld pull them in) — no new external dep tree. +ed25519-dalek = "2.1" +sha2 = { workspace = true } +hex = "0.4" +base64 = "0.22" + # Optional Wasmtime runtime (P2, default-off — 30 MB dep). # Bumped from 25.0.3 → 42 to remediate RUSTSEC-2026-0095 and RUSTSEC-2026-0096 # (Cranelift/Winch sandbox-escape CVEs, CVSS 9.0 — iter-11 security sprint HC-03/04). diff --git a/v2/crates/homecore-plugins/src/error.rs b/v2/crates/homecore-plugins/src/error.rs index 6fe4bb9a..468aa801 100644 --- a/v2/crates/homecore-plugins/src/error.rs +++ b/v2/crates/homecore-plugins/src/error.rs @@ -25,6 +25,18 @@ pub enum PluginError { #[error("plugin setup failed: {0}")] SetupFailed(String), + /// The plugin failed signature/integrity verification (ADR-162 P4): + /// hash mismatch, bad signature, untrusted publisher, or unsigned + /// module under a non-dev trust policy. + #[error("plugin signature rejected: {0}")] + SignatureRejected(String), + + /// A plugin attempted a host call (e.g. `hc_state_set`) on an entity + /// it did not declare in `homecore_permissions` (ADR-162 P5 authority + /// isolation). + #[error("plugin permission denied: {0}")] + PermissionDenied(String), + /// The plugin's `unload` hook returned an error. #[error("plugin unload failed: {0}")] UnloadFailed(String), diff --git a/v2/crates/homecore-plugins/src/lib.rs b/v2/crates/homecore-plugins/src/lib.rs index 296d4aa5..c64d7d9f 100644 --- a/v2/crates/homecore-plugins/src/lib.rs +++ b/v2/crates/homecore-plugins/src/lib.rs @@ -22,8 +22,16 @@ //! - Host ABI wiring: `hc_state_get`, `hc_state_set`, `hc_event_fire`, etc. //! (P2 — requires ADR-127 state machine API freeze first). //! - Config entry lifecycle + hot-load (P3). -//! - Cog registry distribution + Ed25519 signature verification (P4). -//! - Permission enforcement (P5). +//! +//! ## Now enforced (ADR-162) +//! +//! - **Ed25519 signature + SHA-256 integrity verification (P4)** — see +//! [`verify`]: the plugin load path hashes the real `.wasm` bytes, checks +//! the manifest `wasm_module_hash`, verifies `wasm_module_sig` against +//! `publisher_key`, and enforces a [`verify::PluginPolicy`] allowlist. +//! - **Permission / authority isolation (P5)** — see [`permissions`]: a +//! plugin's `hc_state_set` writes are gated against the entity domains/ +//! globs it declared in `homecore_permissions`. //! //! ## Feature flags //! @@ -35,9 +43,11 @@ pub mod error; pub mod host_abi; pub mod manifest; +pub mod permissions; pub mod plugin; pub mod registry; pub mod runtime; +pub mod verify; #[cfg(feature = "wasmtime")] pub mod wasmtime_runtime; @@ -45,9 +55,11 @@ pub mod wasmtime_runtime; pub use error::PluginError; pub use host_abi::{ConfigEntryJson, StateChangedEventJson}; pub use manifest::{IotClass, IntegrationType, PluginManifest}; +pub use permissions::PermissionSet; pub use plugin::{HomeCorePlugin, PluginId}; pub use registry::PluginRegistry; pub use runtime::{InProcessRuntime, LoadedPlugin, PluginRuntime}; +pub use verify::{verify_module, PluginPolicy}; #[cfg(feature = "wasmtime")] pub use wasmtime_runtime::{WasmPlugin, WasmtimeRuntime}; diff --git a/v2/crates/homecore-plugins/src/manifest.rs b/v2/crates/homecore-plugins/src/manifest.rs index 4480fc4d..106bc29b 100644 --- a/v2/crates/homecore-plugins/src/manifest.rs +++ b/v2/crates/homecore-plugins/src/manifest.rs @@ -85,24 +85,26 @@ pub struct PluginManifest { /// [HOMECORE] `sha256:` hash of the wasm binary. /// - /// **(P4 — not yet enforced, ADR-161/B5):** this field is parsed and - /// round-tripped but is NOT verified before execution. The hash/sig - /// gate lands in P4; until then the presence of this field implies no - /// integrity guarantee. + /// **(P4 — ENFORCED, ADR-162):** `verify::verify_module` computes the + /// SHA-256 of the real `.wasm` bytes on load and rejects the module if + /// it does not equal this hash (tamper detection). See [`crate::verify`]. #[serde(default, skip_serializing_if = "Option::is_none")] pub wasm_module_hash: Option, /// [HOMECORE] Ed25519 signature of the wasm binary hash (`ed25519:`). /// - /// **(P4 — not yet enforced, ADR-161/B5):** parsed but never checked. - /// No signature verification happens before a plugin runs. + /// **(P4 — ENFORCED, ADR-162):** verified against `publisher_key` over + /// the SHA-256 module digest before instantiation. A bad/forged/absent + /// signature is rejected under the secure trust policy (the + /// `cog-ha-matter::witness_signing` Ed25519 pattern is reused). #[serde(default, skip_serializing_if = "Option::is_none")] pub wasm_module_sig: Option, /// [HOMECORE] Ed25519 public key of the plugin publisher. /// - /// **(P4 — not yet enforced, ADR-161/B5):** parsed but never used to - /// verify `wasm_module_sig`. + /// **(P4 — ENFORCED, ADR-162):** used to verify `wasm_module_sig`, and + /// checked against the host's [`crate::verify::PluginPolicy`] trust + /// allowlist — an unknown publisher is rejected by the secure default. #[serde(default, skip_serializing_if = "Option::is_none")] pub publisher_key: Option, @@ -115,6 +117,12 @@ pub struct PluginManifest { pub host_imports_required: Vec, /// [HOMECORE] Coarse-grained permission claims (glob patterns). + /// + /// **(P5 — ENFORCED, ADR-162):** `state:write:` (or a bare entity + /// glob like `light.*`) grants are parsed into a + /// [`crate::permissions::PermissionSet` ] and consulted by the + /// `hc_state_set` host import. A plugin can no longer write an entity it + /// did not declare; a plugin with no write grants can write nothing. #[serde(default)] pub homecore_permissions: Vec, diff --git a/v2/crates/homecore-plugins/src/permissions.rs b/v2/crates/homecore-plugins/src/permissions.rs new file mode 100644 index 00000000..67eef26e --- /dev/null +++ b/v2/crates/homecore-plugins/src/permissions.rs @@ -0,0 +1,168 @@ +//! Plugin authority / capability isolation (ADR-162, P5). +//! +//! Wasmtime already gives a plugin **memory** isolation — it cannot read +//! another plugin's linear memory. It does NOT, by itself, stop a plugin +//! from using a host import to write any entity it likes. Before this fix +//! `hc_state_set` happily let any plugin write `lock.front_door` or +//! `alarm_control_panel.*`, and the manifest's `homecore_permissions` +//! claims were parsed but **never consulted** (ADR-161 deferred P5). +//! +//! This module adds **authority isolation**: a plugin may only write +//! entities its manifest declared. The host import consults a +//! [`PermissionSet`] before applying any state write and returns a typed +//! error to the guest (it does **not** panic the host) on a violation. +//! +//! ## Permission grammar +//! +//! Each entry in `homecore_permissions` is one of: +//! +//! * a bare entity glob — `"light.*"`, `"light.kitchen"`, `"*"`; +//! * the explicit capability form `"state:write:"` (the form the +//! ADR-128 manifest doc shows), e.g. `"state:write:sensor.*"`. +//! +//! A glob supports a single trailing `*` (HA-style domain wildcards: +//! `light.*` matches every `light` entity) and a leading-or-bare `*` +//! (`*` = everything). Exact strings match exactly. A plugin with **no** +//! `state:write` entries can write **nothing** — the secure default. + +use crate::manifest::PluginManifest; + +/// The set of entity-write permissions a plugin holds, distilled from its +/// manifest `homecore_permissions` at load time. +#[derive(Debug, Clone, Default)] +pub struct PermissionSet { + /// Glob patterns the plugin may write (state:write authority). Empty = + /// the plugin may write nothing. + write_globs: Vec, +} + +impl PermissionSet { + /// Build a permission set from a manifest's `homecore_permissions`. + /// + /// Only `state:write` authority is modelled here (the host import this + /// gates is `hc_state_set`). A bare glob (`"light.*"`) is treated as a + /// write grant; the explicit `"state:write:"` form is also + /// accepted. Other capability strings (`state:read:*`, future verbs) + /// are ignored for write-gating purposes. + pub fn from_manifest(manifest: &PluginManifest) -> Self { + let mut write_globs = Vec::new(); + for claim in &manifest.homecore_permissions { + let claim = claim.trim(); + if let Some(glob) = claim.strip_prefix("state:write:") { + write_globs.push(glob.trim().to_string()); + } else if claim.starts_with("state:read:") { + // read authority — not relevant to write gating. + } else if !claim.is_empty() { + // Bare glob — treat as a write grant. + write_globs.push(claim.to_string()); + } + } + Self { write_globs } + } + + /// An all-allowing set (equivalent to a `"*"` grant). Used by the + /// legacy permission-free `WasmtimeRuntime::load_wasm` path so existing + /// callers/tests that do not supply a manifest keep working; the + /// permission-gated path uses [`Self::from_manifest`]. + pub fn allow_all() -> Self { + Self { + write_globs: vec!["*".to_string()], + } + } + + /// May this plugin write the given entity id (e.g. `"light.kitchen"`)? + pub fn may_write(&self, entity_id: &str) -> bool { + self.write_globs.iter().any(|g| glob_matches(g, entity_id)) + } + + /// Number of write-grant globs (0 = can write nothing). + pub fn write_grant_count(&self) -> usize { + self.write_globs.len() + } +} + +/// Match `entity_id` against a single glob pattern. +/// +/// Supported forms: +/// * `"*"` → matches anything. +/// * `"light.*"` → trailing wildcard: any id with the `light.` prefix. +/// * `"light.kitchen"` → exact match. +fn glob_matches(pattern: &str, entity_id: &str) -> bool { + if pattern == "*" { + return true; + } + if let Some(prefix) = pattern.strip_suffix('*') { + return entity_id.starts_with(prefix); + } + pattern == entity_id +} + +#[cfg(test)] +mod tests { + use super::*; + + fn manifest_with(perms: &[&str]) -> PluginManifest { + PluginManifest { + domain: "p".into(), + name: "P".into(), + version: "1".into(), + documentation: None, + iot_class: None, + config_flow: false, + integration_type: None, + dependencies: vec![], + requirements: vec![], + wasm_module: None, + wasm_module_hash: None, + wasm_module_sig: None, + publisher_key: None, + min_homecore_version: None, + host_imports_required: vec![], + homecore_permissions: perms.iter().map(|s| s.to_string()).collect(), + cog_id: None, + } + } + + #[test] + fn domain_glob_allows_same_domain_only() { + let ps = PermissionSet::from_manifest(&manifest_with(&["light.*"])); + assert!(ps.may_write("light.kitchen")); + assert!(ps.may_write("light.bedroom")); + assert!(!ps.may_write("lock.front_door")); + assert!(!ps.may_write("alarm_control_panel.home")); + } + + #[test] + fn no_permissions_can_write_nothing() { + let ps = PermissionSet::from_manifest(&manifest_with(&[])); + assert_eq!(ps.write_grant_count(), 0); + assert!(!ps.may_write("light.kitchen")); + assert!(!ps.may_write("sensor.temp")); + } + + #[test] + fn explicit_state_write_form_is_honored() { + let ps = PermissionSet::from_manifest(&manifest_with(&["state:write:sensor.*"])); + assert!(ps.may_write("sensor.temp")); + assert!(!ps.may_write("light.kitchen")); + } + + #[test] + fn read_grants_do_not_confer_write() { + let ps = PermissionSet::from_manifest(&manifest_with(&["state:read:lock.*"])); + assert!(!ps.may_write("lock.front_door")); + } + + #[test] + fn exact_entity_grant_is_scoped() { + let ps = PermissionSet::from_manifest(&manifest_with(&["light.kitchen"])); + assert!(ps.may_write("light.kitchen")); + assert!(!ps.may_write("light.bedroom")); + } + + #[test] + fn wildcard_grants_everything() { + let ps = PermissionSet::from_manifest(&manifest_with(&["*"])); + assert!(ps.may_write("lock.front_door")); + } +} diff --git a/v2/crates/homecore-plugins/src/verify.rs b/v2/crates/homecore-plugins/src/verify.rs new file mode 100644 index 00000000..04b31ae4 --- /dev/null +++ b/v2/crates/homecore-plugins/src/verify.rs @@ -0,0 +1,397 @@ +//! Plugin signature & integrity verification (ADR-162, P4). +//! +//! ADR-161/B5 honestly relabelled the manifest's `wasm_module_hash` / +//! `wasm_module_sig` / `publisher_key` fields as "(P4 — not yet enforced)": +//! they were parsed and round-tripped but **never checked** before a plugin +//! ran. This module makes that claim TRUE — it is the real verification gate +//! the plugin load path runs before instantiating any `.wasm` module. +//! +//! ## What is verified, in order +//! +//! 1. **Module hash** — SHA-256 of the actual `.wasm` bytes must equal the +//! manifest's `wasm_module_hash` (`sha256:`). A tampered module +//! (one byte changed) fails here. +//! 2. **Ed25519 signature** — `wasm_module_sig` (`ed25519:`, 64-byte +//! raw signature) must verify over the **32-byte SHA-256 digest** under +//! the `publisher_key` (`ed25519:`, 32-byte raw verifying key). +//! 3. **Trust policy** — the `publisher_key` must be on the configured +//! allowlist, unless [`PluginPolicy::AllowUnsigned`] is in force (a loud +//! dev escape hatch). +//! +//! The crypto mirrors the in-repo Ed25519 pattern from +//! `cog-ha-matter::witness_signing` (same `ed25519-dalek` 2.x API, same +//! deterministic-test-key convention). SHA-256 matches the `sha256:` prefix +//! the manifest doc already declared for `wasm_module_hash`, and the +//! `cog-ha-matter` cog manifest's `binary_sha256` hex convention. +//! +//! ## Secure default +//! +//! [`PluginPolicy::trusted`] (the production constructor) **rejects**: +//! * an unsigned module (no hash / sig / key), +//! * a signature from a key not on the allowlist, +//! * any hash or signature mismatch. +//! +//! Only [`PluginPolicy::AllowUnsigned`] loosens this, and every load it +//! waves through emits a `warn`-level log line so it cannot pass silently. + +use base64::Engine as _; +use ed25519_dalek::{Signature, Verifier, VerifyingKey}; +use sha2::{Digest, Sha256}; + +use crate::error::PluginError; +use crate::manifest::PluginManifest; + +/// Trust policy governing which plugins may load. +/// +/// The production path uses [`PluginPolicy::trusted`] with an explicit +/// allowlist of publisher verifying keys. [`PluginPolicy::AllowUnsigned`] +/// is the dev escape hatch — it loads anything (even unsigned modules) but +/// logs a loud warning per load. +#[derive(Debug, Clone)] +pub enum PluginPolicy { + /// Secure default: a plugin loads only if its module hash matches, its + /// Ed25519 signature verifies, AND its publisher key is in this + /// allowlist. Each entry is the 32-byte raw Ed25519 verifying key. + Trusted { allowlist: Vec<[u8; 32]> }, + /// Dev-only: skip signature/allowlist enforcement. Hash is still + /// checked when a `wasm_module_hash` is present (cheap integrity), but + /// unsigned / unknown-publisher modules are allowed. Every load logs a + /// loud `warn`. + AllowUnsigned, +} + +impl PluginPolicy { + /// Construct the secure (production) policy from a list of trusted + /// publisher keys, each encoded as `ed25519:` (the same form + /// the manifest `publisher_key` uses). + pub fn trusted(publisher_keys: &[&str]) -> Result { + let mut allowlist = Vec::with_capacity(publisher_keys.len()); + for k in publisher_keys { + allowlist.push(decode_verifying_key(k)?.to_bytes()); + } + Ok(PluginPolicy::Trusted { allowlist }) + } + + /// Secure policy that trusts no publisher at all — every signed or + /// unsigned module is rejected. Useful as a strict default. + pub fn deny_all() -> Self { + PluginPolicy::Trusted { allowlist: vec![] } + } + + fn is_dev(&self) -> bool { + matches!(self, PluginPolicy::AllowUnsigned) + } + + fn allows(&self, key: &VerifyingKey) -> bool { + match self { + PluginPolicy::AllowUnsigned => true, + PluginPolicy::Trusted { allowlist } => { + allowlist.iter().any(|k| k == &key.to_bytes()) + } + } + } +} + +/// Verify a `.wasm` module's integrity and signature against its manifest, +/// under the given trust `policy`. Returns `Ok(())` only if the module may +/// be instantiated. +/// +/// On [`PluginPolicy::AllowUnsigned`] this still checks any present hash, +/// but waves through missing/untrusted signatures with a loud `warn`. +pub fn verify_module( + manifest: &PluginManifest, + wasm_bytes: &[u8], + policy: &PluginPolicy, +) -> Result<(), PluginError> { + let signed = manifest.wasm_module_hash.is_some() + || manifest.wasm_module_sig.is_some() + || manifest.publisher_key.is_some(); + + if !signed { + // No integrity material at all. + if policy.is_dev() { + eprintln!( + "[PLUGIN WARN] loading UNSIGNED plugin `{}` — no wasm_module_hash/sig/publisher_key. \ + AllowUnsigned dev policy is active; this is INSECURE and must not be used in production.", + manifest.domain + ); + return Ok(()); + } + return Err(PluginError::SignatureRejected(format!( + "plugin `{}` is unsigned (no wasm_module_hash/sig/publisher_key) and the trust policy \ + rejects unsigned modules; set PluginPolicy::AllowUnsigned to override in dev", + manifest.domain + ))); + } + + // (1) Hash check — always enforced when a hash is declared. + let digest = sha256_digest(wasm_bytes); + if let Some(declared) = &manifest.wasm_module_hash { + let expected = parse_sha256(declared)?; + if expected != digest { + return Err(PluginError::SignatureRejected(format!( + "plugin `{}` wasm hash mismatch: module does not match manifest wasm_module_hash \ + (tampered or wrong binary)", + manifest.domain + ))); + } + } else if !policy.is_dev() { + return Err(PluginError::SignatureRejected(format!( + "plugin `{}` carries a signature/publisher_key but no wasm_module_hash to bind it to", + manifest.domain + ))); + } + + // (2) Signature check + (3) allowlist. + match (&manifest.wasm_module_sig, &manifest.publisher_key) { + (Some(sig_str), Some(key_str)) => { + let key = decode_verifying_key(key_str)?; + let sig = decode_signature(sig_str)?; + key.verify(&digest, &sig).map_err(|_| { + PluginError::SignatureRejected(format!( + "plugin `{}` Ed25519 signature does not verify over the module hash under \ + publisher_key", + manifest.domain + )) + })?; + if !policy.allows(&key) { + if policy.is_dev() { + eprintln!( + "[PLUGIN WARN] plugin `{}` is validly signed but its publisher_key is NOT on \ + the trust allowlist; AllowUnsigned dev policy loads it anyway.", + manifest.domain + ); + return Ok(()); + } + return Err(PluginError::SignatureRejected(format!( + "plugin `{}` is validly signed but its publisher_key is not on the trust \ + allowlist (untrusted publisher)", + manifest.domain + ))); + } + Ok(()) + } + _ => { + // Hash present but signature/key incomplete. + if policy.is_dev() { + eprintln!( + "[PLUGIN WARN] plugin `{}` has a hash but no complete Ed25519 signature; \ + AllowUnsigned dev policy loads it anyway.", + manifest.domain + ); + return Ok(()); + } + Err(PluginError::SignatureRejected(format!( + "plugin `{}` is missing a complete wasm_module_sig + publisher_key pair; the trust \ + policy requires a valid signature", + manifest.domain + ))) + } + } +} + +/// SHA-256 of `bytes` as a 32-byte digest. +fn sha256_digest(bytes: &[u8]) -> [u8; 32] { + let mut hasher = Sha256::new(); + hasher.update(bytes); + hasher.finalize().into() +} + +/// Parse a `sha256:` manifest hash into a 32-byte digest. +fn parse_sha256(s: &str) -> Result<[u8; 32], PluginError> { + let hex_part = s.strip_prefix("sha256:").ok_or_else(|| { + PluginError::InvalidManifest(format!( + "wasm_module_hash must be `sha256:`, got {s:?}" + )) + })?; + let raw = hex::decode(hex_part).map_err(|e| { + PluginError::InvalidManifest(format!("wasm_module_hash hex decode: {e}")) + })?; + raw.try_into().map_err(|v: Vec| { + PluginError::InvalidManifest(format!( + "wasm_module_hash must decode to 32 bytes, got {}", + v.len() + )) + }) +} + +/// Decode an `ed25519:` 32-byte verifying key. +fn decode_verifying_key(s: &str) -> Result { + let b64 = s.strip_prefix("ed25519:").ok_or_else(|| { + PluginError::InvalidManifest(format!( + "publisher_key must be `ed25519:`, got {s:?}" + )) + })?; + let raw = base64::engine::general_purpose::STANDARD + .decode(b64) + .map_err(|e| PluginError::InvalidManifest(format!("publisher_key base64: {e}")))?; + let bytes: [u8; 32] = raw.try_into().map_err(|v: Vec| { + PluginError::InvalidManifest(format!( + "publisher_key must decode to 32 bytes, got {}", + v.len() + )) + })?; + VerifyingKey::from_bytes(&bytes) + .map_err(|e| PluginError::InvalidManifest(format!("publisher_key not a valid Ed25519 point: {e}"))) +} + +/// Decode an `ed25519:` 64-byte signature. +fn decode_signature(s: &str) -> Result { + let b64 = s.strip_prefix("ed25519:").ok_or_else(|| { + PluginError::InvalidManifest(format!( + "wasm_module_sig must be `ed25519:`, got {s:?}" + )) + })?; + let raw = base64::engine::general_purpose::STANDARD + .decode(b64) + .map_err(|e| PluginError::InvalidManifest(format!("wasm_module_sig base64: {e}")))?; + let bytes: [u8; 64] = raw.try_into().map_err(|v: Vec| { + PluginError::InvalidManifest(format!( + "wasm_module_sig must decode to 64 bytes, got {}", + v.len() + )) + })?; + Ok(Signature::from_bytes(&bytes)) +} + +/// Encode a SHA-256 digest as the manifest `sha256:` form. Exposed so +/// tooling (and tests) can produce a manifest hash for real `.wasm` bytes. +pub fn encode_sha256(wasm_bytes: &[u8]) -> String { + format!("sha256:{}", hex::encode(sha256_digest(wasm_bytes))) +} + +/// Encode an Ed25519 verifying key as the manifest `ed25519:` form. +pub fn encode_verifying_key(key: &VerifyingKey) -> String { + format!( + "ed25519:{}", + base64::engine::general_purpose::STANDARD.encode(key.to_bytes()) + ) +} + +/// Encode an Ed25519 signature as the manifest `ed25519:` form. +pub fn encode_signature(sig: &Signature) -> String { + format!( + "ed25519:{}", + base64::engine::general_purpose::STANDARD.encode(sig.to_bytes()) + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use ed25519_dalek::{Signer, SigningKey}; + + /// Deterministic publisher key (mirrors witness_signing's fixed-bytes + /// seed convention — DO NOT use in production). + fn publisher() -> SigningKey { + SigningKey::from_bytes(b"homecore-plugins-pub-test-seed--") + } + + fn attacker() -> SigningKey { + SigningKey::from_bytes(b"homecore-plugins-attacker-seed--") + } + + /// Sign `wasm_bytes` with `key` and produce a manifest carrying the real + /// hash + signature + publisher key. + fn signed_manifest(wasm_bytes: &[u8], key: &SigningKey) -> PluginManifest { + let digest = sha256_digest(wasm_bytes); + let sig = key.sign(&digest); + PluginManifest { + domain: "demo".into(), + name: "Demo".into(), + version: "1.0.0".into(), + documentation: None, + iot_class: None, + config_flow: false, + integration_type: None, + dependencies: vec![], + requirements: vec![], + wasm_module: Some("demo.wasm".into()), + wasm_module_hash: Some(encode_sha256(wasm_bytes)), + wasm_module_sig: Some(encode_signature(&sig)), + publisher_key: Some(encode_verifying_key(&key.verifying_key())), + min_homecore_version: None, + host_imports_required: vec![], + homecore_permissions: vec![], + cog_id: None, + } + } + + #[test] + fn valid_sig_from_trusted_key_passes() { + let wasm = b"\0asm\x01\0\0\0fake module bytes"; + let key = publisher(); + let manifest = signed_manifest(wasm, &key); + let policy = + PluginPolicy::trusted(&[&encode_verifying_key(&key.verifying_key())]).unwrap(); + verify_module(&manifest, wasm, &policy).expect("trusted signed module should load"); + } + + #[test] + fn tampered_module_is_rejected() { + let wasm = b"\0asm\x01\0\0\0fake module bytes"; + let key = publisher(); + let manifest = signed_manifest(wasm, &key); + let policy = + PluginPolicy::trusted(&[&encode_verifying_key(&key.verifying_key())]).unwrap(); + // Flip a byte: hash no longer matches. + let tampered = b"\0asm\x01\0\0\0FAKE module bytes"; + let err = verify_module(&manifest, tampered, &policy).unwrap_err(); + assert!(matches!(err, PluginError::SignatureRejected(_)), "got {err:?}"); + } + + #[test] + fn valid_sig_from_untrusted_key_is_rejected() { + let wasm = b"\0asm\x01\0\0\0fake module bytes"; + // Signed correctly by the attacker, but the attacker is not trusted. + let manifest = signed_manifest(wasm, &attacker()); + let policy = + PluginPolicy::trusted(&[&encode_verifying_key(&publisher().verifying_key())]).unwrap(); + let err = verify_module(&manifest, wasm, &policy).unwrap_err(); + assert!(matches!(err, PluginError::SignatureRejected(_)), "got {err:?}"); + } + + #[test] + fn forged_signature_is_rejected() { + // Manifest claims the trusted publisher_key but the signature was + // produced by the attacker (a forged sig under a trusted identity). + let wasm = b"\0asm\x01\0\0\0fake module bytes"; + let digest = sha256_digest(wasm); + let forged = attacker().sign(&digest); + let mut manifest = signed_manifest(wasm, &publisher()); + manifest.wasm_module_sig = Some(encode_signature(&forged)); + let policy = + PluginPolicy::trusted(&[&encode_verifying_key(&publisher().verifying_key())]).unwrap(); + let err = verify_module(&manifest, wasm, &policy).unwrap_err(); + assert!(matches!(err, PluginError::SignatureRejected(_)), "got {err:?}"); + } + + #[test] + fn unsigned_module_rejected_under_default_policy() { + let wasm = b"\0asm\x01\0\0\0unsigned"; + let manifest = PluginManifest { + domain: "u".into(), + name: "U".into(), + version: "1".into(), + documentation: None, + iot_class: None, + config_flow: false, + integration_type: None, + dependencies: vec![], + requirements: vec![], + wasm_module: Some("u.wasm".into()), + wasm_module_hash: None, + wasm_module_sig: None, + publisher_key: None, + min_homecore_version: None, + host_imports_required: vec![], + homecore_permissions: vec![], + cog_id: None, + }; + let err = verify_module(&manifest, wasm, &PluginPolicy::deny_all()).unwrap_err(); + assert!(matches!(err, PluginError::SignatureRejected(_)), "got {err:?}"); + // ...but AllowUnsigned loads it (with a warn). + verify_module(&manifest, wasm, &PluginPolicy::AllowUnsigned) + .expect("AllowUnsigned should load an unsigned module"); + } +} diff --git a/v2/crates/homecore-plugins/src/wasmtime_runtime.rs b/v2/crates/homecore-plugins/src/wasmtime_runtime.rs index 023e6a67..3d4ca570 100644 --- a/v2/crates/homecore-plugins/src/wasmtime_runtime.rs +++ b/v2/crates/homecore-plugins/src/wasmtime_runtime.rs @@ -30,16 +30,27 @@ use wasmtime::{Engine, Linker, Module, Store}; use crate::error::PluginError; use crate::host_abi::{LogLevel, StateChangedEventJson, MAX_ABI_BUFFER_BYTES}; +use crate::manifest::PluginManifest; +use crate::permissions::PermissionSet; +use crate::verify::{verify_module, PluginPolicy}; // ── Store data ───────────────────────────────────────────────────────────── /// Per-plugin state stored inside the Wasmtime [`Store`]. /// /// Wasmtime's `Store` exposes `T` to host functions via `caller.data()`. -/// We store the `HomeCore` handle and a list of subscribed entity IDs here. +/// We store the `HomeCore` handle, a list of subscribed entity IDs, and the +/// plugin's write-permission set (ADR-162 P5 authority isolation). pub struct PluginStoreData { pub hc: HomeCore, pub subscriptions: Vec, + /// Entity-write authority distilled from the manifest's + /// `homecore_permissions`. Consulted by `hc_state_set`. The + /// permission-free [`WasmtimeRuntime::load_wasm`] path installs an + /// all-allowing set for backward compatibility; the + /// [`WasmtimeRuntime::load_plugin`] path installs the manifest's + /// declared set. + pub permissions: PermissionSet, } // ── WasmtimeRuntime ──────────────────────────────────────────────────────── @@ -59,14 +70,53 @@ impl WasmtimeRuntime { Ok(Self { engine }) } - /// Compile and instantiate a WASM plugin from raw bytes. + /// Compile and instantiate a WASM plugin from raw bytes, **without** + /// signature verification or permission gating (the plugin gets + /// all-write authority). /// - /// Returns a [`WasmPlugin`] handle that owns the `Store` and the - /// `Instance`. The handle can be used to call into the WASM module. + /// Retained for the legacy/test path and first-party trusted modules. + /// Production plugin loading should go through [`Self::load_plugin`], + /// which verifies the module (ADR-162 P4) and scopes its write + /// authority to the manifest (P5). pub fn load_wasm( &self, wasm_bytes: &[u8], hc: HomeCore, + ) -> Result { + self.instantiate(wasm_bytes, hc, PermissionSet::allow_all()) + } + + /// Verify and instantiate a WASM plugin from its manifest + raw bytes. + /// + /// This is the secure load path (ADR-162): + /// 1. **P4** — [`verify_module`] checks the SHA-256 module hash and + /// Ed25519 signature against the manifest under `policy`. A + /// tampered module, bad/forged signature, untrusted publisher, or + /// (under the secure default) an unsigned module is rejected + /// **before** any guest code runs. + /// 2. **P5** — the plugin's `homecore_permissions` are distilled into + /// a [`PermissionSet`] installed in the store, so `hc_state_set` + /// can only write entities the plugin declared. + pub fn load_plugin( + &self, + manifest: &PluginManifest, + wasm_bytes: &[u8], + hc: HomeCore, + policy: &PluginPolicy, + ) -> Result { + // P4: verify before instantiation. + verify_module(manifest, wasm_bytes, policy)?; + // P5: scope write authority to the manifest's declared permissions. + let permissions = PermissionSet::from_manifest(manifest); + self.instantiate(wasm_bytes, hc, permissions) + } + + /// Shared compile + instantiate, installing the given permission set. + fn instantiate( + &self, + wasm_bytes: &[u8], + hc: HomeCore, + permissions: PermissionSet, ) -> Result { let module = Module::new(&self.engine, wasm_bytes) .map_err(|e| PluginError::RuntimeError(format!("WASM compile: {e}")))?; @@ -77,6 +127,7 @@ impl WasmtimeRuntime { let store_data = PluginStoreData { hc, subscriptions: Vec::new(), + permissions, }; let mut store = Store::new(&self.engine, store_data); @@ -183,7 +234,9 @@ fn register_hc_state_get( /// Sets the state for the entity whose UTF-8 ID is at `[eid_ptr,eid_ptr+eid_len)`. /// The new state string is at `[state_ptr,state_ptr+state_len)`. /// The attributes JSON is at `[attrs_ptr,attrs_ptr+attrs_len)`. -/// Returns 0 on success, negative on error. +/// Returns 0 on success, negative on error: -1 (bad memory/args), -2 +/// (invalid entity id), -3 (permission denied — entity not in the +/// plugin's declared `homecore_permissions`, ADR-162 P5). fn register_hc_state_set( linker: &mut Linker, ) -> Result<(), PluginError> { @@ -224,6 +277,20 @@ fn register_hc_state_set( Ok(id) => id, Err(_) => return -2, }; + + // ── P5 authority isolation (ADR-162) ────────────────────── + // Reject a write to an entity the plugin did not declare in + // `homecore_permissions`. Return a typed error code to the + // guest (-3); do NOT panic the host. + if !caller.data().permissions.may_write(entity_id.as_str()) { + eprintln!( + "[PLUGIN WARN] denied hc_state_set on `{}` — not in plugin's declared \ + homecore_permissions (P5 authority isolation)", + entity_id.as_str() + ); + return -3; + } + let attrs: serde_json::Value = serde_json::from_str(&attrs_str).unwrap_or(serde_json::json!({})); diff --git a/v2/crates/homecore-plugins/tests/integration.rs b/v2/crates/homecore-plugins/tests/integration.rs index 88b1386e..34af267e 100644 --- a/v2/crates/homecore-plugins/tests/integration.rs +++ b/v2/crates/homecore-plugins/tests/integration.rs @@ -371,4 +371,259 @@ mod wasmtime_tests { let r = plugin.call_setup("{}").expect("setup"); assert_eq!(r, 0); } + + // ── ADR-162 P4: signature/integrity verification ──────────────────────── + // + // Each of these FAILS on the pre-ADR-162 code, which had no + // `load_plugin` / `verify_module` at all — the manifest hash/sig/key + // were parsed and discarded. They drive the real verification gate. + + use ed25519_dalek::{Signer, SigningKey}; + use homecore_plugins::manifest::PluginManifest; + use homecore_plugins::verify::{encode_sha256, encode_signature, encode_verifying_key}; + use homecore_plugins::PluginPolicy; + + /// Deterministic publisher key (fixed seed — never use in production; + /// mirrors the cog-ha-matter witness_signing test-key convention). + fn publisher_key() -> SigningKey { + SigningKey::from_bytes(b"hc-plugins-integration-pub-seed-") + } + + fn untrusted_key() -> SigningKey { + SigningKey::from_bytes(b"hc-plugins-integration-evil-seed") + } + + /// A minimal valid module that writes `light.kitchen` on setup, plus a + /// `light.*` permission grant. Returns the WAT source. + const WRITE_LIGHT_WAT: &str = r#" +(module + (import "env" "hc_state_get" (func $hc_state_get (param i32 i32 i32 i32) (result i32))) + (import "env" "hc_state_set" (func $hc_state_set (param i32 i32 i32 i32 i32 i32) (result i32))) + (import "env" "hc_state_subscribe" (func $hc_state_subscribe (param i32 i32) (result i32))) + (import "env" "hc_log" (func $hc_log (param i32 i32 i32))) + (memory (export "memory") 1) + (global $bump (mut i32) (i32.const 512)) + (data (i32.const 0) "light.kitchen") + (data (i32.const 64) "on") + (data (i32.const 128) "{}") + (func (export "alloc") (param i32) (result i32) + (local $p i32) + (local.set $p (global.get $bump)) + (global.set $bump (i32.add (global.get $bump) (local.get 0))) + (local.get $p)) + (func (export "dealloc") (param i32 i32)) + (func (export "plugin_setup") (param i32 i32) (result i32) + (call $hc_state_set + (i32.const 0) (i32.const 13) ;; "light.kitchen" + (i32.const 64) (i32.const 2) ;; "on" + (i32.const 128) (i32.const 2)) ;; "{}" + drop + (i32.const 0)) + (func (export "plugin_handle_state_changed") (param i32 i32) (result i32) (i32.const 0)) +) +"#; + + /// Build a manifest signed by `key` over the SHA-256 of `wasm_bytes`, + /// with the given write-permission grants. + fn signed_manifest( + wasm_bytes: &[u8], + key: &SigningKey, + perms: &[&str], + ) -> PluginManifest { + use sha2::{Digest, Sha256}; + let digest: [u8; 32] = Sha256::digest(wasm_bytes).into(); + let sig = key.sign(&digest); + let mut m = PluginManifest::parse_json( + r#"{"domain":"demo","name":"Demo","version":"1.0.0"}"#, + ) + .unwrap(); + m.wasm_module = Some("demo.wasm".into()); + m.wasm_module_hash = Some(encode_sha256(wasm_bytes)); + m.wasm_module_sig = Some(encode_signature(&sig)); + m.publisher_key = Some(encode_verifying_key(&key.verifying_key())); + m.homecore_permissions = perms.iter().map(|s| s.to_string()).collect(); + m + } + + #[test] + fn p4_valid_sig_from_trusted_key_loads() { + let wasm = wat::parse_str(WRITE_LIGHT_WAT).expect("WAT"); + let key = publisher_key(); + let manifest = signed_manifest(&wasm, &key, &["light.*"]); + let policy = + PluginPolicy::trusted(&[&encode_verifying_key(&key.verifying_key())]).unwrap(); + + let rt = WasmtimeRuntime::new().expect("rt"); + let hc = HomeCore::new(); + rt.load_plugin(&manifest, &wasm, hc, &policy) + .expect("a validly-signed, trusted plugin must load"); + } + + #[test] + fn p4_tampered_module_is_rejected() { + let wasm = wat::parse_str(WRITE_LIGHT_WAT).expect("WAT"); + let key = publisher_key(); + // Manifest signs the original bytes; we then load DIFFERENT bytes. + let manifest = signed_manifest(&wasm, &key, &["light.*"]); + let policy = + PluginPolicy::trusted(&[&encode_verifying_key(&key.verifying_key())]).unwrap(); + + // Re-compile a byte-different module (writes "off" not "on"). + let tampered_src = WRITE_LIGHT_WAT.replace(r#""on""#, r#""of""#); + let tampered = wat::parse_str(&tampered_src).expect("WAT"); + assert_ne!(wasm, tampered, "test bug: bytes must differ"); + + let rt = WasmtimeRuntime::new().expect("rt"); + let hc = HomeCore::new(); + match rt.load_plugin(&manifest, &tampered, hc, &policy) { + Err(homecore_plugins::PluginError::SignatureRejected(_)) => {} + Ok(_) => panic!("tampered module must be rejected (hash mismatch), but it loaded"), + Err(e) => panic!("expected SignatureRejected, got {e:?}"), + } + } + + #[test] + fn p4_valid_sig_from_untrusted_key_is_rejected() { + let wasm = wat::parse_str(WRITE_LIGHT_WAT).expect("WAT"); + // Correctly signed by the untrusted key — but it is not on the allowlist. + let manifest = signed_manifest(&wasm, &untrusted_key(), &["light.*"]); + let policy = + PluginPolicy::trusted(&[&encode_verifying_key(&publisher_key().verifying_key())]) + .unwrap(); + + let rt = WasmtimeRuntime::new().expect("rt"); + let hc = HomeCore::new(); + match rt.load_plugin(&manifest, &wasm, hc, &policy) { + Err(homecore_plugins::PluginError::SignatureRejected(_)) => {} + Ok(_) => panic!("untrusted publisher must be rejected, but it loaded"), + Err(e) => panic!("expected SignatureRejected, got {e:?}"), + } + } + + #[test] + fn p4_unsigned_module_rejected_by_default_loads_only_under_allow_unsigned() { + let wasm = wat::parse_str(WRITE_LIGHT_WAT).expect("WAT"); + let mut manifest = PluginManifest::parse_json( + r#"{"domain":"u","name":"U","version":"1"}"#, + ) + .unwrap(); + manifest.wasm_module = Some("u.wasm".into()); + manifest.homecore_permissions = vec!["light.*".into()]; + // No hash/sig/key → unsigned. + + let rt = WasmtimeRuntime::new().expect("rt"); + // Secure default: rejected. + match rt.load_plugin(&manifest, &wasm, HomeCore::new(), &PluginPolicy::deny_all()) { + Err(homecore_plugins::PluginError::SignatureRejected(_)) => {} + Ok(_) => panic!("unsigned module must be rejected under the secure default"), + Err(e) => panic!("expected SignatureRejected, got {e:?}"), + } + // Dev escape hatch: loads (with a loud warn). + rt.load_plugin( + &manifest, + &wasm, + HomeCore::new(), + &PluginPolicy::AllowUnsigned, + ) + .expect("AllowUnsigned dev policy must load an unsigned module"); + } + + // ── ADR-162 P5: authority / capability isolation ──────────────────────── + // + // FAILS on the pre-ADR-162 code, where `hc_state_set` ignored + // `homecore_permissions` entirely and let any plugin write any entity. + + /// Module that writes `lock.front_door` on setup (an over-privileged + /// write a `light.*` plugin must NOT be allowed to perform). + const WRITE_LOCK_WAT: &str = r#" +(module + (import "env" "hc_state_get" (func $hc_state_get (param i32 i32 i32 i32) (result i32))) + (import "env" "hc_state_set" (func $hc_state_set (param i32 i32 i32 i32 i32 i32) (result i32))) + (import "env" "hc_state_subscribe" (func $hc_state_subscribe (param i32 i32) (result i32))) + (import "env" "hc_log" (func $hc_log (param i32 i32 i32))) + (memory (export "memory") 1) + (global $bump (mut i32) (i32.const 512)) + (data (i32.const 0) "lock.front_door") + (data (i32.const 64) "unlocked") + (data (i32.const 128) "{}") + (func (export "alloc") (param i32) (result i32) + (local $p i32) + (local.set $p (global.get $bump)) + (global.set $bump (i32.add (global.get $bump) (local.get 0))) + (local.get $p)) + (func (export "dealloc") (param i32 i32)) + ;; plugin_setup returns the hc_state_set result code so the host test can + ;; assert the guest saw the typed permission-denied error (-3). + (func (export "plugin_setup") (param i32 i32) (result i32) + (call $hc_state_set + (i32.const 0) (i32.const 15) ;; "lock.front_door" + (i32.const 64) (i32.const 8) ;; "unlocked" + (i32.const 128) (i32.const 2))) ;; "{}" + (func (export "plugin_handle_state_changed") (param i32 i32) (result i32) (i32.const 0)) +) +"#; + + #[test] + fn p5_declared_light_plugin_may_write_light_but_not_lock() { + let key = publisher_key(); + let trusted = PluginPolicy::trusted(&[&encode_verifying_key(&key.verifying_key())]).unwrap(); + let rt = WasmtimeRuntime::new().expect("rt"); + + // (a) A `light.*` plugin writing `light.kitchen` → ALLOWED. + let light_wasm = wat::parse_str(WRITE_LIGHT_WAT).expect("WAT"); + let light_manifest = signed_manifest(&light_wasm, &key, &["light.*"]); + let hc_a = HomeCore::new(); + let plugin_a = rt + .load_plugin(&light_manifest, &light_wasm, hc_a.clone(), &trusted) + .expect("light plugin loads"); + let r = plugin_a.call_setup("{}").expect("setup"); + assert_eq!(r, 0, "write to declared light.kitchen should succeed"); + let kitchen = homecore::EntityId::parse("light.kitchen").unwrap(); + assert_eq!( + hc_a.states().get(&kitchen).expect("light.kitchen written").state, + "on" + ); + + // (b) The SAME `light.*` plugin attempting to write `lock.front_door` + // → REJECTED with the typed -3 code, and the lock is NOT written. + let lock_wasm = wat::parse_str(WRITE_LOCK_WAT).expect("WAT"); + let lock_manifest = signed_manifest(&lock_wasm, &key, &["light.*"]); + let hc_b = HomeCore::new(); + let plugin_b = rt + .load_plugin(&lock_manifest, &lock_wasm, hc_b.clone(), &trusted) + .expect("module loads (verification ok); the WRITE is what's gated"); + let denied = plugin_b.call_setup("{}").expect("setup runs without trapping host"); + assert_eq!( + denied, -3, + "over-privileged write to lock.front_door must return -3 (permission denied)" + ); + let lock = homecore::EntityId::parse("lock.front_door").unwrap(); + assert!( + hc_b.states().get(&lock).is_none(), + "lock.front_door must NOT have been written by a light-only plugin" + ); + } + + #[test] + fn p5_plugin_with_no_permissions_can_write_nothing() { + let key = publisher_key(); + let trusted = PluginPolicy::trusted(&[&encode_verifying_key(&key.verifying_key())]).unwrap(); + let rt = WasmtimeRuntime::new().expect("rt"); + + let wasm = wat::parse_str(WRITE_LIGHT_WAT).expect("WAT"); + // No permissions declared at all. + let manifest = signed_manifest(&wasm, &key, &[]); + let hc = HomeCore::new(); + let plugin = rt + .load_plugin(&manifest, &wasm, hc.clone(), &trusted) + .expect("module loads; the write is gated"); + // WRITE_LIGHT_WAT drops the host-import result and returns 0, so we + // assert the denial via the side-effect: the write must NOT land. + plugin.call_setup("{}").expect("setup runs without trapping host"); + let kitchen = homecore::EntityId::parse("light.kitchen").unwrap(); + assert!( + hc.states().get(&kitchen).is_none(), + "no-permission plugin must not write light.kitchen (P5 authority isolation)" + ); + } } From 3292bd2c5da487e9f0740645995eb31364c24621 Mon Sep 17 00:00:00 2001 From: ruv Date: Fri, 12 Jun 2026 01:40:23 -0400 Subject: [PATCH 2/3] =?UTF-8?q?feat(homecore-automation):=20implement=20bo?= =?UTF-8?q?unded=20RunModes=20Restart/Queued/max=20(ADR-162,=20completes?= =?UTF-8?q?=20ADR-161=20=C2=A7A5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-161 implemented RunMode::Single (AtomicBool re-entrancy guard) + Parallel but honestly left Restart/Queued/max as "ACCEPTED-FUTURE / unbounded parallel" — every non-Single mode spawned an unbounded task. This makes them real. New `runmode` module — per-automation RunState owns the machinery: - Restart: aborts the in-flight action task (tokio::task::AbortHandle) and starts a fresh one. - Queued: serializes runs in arrival order via a per-automation async Mutex — sequential, never concurrent, nothing dropped. - max: N: caps concurrency at N via a per-automation Semaphore; triggers beyond N queue (await a permit) rather than running concurrently (HA bounded semantics). Documented in the module table. - Single/IgnoreFirst/Parallel preserved. engine.rs now holds a RunState per registration and calls run_state.dispatch() at all three trigger sites (event loop, timer, fire_time_for_test); the old spawn_run is removed. engine.rs trimmed to 433 lines. Tests (tests/engine_behaviors.rs) — verified to FAIL on the old unbounded- parallel dispatch (simulated and confirmed each panics), pass on the new: - restart_mode_cancels_prior_run (old: both runs complete → 2; new: 1) - queued_mode_runs_sequentially_not_concurrently (old: max concurrency 3; new: all 3 run, max concurrency 1) - max_two_caps_concurrency_at_two (old: 4 concurrent; new: all 4 run, max 2) homecore-automation --no-default-features: 45 passed (lib 37, engine_behaviors 8), 0 failed. Co-Authored-By: claude-flow --- v2/crates/homecore-automation/src/engine.rs | 83 +++------ v2/crates/homecore-automation/src/lib.rs | 1 + v2/crates/homecore-automation/src/runmode.rs | 153 +++++++++++++++++ .../tests/engine_behaviors.rs | 159 ++++++++++++++++++ 4 files changed, 340 insertions(+), 56 deletions(-) create mode 100644 v2/crates/homecore-automation/src/runmode.rs diff --git a/v2/crates/homecore-automation/src/engine.rs b/v2/crates/homecore-automation/src/engine.rs index b39c675b..9962dbf1 100644 --- a/v2/crates/homecore-automation/src/engine.rs +++ b/v2/crates/homecore-automation/src/engine.rs @@ -3,14 +3,15 @@ //! //! ADR-129 §2 design: one Tokio task per running automation instance. //! -//! ## Run modes (ADR-161, HC-WS-05) +//! ## Run modes (ADR-161 §A5 → completed in ADR-162) //! -//! `RunMode::Single` is enforced via a per-automation `AtomicBool` -//! guard: while an instance is executing, a second trigger is skipped. -//! `Parallel` (and the as-yet-unbounded `Restart`/`Queued`) spawn a -//! fresh instance on every trigger. (Before this fix the doc claimed -//! AtomicBool enforcement but every trigger spawned unbounded parallel -//! tasks regardless of `mode`.) +//! Each registered automation owns a [`RunState`] that implements its +//! `RunMode`: `Single`/`IgnoreFirst` skip re-entrant triggers, `Restart` +//! aborts the in-flight run and starts a fresh one, `Queued` serializes +//! runs in arrival order (nothing dropped), `Parallel` spawns on every +//! trigger, and `max: N` caps concurrency via a per-automation semaphore. +//! (ADR-161 only honored Single/Parallel; Restart/Queued/max were +//! honestly documented as unbounded-parallel until ADR-162.) //! //! ## Time triggers (ADR-161, HC-WS-04) //! @@ -26,7 +27,6 @@ //! `EvalContext::with_templates`), so `template:` conditions evaluate //! against live state instead of always returning false. -use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use chrono::{Local, Timelike}; @@ -34,18 +34,18 @@ use tokio::sync::broadcast; use homecore::HomeCore; -use crate::action::ExecutionContext; -use crate::automation::{Automation, RunMode}; +use crate::automation::Automation; use crate::condition::EvalContext; +use crate::runmode::RunState; use crate::template::TemplateEnvironment; use crate::trigger::{Trigger, TriggerContext}; /// An automation registered with the engine, plus its runtime run-state. struct Registered { auto: Arc, - /// `true` while a `Single`-mode instance is executing. Used to - /// skip re-entrant triggers (HC-WS-05). - running: Arc, + /// Run-mode machinery (re-entrancy guard / restart abort handle / + /// queue mutex / concurrency semaphore) for this automation. + run_state: RunState, } /// The automation engine. Holds a HOMECORE handle and a list of registered @@ -69,9 +69,10 @@ impl AutomationEngine { /// Register an automation. Can be called before or after `start()`. pub fn register(&self, automation: Automation) { + let run_state = RunState::new(&automation); self.automations.lock().unwrap().push(Registered { auto: Arc::new(automation), - running: Arc::new(AtomicBool::new(false)), + run_state, }); } @@ -118,13 +119,13 @@ impl AutomationEngine { loop { match rx.recv().await { Ok(event) => { - let snapshot: Vec<(Arc, Arc)> = automations + let snapshot: Vec<(Arc, RunState)> = automations .lock() .unwrap() .iter() - .map(|r| (Arc::clone(&r.auto), Arc::clone(&r.running))) + .map(|r| (Arc::clone(&r.auto), r.run_state.clone())) .collect(); - for (automation, running) in snapshot { + for (automation, run_state) in snapshot { if !automation.enabled { continue; } @@ -148,7 +149,7 @@ impl AutomationEngine { if !conditions_pass(&automation, &eval_ctx).await { continue; } - spawn_run(&hc, automation, running); + run_state.dispatch(&hc, automation); } } Err(broadcast::error::RecvError::Closed) => break, @@ -183,14 +184,14 @@ impl AutomationEngine { if last_fired_sec.as_deref() == Some(hhmmss.as_str()) { continue; } - let snapshot: Vec<(Arc, Arc)> = automations + let snapshot: Vec<(Arc, RunState)> = automations .lock() .unwrap() .iter() - .map(|r| (Arc::clone(&r.auto), Arc::clone(&r.running))) + .map(|r| (Arc::clone(&r.auto), r.run_state.clone())) .collect(); let mut fired_any = false; - for (automation, running) in snapshot { + for (automation, run_state) in snapshot { if !automation.enabled { continue; } @@ -208,7 +209,7 @@ impl AutomationEngine { if !conditions_pass(&automation, &eval_ctx).await { continue; } - spawn_run(&hc, automation, running); + run_state.dispatch(&hc, automation); fired_any = true; } if fired_any { @@ -231,15 +232,15 @@ impl AutomationEngine { /// wall-clock second to roll over. Returns the number of automations /// that fired (passed conditions and were spawned). pub async fn fire_time_for_test(&self, hhmmss: &str) -> usize { - let snapshot: Vec<(Arc, Arc)> = self + let snapshot: Vec<(Arc, RunState)> = self .automations .lock() .unwrap() .iter() - .map(|r| (Arc::clone(&r.auto), Arc::clone(&r.running))) + .map(|r| (Arc::clone(&r.auto), r.run_state.clone())) .collect(); let mut fired = 0usize; - for (automation, running) in snapshot { + for (automation, run_state) in snapshot { if !automation.enabled { continue; } @@ -254,7 +255,7 @@ impl AutomationEngine { if !conditions_pass(&automation, &eval_ctx).await { continue; } - spawn_run(&self.hc, automation, running); + run_state.dispatch(&self.hc, automation); fired += 1; } fired @@ -281,36 +282,6 @@ fn time_at_matches(at: &str, hhmmss: &str) -> bool { normalized == hhmmss } -/// Spawn an automation run, honoring `RunMode::Single` re-entrancy -/// guard (HC-WS-05). For `Single`/`IgnoreFirst` modes a run already in -/// flight causes the new trigger to be skipped; the `running` flag is -/// cleared when the run finishes. -fn spawn_run(hc: &HomeCore, automation: Arc, running: Arc) { - let single = matches!(automation.mode, RunMode::Single | RunMode::IgnoreFirst); - if single { - // Try to claim the running slot; if already running, skip. - if running - .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) - .is_err() - { - return; - } - } - let hc_clone = hc.clone(); - tokio::spawn(async move { - let mut exec_ctx = ExecutionContext::new(hc_clone, automation.id.clone()); - for action in &automation.action { - if let Err(e) = action.execute(&mut exec_ctx).await { - eprintln!("[homecore-automation] action error in {}: {e}", automation.id); - break; - } - } - if single { - running.store(false, Ordering::SeqCst); - } - }); -} - #[cfg(test)] mod tests { use super::*; diff --git a/v2/crates/homecore-automation/src/lib.rs b/v2/crates/homecore-automation/src/lib.rs index 803625af..17922899 100644 --- a/v2/crates/homecore-automation/src/lib.rs +++ b/v2/crates/homecore-automation/src/lib.rs @@ -19,6 +19,7 @@ pub mod condition; pub mod action; pub mod template; pub mod engine; +pub mod runmode; pub mod error; pub use automation::{Automation, RunMode}; diff --git a/v2/crates/homecore-automation/src/runmode.rs b/v2/crates/homecore-automation/src/runmode.rs new file mode 100644 index 00000000..7590264d --- /dev/null +++ b/v2/crates/homecore-automation/src/runmode.rs @@ -0,0 +1,153 @@ +//! Per-automation run-mode machinery (ADR-162, completes ADR-161 §A5). +//! +//! ADR-161 implemented `RunMode::Single` (a per-automation `AtomicBool` +//! re-entrancy guard) and `Parallel`, but honestly left `Restart`, `Queued` +//! and `max: N` as "ACCEPTED-FUTURE / unbounded parallel" — every non-Single +//! mode spawned an unbounded task. This module makes them real: +//! +//! | Mode | Semantics implemented | +//! |------|-----------------------| +//! | `Single` / `IgnoreFirst` | re-entrancy guard: skip while a run is in flight (ADR-161). | +//! | `Restart` | **cancel** the in-flight run (`tokio::task::AbortHandle`) and start a fresh one. | +//! | `Queued` | **serialize**: runs execute sequentially in arrival order via a per-automation async mutex — nothing is dropped. | +//! | `Parallel` | spawn on every trigger (optionally capped, see below). | +//! | `max: N` | cap concurrency at **N** via a per-automation semaphore; triggers beyond N **queue** (await a permit) rather than running concurrently — matching HA's bounded `parallel`/`queued`. | +//! +//! Each registered automation owns one [`RunState`]; the engine calls +//! [`RunState::dispatch`] on every (trigger + conditions-passed) event. + +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; + +use tokio::sync::{Mutex as AsyncMutex, Semaphore}; + +use homecore::HomeCore; + +use crate::action::ExecutionContext; +use crate::automation::{Automation, RunMode}; + +/// Per-automation runtime state backing the run-mode dispatch. +/// +/// Cheap to clone (all fields are `Arc`); the engine clones it into each +/// spawned run so the machinery (abort handle, queue mutex, semaphore) is +/// shared across all triggers of the same automation. +#[derive(Clone)] +pub struct RunState { + /// `Single`/`IgnoreFirst` re-entrancy guard (ADR-161 §A5). + running: Arc, + /// `Restart`: handle to the currently-running action task, so a new + /// trigger can abort it before starting a fresh one. + current: Arc>>, + /// `Queued`: serializes runs in arrival order (one at a time, FIFO via + /// fair async mutex acquisition). + queue_lock: Arc>, + /// `max: N` (and bounded `Parallel`): caps concurrent runs at N. + /// `None` when no cap applies. + semaphore: Option>, +} + +impl RunState { + /// Build run-state for an automation, sizing the concurrency semaphore + /// from its `max:` field (only meaningful for `Queued`/`Parallel`). + pub fn new(automation: &Automation) -> Self { + let semaphore = automation + .max + .filter(|n| *n > 0) + .map(|n| Arc::new(Semaphore::new(n))); + Self { + running: Arc::new(AtomicBool::new(false)), + current: Arc::new(Mutex::new(None)), + queue_lock: Arc::new(AsyncMutex::new(())), + semaphore, + } + } + + /// Dispatch one trigger for `automation` according to its `RunMode`. + /// Honors Single re-entrancy, Restart cancel-and-replace, Queued + /// serialization, and `max:` concurrency capping. + pub fn dispatch(&self, hc: &HomeCore, automation: Arc) { + match automation.mode { + RunMode::Single | RunMode::IgnoreFirst => self.dispatch_single(hc, automation), + RunMode::Restart => self.dispatch_restart(hc, automation), + RunMode::Queued => self.dispatch_queued(hc, automation), + RunMode::Parallel => self.dispatch_parallel(hc, automation), + } + } + + /// `Single`: skip if a run is already in flight; clear the flag on done. + fn dispatch_single(&self, hc: &HomeCore, automation: Arc) { + if self + .running + .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) + .is_err() + { + return; // already running — skip re-entrant trigger. + } + let hc = hc.clone(); + let running = Arc::clone(&self.running); + tokio::spawn(async move { + run_actions(&hc, &automation).await; + running.store(false, Ordering::SeqCst); + }); + } + + /// `Restart`: abort the in-flight run (if any), then start a fresh one + /// and record its abort handle. + fn dispatch_restart(&self, hc: &HomeCore, automation: Arc) { + // Abort any prior run before starting the new one. + if let Some(prev) = self.current.lock().unwrap().take() { + prev.abort(); + } + let hc = hc.clone(); + let slot = Arc::clone(&self.current); + let handle = tokio::spawn(async move { + run_actions(&hc, &automation).await; + }); + *slot.lock().unwrap() = Some(handle.abort_handle()); + } + + /// `Queued`: serialize via the per-automation async mutex. Each trigger + /// spawns a task that waits its turn, so all triggers run in arrival + /// order, one at a time — nothing is dropped. + fn dispatch_queued(&self, hc: &HomeCore, automation: Arc) { + let hc = hc.clone(); + let lock = Arc::clone(&self.queue_lock); + let sem = self.semaphore.clone(); + tokio::spawn(async move { + // Optional `max:` cap still applies on top of serialization. + let _permit = match &sem { + Some(s) => Some(s.acquire().await.expect("semaphore not closed")), + None => None, + }; + let _guard = lock.lock().await; // FIFO turn — sequential execution. + run_actions(&hc, &automation).await; + }); + } + + /// `Parallel`: spawn on every trigger, capped at `max:` if set. + fn dispatch_parallel(&self, hc: &HomeCore, automation: Arc) { + let hc = hc.clone(); + let sem = self.semaphore.clone(); + tokio::spawn(async move { + let _permit = match &sem { + Some(s) => Some(s.acquire().await.expect("semaphore not closed")), + None => None, + }; + run_actions(&hc, &automation).await; + }); + } +} + +/// Execute an automation's action sequence once. +async fn run_actions(hc: &HomeCore, automation: &Automation) { + let mut exec_ctx = ExecutionContext::new(hc.clone(), automation.id.clone()); + for action in &automation.action { + if let Err(e) = action.execute(&mut exec_ctx).await { + eprintln!( + "[homecore-automation] action error in {}: {e}", + automation.id + ); + break; + } + } +} diff --git a/v2/crates/homecore-automation/tests/engine_behaviors.rs b/v2/crates/homecore-automation/tests/engine_behaviors.rs index 01a7c7fa..be2f1088 100644 --- a/v2/crates/homecore-automation/tests/engine_behaviors.rs +++ b/v2/crates/homecore-automation/tests/engine_behaviors.rs @@ -257,3 +257,162 @@ async fn template_condition_evaluates_false_blocks_action() { sleep(Duration::from_millis(50)).await; assert_eq!(log.lock().unwrap().len(), 0, "false template condition should block the action"); } + +// ── ADR-162 (completes ADR-161 §A5): bounded RunModes ─────────────── +// +// ADR-161 honored only Single/Parallel; Restart/Queued/max were honestly +// documented as unbounded-parallel. These tests drive the real +// Restart/Queued/max machinery and FAIL on the old engine (where every +// non-Single mode spawned an unbounded parallel task). + +/// A service that increments a live concurrency gauge on entry, sleeps, +/// then decrements — recording the maximum concurrency ever observed and +/// the total number of completed runs. Returns `(max_concurrency, completed)`. +async fn register_gauge( + hc: &HomeCore, + domain: &str, + service: &str, + work: Duration, +) -> (Arc, Arc) { + let live = Arc::new(AtomicUsize::new(0)); + let max_seen = Arc::new(AtomicUsize::new(0)); + let completed = Arc::new(AtomicUsize::new(0)); + let (l, m, c) = (Arc::clone(&live), Arc::clone(&max_seen), Arc::clone(&completed)); + hc.services() + .register( + ServiceName::new(domain, service), + FnHandler(move |_call: ServiceCall| { + let (l, m, c) = (Arc::clone(&l), Arc::clone(&m), Arc::clone(&c)); + async move { + let now = l.fetch_add(1, Ordering::SeqCst) + 1; + m.fetch_max(now, Ordering::SeqCst); + sleep(work).await; + l.fetch_sub(1, Ordering::SeqCst); + c.fetch_add(1, Ordering::SeqCst); + Ok(serde_json::Value::Null) + } + }), + ) + .await; + (max_seen, completed) +} + +fn state_auto(id: &str, entity: &str, domain: &str, service: &str) -> Automation { + Automation::new( + id, + vec![Trigger::State { + entity_id: EntityId::parse(entity).unwrap(), + from: None, + to: None, + }], + vec![Action::ServiceCall { + domain: domain.into(), + service: service.into(), + data: serde_json::json!({}), + }], + ) +} + +// ── Restart: cancels the in-flight run ───────────────────────────── +#[tokio::test] +async fn restart_mode_cancels_prior_run() { + let hc = HomeCore::new(); + // Each run sleeps 300ms before recording completion. + let (_max, completed) = + register_gauge(&hc, "light", "slow", Duration::from_millis(300)).await; + + let engine = AutomationEngine::new(hc.clone()); + let mut auto = state_auto("restart_auto", "switch.r", "light", "slow"); + auto.mode = RunMode::Restart; + engine.register(auto); + let _handle = engine.start(); + + // Trigger 1 starts the slow run. + hc.states().set(EntityId::parse("switch.r").unwrap(), "a", serde_json::json!({}), Context::new()); + sleep(Duration::from_millis(80)).await; + // Trigger 2 arrives mid-run → must ABORT run 1 and start run 2. + hc.states().set(EntityId::parse("switch.r").unwrap(), "b", serde_json::json!({}), Context::new()); + + // Wait long enough for run 2 (started ~80ms in) to finish, but run 1 + // (aborted at ~80ms, would have finished at ~300ms) must NOT complete. + sleep(Duration::from_millis(400)).await; + assert_eq!( + completed.load(Ordering::SeqCst), + 1, + "Restart must cancel the in-flight run: exactly the restarted run completes (not both). \ + On the old engine both ran to completion → 2." + ); +} + +// ── Queued: serialize N rapid triggers, all run, never concurrent ── +#[tokio::test] +async fn queued_mode_runs_sequentially_not_concurrently() { + let hc = HomeCore::new(); + let (max_seen, completed) = + register_gauge(&hc, "light", "slow", Duration::from_millis(120)).await; + + let engine = AutomationEngine::new(hc.clone()); + let mut auto = state_auto("queued_auto", "switch.q", "light", "slow"); + auto.mode = RunMode::Queued; + engine.register(auto); + let _handle = engine.start(); + + // Three rapid triggers. + for v in ["a", "b", "c"] { + hc.states().set(EntityId::parse("switch.q").unwrap(), v, serde_json::json!({}), Context::new()); + sleep(Duration::from_millis(10)).await; + } + + // 3 runs × 120ms serialized ≈ 360ms; wait generously. + sleep(Duration::from_millis(600)).await; + assert_eq!( + completed.load(Ordering::SeqCst), + 3, + "Queued must run every trigger (nothing dropped)" + ); + assert_eq!( + max_seen.load(Ordering::SeqCst), + 1, + "Queued must never run two instances concurrently. On the old engine all 3 ran in \ + parallel → max concurrency 3." + ); +} + +// ── max: 2 → never more than 2 concurrent ────────────────────────── +#[tokio::test] +async fn max_two_caps_concurrency_at_two() { + let hc = HomeCore::new(); + let (max_seen, completed) = + register_gauge(&hc, "light", "slow", Duration::from_millis(150)).await; + + let engine = AutomationEngine::new(hc.clone()); + let mut auto = state_auto("max_auto", "switch.m", "light", "slow"); + auto.mode = RunMode::Parallel; + auto.max = Some(2); + engine.register(auto); + let _handle = engine.start(); + + // Four rapid triggers — without the cap all 4 would run at once. + for v in ["a", "b", "c", "d"] { + hc.states().set(EntityId::parse("switch.m").unwrap(), v, serde_json::json!({}), Context::new()); + sleep(Duration::from_millis(10)).await; + } + + sleep(Duration::from_millis(600)).await; + assert_eq!( + completed.load(Ordering::SeqCst), + 4, + "max:2 must still run all 4 triggers (queued beyond the cap, not dropped)" + ); + assert!( + max_seen.load(Ordering::SeqCst) <= 2, + "max:2 must never exceed 2 concurrent runs (observed {}). On the old engine all 4 ran \ + concurrently → 4.", + max_seen.load(Ordering::SeqCst) + ); + assert!( + max_seen.load(Ordering::SeqCst) >= 2, + "max:2 should reach the cap of 2 with 4 rapid triggers (observed {})", + max_seen.load(Ordering::SeqCst) + ); +} From e7b1b66f7400e89a881b2259d2afd30531041ea2 Mon Sep 17 00:00:00 2001 From: ruv Date: Fri, 12 Jun 2026 01:47:30 -0400 Subject: [PATCH 3/3] =?UTF-8?q?docs(adr):=20ADR-162=20=E2=80=94=20plugin?= =?UTF-8?q?=20security=20+=20bounded=20RunModes;=20mark=20ADR-161=20P4/P5/?= =?UTF-8?q?=C2=A7A5=20DONE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-162 records the M8 work that makes ADR-161's honestly-deferred plugin security claims TRUE: P4 (Ed25519 signature + SHA-256 integrity verification, secure-default trust policy), P5 (capability/authority isolation on hc_state_set), and §A5 (bounded Restart/Queued/max RunModes). Each fix MEASURED with a failing-on-old test; threat model table (tampered module, untrusted publisher, over-privileged write, run-mode exhaustion); cog-ha-matter Ed25519 reuse cited; remaining honest deferral (key provisioning/rotation, native in-process plugins, HAP pairing). ADR-161 deferred-backlog lines for P4/P5/RunModes struck through and marked DONE → ADR-162; §B5 note points forward to the now-implemented P4 gate. Co-Authored-By: claude-flow --- .../ADR-161-homecore-server-layer-security.md | 23 ++- ...62-plugin-security-and-bounded-runmodes.md | 186 ++++++++++++++++++ 2 files changed, 201 insertions(+), 8 deletions(-) create mode 100644 docs/adr/ADR-162-plugin-security-and-bounded-runmodes.md diff --git a/docs/adr/ADR-161-homecore-server-layer-security.md b/docs/adr/ADR-161-homecore-server-layer-security.md index 393085f5..4404c608 100644 --- a/docs/adr/ADR-161-homecore-server-layer-security.md +++ b/docs/adr/ADR-161-homecore-server-layer-security.md @@ -196,7 +196,8 @@ fields are **never read** for verification (only ever set to `None` in tests). re-doc'd **"(P4 — not yet enforced, ADR-161/B5)"** — parsed and round-tripped, but no integrity/signature check happens before a plugin runs. No verification code was added (that is P4); the doc now matches the code. -**Grade: doc-honesty (no behavior change).** +**Grade: doc-honesty (no behavior change).** *(Superseded by ADR-162 §P4: +the hash/signature gate is now implemented and enforced.)* ## Negative Results (NO-ACTION positives — audited, found correct, cited not edited) @@ -213,17 +214,23 @@ touched: ## Deferred Backlog (Nothing Dropped) -- **Plugin authority-isolation (P5)** — `homecore_permissions` claims are parsed - but not enforced at the host-call boundary. **ACCEPTED-FUTURE.** -- **Plugin signature/hash verification (P4)** — implement the +- **Plugin authority-isolation (P5)** — ~~`homecore_permissions` claims are parsed + but not enforced at the host-call boundary.~~ **DONE — ADR-162 §P5.** + `hc_state_set` now consults a `PermissionSet` distilled from the manifest; + an undeclared write returns a typed `-3` to the guest. +- **Plugin signature/hash verification (P4)** — ~~implement the `wasm_module_hash`/`wasm_module_sig`/`publisher_key` gate that B5 now honestly - says is absent. **ACCEPTED-FUTURE.** + says is absent.~~ **DONE — ADR-162 §P4.** `WasmtimeRuntime::load_plugin` now + SHA-256-checks the module, Ed25519-verifies the signature against + `publisher_key`, and enforces a `PluginPolicy` trust allowlist + (secure-default rejects unsigned/untrusted/tampered modules). - **HAP real pairing (P2)** — SRP/HKDF pairing + encrypted sessions; current bridge is an accessory-mapping surface. **ACCEPTED-FUTURE (honestly stubbed).** -- **`RunMode::Queued`/`Restart`/`max` ordering** — `Single`/`Parallel` are +- **`RunMode::Queued`/`Restart`/`max` ordering** — ~~`Single`/`Parallel` are honored; bounded queueing, restart-kill, and `max` concurrency are not yet - wired (every non-Single mode is parallel). **ACCEPTED-FUTURE** — the - `engine.rs` doc states exactly this, no over-claim. + wired (every non-Single mode is parallel).~~ **DONE — ADR-162 §A5.** Restart + aborts the in-flight task, Queued serializes via a per-automation async mutex, + and `max: N` caps concurrency via a per-automation semaphore. - **Automation YAML load-at-boot** — the engine starts empty; a YAML loader is P-next. The bin log states "0 automations registered" honestly. diff --git a/docs/adr/ADR-162-plugin-security-and-bounded-runmodes.md b/docs/adr/ADR-162-plugin-security-and-bounded-runmodes.md new file mode 100644 index 00000000..dd69042e --- /dev/null +++ b/docs/adr/ADR-162-plugin-security-and-bounded-runmodes.md @@ -0,0 +1,186 @@ +# ADR-162: HOMECORE Plugin Security (Signature + Capability Isolation) & Bounded Automation RunModes — Making ADR-161's Deferred Claims TRUE + +- **Status**: accepted +- **Date**: 2026-06-12 +- **Deciders**: ruv +- **Tags**: homecore, homecore-plugins, homecore-automation, plugin-security, wasm-signature-verification, ed25519, capability-isolation, runmode, prove-everything, soundness, honest-labeling +- **Amends**: ADR-161 (relabelled P4/P5 + §A5 deferrals → now enforced), ADR-128 (plugin manifest), ADR-129 (automation engine) + +## Context + +Beyond-SOTA sweep **Milestone 8**, scoped to `homecore-plugins` and +`homecore-automation` only, under the project's **prove-everything / +anti-"AI-slop"** directive. + +ADR-161 (Milestone 7) did the honest thing with three plugin/automation +items it could not finish in that window: rather than fake them, it **relabelled +them as deferred** — + +- **P4** (plugin signature verification): the manifest's `wasm_module_hash` / + `wasm_module_sig` / `publisher_key` were re-doc'd "(P4 — not yet enforced, + ADR-161/B5)" — parsed and round-tripped, but **never checked** before a + plugin runs. +- **P5** (plugin authority isolation): `homecore_permissions` claims were + parsed but **never consulted**; `hc_state_set` let any plugin write any + entity, including `lock.*` / `alarm_control_panel.*`. +- **§A5** (`RunMode`): `Single`/`Parallel` were honored; `Restart`/`Queued`/ + `max: N` were honestly documented as still **unbounded-parallel**. + +### Headline — the deferred security items are now ENFORCED + TESTED + +M8 turns those honest deferrals into real, tested behavior. The plugin trust +boundary is now sound (a tampered module, an untrusted publisher, or an +unsigned module is rejected by the secure default), an over-privileged plugin +write is denied with a typed error, and the bounded run-modes actually bound. +**Every fix is pinned by a test that FAILS on the pre-M8 code** — each of the +three RunMode tests was additionally run against a simulated unbounded-parallel +dispatch and confirmed to panic. + +The Ed25519 crypto reuses the in-repo `cog-ha-matter::witness_signing` pattern +(same `ed25519-dalek` 2.x API, same deterministic-test-key convention). SHA-256 +matches the `sha256:` prefix the manifest already declared and the +`cog-ha-matter` cog manifest's `binary_sha256` hex convention. No new external +dependency tree was introduced — `ed25519-dalek` / `sha2` / `hex` / `base64` +were already in the workspace `Cargo.lock` (cog-ha-matter / bfld pull them in); +only new dependency *edges* were added to `homecore-plugins`. + +Grading vocabulary (ADR-152 / ADR-158 / ADR-160 / ADR-161): +- **MEASURED** — reproduced in this worktree, command + failing-on-old test recorded. +- **ACCEPTED-FUTURE** — deliberately deferred, nothing dropped. + +## Decision — Fixes Landed + +### §P4 — Plugin signature & integrity verification (SECURITY) — MEASURED + +`homecore-plugins/src/manifest.rs` declared `wasm_module_hash` / +`wasm_module_sig` / `publisher_key` but they were **never read** for +verification; the load path (`wasmtime_runtime.rs`) instantiated any `.wasm` +bytes handed to it. + +**Real fix** (`src/verify.rs`, wired into `WasmtimeRuntime::load_plugin`): +before instantiation the runtime now — + +1. computes the **SHA-256** of the actual `.wasm` bytes and rejects if it ≠ the + manifest's `wasm_module_hash` (`sha256:`) — tamper detection; +2. verifies the **Ed25519** `wasm_module_sig` (`ed25519:`, 64-byte raw) + over the 32-byte digest against `publisher_key` (`ed25519:`, 32-byte + raw) and rejects on failure; +3. enforces a configurable **trust policy** — `PluginPolicy::trusted(&[keys])` + is an allowlist of publisher verifying keys; `PluginPolicy::AllowUnsigned` + is an explicit dev escape hatch that LOGS a loud `warn` on every load it + waves through. The **secure default rejects unsigned and unknown-publisher + modules.** `PluginPolicy::deny_all()` trusts no publisher. + +A typed `PluginError::SignatureRejected` is returned (no host panic). The +legacy permission-free `load_wasm` is retained for first-party/trusted/test +modules; production loading goes through `load_plugin`. + +**Failing-on-old tests** (`tests/integration.rs`, `--features wasmtime`) — all +drive `load_plugin`, which **did not exist** on the old code (so the gate is +genuinely new): +- `p4_tampered_module_is_rejected` — a byte-flipped `.wasm` → hash mismatch → rejected. +- `p4_valid_sig_from_trusted_key_loads` — a valid sig from an allowlisted key loads. +- `p4_valid_sig_from_untrusted_key_is_rejected` — a correctly-signed module from a key NOT on the allowlist is rejected. +- `p4_unsigned_module_rejected_by_default_loads_only_under_allow_unsigned` — unsigned rejected under `deny_all`, loads (with warn) only under `AllowUnsigned`. +- Unit (`src/verify.rs`): `valid_sig_from_trusted_key_passes`, `tampered_module_is_rejected`, `valid_sig_from_untrusted_key_is_rejected`, `forged_signature_is_rejected`, `unsigned_module_rejected_under_default_policy`. + +A real deterministic keypair signs real `.wasm` bytes in the tests. +The manifest doc now reads **"(P4 — ENFORCED, ADR-162)"**. **Grade: MEASURED. Milestone headline.** + +### §P5 — Plugin authority / capability isolation (SECURITY) — MEASURED + +`wasmtime_runtime.rs::hc_state_set` applied any write a plugin requested, +ignoring the manifest's `homecore_permissions`. + +**Real fix** (`src/permissions.rs` + `hc_state_set`): the manifest's +`homecore_permissions` (the `state:write:` form, or a bare entity glob +like `light.*`) are distilled into a `PermissionSet` installed in the plugin's +Wasmtime store. The `hc_state_set` host import consults +`permissions.may_write(entity_id)` before applying a write and returns a typed +`-3` (permission denied) to the guest on a violation — **the host is not +panicked.** Wasmtime already gives memory isolation; this adds **authority** +isolation. A plugin with **no** write grants can write nothing (secure default). + +**Failing-on-old tests** (`tests/integration.rs`, `--features wasmtime`): +- `p5_declared_light_plugin_may_write_light_but_not_lock` — a `light.*` plugin writes `light.kitchen` (succeeds) but is REJECTED (`-3`, and the entity is not written) when it tries `lock.front_door`. +- `p5_plugin_with_no_permissions_can_write_nothing` — a plugin with empty `homecore_permissions` cannot write `light.kitchen`. +- Unit (`src/permissions.rs`): domain-glob, exact-grant, wildcard, read-grants-don't-confer-write, no-permissions, and explicit `state:write:` form. + +The manifest doc now reads **"(P5 — ENFORCED, ADR-162)"**. **Grade: MEASURED.** + +### §A5 — Bounded automation RunModes (Restart / Queued / max) — MEASURED + +`homecore-automation/src/engine.rs` (per ADR-161) honored `Single`/`Parallel` +but spawned an unbounded parallel task for `Restart`/`Queued`/`max`. + +**Real fix** (`src/runmode.rs`, a per-automation `RunState` the engine owns and +dispatches through at all three trigger sites — event loop, timer, test hook): +- **Restart** — aborts the in-flight action task via `tokio::task::AbortHandle`, then starts a fresh one. +- **Queued** — serializes runs in arrival order via a per-automation async `Mutex`: sequential, never concurrent, nothing dropped. +- **max: N** — caps concurrency at N via a per-automation `Semaphore`; triggers beyond N **queue** (await a permit) rather than running concurrently. (HA bounded `parallel`/`queued` semantics — chosen and documented as *queue beyond N*, not drop.) +- `Single`/`IgnoreFirst` re-entrancy guard and `Parallel` preserved. + +`engine.rs` trimmed to **433 lines**; the run-mode machinery lives in the new +`runmode.rs` (153 lines) to keep both under the 500-line guideline. + +**Failing-on-old tests** (`tests/engine_behaviors.rs`) — each was run against a +simulated unbounded-parallel dispatch and confirmed to panic: +- `restart_mode_cancels_prior_run` — prior run is aborted: exactly **1** completion (old: both ran → 2). +- `queued_mode_runs_sequentially_not_concurrently` — 3 rapid triggers all run, **max observed concurrency = 1** (old: 3). +- `max_two_caps_concurrency_at_two` — 4 rapid triggers all run, **max observed concurrency ≤ 2** (old: 4). + +**Grade: MEASURED. Restart, Queued, and `max: N` all implemented — no remaining RunMode deferral.** + +## Threat model closed + +| Threat | Before (ADR-161) | After (ADR-162) | +|--------|------------------|-----------------| +| **Tampered module** — attacker swaps `.wasm` bytes after signing | loaded unconditionally (hash never checked) | rejected: SHA-256 mismatch | +| **Untrusted publisher** — valid sig from a key the host doesn't trust | loaded (sig/key never read) | rejected: publisher_key not on allowlist | +| **Unsigned module** — no integrity material at all | loaded | rejected by secure default; loads only under explicit `AllowUnsigned` (loud warn) | +| **Over-privileged plugin write** — a `light.*` plugin writes `lock.front_door` / `alarm_control_panel.*` | applied (permissions never consulted) | denied: typed `-3` to guest, write not applied | +| **Run-mode resource exhaustion** — `max`/`Queued` spawn unbounded tasks | unbounded parallel | bounded: Restart cancels, Queued serializes, `max: N` caps at N | + +## Remaining honest deferral (Nothing Dropped) + +- **Plugin-key provisioning / rotation** — the host's trust allowlist + (`PluginPolicy::trusted`) is supplied by the caller; sourcing it from the + Cognitum control-plane key store (as `cog-ha-matter` does for Seed keys) and + key rotation are **ACCEPTED-FUTURE** (out of M8 scope — same boundary + `witness_signing` draws). +- **`InProcessRuntime` (native first-party plugins)** — has no `.wasm` bytes to + hash, so P4/P5 apply only to the WASM (`wasmtime`) path; native plugins remain + trusted-by-compilation. Honestly noted, not over-claimed. +- **HAP real pairing (P2)** — unchanged from ADR-161; out of M8 scope. + +## Reproduction (MEASURED) + +```bash +cd v2 +# P4/P5 (wasmtime feature needs rustc 1.91+; workspace pins 1.89 for the rest): +cargo +1.91.1 test -p homecore-plugins --features wasmtime +# Bounded RunModes: +cargo test -p homecore-automation --no-default-features +# Full workspace still builds (1.89 toolchain, no wasmtime): +cargo build --workspace --no-default-features +``` + +Result at time of writing (all 0 failed): +- **homecore-plugins** `--features wasmtime` — **32 passed** (lib 23; integration 9). (ADR-161 baseline was 15.) +- **homecore-automation** `--no-default-features` — **45 passed** (lib 37; `engine_behaviors` 8). (ADR-161 baseline was 42.) +- Full workspace `cargo build --workspace --no-default-features` succeeds. + +## Consequences + +- A HOMECORE WASM plugin can no longer be loaded with a tampered binary, an + untrusted publisher, or (by default) no signature at all — the trust boundary + ADR-161/B5 honestly said was absent is now real (P4). +- A plugin can no longer write entities outside its declared + `homecore_permissions`; the lock/alarm escalation path is closed (P5). +- The automation engine's `Restart`, `Queued`, and `max: N` run-modes are now + bounded as documented — no run-mode claims a capability the code lacks. +- No new external dependency tree (reuses the cog-ha-matter Ed25519 stack + already in the lock); source files kept under the 500-line guideline + (`engine.rs` 433, `runmode.rs` 153, `verify.rs` 397, `permissions.rs` 168; + `wasmtime_runtime.rs` non-test source < 500, inline WAT tests as ADR-161 left + them).