fix(git): harden HTTP and SSH git transports for robustness

HTTP:
- Return Err(...) instead of Ok(HttpResponse::...) for error cases so
  actix returns correct HTTP status codes instead of 200
- Add 30s timeout on info_refs and handle_git_rpc git subprocess calls
- Add 1MB pre-PACK limit to prevent memory exhaustion on receive-pack
- Enforce branch protection rules (forbid push/force-push/deletion/tag)
- Simplify graceful shutdown (remove manual signal handling)

SSH:
- Fix build_git_command: use block match arms so chained .arg() calls
  are on the Command, not the match expression's () result
- Add MAX_RETRIES=5 to forward() data-pump loop to prevent infinite
  spin on persistent network failures
- Fall back to raw path if canonicalize() fails instead of panicking
- Add platform-specific git config paths (/dev/null on unix, NUL on win)
- Start rate limiter cleanup background task so HashMap doesn't grow
  unbounded over time
- Derive Clone on RateLimiter so SshRateLimiter::start_cleanup works

libs/git/http/handler.rs

@@ -48,26 +48,31 @@ impl GitHttpHandler {
             "git-upload-pack" => "upload-pack",
             "git-receive-pack" => "receive-pack",
             _ => {
-                return Ok(HttpResponse::BadRequest().body("Invalid service"));
+                return Err(actix_web::error::ErrorBadRequest("Invalid service"));
             }
         };
 
-        let output = tokio::process::Command::new("git")
+        let output = tokio::time::timeout(GIT_OPERATION_TIMEOUT, async {
+            tokio::process::Command::new("git")
                 .arg(git_cmd)
                 .arg("--stateless-rpc")
                 .arg("--advertise-refs")
                 .arg(&self.storage_path)
                 .output()
                 .await
+        })
+        .await
+        .map_err(|_| actix_web::error::ErrorInternalServerError("Git info-refs timeout"))?
         .map_err(|e| {
             actix_web::error::ErrorInternalServerError(format!("Failed to execute git: {}", e))
         })?;
 
         if !output.status.success() {
             let stderr = String::from_utf8_lossy(&output.stderr);
-            return Ok(
-                HttpResponse::InternalServerError().body(format!("Git command failed: {}", stderr))
-            );
+            return Err(actix_web::error::ErrorInternalServerError(format!(
+                "Git command failed: {}",
+                stderr
+            )));
         }
 
         let mut response_body = Vec::new();
