fix(server,agent): apply Tasks 3-5 review fixes (non-blocking)

From the secure-session-core Tasks 3-5 code review (APPROVE-WITH-FIXES):
- MEDIUM-2: delete the dead `validate_agent_key` "accept-any-key" placeholder +
  its AuthenticatedAgent/AuthState scaffolding (zero callers; the real agent
  auth is validate_agent_api_key + per-agent cak_ keys). Removes an auth landmine.
- LOW-3: stop interpolating support-code values into 3 relay log lines (bearer
  credentials).
- LOW-1: document the X-Real-IP trust requirement in ip_extract.rs (NPM must set
  it from $remote_addr); behavior unchanged.
- LOW-2: correct the consent/heartbeat comment in agent session loop (the loop
  awaits the dialog; safe because CONSENT_TIMEOUT 60s < HEARTBEAT_TIMEOUT 90s).
cargo fmt/clippy(-D warnings)/test all green on GURU-5070 (89 tests, 0 warnings).
MEDIUM-1 (viewer-token logout revocation) remains a tracked follow-up.

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

agent/src/session/mod.rs

@@ -555,8 +555,14 @@ impl SessionManager {
             access
         );
 
-        // The MessageBox blocks the calling thread; run it on the blocking pool
+        // The MessageBox blocks the calling thread, so it runs on the blocking
-        // so the agent's async loop is not stalled and heartbeats keep flowing.
+        // pool to avoid stalling the tokio runtime. Note, however, that the main
+        // session loop `.await`s this method (see the ConsentRequest arm), so
+        // the loop is SUSPENDED for the user's entire think-time and does NOT
+        // process or respond to server heartbeats while the dialog is open.
+        // This is safe because CONSENT_TIMEOUT_SECS (60s, server-side) is within
+        // the server's 90s HEARTBEAT_TIMEOUT_SECS: the prompt resolves before the
+        // server would consider the agent dead, so the session is not torn down.
         let granted = tokio::task::spawn_blocking(move || prompt_consent(&technician_name, access))
             .await
             .unwrap_or_else(|e| {

server/src/auth/mod.rs

@@ -58,30 +58,6 @@ impl From<Claims> for AuthenticatedUser {
     }
 }
 
-/// Authenticated agent from API key
-#[derive(Debug, Clone)]
-#[allow(dead_code)] // TODO(native-remote-control): consumed by the integration API; see docs/specs/native-remote-control/
-pub struct AuthenticatedAgent {
-    pub agent_id: String,
-    pub org_id: String,
-}
-
-/// JWT configuration stored in app state
-#[derive(Clone)]
-#[allow(dead_code)] // TODO(native-remote-control): consumed by the integration API; see docs/specs/native-remote-control/
-pub struct AuthState {
-    pub jwt_config: Arc<JwtConfig>,
-}
-
-impl AuthState {
-    #[allow(dead_code)] // TODO(native-remote-control): consumed by the integration API; see docs/specs/native-remote-control/
-    pub fn new(jwt_secret: String, expiry_hours: i64) -> Self {
-        Self {
-            jwt_config: Arc::new(JwtConfig::new(jwt_secret, expiry_hours)),
-        }
-    }
-}
-
 /// Extract authenticated user from request
 #[axum::async_trait]
 impl<S> FromRequestParts<S> for AuthenticatedUser
@@ -169,14 +145,3 @@ where
         }
     }
 }
-
-/// Validate an agent API key (placeholder for MVP)
-#[allow(dead_code)] // TODO(native-remote-control): consumed by the integration API; see docs/specs/native-remote-control/
-pub fn validate_agent_key(_api_key: &str) -> Option<AuthenticatedAgent> {
-    // TODO: Implement actual API key validation against database
-    // For now, accept any key for agent connections
-    Some(AuthenticatedAgent {
-        agent_id: "mvp-agent".to_string(),
-        org_id: "mvp-org".to_string(),
-    })
-}

server/src/main.rs

@@ -443,7 +443,10 @@ async fn main() -> Result<()> {
         // fallback below — CLAUDE.md documents this as the public download URL.
         // `nest_service` is matched BEFORE `fallback_service`, so these binaries
         // are served from disk and never fall through to the SPA index.html.
-        .nest_service("/downloads", ServeDir::new(format!("{STATIC_DIR}/downloads")))
+        .nest_service(
+            "/downloads",
+            ServeDir::new(format!("{STATIC_DIR}/downloads")),
+        )
         // NOTE: there are intentionally no /login, /dashboard, /users routes.
         // The v2 SPA (BrowserRouter) owns those paths and resolves them via the
         // fallback_service below; registering server-side handlers for them would
@@ -909,4 +912,3 @@ async fn trigger_machine_update(
         ))
     }
 }
-

server/src/relay/mod.rs

@@ -561,7 +561,7 @@ async fn handle_agent_connection(
     if let Some(ref code) = support_code {
         // Check if the code is cancelled or invalid
         if support_codes.is_cancelled(code).await {
-            warn!("Agent tried to connect with cancelled code: {}", code);
+            warn!("Agent tried to connect with a cancelled support code");
             // Send disconnect message to agent
             let disconnect_msg = proto::Message {
                 payload: Some(proto::message::Payload::Disconnect(proto::Disconnect {
@@ -740,7 +740,7 @@ async fn handle_agent_connection(
             interval.tick().await;
             if let Some(ref code) = support_code_check {
                 if support_codes_check.is_cancelled(code).await {
-                    info!("Support code {} was cancelled, disconnecting agent", code);
+                    info!("Support code was cancelled, disconnecting agent");
                     // Send disconnect message
                     let disconnect_msg = proto::Message {
                         payload: Some(proto::message::Payload::Disconnect(proto::Disconnect {
@@ -917,7 +917,7 @@ async fn handle_agent_connection(
                 let _ = db::support_codes::mark_code_completed(db.pool(), code).await;
             }
 
-            info!("Support code {} marked as completed", code);
+            info!("Support code marked as completed");
         }
     }
 

server/src/utils/ip_extract.rs

@@ -154,6 +154,14 @@ pub fn client_ip(peer: &SocketAddr, headers: &HeaderMap, trusted: &TrustedProxie
     }
 
     // Trusted peer: prefer the single-value X-Real-IP if the proxy set it.
+    //
+    // SECURITY: we take X-Real-IP verbatim here, trusting it as set by the
+    // reverse proxy. The proxy (NPM) MUST overwrite it from the real TCP peer:
+    //   proxy_set_header X-Real-IP $remote_addr;
+    // It must NOT pass through a client-supplied X-Real-IP. A trusted peer that
+    // forwards an attacker-controlled value would let the client spoof the IP
+    // used for rate-limiting and audit logging. The trusted-proxy gate above
+    // only authenticates the immediate hop, not the contents of this header.
     if let Some(ip) = header_single_ip(headers, X_REAL_IP) {
         return ip;
     }