fc-server: session cleanup; conditional Secure cookie; kill_on_drop on NAR processes

Signed-off-by: NotAShelf <raf@notashelf.dev>
Change-Id: Ie7b9002bbdc0ad91bb041f89979881956a6a6964

crates/server/src/main.rs

@@ -63,6 +63,9 @@ async fn main() -> anyhow::Result<()> {
     http_client: reqwest::Client::new(),
   };
 
+  // Start background session cleanup to prevent memory leaks
+  state.spawn_session_cleanup();
+
   let app = routes::router(state, &config.server);
 
   let bind_addr = format!("{host}:{port}");

crates/server/src/routes/cache.rs

@@ -132,23 +132,20 @@ async fn narinfo(
 
   let file_hash = nar_hash;
 
+  use std::fmt::Write;
+
+  let refs_joined = refs.join(" ");
   let mut narinfo_text = format!(
     "StorePath: {store_path}\nURL: nar/{hash}.nar.zst\nCompression: \
      zstd\nFileHash: {file_hash}\nFileSize: {nar_size}\nNarHash: \
-     {nar_hash}\nNarSize: {nar_size}\nReferences: {refs}\n",
+     {nar_hash}\nNarSize: {nar_size}\nReferences: {refs_joined}\n",
-    store_path = store_path,
-    hash = hash,
-    file_hash = file_hash,
-    nar_size = nar_size,
-    nar_hash = nar_hash,
-    refs = refs.join(" "),
   );
 
   if let Some(deriver) = deriver {
-    narinfo_text.push_str(&format!("Deriver: {deriver}\n"));
+    let _ = write!(narinfo_text, "Deriver: {deriver}\n");
   }
   if let Some(ca) = ca {
-    narinfo_text.push_str(&format!("CA: {ca}\n"));
+    let _ = write!(narinfo_text, "CA: {ca}\n");
   }
 
   // Optionally sign if secret key is configured
@@ -248,7 +245,10 @@ async fn serve_nar_zst(
     _ => return Ok(StatusCode::NOT_FOUND.into_response()),
   };
 
-  // Use two piped processes instead of sh -c to prevent command injection
+  // Use two piped processes instead of sh -c to prevent command injection.
+  // nix uses std::process (sync) for piping stdout to zstd stdin.
+  // zstd uses tokio::process with kill_on_drop(true) to ensure cleanup
+  // if the client disconnects.
   let mut nix_child = std::process::Command::new("nix")
     .args(["store", "dump-path", &store_path])
     .stdout(std::process::Stdio::piped())
@@ -270,6 +270,7 @@ async fn serve_nar_zst(
     .stdin(nix_stdout)
     .stdout(std::process::Stdio::piped())
     .stderr(std::process::Stdio::null())
+    .kill_on_drop(true)
     .spawn()
     .map_err(|_| {
       ApiError(fc_common::CiError::Build(
@@ -316,6 +317,7 @@ async fn serve_nar(
     .args(["store", "dump-path", &store_path])
     .stdout(std::process::Stdio::piped())
     .stderr(std::process::Stdio::null())
+    .kill_on_drop(true)
     .spawn();
 
   let mut child = match child {

crates/server/src/routes/dashboard.rs

@@ -25,7 +25,7 @@ use uuid::Uuid;
 
 use crate::state::AppState;
 
-// --- View models (pre-formatted for templates) ---
+// View models (pre-formatted for templates)
 
 struct BuildView {
   id:            Uuid,
@@ -237,7 +237,7 @@ fn auth_name(extensions: &Extensions) -> String {
     .unwrap_or_default()
 }
 
-// --- Templates ---
+// Askama templates
 
 #[derive(Template)]
 #[template(path = "home.html")]
@@ -430,7 +430,7 @@ struct MetricsTemplate {
   auth_name: String,
 }
 
-// --- Handlers ---
+// Route handlers
 
 async fn home(
   State(state): State<AppState>,
@@ -1201,7 +1201,7 @@ async fn admin_page(
   )
 }
 
-// --- Setup Wizard ---
+// Setup wizard
 
 async fn project_setup_page(extensions: Extensions) -> Html<String> {
   let tmpl = ProjectSetupTemplate {
@@ -1215,7 +1215,7 @@ async fn project_setup_page(extensions: Extensions) -> Html<String> {
   )
 }
 
-// --- Login / Logout ---
+// Login / Logout
 
 async fn login_page() -> Html<String> {
   let tmpl = LoginTemplate { error: None };
@@ -1258,9 +1258,17 @@ async fn login_action(
           created_at: std::time::Instant::now(),
         });
 
+      let secure_flag = if !state.config.server.cors_permissive
+        && state.config.server.host != "127.0.0.1"
+        && state.config.server.host != "localhost"
+      {
+        "; Secure"
+      } else {
+        ""
+      };
       let cookie = format!(
         "fc_user_session={session_id}; HttpOnly; SameSite=Strict; Path=/; \
-         Max-Age=86400"
+         Max-Age=86400{secure_flag}"
       );
       return (
         [(axum::http::header::SET_COOKIE, cookie)],
@@ -1314,9 +1322,17 @@ async fn login_action(
           created_at: std::time::Instant::now(),
         });
 
+      let secure_flag = if !state.config.server.cors_permissive
+        && state.config.server.host != "127.0.0.1"
+        && state.config.server.host != "localhost"
+      {
+        "; Secure"
+      } else {
+        ""
+      };
       let cookie = format!(
         "fc_session={session_id}; HttpOnly; SameSite=Strict; Path=/; \
-         Max-Age=86400"
+         Max-Age=86400{secure_flag}"
       );
       (
         [(axum::http::header::SET_COOKIE, cookie)],

crates/server/src/state.rs

@@ -7,6 +7,14 @@ use fc_common::{
 };
 use sqlx::PgPool;
 
+/// Maximum session lifetime before automatic eviction (24 hours).
+const SESSION_MAX_AGE: std::time::Duration =
+  std::time::Duration::from_secs(24 * 60 * 60);
+
+/// How often the background cleanup task runs (every 5 minutes).
+const SESSION_CLEANUP_INTERVAL: std::time::Duration =
+  std::time::Duration::from_secs(5 * 60);
+
 /// Session data supporting both API key and user authentication
 #[derive(Clone)]
 pub struct SessionData {
@@ -69,3 +77,27 @@ pub struct AppState {
   pub sessions:    Arc<DashMap<String, SessionData>>,
   pub http_client: reqwest::Client,
 }
+
+impl AppState {
+  /// Spawn a background task that periodically evicts expired sessions.
+  /// This prevents unbounded memory growth from the in-memory session store.
+  pub fn spawn_session_cleanup(&self) {
+    let sessions = self.sessions.clone();
+    tokio::spawn(async move {
+      loop {
+        tokio::time::sleep(SESSION_CLEANUP_INTERVAL).await;
+        let before = sessions.len();
+        sessions
+          .retain(|_, session| session.created_at.elapsed() < SESSION_MAX_AGE);
+        let evicted = before - sessions.len();
+        if evicted > 0 {
+          tracing::debug!(
+            evicted = evicted,
+            remaining = sessions.len(),
+            "Evicted expired sessions"
+          );
+        }
+      }
+    });
+  }
+}