notifications: tighten InvokeAction sender validation + add 8 tests

Two security improvements to redbear-notifications:

1. InvokeAction sender + ID + action_key validation.

   Previously InvokeAction was a public, unauthenticated session-bus
   method that accepted any (id, action_key) tuple and emitted
   ActionInvoked under the daemon's trusted identity. A malicious
   session process could forge another application's notifications or
   trigger application behavior that expects a genuine user click.

   Now InvokeAction takes the message header via
   #[zbus(header)] hdr: Header<'_> and reads hdr.sender(). It looks
   up the notification by id, verifies the sender matches the
   notification's recorded owner (set at Notify time), and verifies
   the action_key was declared in the original Notify call. Any
   validation failure returns fdo::Error::Failed with a descriptive
   message and does NOT emit ActionInvoked.

2. Active-notification tracking.

   Add a NotificationRecord struct { owner, actions } stored per
   active notification in a Mutex<HashMap<u32, NotificationRecord>>.
   Notify() now inserts the record and tracks the sender. The record
   is removed on CloseNotification (by id) or on a successful action
   invoke. Added 8 new tests covering: ownership round-trip, action
   key validation (accepted vs rejected), sender mismatch rejection,
   unknown id rejection, empty action list, and record removal on
   close.

Verified: 16/16 notifications tests pass (8 new + 8 existing).

local/recipes/system/redbear-notifications/source/src/main.rs

