fix(node,git): bound a hung served git with a total-duration timeout (#62)

.env.example

@@ -90,6 +90,12 @@ GITLAWB_PUBLIC_READ=true
 # Maximum git smart-HTTP pack request size, in bytes.
 GITLAWB_MAX_PACK_BYTES=2147483648
 
+# Max seconds a served git upload-pack / receive-pack (clone / push) may run
+# before it is aborted with a 504. Bounds a hung git that would otherwise pin a
+# worker and, on push, the repo write lock. Does NOT cover the info/refs
+# advertisement or the withheld-blob path, which remain unbounded. Default 600.
+GITLAWB_GIT_SERVICE_TIMEOUT_SECS=600
+
 # ── Push rate limiting (git-receive-pack flood brake) ─────────────────────
 # Max receive-pack requests (info/refs advertisement + push POST) per client
 # IP per hour. 0 disables. Default 600.

README.md

@@ -343,6 +343,7 @@ Important node settings:
 | `GITLAWB_REQUIRE_SIGNED_PEER_WRITES` | Require signed peer announce/sync writes. |
 | `GITLAWB_AUTO_SYNC` | Enable automatic sync from known peers. |
 | `GITLAWB_MAX_PACK_BYTES` | Max git pack body size for smart-HTTP routes. |
+| `GITLAWB_GIT_SERVICE_TIMEOUT_SECS` | Max seconds a served git upload-pack/receive-pack may run before it is aborted (504). Default 600. Does not bound `info/refs` or the withheld-blob path. |
 | `GITLAWB_TIGRIS_BUCKET` | Optional S3/Tigris shared repo storage bucket. |
 | `GITLAWB_PINATA_JWT` | Optional Pinata/IPFS warm-storage pinning. |
 | `GITLAWB_IRYS_URL` | Optional Irys/Arweave permanent anchoring. |

crates/gitlawb-node/src/api/repos.rs

@@ -578,6 +578,26 @@ pub async fn git_info_refs(
         })
 }
 
