feat(agent): v2 secure-session-core Task 6 - full key fidelity

SPEC-002 Phase 1 Task 6, code-reviewed APPROVED (2 rounds), locally verified
(cargo fmt + clippy -D warnings exit 0 + cargo test --workspace 70 pass + build).

- Viewer WH_KEYBOARD_LL hook diverts system combos (Win/Win+R, Alt+Tab, Alt+Esc,
  Ctrl+Esc) to the remote as a full KeyEvent (vk + scan + is_extended + modifiers)
  and suppresses local handling - GATED on the viewer window having focus AND a
  "send system keys" toggle (default on; Pause/Break host-key), so it never bricks
  the technician's local keyboard when unfocused.
- Agent injection via SendInput KEYEVENTF_SCANCODE + correct KEYEVENTF_EXTENDEDKEY
  (right Ctrl/Alt, arrows, nav, Win, NumLock, numpad Divide) - layout-independent,
  extended-key-correct.
- Ctrl+Alt+Del completes through the SAS helper (SYSTEM SendSAS); installer sets
  the SoftwareSASGeneration policy; 3-tier fail-loud (no false success). SAS named
  pipe DACL tightened from NULL/Everyone to Authenticated Users.
- Modifier hygiene: viewer emits key-ups for held Ctrl/Alt/Shift/Win on focus loss
  / close so modifiers never stick on the remote.
- proto: KeyEvent.is_extended = 7 (additive; older agents derive the flag).

Closes Win+R / Ctrl+C-V / Ctrl+Alt+Del / arrows-vs-numpad fidelity. Live on-device
testing is plan Task 8.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

agent/src/input/keyboard.rs

@@ -1,22 +1,30 @@
 //! Keyboard input simulation using Windows SendInput API
+//!
+//! Injection is **scan-code based** (`KEYEVENTF_SCANCODE`) rather than virtual-key
+//! based. Scan codes are layout-independent: the same physical key produces the same
+//! scan code regardless of the remote keyboard layout, so the remote machine's active
+//! layout (not the technician's) decides what character a key produces. The viewer
+//! still carries the virtual-key code for logic that needs it, and we fall back to
+//! deriving a scan code from the VK when the wire frame did not supply one.
 
 use anyhow::Result;
 
 #[cfg(windows)]
 use windows::Win32::UI::Input::KeyboardAndMouse::{
     MapVirtualKeyW, SendInput, INPUT, INPUT_0, INPUT_KEYBOARD, KEYBDINPUT, KEYBD_EVENT_FLAGS,
-    KEYEVENTF_EXTENDEDKEY, KEYEVENTF_KEYUP, KEYEVENTF_UNICODE, MAPVK_VK_TO_VSC_EX,
+    KEYEVENTF_EXTENDEDKEY, KEYEVENTF_KEYUP, KEYEVENTF_SCANCODE, KEYEVENTF_UNICODE,
+    MAPVK_VK_TO_VSC_EX,
 };
 
 /// Keyboard input controller
 pub struct KeyboardController {
-    // Track modifier states for proper handling
-    #[allow(dead_code)]
+    /// Tracks which modifier keys this controller currently holds DOWN on the remote.
+    /// Used so a focus-loss / session-end re-sync can release any still-held modifier
+    /// and avoid "stuck" Ctrl/Alt/Shift/Win on the remote desktop.
     modifiers: ModifierState,
 }
 