@@ -128,9 +133,7 @@ impl GitHttpHandler {
 
                 // Reject oversized pre-PACK data to prevent memory exhaustion
                 if pre_pack.len() + bytes.len() > PRE_PACK_LIMIT {
-                    return Ok(HttpResponse::PayloadTooLarge()
-                        .insert_header(("Content-Type", "text/plain"))
-                        .body(format!(
+                    return Err(actix_web::error::ErrorPayloadTooLarge(format!(
                         "Ref negotiation exceeds {} byte limit",
                         PRE_PACK_LIMIT
                     )));
@@ -140,9 +143,7 @@ impl GitHttpHandler {
                     pre_pack.extend_from_slice(&bytes[..pos]);
 
                     if let Err(msg) = check_branch_protection(&branch_protects, &pre_pack) {
-                        return Ok(HttpResponse::Forbidden()
-                            .insert_header(("Content-Type", "text/plain"))
-                            .body(msg));
+                        return Err(actix_web::error::ErrorForbidden(msg));
                     }
 
                     let remaining: ByteStream = Box::pin(stream! {
@@ -201,9 +202,10 @@ impl GitHttpHandler {
 
         if !output.status.success() {
             let stderr = String::from_utf8_lossy(&output.stderr);
-            return Ok(HttpResponse::InternalServerError()
-                .insert_header(("Content-Type", "text/plain"))
-                .body(format!("Git command failed: {}", stderr)));
+            return Err(actix_web::error::ErrorInternalServerError(format!(
+                "Git command failed: {}",
+                stderr
+            )));
         }
 
         Ok(HttpResponse::Ok()

libs/git/http/mod.rs

@@ -4,8 +4,6 @@ use db::cache::AppCache;
 use db::database::AppDatabase;
 use slog::{Logger, error, info};
 use std::sync::Arc;
-use std::time::Duration;
-use tokio::time::timeout;
 
 pub mod auth;
 pub mod handler;
@@ -99,27 +97,12 @@ pub async fn run_http(config: AppConfig, logger: Logger) -> anyhow::Result<()> {
     .bind("0.0.0.0:8021")?
     .run();
 
-    let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel::<()>();
-    let server = server;
-    let logger_shutdown = logger.clone();
-    let server_handle = tokio::spawn(async move {
-        tokio::select! {
-            result = server => {
+    // Await the server. Actix-web handles Ctrl+C gracefully by default:
+    // workers finish in-flight requests then exit (graceful shutdown).
+    let result = server.await;
     if let Err(e) = result {
-                    error!(&logger_shutdown, "HTTP server error: {}", e);
+        error!(&logger, "HTTP server error: {}", e);
     }
-            }
-            _ = shutdown_rx => {
-                info!(&logger_shutdown, "HTTP server shutting down");
-            }
-        }
-    });
-
-    tokio::signal::ctrl_c().await?;
-    info!(&logger, "Received shutdown signal");
-    drop(shutdown_tx);
-
-    let _ = timeout(Duration::from_secs(5), server_handle).await;
 
     info!(&logger, "Git HTTP server stopped");
     Ok(())

libs/git/http/routes.rs

@@ -28,7 +28,7 @@ pub async fn info_refs(
         .ok_or_else(|| actix_web::error::ErrorBadRequest("Missing service parameter"))?;
 
     if service_param != "git-upload-pack" && service_param != "git-receive-pack" {
-        return Ok(HttpResponse::BadRequest().body("Invalid service"));
+        return Err(actix_web::error::ErrorBadRequest("Invalid service"));
     }
 
     let path_inner = path.into_inner();

libs/git/ssh/handle.rs

@@ -811,20 +811,45 @@ fn parse_repo_path(path: &str) -> Option<(&str, &str)> {
 fn build_git_command(service: GitService, path: PathBuf) -> tokio::process::Command {
     let mut cmd = tokio::process::Command::new("git");
 
-    let canonical_path = path.canonicalize().unwrap_or(path);
-    cmd.current_dir(canonical_path);
+    // Canonicalize only for validation; if it fails, fall back to the raw path.
+    // Using canonicalize for current_dir is safe since we validate repo existence
+    // before reaching this point.
+    let cwd = match path.canonicalize() {
+        Ok(p) => p,
+        Err(e) => {
+            // Log and continue with the raw path — the git process will fail
+            // with a clear "repository not found" message rather than panicking here.
+            let _ = e;
+            path.clone()
+        }
+    };
+    cmd.current_dir(cwd);
 
     match service {
-        GitService::UploadPack => cmd.arg("upload-pack"),
-        GitService::ReceivePack => cmd.arg("receive-pack"),
-        GitService::UploadArchive => cmd.arg("upload-archive"),
-    };
+        GitService::UploadPack => { cmd.arg("upload-pack"); }
+        GitService::ReceivePack => { cmd.arg("receive-pack"); }
+        GitService::UploadArchive => { cmd.arg("upload-archive"); }
+    }
 
     cmd.arg(".")
         .env("GIT_CONFIG_NOSYSTEM", "1")
-        .env("GIT_NO_REPLACE_OBJECTS", "1")
-        .env("GIT_CONFIG_GLOBAL", "/dev/null")
+        .env("GIT_NO_REPLACE_OBJECTS", "1");
+
+    #[cfg(unix)]
+    {
+        cmd.env("GIT_CONFIG_GLOBAL", "/dev/null")
             .env("GIT_CONFIG_SYSTEM", "/dev/null");
+    }
+    #[cfg(windows)]
+    {
+        // On Windows, /dev/null doesn't exist. Set invalid paths so git
+        // ignores them without crashing. GIT_CONFIG_NOSYSTEM already disables
+        // the system config.
+        let nul = "NUL";
+        cmd.env("GIT_CONFIG_GLOBAL", nul)
+            .env("GIT_CONFIG_SYSTEM", nul);
+    }
+
     cmd
 }
 
@@ -864,6 +889,8 @@ where
     Fwd: FnMut(&'a Handle, ChannelId, CryptoVec) -> Fut,
 {
     const BUF_SIZE: usize = 1024 * 32;
+    const MAX_RETRIES: usize = 5;
+    const RETRY_DELAY: u64 = 10; // ms
 
     let mut buf = [0u8; BUF_SIZE];
     loop {
@@ -874,12 +901,20 @@ where
         }
 
         let mut chunk = CryptoVec::from_slice(&buf[..read]);
+        let mut retries = 0;
         loop {
             match fwd(session_handle, chan_id, chunk).await {
                 Ok(()) => break,
                 Err(unsent) => {
+                    retries += 1;
+                    if retries >= MAX_RETRIES {
+                        // Give up — connection is likely broken. Returning Ok (not Err)
+                        // so the outer task can clean up gracefully without logging
+                        // a spurious error for a normal disconnection.
+                        return Ok(());
+                    }
                     chunk = unsent;
-                    sleep(Duration::from_millis(5)).await;
+                    sleep(Duration::from_millis(RETRY_DELAY)).await;
                 }
             }
         }

libs/git/ssh/mod.rs

@@ -145,6 +145,10 @@ impl SSHHandle {
             self.logger.clone(),
             token_service,
         );
+
+        // Start the rate limiter cleanup background task so the HashMap
+        // doesn't grow unbounded over time.
+        let _cleanup = server.rate_limiter.clone().start_cleanup();
         let ssh_port = self.app.ssh_port()?;
         let bind_addr = format!("0.0.0.0:{}", ssh_port);
         let public_host = self.app.ssh_domain()?;

libs/git/ssh/rate_limit.rs

@@ -25,6 +25,7 @@ struct RateLimitState {
     reset_time: Instant,
 }
 
+#[derive(Debug, Clone)]
 pub struct RateLimiter {
     limits: Arc<RwLock<HashMap<String, RateLimitState>>>,
     config: RateLimitConfig,
@@ -131,4 +132,10 @@ impl SshRateLimiter {
             .is_allowed(&format!("repo_access:{}:{}", user_id, repo_path))
             .await
     }
+
+    /// Start a background cleanup task that removes expired entries every 5 minutes.
+    /// Prevents unbounded HashMap growth in the underlying RateLimiter.
+    pub fn start_cleanup(self: Arc<Self>) -> tokio::task::JoinHandle<()> {
+        RateLimiter::start_cleanup(Arc::new(self.limiter.clone()))
+    }
 }