Merge pull request #5 from Spacecraft-Software/clipboard-hardening

feat(vault): clipboard hardening — clear-on-lock, backend trait, OSC52, configurable interval

Author: UnbreakableMJ

CHANGELOG.md

@@ -10,6 +10,44 @@ range may break in any release.
 
 ### Added
 
+- **Clipboard hardening — clear-on-lock sweep, backend trait, OSC52 fallback,
+  configurable interval.** Closes the secret-can-outlive-the-agent limitation
+  tracked since the M5 copy slice, and delivers PRD §7.5 in its
+  architecturally honest shape.
+  - **Clear-on-lock sweep.** The agent remembers the last value it copied
+    (`Zeroizing`, dropped once a timer clears it) and `lock()` now sweeps it
+    off the clipboard — covering `vault lock`, `Quit`/`stop-agent`,
+    idle-lock, and the new **SIGTERM handler** (PRD §7.3: the agent locks,
+    sweeps, removes its socket, and exits on SIGTERM). The sweep keeps the
+    only-if-still-ours rule: a newer copy by the user is never clobbered.
+  - **Backend trait (PRD §7.5).** New `vault-agent/src/clipboard.rs` defines
+    a `Backend` trait (`arboard` is the one system implementation; detection
+    unchanged) so tests inject a fake clipboard — the sweep logic is now
+    exercised end-to-end on headless CI — and a config-selected backend can
+    slot in later. `Status` gains a serde-defaulted `clipboard_backend`
+    field (`"arboard"` / absent) so clients can see what they're talking to;
+    old-agent frames still decode (regression-tested).
+  - **OSC52 fallback (client-side, by design).** OSC52 copies by escaping to
+    the user's *terminal* — something the detached agent fundamentally cannot
+    do, so PRD §7.5's SSH/tmux fallback lives in the TUI: when the agent
+    declines with the new typed `Error::ClipboardUnavailable`, the TUI
+    fetches the value (id-targeted `Get` — the one path where the secret
+    crosses the local UDS, same as `vault get`) and emits the OSC52 sequence
+    itself, with tmux DCS-passthrough wrapping when `$TMUX` is set
+    (`set-clipboard on` required). The TUI runs its own 30 s timed clear
+    (OSC52 `!` payload) while it lives, and sweeps on quit; a generated
+    password falls back without any `Get` since it's already local.
+  - **Configurable interval.** `vault-agent --clipboard-clear-secs N` (then
+    `$VAULT_CLIPBOARD_CLEAR_SECS`, then 30; `0` disables) sets the agent's
+    default; `Request::Copy`/`CopyText` still override per call. The new
+    `Response::Copied { clear_after_secs }` reports the *effective* window,
+    so the TUI's toast now shows the agent's real interval instead of a
+    hardcoded 30. The future `vault config` file maps onto the same knob.
+  - Tests: fake-backend units for sweep-on-lock, never-clobber-newer-copy,
+    and timer/sweep marker interplay; `Status` backend-name reporting;
+    interval-resolution precedence; OSC52 sequence/clear/tmux-wrapping
+    units; and the old-agent `Status` decode regression.
+
 - **M5 (slice 5) — CLI agent auto-spawn + headless feature gate.** The two
   non-TUI M5 items, closing out the milestone.
   - **Auto-spawn (PRD §7.3).** Any `vault` verb now starts `vault-agent`

Cargo.lock

@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+//! Clipboard backends (PRD §7.5) — the agent-side abstraction.
+//!
+//! The agent owns one [`Backend`] for its lifetime (on X11 the owning process
+//! serves the selection, so the handle can't be transient). `arboard` covers
+//! Wayland, X11, and macOS; when it can't initialise (headless box, no
+//! display) the agent runs with no backend and `Copy`/`CopyText` decline with
+//! a typed `ClipboardUnavailable` — clients fall back from there (the TUI
+//! emits OSC52 itself; a daemon has no terminal to escape to, so OSC52 can
+//! never live here). The trait exists so tests can inject a fake and so a
+//! config-selected backend can slot in later without touching `AgentState`.
+
+use vault_ipc::proto::Error as IpcError;
+
+/// One system clipboard the agent can write to.
+///
+/// Object-safe and `Send` so the boxed backend can live inside the
+/// tokio-shared `AgentState`.
+pub trait Backend: Send {
+    /// Short identifier surfaced in `Status.clipboard_backend`.
+    fn name(&self) -> &'static str;
+
+    /// Place `value` on the clipboard.
+    ///
+    /// # Errors
+    ///
+    /// Returns an [`IpcError`] when the underlying clipboard write fails.
+    fn set_text(&mut self, value: &str) -> Result<(), IpcError>;
+
+    /// Current clipboard contents, `None` if unreadable.
+    fn get_text(&mut self) -> Option<String>;
+
+    /// Best-effort clear; errors are not actionable mid-shutdown.
+    fn clear(&mut self);
+}
+
+/// The `arboard`-backed system clipboard (Wayland / X11 / macOS).
+pub struct Arboard(arboard::Clipboard);
+
+impl Backend for Arboard {
+    fn name(&self) -> &'static str {
+        "arboard"
+    }
+
+    fn set_text(&mut self, value: &str) -> Result<(), IpcError> {
+        self.0
+            .set_text(value.to_owned())
+            .map_err(|e| IpcError::Internal(format!("clipboard write failed: {e}")))
+    }
+
+    fn get_text(&mut self) -> Option<String> {
+        self.0.get_text().ok()
+    }
+
+    fn clear(&mut self) {
+        let _ = self.0.clear();
+    }
+}
+
+/// Detect a usable backend, degrading to `None` (with a warning) when no
+/// display/compositor is reachable — copy requests then decline cleanly.
+pub fn detect() -> Option<Box<dyn Backend>> {
+    match arboard::Clipboard::new() {
+        Ok(cb) => Some(Box::new(Arboard(cb))),
+        Err(e) => {
+            eprintln!("vault-agent: clipboard unavailable, copy will be declined: {e}");
+            None
+        }
+    }
+}