+/// Map an error from a `smart_http` git service call to the right `AppError`:
+/// [`smart_http::GitServiceTimeout`] to 504, a malformed client request to 400,
+/// anything else to a 500 git error. Pure (no logging) so it is unit-testable;
+/// callers add their own tracing.
+fn git_service_app_error(err: &anyhow::Error) -> AppError {
+    if err
+        .downcast_ref::<smart_http::GitServiceTimeout>()
+        .is_some()
+    {
+        AppError::Timeout("git service timed out".into())
+    } else {
+        let msg = err.to_string();
+        if msg.contains("bad line length") || msg.contains("protocol error") {
+            AppError::BadRequest(msg)
+        } else {
+            AppError::Git(msg)
+        }
+    }
+}
+
 /// POST /:owner/:repo.git/git-upload-pack
 pub async fn git_upload_pack(
     State(state): State<AppState>,
@@ -615,8 +635,9 @@ pub async fn git_upload_pack(
     // No path-scoped rule can withhold an individual blob, and the whole-repo
     // "/" gate above already enforced repo-level access. Skip the per-blob
     // withheld walk and serve the pack directly.
+    let git_timeout = std::time::Duration::from_secs(state.config.git_service_timeout_secs);
     let resp = if !visibility_pack::has_path_scoped_rule(&rules) {
-        smart_http::upload_pack(&disk_path, body).await
+        smart_http::upload_pack(&disk_path, body, git_timeout).await
     } else {
         // withheld_blob_oids walks every ref with blocking `git ls-tree`; keep
         // that off the async worker thread.
@@ -641,21 +662,22 @@ pub async fn git_upload_pack(
         };
 
         if withheld.is_empty() {
-            smart_http::upload_pack(&disk_path, body).await
+            smart_http::upload_pack(&disk_path, body, git_timeout).await
         } else {
             tracing::info!(repo = %name, caller = ?caller, withheld = withheld.len(), "serving filtered pack");
             smart_http::upload_pack_excluding(&disk_path, body, &withheld).await
         }
     }
     .map_err(|e| {
-        let msg = e.to_string();
-        if msg.contains("bad line length") || msg.contains("protocol error") {
-            tracing::warn!(repo = %name, err = %msg, "git-upload-pack: bad client request");
-            AppError::BadRequest(msg)
-        } else {
-            tracing::error!(repo = %name, err = %msg, "git-upload-pack failed");
-            AppError::Git(msg)
+        let app = git_service_app_error(&e);
+        match &app {
+            AppError::Timeout(_) => tracing::warn!(repo = %name, "git-upload-pack timed out"),
+            AppError::BadRequest(msg) => {
+                tracing::warn!(repo = %name, err = %msg, "git-upload-pack: bad client request")
+            }
+            _ => tracing::error!(repo = %name, err = %e, "git-upload-pack failed"),
         }
+        app
     })?;
     crate::metrics::record_fetch(&format!("{owner}/{name}"));
     crate::metrics::observe_pack_size(body_len as f64);
@@ -902,16 +924,24 @@ pub async fn git_receive_pack(
     let disk_path = guard.path().to_path_buf();
     tracing::debug!(repo = %name, path = %disk_path.display(), "running git receive-pack");
     let body_len = body.len();
-    let receive_result = smart_http::receive_pack(&disk_path, body).await;
+    let git_timeout = std::time::Duration::from_secs(state.config.git_service_timeout_secs);
+    let receive_result = smart_http::receive_pack(&disk_path, body, git_timeout).await;
 
     // Always release the advisory lock — even on error — to prevent stale locks
     // from blocking subsequent pushes. Only upload to Tigris when the push
     // succeeded; uploading a half-applied repo would propagate corruption.
     guard.release(receive_result.is_ok()).await;
 
     let result = receive_result.map_err(|e| {
-        tracing::error!(repo = %name, err = %e, "git receive-pack failed");
-        AppError::Git(e.to_string())
+        let app = git_service_app_error(&e);
+        match &app {
+            AppError::Timeout(_) => tracing::warn!(repo = %name, "git receive-pack timed out"),
+            AppError::BadRequest(msg) => {
+                tracing::warn!(repo = %name, err = %msg, "git receive-pack: bad client request")
+            }
+            _ => tracing::error!(repo = %name, err = %e, "git receive-pack failed"),
+        }
+        app
     })?;
 
     // Update the repo's updated_at timestamp after a successful push
@@ -1795,6 +1825,32 @@ mod tests {
     const OWNER_SHORT: &str = "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH";
     const STRANGER_DID: &str = "did:key:z6Mkffonly5tranger0000000000000000000000000000000";
 
+    #[test]
+    fn git_service_app_error_classifies_timeout_bad_request_and_git() {
+        // GitServiceTimeout carried through anyhow -> 504 Timeout.
+        let timeout_err: anyhow::Error = smart_http::GitServiceTimeout.into();
+        assert!(matches!(
+            git_service_app_error(&timeout_err),
+            AppError::Timeout(_)
+        ));
+        // A malformed client request -> 400.
+        let bad = anyhow::anyhow!("fatal: bad line length character: 0000");
+        assert!(matches!(
+            git_service_app_error(&bad),
+            AppError::BadRequest(_)
+        ));
+        // The `protocol error` marker (with no "bad line length" substring) also
+        // -> 400, exercising the second arm of the classifier independently.
+        let proto = anyhow::anyhow!("fatal: protocol error: unexpected flush packet");
+        assert!(matches!(
+            git_service_app_error(&proto),
+            AppError::BadRequest(_)
+        ));
+        // Anything else -> 500 git error.
+        let other = anyhow::anyhow!("some other git failure");
+        assert!(matches!(git_service_app_error(&other), AppError::Git(_)));
+    }
+
     fn repo_owned_by(owner_did: &str) -> crate::db::RepoRecord {
         let now = chrono::Utc::now();
         crate::db::RepoRecord {

crates/gitlawb-node/src/config.rs

@@ -166,6 +166,22 @@ pub struct Config {
     /// in flight and exits. Default: 30s.
     #[arg(long, env = "GITLAWB_SHUTDOWN_GRACE_SECS", default_value_t = 30)]
     pub shutdown_grace_secs: u64,
+
+    /// Maximum wall-clock time a single served git operation (upload-pack /
+    /// receive-pack through `run_git_service`) may run before it is aborted and
+    /// its process group torn down, in seconds. Bounds a git that neither
+    /// finishes nor disconnects. Must be positive; set it very large to
+    /// effectively disable the bound. Default: 600s (10 min), generous for large
+    /// clones. Does not cover the ref advertisement (`info/refs`) or the
+    /// withheld-blob fetch path (`upload_pack_excluding`, a blocking
+    /// `spawn_blocking` a tokio timeout cannot cancel); both remain unbounded.
+    #[arg(
+        long,
+        env = "GITLAWB_GIT_SERVICE_TIMEOUT_SECS",
+        default_value_t = 600,
+        value_parser = clap::value_parser!(u64).range(1..)
+    )]
+    pub git_service_timeout_secs: u64,
 }
 
 impl Config {
@@ -183,3 +199,25 @@ impl Config {
         PathBuf::from(&self.key_path)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn git_service_timeout_defaults_to_600_and_rejects_zero() {
+        assert_eq!(
+            Config::parse_from(["gitlawb-node"]).git_service_timeout_secs,
+            600
+        );
+        assert_eq!(
+            Config::parse_from(["gitlawb-node", "--git-service-timeout-secs", "30"])
+                .git_service_timeout_secs,
+            30
+        );
+        // 0 is a footgun (immediate-504 on every request); clap must reject it.
+        assert!(
+            Config::try_parse_from(["gitlawb-node", "--git-service-timeout-secs", "0"]).is_err()
+        );
+    }
+}

crates/gitlawb-node/src/error.rs

@@ -44,6 +44,9 @@ pub enum AppError {
     #[error("git error: {0}")]
     Git(String),
 
+    #[error("git service timed out: {0}")]
+    Timeout(String),
+
     #[error("database error: {0}")]
     Db(#[from] sqlx::Error),
 
@@ -105,6 +108,9 @@ impl IntoResponse for AppError {
                 (StatusCode::UNPROCESSABLE_ENTITY, "incomplete", msg.clone())
             }
             AppError::Git(msg) => (StatusCode::INTERNAL_SERVER_ERROR, "git_error", msg.clone()),
+            // 504, distinct from the 500 git_error and from the read-gate's 404 /
+            // the auth 401, so the client can tell a deadline from a failure.
+            AppError::Timeout(msg) => (StatusCode::GATEWAY_TIMEOUT, "git_timeout", msg.clone()),
             AppError::Db(e) => (StatusCode::INTERNAL_SERVER_ERROR, "db_error", e.to_string()),
             AppError::Internal(e) => (
                 StatusCode::INTERNAL_SERVER_ERROR,
@@ -123,3 +129,21 @@ impl IntoResponse for AppError {
 }
 
 pub type Result<T> = std::result::Result<T, AppError>;
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn timeout_maps_to_504_distinct_from_git_500() {
+        assert_eq!(
+            AppError::Timeout("x".into()).into_response().status(),
+            StatusCode::GATEWAY_TIMEOUT
+        );
+        // Guard against a swap with the generic git failure (500).
+        assert_eq!(
+            AppError::Git("x".into()).into_response().status(),
+            StatusCode::INTERNAL_SERVER_ERROR
+        );
+    }
+}