diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index 31abfc0..294921c 100644 --- a/crates/server/src/main.rs +++ b/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}"); diff --git a/crates/server/src/routes/cache.rs b/crates/server/src/routes/cache.rs index ce63418..fea7428 100644 --- a/crates/server/src/routes/cache.rs +++ b/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", - store_path = store_path, - hash = hash, - file_hash = file_hash, - nar_size = nar_size, - nar_hash = nar_hash, - refs = refs.join(" "), + {nar_hash}\nNarSize: {nar_size}\nReferences: {refs_joined}\n", ); 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 { diff --git a/crates/server/src/routes/dashboard.rs b/crates/server/src/routes/dashboard.rs index d834a6b..0882311 100644 --- a/crates/server/src/routes/dashboard.rs +++ b/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, @@ -1201,7 +1201,7 @@ async fn admin_page( ) } -// --- Setup Wizard --- +// Setup wizard async fn project_setup_page(extensions: Extensions) -> Html { let tmpl = ProjectSetupTemplate { @@ -1215,7 +1215,7 @@ async fn project_setup_page(extensions: Extensions) -> Html { ) } -// --- Login / Logout --- +// Login / Logout async fn login_page() -> Html { 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)], diff --git a/crates/server/src/state.rs b/crates/server/src/state.rs index b47de28..ea2836d 100644 --- a/crates/server/src/state.rs +++ b/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>, 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" + ); + } + } + }); + } +}