diff --git a/Documentation.md b/Documentation.md index 1520ab5..14dc9b0 100644 --- a/Documentation.md +++ b/Documentation.md @@ -1036,7 +1036,7 @@ Two dotted-name segments are reserved, permanent parts of the schema — not one - **`bread..*`** — inbound events published *by* a sibling `bread*` application about its own state (e.g. `bread.clip.copied`). An app may only publish within its own segment; the daemon enforces this at the IPC boundary (a socket client claiming a `source` of an app id it doesn't own is rejected the same way spoofing `power`/`hyprland` is rejected today). - **`bread.command..`** — outbound commands *to* a sibling application (e.g. `bread.command.clip.clear`). Any module or app may publish; only the target app subscribes. This reuses the existing event bus in both directions — there is no separate request/response protocol. -- The second dotted segment is drawn from a small known-apps registry (`bread_shared::apps::KNOWN_APPS`); daemon-internal domains (`terminal`, `git`, `hyprland`, `device`, `power`, `network`, `service`, `container`, `project`, `remote`, `system`, `profile`, `notify`, `command`, `workflow`) are reserved and cannot be claimed as app ids. +- The second dotted segment is drawn from a small known-apps registry (`bread_shared::apps::KNOWN_APPS`); daemon-internal domains (`terminal`, `git`, `hyprland`, `device`, `power`, `network`, `bluetooth`, `workspace`, `window`, `monitor`, `service`, `container`, `project`, `remote`, `system`, `profile`, `notify`, `command`, `workflow`) are reserved and cannot be claimed as app ids. *Since: v1.5 — `bluetooth`, `workspace`, `window`, and `monitor` added to this list (event families the Bluetooth and Hyprland adapters already published under, but that were missing from it); this same list is now also the boundary the IPC `emit` method's no-`source` path checks event names against, see [Dictionary: IPC protocol](#dictionary-ipc-protocol).* - **Commands are best-effort.** Publishing `bread.command..` with no subscriber (the app isn't installed or isn't running) is a silent no-op — there is nothing to special-case, and no error is raised. An app that acts on a command *should* emit a corresponding `bread...done` (or `.failed`) confirmation; a module that needs to know a command was actually honored must `bread.wait`/`bread.wait_any` on that confirmation with a timeout rather than assume success. There is no mandatory request/response correlation layer — most commands are legitimately fire-and-forget, and building one would contradict the "no listener, no-op" degradation property. - **`bread.exec(" ...")`** remains the zero-infrastructure fallback for triggering a sibling app that has a synchronous CLI and no need for a structured response. @@ -1173,8 +1173,13 @@ Available methods: | `profile.activate` | `name` | Switch active profile | | `events.subscribe` | — | Upgrade to streaming mode; pushes events line by line | | `events.replay` | `since_ms` | Replay buffered events from the last N ms | -| `emit` | `event`, `data`, optional `source`, `kind` | Inject an event. Without `source`, builds a `BreadEvent` directly tagged `System` (legacy path). With `source` set to `terminal`/`git`/`remote`, or a registered sibling-app id (see [Namespaces](#namespaces)), builds a real `RawEvent` (requires `kind` too) that goes through the normalizer like any adapter. Any other `source` value is rejected — this is the anti-spoofing boundary that stops a socket client from forging e.g. `power`/`hyprland` events. | +| `emit` | `event`, `data`, optional `source`, `kind` | Inject an event. Without `source`, builds a `BreadEvent` directly, tagged `Manual` *(Since: v1.5 — previously tagged `System`; see below)*, for manually testing Lua handlers (this is what `bread emit ` and `bread-emit` use). With `source` set to `terminal`/`git`/`remote`, or a registered sibling-app id (see [Namespaces](#namespaces)), builds a real `RawEvent` (requires `kind` too) that goes through the normalizer like any adapter. Any other `source` value is rejected — this is the anti-spoofing boundary that stops a socket client from forging e.g. `power`/`hyprland` events. | | `workflows.list` | — | List running/completed workflow instances and their step/status *(Since: v1.2)* | | `widgets.list` | — | List all registered widgets across every module *(Since: v1.3)* | The `health` response's `api_version` field lets a client — the CLI, a Lua module via `bread.exec`, or a `bread-client`-linked sibling app — assert compatibility with this document's versioned schema at connect time (see [API Stability & Versioning](#api-stability--versioning)). + +*Since: v1.5 — `emit` without `source` closed a spoofing gap: previously any event name was accepted with zero validation and tagged `System`, the same tag the daemon uses internally for events it originates itself in Rust code (`bread.system.startup`, `bread.profile.activated`, ...). That made a manually-injected event indistinguishable from a trusted, daemon-originated one. Now:* +- *The unsourced path is tagged `AdapterSource::Manual`, not `System` — `System` is reserved for the daemon's own Rust-originated sends and can no longer be produced from data that arrived over the IPC socket.* +- *The event name is rejected if its top-level dotted segment (the part right after `bread.`) is one of the reserved, adapter-owned domains in `bread_shared::apps::RESERVED_DOMAINS` — `terminal`, `git`, `hyprland`, `device`, `power`, `network`, `bluetooth`, `workspace`, `window`, `monitor`, `service`, `container`, `project`, `remote`, `system`, `profile`, `notify`, `command`, `workflow` (see [Namespaces](#namespaces)) — since a socket client emitting e.g. `bread.power.ac.connected` this way would otherwise be indistinguishable from the real power adapter observing it.* +- *Freely-named custom/test event names (anything outside those reserved domains, including names with no `bread.` prefix at all) remain unrestricted — this is what keeps `bread emit ` useful for testing Lua handlers without unplugging cables, and what `bread-emit`'s fire-and-forget, no-reply-wait design still works against unchanged (a single JSON line write is still sufficient; no handshake was added).* diff --git a/bread-shared/src/apps.rs b/bread-shared/src/apps.rs index 3537a4d..3099690 100644 --- a/bread-shared/src/apps.rs +++ b/bread-shared/src/apps.rs @@ -18,6 +18,18 @@ pub const KNOWN_APPS: &[&str] = &[ /// app id, even if a future `bread*` app would otherwise want that name — /// these are the top-level segments the normalizer and built-in event /// families already use. +/// +/// This is also the single source of truth the IPC boundary checks before +/// allowing a manual (no-`source`) `emit` request to use an event name — a +/// socket client may freely emit a custom/test event, but not one whose +/// top-level segment is one of these, since that would let it impersonate +/// a real adapter (or another daemon-internal event family) rather than +/// producing an obviously-manual one. See [`is_reserved_domain`] and +/// `breadd/src/ipc/mod.rs`'s `emit` handler. *Since: v1.5 — `bluetooth`, +/// `workspace`, `window`, and `monitor` added (event families the Hyprland +/// and Bluetooth adapters already published under, but that were missing +/// from this list) when this became a spoofing-prevention boundary and not +/// just an app-id-conflict one.* const RESERVED_DOMAINS: &[&str] = &[ "terminal", "git", @@ -34,6 +46,10 @@ const RESERVED_DOMAINS: &[&str] = &[ "notify", "command", "workflow", + "bluetooth", + "workspace", + "window", + "monitor", ]; /// Whether `id` is a registered sibling-app id. @@ -55,6 +71,15 @@ pub fn validate_app_namespace(app: &str, event: &str) -> bool { event.starts_with(&format!("bread.{app}.")) } +/// The top-level dotted segment after `bread.` in an event name — e.g. +/// `Some("power")` for `"bread.power.ac.connected"`. Returns `None` for +/// event names that don't start with `bread.` at all, which are always +/// outside any reserved namespace (freely-named custom/test events, the +/// `bread emit ` debug use case, never take this prefix). +pub fn event_domain(event: &str) -> Option<&str> { + event.strip_prefix("bread.")?.split('.').next() +} + #[cfg(test)] mod tests { use super::*; @@ -88,6 +113,35 @@ mod tests { assert!(!is_reserved_domain("clip")); } + #[test] + fn reserved_domains_cover_every_adapter_owned_event_family() { + // Every top-level segment a real adapter (via the normalizer) or the + // daemon itself publishes under must be reserved, or a manual/no-source + // `emit` over the IPC socket could impersonate it undetected. + for domain in [ + "power", "network", "device", "bluetooth", "hyprland", "workspace", "monitor", + "window", "system", + ] { + assert!( + is_reserved_domain(domain), + "'{domain}' is an adapter-owned event family and must be reserved" + ); + } + } + + #[test] + fn event_domain_extracts_top_level_segment() { + assert_eq!(event_domain("bread.power.ac.connected"), Some("power")); + assert_eq!(event_domain("bread.custom.event"), Some("custom")); + assert_eq!(event_domain("bread.test"), Some("test")); + } + + #[test] + fn event_domain_is_none_without_bread_prefix() { + assert_eq!(event_domain("power.ac.connected"), None); + assert_eq!(event_domain(""), None); + } + #[test] fn validate_app_namespace_accepts_own_namespace() { assert!(validate_app_namespace("clip", "bread.clip.copied")); diff --git a/bread-shared/src/lib.rs b/bread-shared/src/lib.rs index 50a2f25..cd4e970 100644 --- a/bread-shared/src/lib.rs +++ b/bread-shared/src/lib.rs @@ -32,9 +32,25 @@ pub enum AdapterSource { Power, /// Network state (rtnetlink / NetworkManager). Network, - /// Internal events synthesized by the daemon itself - /// (e.g. `bread.profile.activated`, `bread.state.changed.*`). + /// Internal events synthesized by the daemon itself, i.e. trusted, + /// Rust-code-originated sends via `emit_tx` (e.g. `bread.system.startup`, + /// `bread.profile.activated`, `bread.state.changed.*`, and Lua's + /// `bread.emit()` binding). Never assignable from data that arrived + /// over the IPC socket — see [`Manual`](AdapterSource::Manual) for that + /// case. *Since: v1.5 — this constraint is now enforced; previously the + /// IPC `emit` method's no-`source` path could also tag events `System`.* System, + /// A manual `emit` IPC request with no `source` param — a human or + /// script poked the daemon's Unix socket directly (e.g. `bread emit + /// ` for testing Lua handlers without unplugging cables). + /// Distinct from [`System`](AdapterSource::System) so downstream Lua + /// modules and tooling can tell "someone manually injected this event" + /// apart from "a real adapter observed this" or "the daemon itself + /// produced this." The IPC boundary restricts which event names may be + /// tagged this way — it may not claim an adapter-owned namespace (see + /// `apps::is_reserved_domain`), but is otherwise free for custom/test + /// event names. *Since: v1.5* + Manual, /// BlueZ Bluetooth stack via D-Bus. Bluetooth, /// Shell precmd/preexec hooks (terminal command lifecycle, cwd changes). @@ -222,6 +238,10 @@ mod tests { serde_json::to_string(&AdapterSource::System).unwrap(), "\"system\"" ); + assert_eq!( + serde_json::to_string(&AdapterSource::Manual).unwrap(), + "\"manual\"" + ); assert_eq!( serde_json::to_string(&AdapterSource::Bluetooth).unwrap(), "\"bluetooth\"" @@ -268,6 +288,7 @@ mod tests { AdapterSource::Power, AdapterSource::Network, AdapterSource::System, + AdapterSource::Manual, AdapterSource::Bluetooth, AdapterSource::Terminal, AdapterSource::Git, diff --git a/breadd/src/core/normalizer.rs b/breadd/src/core/normalizer.rs index 96e40c8..bf01e54 100644 --- a/breadd/src/core/normalizer.rs +++ b/breadd/src/core/normalizer.rs @@ -45,6 +45,19 @@ impl EventNormalizer { source: raw.source.clone(), data: raw.payload.clone(), }], + // `Manual` is never constructed as a `RawEvent::source` in this + // codebase — the IPC boundary's unsourced `emit` path builds a + // `BreadEvent` directly (see `breadd/src/ipc/mod.rs`), bypassing + // this normalizer entirely, exactly as `System` did before it. + // This arm exists only so the match stays exhaustive; if that + // ever changes, pass-through (like `System`) is the sane default + // rather than silently dropping the event. + AdapterSource::Manual => vec![BreadEvent { + event: raw.kind.clone(), + timestamp: raw.timestamp, + source: raw.source.clone(), + data: raw.payload.clone(), + }], }; out.retain(|ev| self.accept(ev)); diff --git a/breadd/src/ipc/mod.rs b/breadd/src/ipc/mod.rs index e132770..a4769c2 100644 --- a/breadd/src/ipc/mod.rs +++ b/breadd/src/ipc/mod.rs @@ -8,7 +8,7 @@ use std::sync::Arc; use std::time::Instant; use anyhow::{anyhow, Result}; -use bread_shared::apps::{is_known_app, validate_app_namespace}; +use bread_shared::apps::{event_domain, is_known_app, is_reserved_domain, validate_app_namespace}; use bread_shared::{now_unix_ms, AdapterSource, BreadEvent, RawEvent}; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; @@ -27,7 +27,7 @@ use crate::lua::RuntimeHandle; /// something new-but-additive (a binding, an event, an IPC param); bump the /// major version only for a breaking change, which should not happen inside /// this daemon's v1 lifetime per that section's stated policy. -const API_VERSION: &str = "1.4.0"; +const API_VERSION: &str = "1.5.0"; #[derive(Clone)] pub struct Server { @@ -323,12 +323,35 @@ impl Server { } Ok(json!({ "emitted": true })) } else { + // Unsourced emit: the manual-testing path ("bread emit + // ", used to poke Lua handlers without unplugging + // cables). Tagged `Manual`, never `System` — `System` is + // reserved for events the daemon originates itself in + // Rust code (e.g. `bread.system.startup` in `serve()` + // above), not for anything that arrived over the wire. + // A socket client can still name any custom/test event + // it likes, but not one whose top-level segment is a + // reserved, adapter-owned domain (`bread.power.*`, + // `bread.hyprland.*`, ...) — otherwise this path would + // let any same-UID process impersonate a real adapter + // event with nothing downstream able to tell the + // difference. let Some(event) = req.params.get("event").and_then(Value::as_str) else { return Err((id, "missing event name".to_string())); }; + if let Some(domain) = event_domain(event) { + if is_reserved_domain(domain) { + return Err(( + id, + format!( + "event '{event}' claims the reserved '{domain}' domain — manual emit cannot impersonate an adapter-owned event; use a custom event name, or a sourced emit if this should go through the normalizer" + ), + )); + } + } if self .emit_tx - .send(BreadEvent::new(event, AdapterSource::System, data)) + .send(BreadEvent::new(event, AdapterSource::Manual, data)) .is_err() { return Err((id, "emit channel closed".to_string())); diff --git a/breadd/tests/ipc_integration.rs b/breadd/tests/ipc_integration.rs index 7e7129b..8b3374d 100644 --- a/breadd/tests/ipc_integration.rs +++ b/breadd/tests/ipc_integration.rs @@ -8,7 +8,7 @@ use serde_json::{json, Value}; use tempfile::TempDir; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::UnixStream; -use tokio::time::sleep; +use tokio::time::{sleep, timeout}; #[tokio::test] async fn ping_and_state_dump_work() -> Result<()> { @@ -99,6 +99,155 @@ async fn emit_without_event_errors() -> Result<()> { Ok(()) } +#[tokio::test] +async fn emit_without_source_rejects_adapter_owned_event_name() -> Result<()> { + let harness = TestHarness::spawn()?; + harness.wait_until_ready().await?; + + // A no-`source` emit must not be able to impersonate a real adapter + // event by event name alone — "power" is a reserved, adapter-owned + // domain (see `bread_shared::apps::RESERVED_DOMAINS`). + let result = harness + .send_request( + "emit", + json!({ "event": "bread.power.ac.connected", "data": { "ac_connected": true } }), + ) + .await; + assert!( + result.is_err(), + "manual emit must not be able to claim an adapter-owned event name" + ); + let msg = result.err().unwrap().to_string(); + assert!(msg.contains("reserved"), "got: {msg}"); + + harness.shutdown(); + Ok(()) +} + +#[tokio::test] +async fn emit_without_source_rejects_every_adapter_owned_domain() -> Result<()> { + let harness = TestHarness::spawn()?; + harness.wait_until_ready().await?; + + // Every namespace a real adapter (or the daemon itself) publishes + // under must be closed to the unsourced emit path, not just "power". + for event in [ + "bread.power.ac.connected", + "bread.network.connected", + "bread.device.connected", + "bread.bluetooth.device.paired", + "bread.hyprland.event", + "bread.workspace.changed", + "bread.monitor.connected", + "bread.window.opened", + "bread.system.startup", + ] { + let result = harness + .send_request("emit", json!({ "event": event, "data": {} })) + .await; + assert!( + result.is_err(), + "expected '{event}' to be rejected on the unsourced emit path" + ); + } + + harness.shutdown(); + Ok(()) +} + +#[tokio::test] +async fn emit_without_source_still_allows_custom_event_names() -> Result<()> { + let harness = TestHarness::spawn()?; + harness.wait_until_ready().await?; + + // The documented `bread emit ` debug use case (testing Lua + // handlers without unplugging cables) must keep working for any event + // name that isn't a reserved, adapter-owned domain. + let result = harness + .send_request( + "emit", + json!({ "event": "bread.mymodule.custom_thing", "data": { "n": 1 } }), + ) + .await; + assert!(result.is_ok(), "custom event name must still be emittable"); + assert_eq!( + result.unwrap().get("emitted").and_then(Value::as_bool), + Some(true) + ); + + harness.shutdown(); + Ok(()) +} + +#[tokio::test] +async fn emit_without_source_is_tagged_manual_not_system() -> Result<()> { + let harness = TestHarness::spawn()?; + harness.wait_until_ready().await?; + + let stream = UnixStream::connect(harness.socket_path()).await?; + let (read_half, mut write_half) = stream.into_split(); + let subscribe = json!({ + "id": "sub-manual", + "method": "events.subscribe", + "params": { "filter": "bread.mymodule.*" } + }); + write_half + .write_all(format!("{}\n", serde_json::to_string(&subscribe)?).as_bytes()) + .await?; + + let mut reader = BufReader::new(read_half).lines(); + reader + .next_line() + .await? + .ok_or_else(|| anyhow!("missing subscribe ack"))?; + + // The subscribe ack is written to the client before the server task + // actually registers on the broadcast channel (see `handle_connection`'s + // "events.subscribe" arm — ack first, `stream_events`/`event_tx.subscribe()` + // second). A brief settle delay avoids racing that registration under + // full-suite parallel load, same as the settle delay already used in + // `workflow_reaches_done_via_wait_any_happy_path` above for the same + // class of subscribe-then-fire race. + sleep(Duration::from_millis(100)).await; + + harness + .send_request( + "emit", + json!({ "event": "bread.mymodule.poked", "data": {} }), + ) + .await?; + + // Bounded by an explicit timeout (not just the `Instant`-deadline-checked + // loop used elsewhere in this file) so a real regression here fails the + // test in 5s instead of hanging this test binary — and therefore the + // whole `cargo test --workspace` run — forever. + let event = timeout(Duration::from_secs(5), async { + loop { + let line = reader + .next_line() + .await? + .ok_or_else(|| anyhow!("event stream closed before match"))?; + let event: Value = serde_json::from_str(&line)?; + if event.get("event").and_then(Value::as_str) == Some("bread.mymodule.poked") { + return Ok::(event); + } + } + }) + .await + .map_err(|_| anyhow!("timed out waiting for bread.mymodule.poked on the stream"))??; + // A wire-triggered no-source emit must never be indistinguishable from + // a daemon-internal `System` event (e.g. `bread.system.startup`, + // `bread.profile.activated`) — it must carry the distinct `Manual` tag. + assert_eq!( + event.get("source").and_then(Value::as_str), + Some("manual"), + "no-source emit must be tagged 'manual', not 'system': {event:?}" + ); + + harness.shutdown(); + Ok(()) +} + #[tokio::test] async fn emit_with_internal_source_is_rejected() -> Result<()> { let harness = TestHarness::spawn()?; @@ -471,7 +620,12 @@ async fn events_stream_receives_emitted_events() -> Result<()> { "id": "sub-1", "method": "events.subscribe", "params": { - "filter": "bread.system.*" + // "system" is a reserved, adapter/daemon-owned domain (see + // `emit_without_source_rejects_reserved_domain` below) — a + // manual emit can no longer use it, so this test's custom + // event lives under "custom" instead, same as it always did + // for "match"/"nomatch"/"replay"/etc. elsewhere in this file. + "filter": "bread.custom.*" } }); write_half @@ -497,7 +651,7 @@ async fn events_stream_receives_emitted_events() -> Result<()> { .send_request( "emit", json!({ - "event": "bread.system.test", + "event": "bread.custom.test", "data": { "ok": true } }), ) @@ -510,7 +664,7 @@ async fn events_stream_receives_emitted_events() -> Result<()> { break; }; let event: Value = serde_json::from_str(&line)?; - if event.get("event").and_then(Value::as_str) == Some("bread.system.test") { + if event.get("event").and_then(Value::as_str) == Some("bread.custom.test") { got = true; break; }