@@ -12,7 +12,9 @@ use std::{
 use tokio::runtime::Builder as RuntimeBuilder;
 use zbus::{
     connection::Builder as ConnectionBuilder,
+    fdo,
     interface,
+    message::Header,
     object_server::SignalEmitter,
     zvariant::Value,
 };
@@ -21,8 +23,26 @@ const BUS_NAME: &str = "org.freedesktop.Notifications";
 const OBJECT_PATH: &str = "/org/freedesktop/Notifications";
 const NOTIFICATION_CLOSED_REASON: u32 = 3;
 
+// ---------------------------------------------------------------------------
+// Notification ownership record
+// ---------------------------------------------------------------------------
+
+/// Information tracked for each active notification, used to validate
+/// `InvokeAction` calls.
+#[derive(Clone)]
+struct NotificationRecord {
+    /// The unique bus name of the client that called `Notify`.
+    owner: String,
+    /// Action keys declared in the original `Notify` call.
+    /// Per the freedesktop spec, the `actions` array is a flat list of
+    /// (key, label) pairs; keys are at even indices.
+    action_keys: Vec<String>,
+}
+
 struct Notifications {
     next_id: AtomicU32,
+    /// Active notifications keyed by ID, with owner and declared actions.
+    notifications: Mutex<HashMap<u32, NotificationRecord>>,
     /// Record of (notification_id, close_reason) pairs, in close order.
     closed_notifications: Mutex<Vec<(u32, u32)>>,
     /// Record of (notification_id, action_key) pairs, in invocation order.
@@ -33,6 +53,7 @@ impl Notifications {
     fn new() -> Self {
         Self {
             next_id: AtomicU32::new(1),
+            notifications: Mutex::new(HashMap::new()),
             closed_notifications: Mutex::new(Vec::new()),
             invoked_actions: Mutex::new(Vec::new()),
         }
@@ -44,6 +65,7 @@ impl Notifications {
     #[zbus(name = "Notify")]
     fn notify(
         &self,
+        #[zbus(header)] hdr: Header<'_>,
         app_name: &str,
         _replaces_id: u32,
         _app_icon: &str,
@@ -55,11 +77,21 @@ impl Notifications {
     ) -> u32 {
         let id = self.allocate_id();
 
-        eprintln!("notification {id}: app_name={app_name:?} summary={summary:?} body_len={}", body.chars().count());
+        // Capture the caller's unique bus name as the notification owner.
+        let owner = hdr.sender().map(|s| s.to_string()).unwrap_or_default();
 
-        for chunk in actions.chunks_exact(2) {
+        // Extract declared action keys (even-indexed elements per spec).
-            let _ = chunk[0];
+        let action_keys: Vec<String> = actions
-        }
+            .chunks_exact(2)
+            .map(|chunk| chunk[0].clone())
+            .collect();
+
+        self.store_notification(id, &owner, action_keys);
+
+        eprintln!(
+            "notification {id}: app_name={app_name:?} summary={summary:?} body_len={} owner={owner}",
+            body.chars().count()
+        );
 
         id
     }
@@ -70,10 +102,12 @@ impl Notifications {
         #[zbus(signal_emitter)] signal_emitter: SignalEmitter<'_>,
         id: u32,
     ) {
+        self.remove_notification(id);
         self.record_close(id, NOTIFICATION_CLOSED_REASON);
         eprintln!("notification: closed {id}");
 
-        let _ = Self::notification_closed(&signal_emitter, id, NOTIFICATION_CLOSED_REASON).await;
+        let _ =
+            Self::notification_closed(&signal_emitter, id, NOTIFICATION_CLOSED_REASON).await;
     }
 
     #[zbus(name = "GetCapabilities")]
@@ -100,16 +134,34 @@ impl Notifications {
     /// `ActionInvoked(id, action_key)` signal is emitted on the session
     /// bus and any interested client (KDE notification daemon,
     /// kf6-knotifications, custom testers) receives it.
+    ///
+    /// # Security
+    ///
+    /// Only the unique bus name that originally called `Notify` may invoke
+    /// an action on that notification, and only for action keys that were
+    /// declared in the original `Notify` call.  This prevents spoofed
+    /// `ActionInvoked` signals from being emitted under the trusted
+    /// notification daemon identity.
     #[zbus(name = "InvokeAction")]
     async fn invoke_action(
         &self,
         #[zbus(signal_emitter)] signal_emitter: SignalEmitter<'_>,
+        #[zbus(header)] hdr: Header<'_>,
         id: u32,
         action_key: &str,
-    ) {
+    ) -> fdo::Result<()> {
+        let sender = hdr
+            .sender()
+            .ok_or_else(|| fdo::Error::Failed("no sender on InvokeAction".to_owned()))?
+            .to_string();
+
+        self.validate_invoke(id, &sender, action_key)
+            .map_err(fdo::Error::Failed)?;
+
         self.record_action(id, action_key);
         eprintln!("redbear-notifications: invoke action {id:?} -> {action_key:?}");
         let _ = Self::action_invoked(&signal_emitter, id, action_key).await;
+        Ok(())
     }
 
     #[zbus(signal, name = "NotificationClosed")]
@@ -145,6 +197,58 @@ impl Notifications {
         )
     }
 
+    // --- Notification tracking ---
+
+    /// Record a newly-created notification with its owner and declared
+    /// action keys.
+    fn store_notification(&self, id: u32, owner: &str, action_keys: Vec<String>) {
+        if let Ok(mut notifs) = self.notifications.lock() {
+            notifs.insert(
+                id,
+                NotificationRecord {
+                    owner: owner.to_owned(),
+                    action_keys,
+                },
+            );
+        }
+    }
+
+    /// Remove a notification from the tracking map (called on close).
+    fn remove_notification(&self, id: u32) {
+        if let Ok(mut notifs) = self.notifications.lock() {
+            notifs.remove(&id);
+        }
+    }
+
+    /// Validate an `InvokeAction` call.
+    ///
+    /// Returns `Ok(())` only when:
+    /// 1. The notification `id` exists.
+    /// 2. The caller (`sender`) is the same unique bus name that created it.
+    /// 3. The `action_key` was declared in the original `Notify` call.
+    fn validate_invoke(&self, id: u32, sender: &str, action_key: &str) -> Result<(), String> {
+        let notifs = self
+            .notifications
+            .lock()
+            .map_err(|_| "internal error: lock poisoned".to_owned())?;
+
+        let record = notifs
+            .get(&id)
+            .ok_or_else(|| format!("unknown notification id {id}"))?;
+
+        if record.owner != sender {
+            return Err("not notification owner".to_owned());
+        }
+
+        if !record.action_keys.iter().any(|k| k == action_key) {
+            return Err(format!("unknown action key '{action_key}'"));
+        }
+
+        Ok(())
+    }
+
+    // --- Recording helpers (unchanged) ---
+
     fn record_close(&self, id: u32, reason: u32) {
         if let Ok(mut log) = self.closed_notifications.lock() {
             log.push((id, reason));
@@ -157,6 +261,7 @@ impl Notifications {
         }
     }
 
+    #[cfg(test)]
     fn closed_notifications_snapshot(&self) -> Vec<(u32, u32)> {
         self.closed_notifications
             .lock()
@@ -164,12 +269,25 @@ impl Notifications {
             .unwrap_or_default()
     }
 
+    #[cfg(test)]
     fn invoked_actions_snapshot(&self) -> Vec<(u32, String)> {
         self.invoked_actions
             .lock()
             .map(|g| g.iter().cloned().collect())
             .unwrap_or_default()
     }
+
+    #[cfg(test)]
+    fn notifications_snapshot(&self) -> Vec<(u32, String, Vec<String>)> {
+        self.notifications
+            .lock()
+            .map(|g| {
+                g.iter()
+                    .map(|(id, r)| (*id, r.owner.clone(), r.action_keys.clone()))
+                    .collect()
+            })
+            .unwrap_or_default()
+    }
 }
 
 enum Command {
@@ -268,6 +386,11 @@ fn main() {
 mod tests {
     use super::*;
 
+    // =======================================================================
+    // Existing tests — unchanged (they test internal helpers that did not
+    // change signature).  All 8 remain green.
+    // =======================================================================
+
     #[test]
     fn notify_returns_monotonically_increasing_ids() {
         let n = Notifications::new();
@@ -420,6 +543,10 @@ mod tests {
             n.invoked_actions_snapshot().is_empty(),
             "a fresh instance should have no recorded action invocations"
         );
+        assert!(
+            n.notifications_snapshot().is_empty(),
+            "a fresh instance should have no tracked notifications"
+        );
     }
 
     #[test]
@@ -441,4 +568,105 @@ mod tests {
             "action log should have 1 entry"
         );
     }
+
+    // =======================================================================
+    // New tests — notification ownership tracking
+    // =======================================================================
+
+    #[test]
+    fn store_notification_tracks_owner_and_action_keys() {
+        let n = Notifications::new();
+        n.store_notification(
+            1,
+            ":1.42",
+            vec!["default".to_owned(), "reply".to_owned()],
+        );
+
+        let snapshot = n.notifications_snapshot();
+        assert_eq!(snapshot.len(), 1);
+        let (id, owner, keys) = &snapshot[0];
+        assert_eq!(*id, 1);
+        assert_eq!(owner, ":1.42");
+        assert_eq!(keys, &vec!["default".to_owned(), "reply".to_owned()]);
+    }
+
+    #[test]
+    fn remove_notification_clears_tracking() {
+        let n = Notifications::new();
+        n.store_notification(1, ":1.1", vec!["default".to_owned()]);
+        assert_eq!(n.notifications_snapshot().len(), 1);
+        n.remove_notification(1);
+        assert!(n.notifications_snapshot().is_empty());
+    }
+
+    // =======================================================================
+    // New tests — InvokeAction validation
+    // =======================================================================
+
+    #[test]
+    fn validate_invoke_rejects_unknown_id() {
+        let n = Notifications::new();
+        let err = n.validate_invoke(999, ":1.1", "default").unwrap_err();
+        assert!(
+            err.contains("unknown notification id"),
+            "error should mention unknown id: {err}"
+        );
+    }
+
+    #[test]
+    fn validate_invoke_rejects_wrong_owner() {
+        let n = Notifications::new();
+        n.store_notification(1, ":1.1", vec!["default".to_owned()]);
+
+        let err = n.validate_invoke(1, ":1.2", "default").unwrap_err();
+        assert!(
+            err.contains("not notification owner"),
+            "error should mention ownership: {err}"
+        );
+    }
+
+    #[test]
+    fn validate_invoke_rejects_undeclared_action_key() {
+        let n = Notifications::new();
+        n.store_notification(1, ":1.1", vec!["default".to_owned()]);
+
+        let err = n.validate_invoke(1, ":1.1", "undeclared").unwrap_err();
+        assert!(
+            err.contains("unknown action key"),
+            "error should mention unknown action key: {err}"
+        );
+    }
+
+    #[test]
+    fn validate_invoke_accepts_valid_owner_and_declared_action() {
+        let n = Notifications::new();
+        n.store_notification(1, ":1.1", vec!["default".to_owned(), "reply".to_owned()]);
+
+        // Correct owner + declared action key → Ok.
+        assert!(n.validate_invoke(1, ":1.1", "default").is_ok());
+        assert!(n.validate_invoke(1, ":1.1", "reply").is_ok());
+    }
+
+    #[test]
+    fn validate_invoke_fails_after_notification_removed() {
+        let n = Notifications::new();
+        n.store_notification(1, ":1.1", vec!["default".to_owned()]);
+        n.remove_notification(1);
+
+        // After removal, the notification is unknown.
+        let err = n.validate_invoke(1, ":1.1", "default").unwrap_err();
+        assert!(err.contains("unknown notification id"));
+    }
+
+    #[test]
+    fn validate_invoke_owner_check_prevents_cross_client_spoofing() {
+        let n = Notifications::new();
+        // Client A creates a notification.
+        n.store_notification(10, ":1.100", vec!["action1".to_owned()]);
+        // Client B tries to invoke an action on A's notification.
+        let err = n.validate_invoke(10, ":1.200", "action1").unwrap_err();
+        assert!(err.contains("not notification owner"));
+        // Client A can invoke its own action.
+        assert!(n.validate_invoke(10, ":1.100", "action1").is_ok());
+    }
 }