-// Modifier tracking is not yet wired into key dispatch.
-#[allow(dead_code)]
+/// Tracks the down/up state of each modifier the agent has injected.
 #[derive(Default)]
 struct ModifierState {
     ctrl: bool,
@@ -25,6 +33,55 @@ struct ModifierState {
     meta: bool,
 }
 
+impl ModifierState {
+    /// Record a modifier transition for `vk_code`. Returns `true` if `vk_code` is a
+    /// modifier key (and the state was updated), `false` otherwise.
+    fn record(&mut self, vk_code: u16, down: bool) -> bool {
+        match vk_code {
+            // VK_CONTROL / VK_LCONTROL / VK_RCONTROL
+            0x11 | 0xA2 | 0xA3 => {
+                self.ctrl = down;
+                true
+            }
+            // VK_MENU / VK_LMENU / VK_RMENU (Alt)
+            0x12 | 0xA4 | 0xA5 => {
+                self.alt = down;
+                true
+            }
+            // VK_SHIFT / VK_LSHIFT / VK_RSHIFT
+            0x10 | 0xA0 | 0xA1 => {
+                self.shift = down;
+                true
+            }
+            // VK_LWIN / VK_RWIN
+            0x5B | 0x5C => {
+                self.meta = down;
+                true
+            }
+            _ => false,
+        }
+    }
+
+    /// Return the VK codes of every modifier currently held down, then clear the state.
+    fn drain_held(&mut self) -> Vec<u16> {
+        let mut held = Vec::new();
+        if self.ctrl {
+            held.push(0x11);
+        }
+        if self.alt {
+            held.push(0x12);
+        }
+        if self.shift {
+            held.push(0x10);
+        }
+        if self.meta {
+            held.push(0x5B);
+        }
+        *self = ModifierState::default();
+        held
+    }
+}
+
 impl KeyboardController {
     /// Create a new keyboard controller
     pub fn new() -> Result<Self> {
@@ -33,28 +90,75 @@ impl KeyboardController {
         })
     }
 
-    /// Press a key down by virtual key code
+    /// Press a key down by virtual key code (scan code derived from the VK).
     #[cfg(windows)]
     pub fn key_down(&mut self, vk_code: u16) -> Result<()> {
-        self.send_key(vk_code, true)
+        self.send_key(vk_code, 0, false, true)
     }
 
-    /// Release a key by virtual key code
+    /// Release a key by virtual key code (scan code derived from the VK).
     #[cfg(windows)]
     pub fn key_up(&mut self, vk_code: u16) -> Result<()> {
-        self.send_key(vk_code, false)
+        self.send_key(vk_code, 0, false, false)
     }
 
-    /// Send a key event
+    /// Inject a full-fidelity key event.
+    ///
+    /// `scan_code` is the hardware scan code captured by the viewer's low-level hook
+    /// (0 ⇒ derive it from `vk_code`). `is_extended` is the viewer-captured extended-key
+    /// flag (`LLKHF_EXTENDED`); when `false` the agent still derives the flag from the
+    /// VK / scan code so older viewers that don't set it stay correct.
     #[cfg(windows)]
-    fn send_key(&mut self, vk_code: u16, down: bool) -> Result<()> {
-        // Get scan code from virtual key
-        let scan_code = unsafe { MapVirtualKeyW(vk_code as u32, MAPVK_VK_TO_VSC_EX) as u16 };
+    pub fn key_event_full(
+        &mut self,
+        vk_code: u16,
+        scan_code: u16,
+        is_extended: bool,
+        down: bool,
+    ) -> Result<()> {
+        self.send_key(vk_code, scan_code, is_extended, down)
+    }
 
-        let mut flags = KEYBD_EVENT_FLAGS::default();
+    /// Release every modifier this controller currently holds down on the remote.
+    ///
+    /// Called on viewer focus loss and at session end so a Ctrl/Alt/Shift/Win that was
+    /// pressed but whose key-up never arrived (e.g. the technician alt-tabbed away) does
+    /// not stay latched on the remote desktop.
+    #[cfg(windows)]
+    pub fn release_all_modifiers(&mut self) -> Result<()> {
+        for vk in self.modifiers.drain_held() {
+            // Emit the key-up directly; drain_held already cleared the tracked state.
+            if let Err(e) = self.send_key(vk, 0, false, false) {
+                tracing::warn!("Failed to release held modifier vk={:#x}: {}", vk, e);
+            } else {
+                tracing::debug!("Released stuck modifier vk={:#x} on focus loss", vk);
+            }
+        }
+        Ok(())
+    }
 
-        // Add extended key flag for certain keys
-        if Self::is_extended_key(vk_code) || (scan_code >> 8) == 0xE0 {
+    /// Send a key event using scan-code injection.
+    #[cfg(windows)]
+    fn send_key(
+        &mut self,
+        vk_code: u16,
+        scan_code: u16,
+        is_extended: bool,
+        down: bool,
+    ) -> Result<()> {
+        // Track modifier state so we can release stuck modifiers later.
+        self.modifiers.record(vk_code, down);
+
+        // Prefer the viewer-supplied scan code; fall back to deriving one from the VK.
+        // MAPVK_VK_TO_VSC_EX yields a 0xE0-prefixed value for extended keys.
+        let mapped = unsafe { MapVirtualKeyW(vk_code as u32, MAPVK_VK_TO_VSC_EX) as u16 };
+        let effective_scan = if scan_code != 0 { scan_code } else { mapped };
+
+        let mut flags = KEYBD_EVENT_FLAGS::default() | KEYEVENTF_SCANCODE;
+
+        // Add the extended flag if the viewer flagged it, the VK is inherently
+        // extended, or the mapped scan code carries the 0xE0 extended prefix.
+        if is_extended || Self::is_extended_key(vk_code) || (mapped >> 8) == 0xE0 {
             flags |= KEYEVENTF_EXTENDEDKEY;
         }
 
@@ -62,12 +166,16 @@ impl KeyboardController {
             flags |= KEYEVENTF_KEYUP;
         }
 
+        // For scan-code injection the low byte of the scan code is what Windows uses;
+        // the 0xE0 prefix is conveyed via KEYEVENTF_EXTENDEDKEY, not the wScan value.
+        let w_scan = (effective_scan & 0x00FF) as u16;
+
         let input = INPUT {
             r#type: INPUT_KEYBOARD,
             Anonymous: INPUT_0 {
                 ki: KEYBDINPUT {
-                    wVk: windows::Win32::UI::Input::KeyboardAndMouse::VIRTUAL_KEY(vk_code),
-                    wScan: scan_code,
+                    wVk: windows::Win32::UI::Input::KeyboardAndMouse::VIRTUAL_KEY(0),
+                    wScan: w_scan,
                     dwFlags: flags,
                     time: 0,
                     dwExtraInfo: 0,
@@ -132,21 +240,35 @@ impl KeyboardController {
 
     /// Send Secure Attention Sequence (Ctrl+Alt+Delete)
     ///
-    /// This uses a multi-tier approach:
-    /// 1. Try the GuruConnect SAS Service (runs as SYSTEM, handles via named pipe)
-    /// 2. Try the sas.dll directly (requires SYSTEM privileges)
-    /// 3. Fallback to key simulation (won't work on secure desktop)
+    /// Ctrl+Alt+Del is the Secure Attention Sequence and **cannot** be injected via
+    /// `SendInput` — Windows reserves it. It must be raised by `SendSAS`, which only
+    /// works when the caller runs as SYSTEM (or has SeTcbPrivilege) AND the
+    /// `SoftwareSASGeneration` Winlogon policy permits software-generated SAS. The
+    /// managed installer is responsible for installing the SAS helper service (running
+    /// as SYSTEM) and setting that policy. See `set_software_sas_policy` in
+    /// `bin/sas_service.rs` and the `// TODO(installer)` note there.
+    ///
+    /// Tiers, in order:
+    /// 1. The GuruConnect SAS helper service (SYSTEM) via named-pipe IPC — the supported path.
+    /// 2. Direct `sas.dll!SendSAS` — only succeeds if THIS process is already SYSTEM with the policy.
+    /// 3. Fallback key simulation — will NOT reach the secure desktop; logged as a clear failure.
     #[cfg(windows)]
     pub fn send_sas(&mut self) -> Result<()> {
         // Tier 1: Try the SAS service (named pipe IPC to SYSTEM service)
-        if let Ok(()) = crate::sas_client::request_sas() {
-            tracing::info!("SAS sent via GuruConnect SAS Service");
-            return Ok(());
+        match crate::sas_client::request_sas() {
+            Ok(()) => {
+                tracing::info!("SAS sent via GuruConnect SAS Service");
+                return Ok(());
+            }
+            Err(e) => {
+                tracing::warn!(
+                    "SAS helper service unavailable ({}); trying direct sas.dll",
+                    e
+                );
+            }
         }
 
-        tracing::info!("SAS service not available, trying direct sas.dll...");
-
-        // Tier 2: Try using the sas.dll directly (requires SYSTEM privileges)
+        // Tier 2: Try using the sas.dll directly (requires SYSTEM + SoftwareSASGeneration)
         use windows::core::PCWSTR;
         use windows::Win32::System::LibraryLoader::{GetProcAddress, LoadLibraryW};
 
@@ -157,49 +279,33 @@ impl KeyboardController {
             if let Ok(lib) = lib {
                 let proc_name = b"SendSAS\0";
                 if let Some(proc) = GetProcAddress(lib, windows::core::PCSTR(proc_name.as_ptr())) {
-                    // SendSAS takes a BOOL parameter: FALSE for Ctrl+Alt+Del
+                    // SendSAS takes a BOOL parameter: FALSE for Ctrl+Alt+Del.
+                    // It silently no-ops if the caller lacks privilege / the policy is
+                    // unset, so we cannot detect success here — but it is the best
+                    // effort short of the SYSTEM helper.
                     let send_sas: extern "system" fn(i32) = std::mem::transmute(proc);
                     send_sas(0); // FALSE = Ctrl+Alt+Del
-                    tracing::info!("SAS sent via direct sas.dll call");
+                    tracing::info!("SAS attempted via direct sas.dll call (effective only if SYSTEM + SoftwareSASGeneration policy set)");
                     return Ok(());
                 }
             }
         }
 
-        // Tier 3: Fallback - try sending the keys (won't work on secure desktop)
-        tracing::warn!("SAS service and sas.dll not available, Ctrl+Alt+Del may not work");
-
-        // VK codes
-        const VK_CONTROL: u16 = 0x11;
-        const VK_MENU: u16 = 0x12; // Alt
-        const VK_DELETE: u16 = 0x2E;
-
-        // Press keys
-        self.key_down(VK_CONTROL)?;
-        self.key_down(VK_MENU)?;
-        self.key_down(VK_DELETE)?;
-
-        // Release keys
-        self.key_up(VK_DELETE)?;
-        self.key_up(VK_MENU)?;
-        self.key_up(VK_CONTROL)?;
-
-        Ok(())
+        // Tier 3: SAS could not be delivered through any privileged path. A plain
+        // SendInput of Ctrl+Alt+Del never reaches the secure desktop, so report a
+        // clear, actionable error instead of pretending it worked.
+        let msg = "Ctrl+Alt+Del could not be delivered: the GuruConnect SAS helper \
+                   service is not running and sas.dll!SendSAS is unavailable. Ensure the \
+                   SAS service is installed (runs as SYSTEM) and the SoftwareSASGeneration \
+                   policy is enabled by the installer.";
+        tracing::error!("{}", msg);
+        anyhow::bail!("{}", msg)
     }
 
     /// Check if a virtual key code is an extended key
     #[cfg(windows)]
     fn is_extended_key(vk: u16) -> bool {
-        matches!(
-            vk,
-            0x21..=0x28 | // Page Up, Page Down, End, Home, Arrow keys
-            0x2D | 0x2E | // Insert, Delete
-            0x5B | 0x5C | // Left/Right Windows keys
-            0x5D |        // Applications key
-            0x6F |        // Numpad Divide
-            0x90 |        // Num Lock
-            0x91 // Scroll Lock
-        )
+        vk_is_extended(vk)
     }
 
     /// Send input events
@@ -224,6 +330,22 @@ impl KeyboardController {
         anyhow::bail!("Keyboard input only supported on Windows")
     }
 
+    #[cfg(not(windows))]
+    pub fn key_event_full(
+        &mut self,
+        _vk_code: u16,
+        _scan_code: u16,
+        _is_extended: bool,
+        _down: bool,
+    ) -> Result<()> {
+        anyhow::bail!("Keyboard input only supported on Windows")
+    }
+
+    #[cfg(not(windows))]
+    pub fn release_all_modifiers(&mut self) -> Result<()> {
+        anyhow::bail!("Keyboard input only supported on Windows")
+    }
+
     #[cfg(not(windows))]
     pub fn type_char(&mut self, _ch: char) -> Result<()> {
         anyhow::bail!("Keyboard input only supported on Windows")
@@ -293,3 +415,121 @@ pub mod vk {
     pub const LMENU: u16 = 0xA4; // Left Alt
     pub const RMENU: u16 = 0xA5; // Right Alt
 }
+
+/// Whether a Windows virtual-key code is an "extended" key.
+///
+/// Extended keys must be injected with `KEYEVENTF_EXTENDEDKEY`. This is the
+/// platform-independent classifier so the determination can be unit-tested off-Windows;
+/// the `#[cfg(windows)]` injection path delegates here. The viewer-captured
+/// `LLKHF_EXTENDED` flag is authoritative when present; this is the fallback used when
+/// the wire frame did not carry it (older viewers / VK-only synthesis).
+pub fn vk_is_extended(vk: u16) -> bool {
+    matches!(
+        vk,
+        0x21..=0x28 | // Page Up, Page Down, End, Home, Arrow keys
+        0x2D | 0x2E | // Insert, Delete
+        0x5B | 0x5C | // Left/Right Windows keys
+        0x5D |        // Applications key
+        0x6F |        // Numpad Divide
+        0x90 |        // Num Lock
+        0x91 |        // Scroll Lock
+        0xA3 |        // Right Control
+        0xA5 // Right Alt (AltGr)
+    )
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn extended_keys_are_flagged() {
+        // Arrows / navigation block.
+        for vk in [0x21u16, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28] {
+            assert!(vk_is_extended(vk), "vk={:#x} should be extended", vk);
+        }
+        // Insert / Delete.
+        assert!(vk_is_extended(0x2D));
+        assert!(vk_is_extended(0x2E));
+        // Win keys, Apps, NumLock, numpad Divide.
+        assert!(vk_is_extended(0x5B));
+        assert!(vk_is_extended(0x5C));
+        assert!(vk_is_extended(0x5D));
+        assert!(vk_is_extended(0x6F));
+        assert!(vk_is_extended(0x90));
+        // Right Ctrl / Right Alt.
+        assert!(vk_is_extended(0xA3));
+        assert!(vk_is_extended(0xA5));
+    }
+
+    #[test]
+    fn non_extended_keys_are_not_flagged() {
+        // Letters, digits, space, enter, left modifiers, numpad digits.
+        for vk in [
+            0x41u16, // A
+            0x5A,    // Z
+            0x30,    // 0
+            0x20,    // Space
+            0x0D,    // Enter
+            0xA0,    // Left Shift
+            0xA2,    // Left Control
+            0xA4,    // Left Alt
+            0x60,    // Numpad 0
+            0x6A,    // Numpad Multiply (NOT extended; only Divide is)
+        ] {
+            assert!(!vk_is_extended(vk), "vk={:#x} should NOT be extended", vk);
+        }
+    }
+
+    #[test]
+    fn modifier_state_records_ctrl_alt_shift_win() {
+        let mut m = ModifierState::default();
+        // Each of the VK aliases maps to its modifier flag.
+        assert!(m.record(0x11, true)); // VK_CONTROL
+        assert!(m.ctrl);
+        assert!(m.record(0xA4, true)); // VK_LMENU (Alt)
+        assert!(m.alt);
+        assert!(m.record(0xA0, true)); // VK_LSHIFT
+        assert!(m.shift);
+        assert!(m.record(0x5C, true)); // VK_RWIN
+        assert!(m.meta);
+    }
+
+    #[test]
+    fn modifier_state_ignores_non_modifiers() {
+        let mut m = ModifierState::default();
+        assert!(!m.record(0x41, true)); // 'A' is not a modifier
+        assert!(!m.ctrl && !m.alt && !m.shift && !m.meta);
+    }
+
+    #[test]
+    fn modifier_state_tracks_down_then_up() {
+        let mut m = ModifierState::default();
+        m.record(0x11, true); // Ctrl down
+        assert!(m.ctrl);
+        m.record(0x11, false); // Ctrl up
+        assert!(!m.ctrl);
+    }
+
+    #[test]
+    fn drain_held_returns_and_clears_held_modifiers() {
+        let mut m = ModifierState::default();
+        m.record(0xA2, true); // Left Ctrl -> ctrl
+        m.record(0x12, true); // Alt
+                              // Shift and Win were never pressed.
+        let mut held = m.drain_held();
+        held.sort_unstable();
+        // Canonical VKs returned: Ctrl(0x11), Alt(0x12).
+        assert_eq!(held, vec![0x11u16, 0x12]);
+        // State is cleared after draining.
+        assert!(!m.ctrl && !m.alt && !m.shift && !m.meta);
+        // A second drain yields nothing.
+        assert!(m.drain_held().is_empty());
+    }
+
+    #[test]
+    fn drain_held_empty_when_nothing_pressed() {
+        let mut m = ModifierState::default();
+        assert!(m.drain_held().is_empty());
+    }
+}