crates/vault-agent/src/clipboard.rs

@@ -19,6 +19,8 @@ use tokio::sync::Mutex;
 
 use vault_ipc::default_socket_path;
 
+#[cfg(feature = "clipboard")]
+mod clipboard;
 mod server;
 mod state;
 mod unlock;
@@ -57,23 +59,64 @@ struct Args {
     /// many seconds with no client activity. `0` disables auto-lock.
     #[arg(long, default_value_t = 900)]
     idle_lock_secs: u64,
+    /// Default seconds before a copied secret is wiped from the clipboard
+    /// when the client doesn't specify; `0` disables the default auto-clear.
+    /// Falls back to `$VAULT_CLIPBOARD_CLEAR_SECS`, then 30. No effect on a
+    /// build without clipboard support.
+    #[arg(long)]
+    clipboard_clear_secs: Option<u64>,
 }
 
 #[tokio::main(flavor = "current_thread")]
 async fn main() -> anyhow::Result<()> {
     let args = Args::parse();
     let path = pick_socket(args.socket)?;
-    let state = Arc::new(Mutex::new(AgentState::new(args.idle_lock_secs)));
+    #[cfg(feature = "clipboard")]
+    let agent = {
+        let mut a = AgentState::new(args.idle_lock_secs);
+        a.clipboard_clear_secs = resolve_clear_secs(
+            args.clipboard_clear_secs,
+            std::env::var("VAULT_CLIPBOARD_CLEAR_SECS").ok().as_deref(),
+        );
+        a
+    };
+    #[cfg(not(feature = "clipboard"))]
+    let agent = AgentState::new(args.idle_lock_secs);
+    let state = Arc::new(Mutex::new(agent));
 
     if args.idle_lock_secs > 0 {
         let st = state.clone();
         tokio::spawn(server::idle_lock_loop(st));
     }
 
+    // PRD §7.3: lock on SIGTERM. Locking also sweeps a still-pending
+    // clipboard copy; the socket file is removed so the next CLI call
+    // auto-spawns cleanly instead of hitting a stale socket.
+    {
+        let st = state.clone();
+        let sock = path.clone();
+        let mut term = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate())?;
+        tokio::spawn(async move {
+            term.recv().await;
+            st.lock().await.lock();
+            let _ = std::fs::remove_file(&sock);
+            eprintln!("vault-agent: SIGTERM — keys dropped, exiting");
+            std::process::exit(0);
+        });
+    }
+
     server::run(path, state).await?;
     Ok(())
 }
 
