Comprehensive remediation: fix ssh-agent control + UX cohesion + security (P0–P2)

README.md

@@ -21,13 +21,13 @@ SSHWarden 的目标不是复刻完整 Bitwarden Desktop，而是提供一个专
 | Bitwarden 登录与 SSH Key 同步 | 已实现 |
 | SSH Agent 协议服务 | Windows 已实现；Unix socket 有基础实现 |
 | 签名授权对话框 | Slint 跨平台 UI 已实现 |
-| PIN 解锁 | 已实现，后续需迁移到 envelope encryption 模型 |
-| Windows Hello | 已有实现，后续需迁移到 envelope encryption 模型 |
-| IPC 控制命令 | Windows 已实现；Linux/macOS 待补独立 control socket |
-| Local Key Cache | 已实现旧模型；目标模型见 ADR |
-| 标准平台存储目录 | 待实现；当前代码仍使用 exe 同目录 |
+| PIN 解锁 | 已实现（信封加密 + 每库随机盐 + 失败延迟/锁定） |
+| Windows Hello | 已实现（信封加密模型） |
+| IPC 控制命令 | 已实现；Windows Named Pipe 与 Unix socket 均含调用方鉴权 |
+| Local Key Cache | 已实现信封加密模型（格式 v3，PIN 随机盐） |
+| 标准平台存储目录 | 已实现，默认平台标准目录（`%APPDATA%`/`$XDG_CONFIG_HOME`/`~/Library/Application Support`）；便携模式可选 |
 | 自启动 | Windows 已实现；macOS/Linux 待实现 |
-| Shell integration | 待实现 `sshwarden env` |
+| Shell integration | `sshwarden env` 已实现（sh/fish/powershell/cmd） |
 | macOS native unlock | 已实现（Phase 6，Keychain；仍需平台实机验证） |
 | Linux native unlock | 已实现（Phase 6，Secret Service；仍需平台实机验证） |
 

config.toml.example

@@ -46,6 +46,9 @@ email = ""
 # 可选值:
 #   - "always": 每次请求都提示（最安全）
 #   - "never": 从不提示（自动批准所有请求）
+#       ⚠ 不安全:SSHWarden 不对调用方做进程级鉴权,因此本机上任何以你的用户身份
+#       运行的进程都能在你不知情的情况下让 agent 静默签名。仅在完全受信任的环境
+#       (如隔离的 CI runner)使用。
 #   - "remember_until_lock": 记住决定直到锁定（平衡安全与便利）
 # 默认: "always"
 prompt_behavior = "always"

crates/sshwarden-agent/Cargo.toml

@@ -16,7 +16,7 @@ serde_json = { workspace = true }
 ssh-key = { workspace = true, features = ["encryption", "ed25519", "rsa", "getrandom"] }
 sysinfo = { workspace = true, features = ["windows"] }
 thiserror = { workspace = true }
-tokio = { workspace = true, features = ["io-util", "sync", "macros", "net"] }
+tokio = { workspace = true, features = ["io-util", "sync", "macros", "net", "time"] }
 tokio-util = { workspace = true, features = ["codec"] }
 tracing = { workspace = true }
 sshwarden-config = { path = "../sshwarden-config" }
@@ -26,8 +26,14 @@ pin-project = "1"
 windows = { version = "0.61", features = [
     "Win32_Foundation",
     "Win32_System_Pipes",
+    "Win32_System_Threading",
+    "Win32_Security",
+    "Win32_Security_Authorization",
 ] }
 
+[target.'cfg(unix)'.dependencies]
+libc = "0.2"
+
 [dev-dependencies]
 tokio = { workspace = true, features = ["rt-multi-thread", "macros"] }
 

crates/sshwarden-agent/src/agent.rs

