diff --git a/agent/src/session/mod.rs b/agent/src/session/mod.rs index 5d749f6..a2f7587 100644 --- a/agent/src/session/mod.rs +++ b/agent/src/session/mod.rs @@ -555,8 +555,14 @@ impl SessionManager { access ); - // The MessageBox blocks the calling thread; run it on the blocking pool - // so the agent's async loop is not stalled and heartbeats keep flowing. + // The MessageBox blocks the calling thread, so it runs on the blocking + // 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| { diff --git a/server/src/auth/mod.rs b/server/src/auth/mod.rs index 950cfb3..da9ab96 100644 --- a/server/src/auth/mod.rs +++ b/server/src/auth/mod.rs @@ -58,30 +58,6 @@ impl From 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, -} - -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 FromRequestParts 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 { - // 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(), - }) -} diff --git a/server/src/main.rs b/server/src/main.rs index c973b6b..94d0981 100644 --- a/server/src/main.rs +++ b/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( )) } } - diff --git a/server/src/relay/mod.rs b/server/src/relay/mod.rs index 7a49849..68e8a13 100644 --- a/server/src/relay/mod.rs +++ b/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"); } } diff --git a/server/src/utils/ip_extract.rs b/server/src/utils/ip_extract.rs index a04ed8e..5f6b097 100644 --- a/server/src/utils/ip_extract.rs +++ b/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; }