+/// Effective default auto-clear interval: `--clipboard-clear-secs` wins, then
+/// `$VAULT_CLIPBOARD_CLEAR_SECS` (ignored when unparsable), then 30 s.
+#[cfg(feature = "clipboard")]
+fn resolve_clear_secs(flag: Option<u64>, env: Option<&str>) -> u64 {
+    flag.or_else(|| env.and_then(|v| v.parse().ok()))
+        .unwrap_or(30)
+}
+
 fn pick_socket(cli: Option<PathBuf>) -> anyhow::Result<PathBuf> {
     if let Some(p) = cli {
         return Ok(p);
@@ -86,3 +129,21 @@ fn pick_socket(cli: Option<PathBuf>) -> anyhow::Result<PathBuf> {
     }
     default_socket_path().ok_or_else(|| anyhow::anyhow!("no XDG_RUNTIME_DIR / TMPDIR available"))
 }
+
+#[cfg(test)]
+mod tests {
+    #[cfg(feature = "clipboard")]
+    #[test]
+    fn resolve_clear_secs_precedence() {
+        use super::resolve_clear_secs;
+        assert_eq!(resolve_clear_secs(Some(45), Some("60")), 45, "flag wins");
+        assert_eq!(resolve_clear_secs(None, Some("60")), 60, "env next");
+        assert_eq!(
+            resolve_clear_secs(None, Some("nope")),
+            30,
+            "bad env ignored"
+        );
+        assert_eq!(resolve_clear_secs(None, None), 30, "default");
+        assert_eq!(resolve_clear_secs(Some(0), Some("60")), 0, "0 disables");
+    }
+}

crates/vault-agent/src/main.rs

@@ -155,20 +155,21 @@ async fn dispatch(req: Request, state: &Arc<Mutex<AgentState>>) -> Response {
                 }
                 Err(e) => Err(e),
             };
+            let secs = clear_after_secs.unwrap_or(s.clipboard_clear_secs);
             s.touch();
             drop(s);
             match outcome {
                 Ok(value) => {
-                    schedule_clipboard_clear(state.clone(), value, clear_after_secs);
-                    Response::Ok
+                    schedule_clipboard_clear(state.clone(), value, secs);
+                    Response::Copied(vault_ipc::proto::Copied {
+                        clear_after_secs: secs,
+                    })
                 }
                 Err(e) => Response::Error(e),
             }
         }
         #[cfg(not(feature = "clipboard"))]
-        Request::Copy { .. } => Response::Error(vault_ipc::proto::Error::Internal(
-            "clipboard support not compiled in".to_owned(),
-        )),
+        Request::Copy { .. } => Response::Error(vault_ipc::proto::Error::ClipboardUnavailable),
         #[cfg(feature = "clipboard")]
         Request::CopyText {
             text,
@@ -192,20 +193,21 @@ async fn dispatch(req: Request, state: &Arc<Mutex<AgentState>>) -> Response {
             } else {
                 Err(vault_ipc::proto::Error::Locked)
             };
+            let secs = clear_after_secs.unwrap_or(s.clipboard_clear_secs);
             s.touch();
             drop(s);
             match outcome {
                 Ok(value) => {
-                    schedule_clipboard_clear(state.clone(), value, clear_after_secs);
-                    Response::Ok
+                    schedule_clipboard_clear(state.clone(), value, secs);
+                    Response::Copied(vault_ipc::proto::Copied {
+                        clear_after_secs: secs,
+                    })
                 }
                 Err(e) => Response::Error(e),
             }
         }
         #[cfg(not(feature = "clipboard"))]
-        Request::CopyText { .. } => Response::Error(vault_ipc::proto::Error::Internal(
-            "clipboard support not compiled in".to_owned(),
-        )),
+        Request::CopyText { .. } => Response::Error(vault_ipc::proto::Error::ClipboardUnavailable),
         Request::Remove { selector } => {
             // Hold the agent mutex across the network call. Vault is
             // single-user / single-agent, so request concurrency is low and
@@ -284,26 +286,18 @@ async fn dispatch(req: Request, state: &Arc<Mutex<AgentState>>) -> Response {
     }
 }
 
-/// Default seconds before the agent wipes a copied secret from the clipboard.
-///
-/// 30 s follows common password-manager practice (and Vault PRD §7.2): long
-/// enough to paste, short enough to bound exposure. `Request::Copy` overrides
-/// per call; config-driven tuning lands in a later slice.
-#[cfg(feature = "clipboard")]
-const DEFAULT_CLIPBOARD_CLEAR_SECS: u64 = 30;
-
-/// Spawn a one-shot task that wipes the clipboard after `clear_after_secs` (or
-/// the default), but only if it still holds the value we copied. `Some(0)`
-/// disables the auto-clear. The task carries the secret so the clear survives
-/// the requesting client quitting.
+/// Spawn a one-shot task that wipes the clipboard after `secs` (the effective
+/// interval — the caller already applied the agent default), but only if it
+/// still holds the value we copied. `0` disables the auto-clear. The task
+/// carries the secret so the clear survives the requesting client quitting;
+/// if the *agent* goes down first, `AgentState::lock`'s sweep covers it.
 #[cfg(feature = "clipboard")]
 fn schedule_clipboard_clear(
     state: Arc<Mutex<AgentState>>,
     value: zeroize::Zeroizing<String>,
-    clear_after_secs: Option<u64>,
+    secs: u64,
 ) {
     use tokio::time::{Duration, sleep};
-    let secs = clear_after_secs.unwrap_or(DEFAULT_CLIPBOARD_CLEAR_SECS);
     if secs == 0 {
         return;
     }