@@ -68,6 +68,11 @@ pub struct SshWardenAgent {
     request_id: Arc<AtomicU32>,
     needs_unlock: Arc<AtomicBool>,
     is_running: Arc<AtomicBool>,
+    /// Reports a fatal agent-transport failure (e.g. the SSH agent endpoint
+    /// could not be claimed). The main loop watches this so the daemon shuts
+    /// down instead of running as a zombie that still answers status/unlock
+    /// while serving no SSH client (RT-01).
+    fatal_tx: Arc<tokio::sync::watch::Sender<Option<String>>>,
 }
 
 impl ssh_agent::Agent<PeerInfo, SshWardenKey> for SshWardenAgent {
@@ -102,7 +107,8 @@ impl ssh_agent::Agent<PeerInfo, SshWardenKey> for SshWardenAgent {
         );
 
         let mut rx_channel = self.ui_response_tx.subscribe();
-        self.show_ui_request_tx
+        if self
+            .show_ui_request_tx
             .send(SshAgentUIRequest {
                 request_id,
                 cipher_id: Some(ssh_key.cipher_uuid.clone()),
@@ -114,7 +120,13 @@ impl ssh_agent::Agent<PeerInfo, SshWardenKey> for SshWardenAgent {
                 is_forwarding: info.is_forwarding(),
             })
             .await
-            .expect("Should send request to ui");
+            .is_err()
+        {
+            // The host application's UI channel is gone (daemon shutting down or
+            // main loop exited). Fail closed instead of panicking the serve task.
+            error!("UI request channel closed; denying sign request");
+            return false;
+        }
         while let Ok((id, response)) = rx_channel.recv().await {
             if id == request_id {
                 return response;
@@ -141,10 +153,11 @@ impl ssh_agent::Agent<PeerInfo, SshWardenKey> for SshWardenAgent {
             operation_kind: crate::request_parser::OperationKind::SshAuthentication,
             is_forwarding: info.is_forwarding(),
         };
-        self.show_ui_request_tx
-            .send(message)
-            .await
-            .expect("Should send request to ui");
+        if self.show_ui_request_tx.send(message).await.is_err() {
+            // UI channel gone (daemon shutting down); fail closed, don't panic.
+            error!("UI request channel closed; denying list request");
+            return false;
+        }
         while let Ok((id, response)) = rx_channel.recv().await {
             if id == request_id {
                 return response;
@@ -175,6 +188,7 @@ impl SshWardenAgent {
         auth_request_tx: tokio::sync::mpsc::Sender<SshAgentUIRequest>,
         auth_response_tx: Arc<tokio::sync::broadcast::Sender<(u32, bool)>>,
     ) -> Self {
+        let (fatal_tx, _fatal_rx) = tokio::sync::watch::channel(None);
         Self {
             keystore: ssh_agent::KeyStore(Arc::new(RwLock::new(HashMap::new()))),
             cancellation_token: CancellationToken::new(),
@@ -183,6 +197,7 @@ impl SshWardenAgent {
             request_id: Arc::new(AtomicU32::new(0)),
             needs_unlock: Arc::new(AtomicBool::new(true)),
             is_running: Arc::new(AtomicBool::new(false)),
+            fatal_tx: Arc::new(fatal_tx),
         }
     }
 
@@ -229,7 +244,7 @@ impl SshWardenAgent {
                         .public_key()
                         .to_bytes()
                         .expect("Cipher private key is always correctly parsed");
-                    keystore.0.write().expect("RwLock is not poisoned").insert(
+                    let displaced = keystore.0.write().expect("RwLock is not poisoned").insert(
                         public_key_bytes.clone(),
                         SshWardenKey {
                             private_key: Some(private_key),
@@ -238,6 +253,17 @@ impl SshWardenAgent {
                             cipher_uuid: cipher_id.clone(),
                         },
                     );
+                    if displaced.is_some() {
+                        // LOGIC-7: the keystore is keyed by public key bytes, so
+                        // two vault items sharing a public key collapse into one.
+                        // Warn so the operator understands why status/key_count
+                        // can be lower than the number of vault keys.
+                        tracing::warn!(
+                            key = %name,
+                            "Duplicate public key: overwrote an earlier agent keystore entry; \
+                             status key_count will be lower than the vault key count"
+                        );
+                    }
                 }
                 Err(e) => {
                     error!(error=%e, "Error while parsing key");
@@ -324,6 +350,20 @@ impl SshWardenAgent {
             .len()
     }
 
+    /// Number of loaded keys that actually hold private material and can sign.
+    /// While the vault is locked this is 0 even though `key_count()` (listed
+    /// identities) may be non-zero. status reports this so what it shows matches
+    /// what the agent can actually sign.
+    pub fn signable_key_count(&self) -> usize {
+        self.keystore
+            .0
+            .read()
+            .expect("RwLock is not poisoned")
+            .values()
+            .filter(|k| k.private_key.is_some())
+            .count()
+    }
+
     pub fn start_server(
         auth_request_tx: tokio::sync::mpsc::Sender<SshAgentUIRequest>,
         auth_response_tx: Arc<tokio::sync::broadcast::Sender<(u32, bool)>>,
@@ -349,6 +389,18 @@ impl SshWardenAgent {
         self.is_running.clone()
     }
 
+    /// A receiver the main loop watches for a fatal agent-transport failure.
+    /// When a value is sent, the daemon should shut down rather than keep
+    /// running as a zombie that still answers status/unlock (RT-01).
+    pub fn fatal_rx(&self) -> tokio::sync::watch::Receiver<Option<String>> {
+        self.fatal_tx.subscribe()
+    }
+
+    /// A clonable sender the transport listener uses to report a fatal failure.
+    pub fn fatal_tx(&self) -> Arc<tokio::sync::watch::Sender<Option<String>>> {
+        self.fatal_tx.clone()
+    }
+
     pub fn keystore_clone(&self) -> ssh_agent::KeyStore<SshWardenKey> {
         self.keystore.clone()
     }
@@ -430,4 +482,32 @@ nGZV/aEAZ3ZMrsrA3g32AAAAEHRlc3RAZXhhbXBsZS5jb20BAgMEBQ==
         assert!(!signature.as_bytes().is_empty());
         assert_eq!(signature.algorithm(), ssh_key::Algorithm::Ed25519);
     }
+
+    // lock-keystore: after lock(), identities remain listable (ssh-add -l keeps
+    // working so auto-unlock-on-request can fire) but hold no private material,
+    // so signable_key_count drops to 0 and status can report the truth.
+    #[test]
+    fn lock_keeps_keys_listable_but_not_signable() {
+        let (mut agent, _request_rx, _response_tx) = create_test_agent();
+        agent.is_running.store(true, Ordering::Relaxed);
+        agent
+            .set_keys(vec![(
+                TEST_ED25519_KEY.to_string(),
+                "ed25519-key".to_string(),
+                "ed25519-uuid".to_string(),
+            )])
+            .expect("set_keys should succeed");
+
+        assert_eq!(agent.key_count(), 1);
+        assert_eq!(agent.signable_key_count(), 1);
+
+        agent.lock().expect("lock should succeed");
+
+        assert_eq!(agent.key_count(), 1, "identity stays listable while locked");
+        assert_eq!(
+            agent.signable_key_count(),
+            0,
+            "no private material is signable while locked"
+        );
+    }
 }