From 885c5990b164a3b545bb15287456766a4d6163ac Mon Sep 17 00:00:00 2001
From: derekbarbosa <derekasobrab@gmail.com>
Date: Mon, 6 Jul 2026 13:59:38 -0400
Subject: [PATCH 1/5] security: add input validation and body size limits for
 webhook endpoints

Add validation for untrusted input on HTTP ingestion endpoints:

- Explicit 2 MiB DefaultBodyLimit on the axum router
- Download size cap (10 MiB) and decompression limit (50 MiB) on
  lore.kernel.org mbox fetches to mitigate gzip bombs
- SHA format validation (40 or 64 char hex) in both forge
  parse_payload methods to reject malformed commit hashes
- Repository URL validation with SSRF blocklist (cloud metadata,
  loopback, localhost) in forge parse_payload
- PR/MR number must be positive in both forge providers
- Message-ID path separator sanitization in Thread submit handler

SHA and URL validation are applied only in forge parse_payload (webhook
path), not in submit_patch (CLI path), because the CLI legitimately
sends git refs (HEAD, branch names, short SHAs) and local filesystem
paths that are resolved server-side via git rev-parse.

Assisted-by: claude-opus-4.6 <noreply@anthropic.com>
Signed-off-by: derekbarbosa <derekasobrab@gmail.com>
---
 src/api.rs   |  24 ++++-
 src/forge.rs | 272 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 294 insertions(+), 2 deletions(-)

diff --git a/src/api.rs b/src/api.rs
index a1944cfe0..2cb73d1b4 100644
--- a/src/api.rs
+++ b/src/api.rs
@@ -309,6 +309,7 @@ pub fn build_router(
         .route("/", get_service(ServeFile::new("static/index.html")))
         .nest_service("/static", ServeDir::new("static"))
         .layer(middleware::from_fn(redirect_www))
+        .layer(axum::extract::DefaultBodyLimit::max(2 * 1024 * 1024))
         .with_state(state)
 }
 
@@ -472,6 +473,13 @@ async fn submit_patch(
         SubmitRequest::Thread { msgid } => {
             let id = generate_synthetic_id("thread");
             let clean_msgid = msgid.trim_matches(|c| c == '<' || c == '>').to_string();
+
+            // Reject message-IDs with path separators to prevent URL path traversal
+            // when constructing the lore.kernel.org fetch URL. Note: ".." is valid
+            // in RFC 5322 local-parts and is not rejected here.
+            if clean_msgid.contains('/') || clean_msgid.contains('\\') {
+                return Err(StatusCode::BAD_REQUEST);
+            }
             info!(
                 "Received thread fetch request: {} (msgid: {})",
                 id, clean_msgid
@@ -535,14 +543,26 @@ async fn fetch_and_inject_thread(
         .into());
     }
 
+    const MAX_MBOX_DOWNLOAD: usize = 10 * 1024 * 1024;
+    const MAX_MBOX_DECOMPRESSED: u64 = 50 * 1024 * 1024;
+
     let bytes = response.bytes().await?;
+    if bytes.len() > MAX_MBOX_DOWNLOAD {
+        return Err(format!(
+            "Mbox download {} bytes exceeds {} byte limit",
+            bytes.len(),
+            MAX_MBOX_DOWNLOAD
+        )
+        .into());
+    }
 
     // Decompress the gzip data using a blocking task to avoid blocking the async runtime
     let raw = tokio::task::spawn_blocking(move || -> Result<String, std::io::Error> {
         use std::io::Read;
-        let mut decoder = flate2::read::GzDecoder::new(&bytes[..]);
+        let decoder = flate2::read::GzDecoder::new(&bytes[..]);
+        let mut limited = decoder.take(MAX_MBOX_DECOMPRESSED);
         let mut raw = String::new();
-        decoder.read_to_string(&mut raw)?;
+        limited.read_to_string(&mut raw)?;
         Ok(raw)
     })
     .await??;
diff --git a/src/forge.rs b/src/forge.rs
index 2d082a97e..2f23c62d0 100644
--- a/src/forge.rs
+++ b/src/forge.rs
@@ -18,6 +18,32 @@ use bytes::Bytes;
 use std::collections::HashMap;
 use std::sync::Arc;
 
+/// Validate a git commit SHA (40-char SHA-1 or 64-char SHA-256, lowercase hex).
+pub fn is_valid_git_sha(s: &str) -> bool {
+    (s.len() == 40 || s.len() == 64) && s.bytes().all(|b| b.is_ascii_hexdigit())
+}
+
+/// Check that a URL uses an acceptable scheme for git operations.
+pub fn is_valid_repo_url(url: &str) -> bool {
+    url.starts_with("https://") || url.starts_with("http://") || url.starts_with("git@")
+}
+
+/// Check that a repository URL does not target known internal or metadata
+/// endpoints. Returns false for cloud metadata services, loopback addresses,
+/// and other destinations that should never be cloned.
+pub fn is_safe_repo_url(url: &str) -> bool {
+    if !is_valid_repo_url(url) {
+        return false;
+    }
+    let lower = url.to_lowercase();
+    !lower.contains("169.254.")
+        && !lower.contains("metadata.google")
+        && !lower.contains("localhost")
+        && !lower.contains("127.0.0.1")
+        && !lower.contains("[::1]")
+        && !lower.contains("0.0.0.0")
+}
+
 /// Metadata extracted from forge webhook
 #[derive(Debug, Clone)]
 pub struct ForgeMetadata {
@@ -87,7 +113,14 @@ impl ForgeProvider for GitHubForge {
             .ok_or(StatusCode::BAD_REQUEST)?
             .to_string();
 
+        if !is_valid_git_sha(&head_sha) || !is_valid_git_sha(&base_sha) {
+            return Err(StatusCode::BAD_REQUEST);
+        }
+
         let pr_number = pr["number"].as_i64().ok_or(StatusCode::BAD_REQUEST)?;
+        if pr_number <= 0 {
+            return Err(StatusCode::BAD_REQUEST);
+        }
 
         let pr_title = pr["title"].as_str().map(|s| s.to_string());
         let pr_url = pr["html_url"].as_str().map(|s| s.to_string());
@@ -96,6 +129,12 @@ impl ForgeProvider for GitHubForge {
             .as_str()
             .map(|s| s.to_string());
 
+        if let Some(ref url) = repo_url
+            && !is_safe_repo_url(url)
+        {
+            return Err(StatusCode::BAD_REQUEST);
+        }
+
         let metadata = ForgeMetadata {
             repo_url,
             base_sha,
@@ -155,7 +194,14 @@ impl ForgeProvider for GitLabForge {
             .map(|s| s.to_string())
             .unwrap_or_else(|| head_sha.clone());
 
+        if !is_valid_git_sha(&head_sha) || !is_valid_git_sha(&base_sha) {
+            return Err(StatusCode::BAD_REQUEST);
+        }
+
         let pr_number = attrs["iid"].as_i64().ok_or(StatusCode::BAD_REQUEST)?;
+        if pr_number <= 0 {
+            return Err(StatusCode::BAD_REQUEST);
+        }
 
         let pr_title = attrs["title"].as_str().map(|s| s.to_string());
         let pr_url = attrs["url"].as_str().map(|s| s.to_string());
@@ -164,6 +210,12 @@ impl ForgeProvider for GitLabForge {
             .as_str()
             .map(|s| s.to_string());
 
+        if let Some(ref url) = repo_url
+            && !is_safe_repo_url(url)
+        {
+            return Err(StatusCode::BAD_REQUEST);
+        }
+
         let metadata = ForgeMetadata {
             repo_url,
             base_sha,
@@ -236,3 +288,223 @@ impl Default for ForgeRegistry {
         Self::new()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_is_valid_git_sha_40_char() {
+        assert!(is_valid_git_sha("a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"));
+        assert!(is_valid_git_sha("0000000000000000000000000000000000000000"));
+        assert!(is_valid_git_sha("abcdef0123456789abcdef0123456789abcdef01"));
+    }
+
+    #[test]
+    fn test_is_valid_git_sha_rejects_non_hex() {
+        assert!(!is_valid_git_sha(
+            "g1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"
+        ));
+        assert!(!is_valid_git_sha("../../etc/passwd/../../../../etc/shadow"));
+        // Uppercase hex is valid — git accepts both cases
+        assert!(is_valid_git_sha("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"));
+    }
+
+    #[test]
+    fn test_is_valid_git_sha_64_char() {
+        let sha256 = "a".repeat(64);
+        assert!(is_valid_git_sha(&sha256));
+    }
+
+    #[test]
+    fn test_is_valid_git_sha_rejects_wrong_length() {
+        assert!(!is_valid_git_sha("abc123"));
+        assert!(!is_valid_git_sha("a".repeat(39).as_str()));
+        assert!(!is_valid_git_sha("a".repeat(41).as_str()));
+        assert!(!is_valid_git_sha(""));
+    }
+
+    #[test]
+    fn test_is_valid_repo_url_accepts_valid_schemes() {
+        assert!(is_valid_repo_url("https://gitlab.com/org/repo.git"));
+        assert!(is_valid_repo_url("http://gitlab.internal/org/repo.git"));
+        assert!(is_valid_repo_url("git@github.com:org/repo.git"));
+    }
+
+    #[test]
+    fn test_is_valid_repo_url_rejects_invalid_schemes() {
+        assert!(!is_valid_repo_url("ftp://files.example.com/repo.tar"));
+        assert!(!is_valid_repo_url("file:///etc/passwd"));
+        assert!(!is_valid_repo_url("javascript:alert(1)"));
+        assert!(!is_valid_repo_url(""));
+    }
+
+    #[test]
+    fn test_is_safe_repo_url_blocks_ssrf() {
+        assert!(!is_safe_repo_url(
+            "http://169.254.169.254/latest/meta-data/"
+        ));
+        assert!(!is_safe_repo_url("http://metadata.google.internal/"));
+        assert!(!is_safe_repo_url("http://localhost:5432/"));
+        assert!(!is_safe_repo_url("http://127.0.0.1:8080/repo"));
+        assert!(!is_safe_repo_url("http://[::1]:8080/repo"));
+        assert!(!is_safe_repo_url("http://0.0.0.0/repo"));
+    }
+
+    #[test]
+    fn test_is_safe_repo_url_accepts_legitimate() {
+        assert!(is_safe_repo_url("https://gitlab.com/org/repo.git"));
+        assert!(is_safe_repo_url("https://github.com/org/repo.git"));
+        assert!(is_safe_repo_url("git@gitlab.example.com:org/repo.git"));
+        assert!(is_safe_repo_url(
+            "http://gitlab.internal:8929/group/project.git"
+        ));
+    }
+
+    #[test]
+    fn test_github_parse_payload_rejects_invalid_sha() {
+        let forge = GitHubForge;
+        let payload = serde_json::json!({
+            "action": "opened",
+            "pull_request": {
+                "head": {"sha": "not-a-valid-sha"},
+                "base": {"sha": "also-not-valid"},
+                "number": 1,
+                "title": "test"
+            },
+            "repository": {"clone_url": "https://github.com/org/repo.git"}
+        });
+        let body = Bytes::from(serde_json::to_vec(&payload).unwrap());
+        assert_eq!(
+            forge.parse_payload(&body).unwrap_err(),
+            StatusCode::BAD_REQUEST
+        );
+    }
+
+    #[test]
+    fn test_github_parse_payload_rejects_negative_pr() {
+        let forge = GitHubForge;
+        let valid_sha = "a".repeat(40);
+        let payload = serde_json::json!({
+            "action": "opened",
+            "pull_request": {
+                "head": {"sha": &valid_sha},
+                "base": {"sha": &valid_sha},
+                "number": -1,
+                "title": "test"
+            },
+            "repository": {"clone_url": "https://github.com/org/repo.git"}
+        });
+        let body = Bytes::from(serde_json::to_vec(&payload).unwrap());
+        assert_eq!(
+            forge.parse_payload(&body).unwrap_err(),
+            StatusCode::BAD_REQUEST
+        );
+    }
+
+    #[test]
+    fn test_github_parse_payload_rejects_ssrf_url() {
+        let forge = GitHubForge;
+        let valid_sha = "a".repeat(40);
+        let payload = serde_json::json!({
+            "action": "opened",
+            "pull_request": {
+                "head": {"sha": &valid_sha},
+                "base": {"sha": &valid_sha},
+                "number": 1,
+                "title": "test"
+            },
+            "repository": {"clone_url": "http://169.254.169.254/latest/meta-data/"}
+        });
+        let body = Bytes::from(serde_json::to_vec(&payload).unwrap());
+        assert_eq!(
+            forge.parse_payload(&body).unwrap_err(),
+            StatusCode::BAD_REQUEST
+        );
+    }
+
+    #[test]
+    fn test_github_parse_payload_accepts_valid() {
+        let forge = GitHubForge;
+        let valid_sha = "a".repeat(40);
+        let base_sha = "b".repeat(40);
+        let payload = serde_json::json!({
+            "action": "opened",
+            "pull_request": {
+                "head": {"sha": &valid_sha},
+                "base": {"sha": &base_sha},
+                "number": 42,
+                "title": "Fix something",
+                "html_url": "https://github.com/org/repo/pull/42"
+            },
+            "repository": {"clone_url": "https://github.com/org/repo.git"}
+        });
+        let body = Bytes::from(serde_json::to_vec(&payload).unwrap());
+        let (action, metadata) = forge.parse_payload(&body).unwrap();
+        assert_eq!(action, "opened");
+        assert_eq!(metadata.pr_number, 42);
+        assert_eq!(metadata.head_sha, valid_sha);
+    }
+
+    #[test]
+    fn test_gitlab_parse_payload_rejects_invalid_sha() {
+        let forge = GitLabForge;
+        let payload = serde_json::json!({
+            "object_kind": "merge_request",
+            "object_attributes": {
+                "last_commit": {"id": "../../etc/passwd"},
+                "diff_refs": {"base_sha": "invalid"},
+                "iid": 1,
+                "title": "test"
+            },
+            "project": {"git_http_url": "https://gitlab.com/org/repo.git"}
+        });
+        let body = Bytes::from(serde_json::to_vec(&payload).unwrap());
+        assert_eq!(
+            forge.parse_payload(&body).unwrap_err(),
+            StatusCode::BAD_REQUEST
+        );
+    }
+
+    #[test]
+    fn test_gitlab_parse_payload_rejects_zero_iid() {
+        let forge = GitLabForge;
+        let valid_sha = "a".repeat(40);
+        let payload = serde_json::json!({
+            "object_kind": "merge_request",
+            "object_attributes": {
+                "last_commit": {"id": &valid_sha},
+                "diff_refs": {"base_sha": &valid_sha},
+                "iid": 0,
+                "title": "test"
+            },
+            "project": {"git_http_url": "https://gitlab.com/org/repo.git"}
+        });
+        let body = Bytes::from(serde_json::to_vec(&payload).unwrap());
+        assert_eq!(
+            forge.parse_payload(&body).unwrap_err(),
+            StatusCode::BAD_REQUEST
+        );
+    }
+
+    #[test]
+    fn test_gitlab_parse_payload_accepts_valid() {
+        let forge = GitLabForge;
+        let valid_sha = "c".repeat(40);
+        let payload = serde_json::json!({
+            "object_kind": "merge_request",
+            "object_attributes": {
+                "last_commit": {"id": &valid_sha},
+                "diff_refs": {"base_sha": &valid_sha},
+                "iid": 10,
+                "title": "Fix bug",
+                "url": "https://gitlab.com/org/repo/-/merge_requests/10"
+            },
+            "project": {"git_http_url": "https://gitlab.com/org/repo.git"}
+        });
+        let body = Bytes::from(serde_json::to_vec(&payload).unwrap());
+        let (action, metadata) = forge.parse_payload(&body).unwrap();
+        assert_eq!(action, "merge_request");
+        assert_eq!(metadata.pr_number, 10);
+    }
+}

From 04005713b336aa26f31b5497aa79e770c30ec6fe Mon Sep 17 00:00:00 2001
From: derekbarbosa <derekasobrab@gmail.com>
Date: Mon, 6 Jul 2026 15:40:44 -0400
Subject: [PATCH 2/5] security: add webhook HMAC signature verification

Implement cryptographic verification for incoming forge webhook
requests, mitigating forged patchset creation, cost amplification via
unauthorized AI review triggers, and SSRF via injected repo URLs.

Three authentication modes are supported, selected by header presence:

- Standard Webhooks HMAC-SHA256 (GitLab 19.0+ signing token):
  verifies webhook-signature header over msg_id.timestamp.body
- GitHub HMAC-SHA256: verifies X-Hub-Signature-256 header
- Legacy GitLab secret token: constant-time comparison of
  X-Gitlab-Token header

All comparisons use constant-time operations via the subtle crate to
prevent timing-based secret extraction.

The access control flow is revised: when webhook_secret is configured,
non-localhost requests are permitted because the signature check is
the access control. The --enable-unsafe-all-submit flag is only needed
for unauthenticated setups. Startup warnings are emitted for insecure
configurations.

Secret type auto-detection: whsec_ prefix indicates a Standard
Webhooks signing token (base64-decoded); plain strings are used
directly for both X-Gitlab-Token and HMAC key bytes. A warning is
logged if whsec_ prefix is present but base64 decoding fails.

The ForgeProvider trait signature changes to accept body and optional
secret, which is a breaking change for downstream trait implementors
but not for API consumers.

New dependencies: hmac 0.13, base64 0.22, subtle 2.

Assisted-by: claude-opus-4.6 <noreply@anthropic.com>
Signed-off-by: derekbarbosa <derekasobrab@gmail.com>
---
 Cargo.lock   |   3 +
 Cargo.toml   |   3 +
 src/api.rs   |  18 ++-
 src/forge.rs | 343 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/main.rs  |  16 +++
 5 files changed, 375 insertions(+), 8 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 5263fbbbb..7904c4c76 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3397,6 +3397,7 @@ dependencies = [
  "aws-sdk-bedrockruntime",
  "aws-smithy-types",
  "axum 0.8.9",
+ "base64 0.22.1",
  "bytes",
  "chrono",
  "clap",
@@ -3405,6 +3406,7 @@ dependencies = [
  "flate2",
  "futures",
  "google-cloud-auth",
+ "hmac 0.13.0",
  "lettre",
  "libsql",
  "mail-parser",
@@ -3413,6 +3415,7 @@ dependencies = [
  "serde",
  "serde_json",
  "sha2 0.11.0",
+ "subtle",
  "tempfile",
  "termcolor",
  "thiserror 2.0.18",
diff --git a/Cargo.toml b/Cargo.toml
index 8803d9178..ad6d6fcc5 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -64,3 +64,6 @@ tracing = "0.1.44"
 tracing-subscriber = { version = "0.3.22", features = ["env-filter"] }
 tree-sitter = "0.26.9"
 tree-sitter-c = "0.24.2"
+hmac = "0.13"
+base64 = "0.22"
+subtle = "2"
diff --git a/src/api.rs b/src/api.rs
index 2cb73d1b4..db29bb748 100644
--- a/src/api.rs
+++ b/src/api.rs
@@ -1117,8 +1117,20 @@ async fn forge_webhook(
     if state.read_only {
         return Err(StatusCode::FORBIDDEN);
     }
-    if !state.allow_all_submit && !addr.ip().to_canonical().is_loopback() {
-        info!("Refused {} webhook from non-localhost: {}", provider, addr);
+
+    let is_loopback = addr.ip().to_canonical().is_loopback();
+    let webhook_secret = state.settings.forge.webhook_secret.as_deref();
+    let has_secret = webhook_secret.is_some();
+
+    // When webhook_secret is configured, the signature check in
+    // validate_event is the access control — no need for
+    // --enable-unsafe-all-submit. Without a secret, fall back to the
+    // existing localhost-only or explicit flag behavior.
+    if !is_loopback && !has_secret && !state.allow_all_submit {
+        info!(
+            "Refused {} webhook from {}: configure webhook_secret or use --enable-unsafe-all-submit",
+            provider, addr
+        );
         return Err(StatusCode::FORBIDDEN);
     }
 
@@ -1127,7 +1139,7 @@ async fn forge_webhook(
         StatusCode::NOT_FOUND
     })?;
 
-    forge.validate_event(&headers)?;
+    forge.validate_event(&headers, &body, webhook_secret)?;
 
     let (action, metadata) = forge.parse_payload(&body)?;
 
diff --git a/src/forge.rs b/src/forge.rs
index 2f23c62d0..75027d506 100644
--- a/src/forge.rs
+++ b/src/forge.rs
@@ -14,11 +14,17 @@
 
 use anyhow::Result;
 use axum::http::{HeaderMap, StatusCode};
+use base64::Engine;
 use bytes::Bytes;
+use hmac::{Hmac, Mac, digest::KeyInit};
+use sha2::Sha256;
 use std::collections::HashMap;
 use std::sync::Arc;
+use subtle::ConstantTimeEq;
 
-/// Validate a git commit SHA (40-char SHA-1 or 64-char SHA-256, lowercase hex).
+type HmacSha256 = Hmac<Sha256>;
+
+/// Validate a git commit SHA (40-char SHA-1 or 64-char SHA-256, hex digits).
 pub fn is_valid_git_sha(s: &str) -> bool {
     (s.len() == 40 || s.len() == 64) && s.bytes().all(|b| b.is_ascii_hexdigit())
 }
@@ -31,6 +37,10 @@ pub fn is_valid_repo_url(url: &str) -> bool {
 /// Check that a repository URL does not target known internal or metadata
 /// endpoints. Returns false for cloud metadata services, loopback addresses,
 /// and other destinations that should never be cloned.
+///
+/// This is a best-effort blocklist, not a complete SSRF mitigation. DNS
+/// rebinding and URL encoding can bypass string-based checks. The primary
+/// access control is webhook signature verification.
 pub fn is_safe_repo_url(url: &str) -> bool {
     if !is_valid_repo_url(url) {
         return false;
@@ -44,6 +54,81 @@ pub fn is_safe_repo_url(url: &str) -> bool {
         && !lower.contains("0.0.0.0")
 }
 
+/// Decode a webhook secret. If prefixed with "whsec_", strip the prefix
+/// and base64-decode the remainder (Standard Webhooks convention). Otherwise
+/// return the raw string bytes.
+fn decode_webhook_secret(secret: &str) -> Vec<u8> {
+    if let Some(encoded) = secret.strip_prefix("whsec_") {
+        match base64::engine::general_purpose::STANDARD.decode(encoded) {
+            Ok(key) => key,
+            Err(e) => {
+                tracing::warn!(
+                    "webhook_secret has whsec_ prefix but base64 decode failed: {}. \
+                     Check that the token was copied correctly from GitLab. \
+                     Falling back to raw string bytes.",
+                    e
+                );
+                secret.as_bytes().to_vec()
+            }
+        }
+    } else {
+        secret.as_bytes().to_vec()
+    }
+}
+
+/// Verify a Standard Webhooks HMAC-SHA256 signature (GitLab 19.0+ signing
+/// token). The signature header may contain multiple space-separated entries,
+/// each in the format "v1,{base64(hmac)}". The HMAC is computed over
+/// "{message_id}.{timestamp}.{body}".
+fn verify_standard_webhook_signature(
+    secret: &str,
+    msg_id: &str,
+    timestamp: &str,
+    body: &[u8],
+    signatures: &str,
+) -> bool {
+    let key = decode_webhook_secret(secret);
+    let mut mac = match HmacSha256::new_from_slice(&key) {
+        Ok(m) => m,
+        Err(_) => return false,
+    };
+    let preamble = format!("{}.{}.", msg_id, timestamp);
+    mac.update(preamble.as_bytes());
+    mac.update(body);
+    let result = mac.finalize().into_bytes();
+    let expected = format!(
+        "v1,{}",
+        base64::engine::general_purpose::STANDARD.encode(result)
+    );
+    signatures
+        .split(' ')
+        .any(|sig| expected.as_bytes().ct_eq(sig.as_bytes()).into())
+}
+
+/// Verify a GitHub HMAC-SHA256 signature. The header value has the format
+/// "sha256={hex_digest}".
+fn verify_github_signature(secret: &str, body: &[u8], signature_header: &str) -> bool {
+    let hex_sig = match signature_header.strip_prefix("sha256=") {
+        Some(s) => s,
+        None => return false,
+    };
+    let mut mac = match HmacSha256::new_from_slice(secret.as_bytes()) {
+        Ok(m) => m,
+        Err(_) => return false,
+    };
+    mac.update(body);
+    let result = mac.finalize().into_bytes();
+    let computed: String = result.iter().map(|b| format!("{:02x}", b)).collect();
+    computed.as_bytes().ct_eq(hex_sig.as_bytes()).into()
+}
+
+/// Verify a legacy GitLab secret token via constant-time comparison.
+/// Note: ct_eq reveals whether the lengths differ (but not the content).
+/// This is acceptable for webhook secrets with sufficient entropy.
+fn verify_secret_token(secret: &str, token_header: &str) -> bool {
+    secret.as_bytes().ct_eq(token_header.as_bytes()).into()
+}
+
 /// Metadata extracted from forge webhook
 #[derive(Debug, Clone)]
 pub struct ForgeMetadata {
@@ -60,8 +145,15 @@ pub trait ForgeProvider: Send + Sync {
     /// Provider name (e.g., "GitHub", "GitLab")
     fn name(&self) -> &str;
 
-    /// Validate webhook event from headers
-    fn validate_event(&self, headers: &HeaderMap) -> Result<(), StatusCode>;
+    /// Validate webhook event type and verify signature when a secret is
+    /// configured. Returns `UNAUTHORIZED` if the signature is missing or
+    /// invalid, `BAD_REQUEST` if the event type is wrong.
+    fn validate_event(
+        &self,
+        headers: &HeaderMap,
+        body: &Bytes,
+        secret: Option<&str>,
+    ) -> Result<(), StatusCode>;
 
     /// Parse webhook payload and extract metadata
     fn parse_payload(&self, body: &Bytes) -> Result<(String, ForgeMetadata), StatusCode>;
@@ -75,7 +167,12 @@ impl ForgeProvider for GitHubForge {
         "GitHub"
     }
 
-    fn validate_event(&self, headers: &HeaderMap) -> Result<(), StatusCode> {
+    fn validate_event(
+        &self,
+        headers: &HeaderMap,
+        body: &Bytes,
+        secret: Option<&str>,
+    ) -> Result<(), StatusCode> {
         let event = headers
             .get("x-github-event")
             .and_then(|v| v.to_str().ok())
@@ -85,6 +182,16 @@ impl ForgeProvider for GitHubForge {
             return Err(StatusCode::BAD_REQUEST);
         }
 
+        if let Some(secret) = secret {
+            let sig = headers
+                .get("x-hub-signature-256")
+                .and_then(|v| v.to_str().ok())
+                .ok_or(StatusCode::UNAUTHORIZED)?;
+            if !verify_github_signature(secret, body, sig) {
+                return Err(StatusCode::UNAUTHORIZED);
+            }
+        }
+
         Ok(())
     }
 
@@ -156,7 +263,12 @@ impl ForgeProvider for GitLabForge {
         "GitLab"
     }
 
-    fn validate_event(&self, headers: &HeaderMap) -> Result<(), StatusCode> {
+    fn validate_event(
+        &self,
+        headers: &HeaderMap,
+        body: &Bytes,
+        secret: Option<&str>,
+    ) -> Result<(), StatusCode> {
         let event = headers
             .get("x-gitlab-event")
             .and_then(|v| v.to_str().ok())
@@ -166,6 +278,35 @@ impl ForgeProvider for GitLabForge {
             return Err(StatusCode::BAD_REQUEST);
         }
 
+        if let Some(secret) = secret {
+            // Try Standard Webhooks signature first (GitLab 19.0+)
+            if let (Some(msg_id), Some(timestamp), Some(sig)) = (
+                headers.get("webhook-id").and_then(|v| v.to_str().ok()),
+                headers
+                    .get("webhook-timestamp")
+                    .and_then(|v| v.to_str().ok()),
+                headers
+                    .get("webhook-signature")
+                    .and_then(|v| v.to_str().ok()),
+            ) {
+                if !verify_standard_webhook_signature(secret, msg_id, timestamp, body, sig) {
+                    return Err(StatusCode::UNAUTHORIZED);
+                }
+                return Ok(());
+            }
+
+            // Fallback: legacy secret token (X-Gitlab-Token)
+            if let Some(token) = headers.get("x-gitlab-token").and_then(|v| v.to_str().ok()) {
+                if !verify_secret_token(secret, token) {
+                    return Err(StatusCode::UNAUTHORIZED);
+                }
+                return Ok(());
+            }
+
+            // Secret configured but no auth header present
+            return Err(StatusCode::UNAUTHORIZED);
+        }
+
         Ok(())
     }
 
@@ -507,4 +648,196 @@ mod tests {
         assert_eq!(action, "merge_request");
         assert_eq!(metadata.pr_number, 10);
     }
+
+    // --- HMAC verification tests ---
+
+    #[test]
+    fn test_verify_github_signature_known_vector() {
+        // Test vector from GitHub docs:
+        // secret: "It's a Secret to Everybody"
+        // payload: "Hello, World!"
+        // expected: sha256=757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e17
+        let secret = "It's a Secret to Everybody";
+        let payload = b"Hello, World!";
+        let sig = "sha256=757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e17";
+        assert!(verify_github_signature(secret, payload, sig));
+    }
+
+    #[test]
+    fn test_verify_github_signature_rejects_invalid() {
+        let secret = "my-secret";
+        let payload = b"test body";
+        let sig = "sha256=0000000000000000000000000000000000000000000000000000000000000000";
+        assert!(!verify_github_signature(secret, payload, sig));
+    }
+
+    #[test]
+    fn test_verify_github_signature_rejects_missing_prefix() {
+        let secret = "my-secret";
+        let payload = b"test body";
+        let sig = "md5=abcdef";
+        assert!(!verify_github_signature(secret, payload, sig));
+    }
+
+    #[test]
+    fn test_verify_standard_webhook_signature() {
+        let secret = "test-secret-key";
+        let msg_id = "msg-123";
+        let timestamp = "1720000000";
+        let body = b"test body";
+
+        // Compute the expected signature manually
+        let key = decode_webhook_secret(secret);
+        let mut mac = HmacSha256::new_from_slice(&key).unwrap();
+        let preamble = format!("{}.{}.", msg_id, timestamp);
+        mac.update(preamble.as_bytes());
+        mac.update(body);
+        let result = mac.finalize().into_bytes();
+        let sig = format!(
+            "v1,{}",
+            base64::engine::general_purpose::STANDARD.encode(result)
+        );
+
+        assert!(verify_standard_webhook_signature(
+            secret, msg_id, timestamp, body, &sig
+        ));
+    }
+
+    #[test]
+    fn test_verify_standard_webhook_signature_rejects_tampered() {
+        let secret = "test-secret";
+        let msg_id = "msg-456";
+        let timestamp = "1720000000";
+        let body = b"original body";
+        let sig = "v1,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
+        assert!(!verify_standard_webhook_signature(
+            secret, msg_id, timestamp, body, sig
+        ));
+    }
+
+    #[test]
+    fn test_verify_standard_webhook_multiple_signatures() {
+        let secret = "multi-test";
+        let msg_id = "msg-789";
+        let timestamp = "1720000000";
+        let body = b"multi sig body";
+
+        let key = decode_webhook_secret(secret);
+        let mut mac = HmacSha256::new_from_slice(&key).unwrap();
+        mac.update(format!("{}.{}.", msg_id, timestamp).as_bytes());
+        mac.update(body);
+        let result = mac.finalize().into_bytes();
+        let valid_sig = format!(
+            "v1,{}",
+            base64::engine::general_purpose::STANDARD.encode(result)
+        );
+
+        // Multiple signatures separated by space — one valid, one garbage
+        let header = format!("v1,garbage_signature {}", valid_sig);
+        assert!(verify_standard_webhook_signature(
+            secret, msg_id, timestamp, body, &header
+        ));
+    }
+
+    #[test]
+    fn test_verify_secret_token() {
+        assert!(verify_secret_token("my-secret", "my-secret"));
+        assert!(!verify_secret_token("my-secret", "wrong-secret"));
+        assert!(!verify_secret_token("my-secret", "my-secre")); // length differs
+    }
+
+    #[test]
+    fn test_decode_webhook_secret_whsec_prefix() {
+        // "whsec_" + base64("test-key") = "whsec_dGVzdC1rZXk="
+        let decoded = decode_webhook_secret("whsec_dGVzdC1rZXk=");
+        assert_eq!(decoded, b"test-key");
+    }
+
+    #[test]
+    fn test_decode_webhook_secret_plain() {
+        let decoded = decode_webhook_secret("my-plain-secret");
+        assert_eq!(decoded, b"my-plain-secret");
+    }
+
+    #[test]
+    fn test_decode_webhook_secret_invalid_base64_falls_back() {
+        // Invalid base64 after whsec_ prefix — falls back to raw bytes
+        let decoded = decode_webhook_secret("whsec_!!!invalid!!!");
+        assert_eq!(decoded, b"whsec_!!!invalid!!!");
+    }
+
+    #[test]
+    fn test_github_validate_event_accepts_without_secret() {
+        let forge = GitHubForge;
+        let mut headers = HeaderMap::new();
+        headers.insert("x-github-event", "pull_request".parse().unwrap());
+        let body = Bytes::from("{}");
+        assert!(forge.validate_event(&headers, &body, None).is_ok());
+    }
+
+    #[test]
+    fn test_github_validate_event_rejects_missing_signature() {
+        let forge = GitHubForge;
+        let mut headers = HeaderMap::new();
+        headers.insert("x-github-event", "pull_request".parse().unwrap());
+        let body = Bytes::from("{}");
+        assert_eq!(
+            forge
+                .validate_event(&headers, &body, Some("my-secret"))
+                .unwrap_err(),
+            StatusCode::UNAUTHORIZED
+        );
+    }
+
+    #[test]
+    fn test_gitlab_validate_event_accepts_legacy_token() {
+        let forge = GitLabForge;
+        let mut headers = HeaderMap::new();
+        headers.insert("x-gitlab-event", "Merge Request Hook".parse().unwrap());
+        headers.insert("x-gitlab-token", "shared-secret".parse().unwrap());
+        let body = Bytes::from("{}");
+        assert!(
+            forge
+                .validate_event(&headers, &body, Some("shared-secret"))
+                .is_ok()
+        );
+    }
+
+    #[test]
+    fn test_gitlab_validate_event_rejects_wrong_token() {
+        let forge = GitLabForge;
+        let mut headers = HeaderMap::new();
+        headers.insert("x-gitlab-event", "Merge Request Hook".parse().unwrap());
+        headers.insert("x-gitlab-token", "wrong-token".parse().unwrap());
+        let body = Bytes::from("{}");
+        assert_eq!(
+            forge
+                .validate_event(&headers, &body, Some("correct-token"))
+                .unwrap_err(),
+            StatusCode::UNAUTHORIZED
+        );
+    }
+
+    #[test]
+    fn test_gitlab_validate_event_rejects_no_auth_headers() {
+        let forge = GitLabForge;
+        let mut headers = HeaderMap::new();
+        headers.insert("x-gitlab-event", "Merge Request Hook".parse().unwrap());
+        let body = Bytes::from("{}");
+        assert_eq!(
+            forge
+                .validate_event(&headers, &body, Some("my-secret"))
+                .unwrap_err(),
+            StatusCode::UNAUTHORIZED
+        );
+    }
+
+    #[test]
+    fn test_gitlab_validate_event_accepts_without_secret() {
+        let forge = GitLabForge;
+        let mut headers = HeaderMap::new();
+        headers.insert("x-gitlab-event", "Merge Request Hook".parse().unwrap());
+        let body = Bytes::from("{}");
+        assert!(forge.validate_event(&headers, &body, None).is_ok());
+    }
 }
diff --git a/src/main.rs b/src/main.rs
index 82b0018c7..6e88dee92 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -709,6 +709,22 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         );
     });
 
+    // Warn about insecure forge webhook configurations
+    if settings.forge.enabled && settings.forge.webhook_secret.is_none() {
+        if cli.enable_unsafe_all_submit {
+            warn!(
+                "Accepting unauthenticated webhook requests from all addresses. \
+                 Configure forge.webhook_secret for production deployments."
+            );
+        } else {
+            warn!(
+                "Forge webhooks enabled without webhook_secret. \
+                 Non-localhost requests require --enable-unsafe-all-submit. \
+                 See docs/WEBHOOK_SECURITY.md"
+            );
+        }
+    }
+
     // Start Ingestor (feeds raw_tx)
     let ingestor_handle = if !(settings.forge.enabled && settings.forge.disable_nntp) {
         let ingestor = Ingestor::new(

From 625891a106edf7e795375e5a279dcf2cd59e1879 Mon Sep 17 00:00:00 2001
From: derekbarbosa <derekasobrab@gmail.com>
Date: Mon, 6 Jul 2026 15:40:57 -0400
Subject: [PATCH 3/5] docs: add webhook security guide and update forge
 documentation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add docs/WEBHOOK_SECURITY.md — a comprehensive guide covering webhook
authentication setup for all deployment topologies:

- Deployment topology decision tree (public server, tunnel, LAN,
  script-based)
- Authentication method comparison (HMAC signing token, HMAC secret,
  plain secret token)
- Step-by-step setup for GitLab signing tokens, GitLab legacy tokens,
  and GitHub HMAC
- Script-based/cronjob curl examples for both plain and HMAC auth
- Production deployment checklist
- Reverse proxy examples (nginx with TLS/rate-limiting, Caddy)
- Network security (GitLab/GitHub IP ranges, SSRF protections)
- Secret management (env vars, file permissions, rotation)
- Troubleshooting (every error code) and FAQ

Update existing docs to replace placeholder security warnings:
- FORGE_SETUP.md: replace security sections with summary + link,
  update comparison table to mark signature validation as implemented
- GITHUB_SETUP.md: replace 'not yet implemented' with setup steps
- GITLAB_SETUP.md: replace 'not validated' with signing token setup
- configuration.md: add [forge] section and webhook_secret env var

Add four topology-specific example configs:
- Settings.forge-gitlab-production.toml (signing token + localhost)
- Settings.forge-gitlab-simple.toml (plain secret + cronjob)
- Settings.forge-github.toml (HMAC secret)
- Settings.forge-selfhosted.toml (signing token + LAN bind)

Update Settings.toml with documented webhook_secret guidance.

Assisted-by: claude-opus-4.6 <noreply@anthropic.com>
Signed-off-by: derekbarbosa <derekasobrab@gmail.com>
---
 Settings.toml                                 |  14 +-
 docs/FORGE_SETUP.md                           |  79 +---
 docs/GITHUB_SETUP.md                          |  35 +-
 docs/GITLAB_SETUP.md                          |  24 +-
 docs/WEBHOOK_SECURITY.md                      | 410 ++++++++++++++++++
 docs/configuration.md                         |  16 +
 docs/examples/Settings.forge-github.toml      |  15 +
 .../Settings.forge-gitlab-production.toml     |  16 +
 .../Settings.forge-gitlab-simple.toml         |  15 +
 docs/examples/Settings.forge-selfhosted.toml  |  14 +
 10 files changed, 534 insertions(+), 104 deletions(-)
 create mode 100644 docs/WEBHOOK_SECURITY.md
 create mode 100644 docs/examples/Settings.forge-github.toml
 create mode 100644 docs/examples/Settings.forge-gitlab-production.toml
 create mode 100644 docs/examples/Settings.forge-gitlab-simple.toml
 create mode 100644 docs/examples/Settings.forge-selfhosted.toml

diff --git a/Settings.toml b/Settings.toml
index 79f1bcf03..f8da84166 100644
--- a/Settings.toml
+++ b/Settings.toml
@@ -7,9 +7,21 @@ description = ""
 [forge]
 enabled = false
 # provider = "github"
-# webhook_secret = "your-webhook-secret"
 # api_token = "your-api-token"
 
+# Webhook authentication (recommended for production).
+# When configured, Sashiko authenticates incoming webhook requests using
+# this token. Non-localhost requests are permitted without
+# --enable-unsafe-all-submit when a secret is set.
+#
+# For GitLab 19.0+ signing token (recommended):
+#   webhook_secret = "whsec_YOUR_BASE64_TOKEN"
+# For GitHub or plain secret token:
+#   webhook_secret = "YOUR_SECRET"
+# Generate a strong secret: openssl rand -hex 32
+# See docs/WEBHOOK_SECURITY.md for full setup guide.
+# webhook_secret = ""
+
 [subsystems]
 # Regex map: email address pattern -> subsystem name
 # mapping = [
diff --git a/docs/FORGE_SETUP.md b/docs/FORGE_SETUP.md
index e62e70edf..837078845 100644
--- a/docs/FORGE_SETUP.md
+++ b/docs/FORGE_SETUP.md
@@ -136,11 +136,12 @@ host = "127.0.0.1"  # Listen address
 port = 8080          # Port for webhook endpoint
 ```
 
-**Security considerations:**
-- By default, Sashiko only accepts webhooks from localhost
-- For production, use `--enable-unsafe-all-submit` flag (behind firewall/proxy)
-- Always use HTTPS in production with valid certificates
-- Implement webhook signature validation when available
+**Security:** Sashiko supports webhook signature verification via HMAC-SHA256
+(GitHub and GitLab 19.0+ signing tokens) and legacy secret tokens. When
+`webhook_secret` is configured, non-localhost requests are authenticated
+without requiring `--enable-unsafe-all-submit`. See the
+[Webhook Security Guide](WEBHOOK_SECURITY.md) for setup instructions and
+production deployment recommendations.
 
 ### Git Configuration
 
@@ -345,67 +346,9 @@ api_endpoint = "https://api.yourforge.com"
 
 ## Security Best Practices
 
-### Production Deployment
-
-1. **Use HTTPS with valid certificates**
-   - Set up reverse proxy (nginx, Apache, Caddy)
-   - Obtain SSL/TLS certificates (Let's Encrypt)
-   - Terminate TLS at proxy, forward to Sashiko
-
-2. **Implement authentication**
-   - Use webhook secrets when available
-   - Validate webhook signatures
-   - Restrict by source IP (if forge IPs are known)
-
-3. **Network isolation**
-   - Run Sashiko on private network
-   - Use VPN or SSH tunneling for access
-   - Firewall rules to limit exposure
-
-4. **Rate limiting**
-   - Configure at reverse proxy level
-   - Prevent abuse and DoS attempts
-   - Monitor webhook delivery rates
-
-5. **Audit logging**
-   - Log all webhook deliveries
-   - Track review queue metrics
-   - Monitor for unusual patterns
-
-### Example Nginx Configuration
-
-```nginx
-server {
-    listen 443 ssl http2;
-    server_name sashiko.example.com;
-
-    ssl_certificate /etc/letsencrypt/live/sashiko.example.com/fullchain.pem;
-    ssl_certificate_key /etc/letsencrypt/live/sashiko.example.com/privkey.pem;
-
-    # Security headers
-    add_header Strict-Transport-Security "max-age=31536000" always;
-    add_header X-Frame-Options DENY;
-    add_header X-Content-Type-Options nosniff;
-
-    # Rate limiting
-    limit_req_zone $binary_remote_addr zone=webhook:10m rate=10r/m;
-    limit_req zone=webhook burst=5;
-
-    location /api/webhook/ {
-        proxy_pass http://127.0.0.1:8080;
-        proxy_set_header Host $host;
-        proxy_set_header X-Real-IP $remote_addr;
-        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
-        proxy_set_header X-Forwarded-Proto $scheme;
-    }
-
-    location / {
-        proxy_pass http://127.0.0.1:8080;
-        proxy_set_header Host $host;
-        proxy_set_header X-Real-IP $remote_addr;
-    }
-}
-```
+Sashiko supports webhook signature verification. For complete setup
+instructions, deployment topologies, reverse proxy examples, and a
+production checklist, see the [Webhook Security Guide](WEBHOOK_SECURITY.md).
 
 ## Forge Comparison
 
@@ -414,8 +357,8 @@ server {
 | Webhook delivery | ✅ | ✅ | **Required** |
 | JSON payloads | ✅ | ✅ | **Required** |
 | Event filtering | ✅ | ✅ | **Required** |
-| Webhook secrets | ✅ | ✅ | Recommended |
-| Signature validation | ✅ | ✅ | Recommended |
+| Webhook secrets | ✅ | ✅ | ✅ Implemented |
+| Signature validation | ✅ | ✅ | ✅ Implemented |
 | Delivery logs | ✅ | ✅ | Recommended |
 | Public API | ✅ | ✅ | **Required** |
 | Token auth | ✅ | ✅ | **Required** |
diff --git a/docs/GITHUB_SETUP.md b/docs/GITHUB_SETUP.md
index c91356540..2f04954bc 100644
--- a/docs/GITHUB_SETUP.md
+++ b/docs/GITHUB_SETUP.md
@@ -148,7 +148,9 @@ cargo run --release -- --enable-unsafe-all-submit
      - Replace `your-server` with your actual server address
      - Use port `8080` (or your configured server port)
    - **Content type:** `application/json`
-   - **Secret:** Leave empty (signature validation not yet implemented)
+   - **Secret:** Enter a strong random token (generate with `openssl rand -hex 32`).
+     Add the same value as `webhook_secret` in Sashiko's `Settings.toml`.
+     See the [Webhook Security Guide](WEBHOOK_SECURITY.md#github-hmac-setup) for details.
    - **Which events would you like to trigger this webhook?**
      - Select: **Let me select individual events**
      - Check: ✓ **Pull requests**
@@ -241,32 +243,13 @@ Check the web UI at `http://localhost:8080/` to see the review progress.
 
 ## Security Considerations
 
-**⚠️ IMPORTANT:** GitHub webhook signature validation is not yet implemented.
+Sashiko verifies GitHub webhook signatures using HMAC-SHA256 when
+`webhook_secret` is configured. This mitigates forged webhook requests that
+could trigger unauthorized reviews.
 
-For production deployments:
-
-1. **Use HTTPS:** Set up a reverse proxy with TLS
-   ```nginx
-   # Example nginx config
-   server {
-       listen 443 ssl;
-       server_name sashiko.example.com;
-
-       ssl_certificate /path/to/cert.pem;
-       ssl_certificate_key /path/to/key.pem;
-
-       location /api/webhook/ {
-           proxy_pass http://localhost:8080;
-           proxy_set_header X-Real-IP $remote_addr;
-       }
-   }
-   ```
-
-2. **Implement webhook secrets:** Future enhancement - see GitHub's [webhook security guide](https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries)
-
-3. **Network isolation:** Run Sashiko on private network and use SSH tunneling or VPN
-
-4. **Rate limiting:** Configure reverse proxy or firewall to prevent abuse
+For production deployment instructions, reverse proxy examples, and a
+complete security checklist, see the
+[Webhook Security Guide](WEBHOOK_SECURITY.md).
 
 ## Advanced Configuration
 
diff --git a/docs/GITLAB_SETUP.md b/docs/GITLAB_SETUP.md
index 023c9280f..780cb75fc 100644
--- a/docs/GITLAB_SETUP.md
+++ b/docs/GITLAB_SETUP.md
@@ -94,7 +94,14 @@ Restart Sashiko after changing the configuration.
    http://localhost:9080/api/webhook/gitlab
    ```
 
-   **Secret token:** (Optional - currently not validated, see security note below)
+   **Signing token** (GitLab 19.0+): Select **Generate signing token** and copy the
+   value. Add it as `webhook_secret` in Sashiko's `Settings.toml`.
+
+   **Secret token** (older GitLab versions): Enter a shared secret. Add the same
+   value as `webhook_secret` in Sashiko's `Settings.toml`.
+
+   See the [Webhook Security Guide](WEBHOOK_SECURITY.md#gitlab-signing-token-setup)
+   for detailed setup instructions.
 
    **Trigger:**
    - ✓ Merge request events
@@ -200,15 +207,14 @@ If GitLab cannot reach your server:
 
 ## Security Notes
 
-⚠️ **IMPORTANT:** The current implementation does NOT validate webhook signatures.
-
-This means anyone who knows your webhook URL can trigger reviews.
+Sashiko verifies GitLab webhook signatures using HMAC-SHA256 signing tokens
+(GitLab 19.0+) or legacy secret tokens when `webhook_secret` is configured.
+This mitigates forged webhook requests that could trigger unauthorized
+reviews.
 
-**Recommended for production:**
-1. Implement webhook signature validation (see issue in code review)
-2. Use HTTPS with valid certificates
-3. Restrict network access to known GitLab IPs
-4. Set a strong webhook secret token in GitLab
+For production deployment instructions, reverse proxy examples, and a
+complete security checklist, see the
+[Webhook Security Guide](WEBHOOK_SECURITY.md).
 
 ## Manual Testing
 
diff --git a/docs/WEBHOOK_SECURITY.md b/docs/WEBHOOK_SECURITY.md
new file mode 100644
index 000000000..3fa1868f6
--- /dev/null
+++ b/docs/WEBHOOK_SECURITY.md
@@ -0,0 +1,410 @@
+# Webhook Security Guide
+
+This guide covers how to authenticate incoming webhook requests from GitHub
+and GitLab to your Sashiko instance. Configuring webhook authentication
+mitigates forged requests that could trigger unauthorized AI reviews, inject
+arbitrary repository URLs, or pollute the review database.
+
+## Quick start
+
+1. Choose your [deployment topology](#deployment-topologies)
+2. Generate a signing token or shared secret
+3. Configure it in your forge's webhook settings (GitLab or GitHub UI)
+4. Add the same value as `webhook_secret` in Sashiko's `Settings.toml`
+5. Restart Sashiko and test with a real PR/MR event
+
+## Deployment topologies
+
+### Public server with reverse proxy
+
+```
+GitLab.com ──HTTPS──▶ nginx (TLS) ──HTTP──▶ Sashiko :8080 (localhost)
+```
+
+Sashiko binds to localhost; a reverse proxy (nginx, Caddy) terminates TLS.
+Configure `webhook_secret` in `Settings.toml` — non-localhost requests from
+the proxy are authenticated via the signature check.
+
+- No `--enable-unsafe-all-submit` flag needed
+- Example config: `docs/examples/Settings.forge-gitlab-production.toml`
+- See [Reverse proxy examples](#reverse-proxy-examples) for nginx/Caddy setup
+
+### Tunnel-based (ngrok, Cloudflare Tunnel, SSH)
+
+```
+GitLab.com ──HTTPS──▶ tunnel ──HTTP──▶ Sashiko :8080 (localhost)
+```
+
+The tunnel terminates locally, so requests appear as localhost. A
+`webhook_secret` is still recommended because tunnel URLs can be discovered.
+
+- Uses the same config as the public server topology
+- No `--enable-unsafe-all-submit` flag needed
+
+### Self-hosted forge on same LAN
+
+```
+GitLab (internal) ──HTTP──▶ Sashiko :8080 (LAN-accessible)
+```
+
+Sashiko binds to all interfaces (`host = "::"`). Configure `webhook_secret`
+— requests from the GitLab server's IP are authenticated via the signature
+check.
+
+- No `--enable-unsafe-all-submit` flag needed
+- Example config: `docs/examples/Settings.forge-selfhosted.toml`
+
+### Script-based polling (cronjob + curl)
+
+```
+cron ──▶ poll GitLab API ──▶ curl POST ──▶ Sashiko :8080 (localhost)
+```
+
+No forge webhook — a script polls the API and posts results to Sashiko via
+curl. The plain secret token method works well here (simple `-H` flag in
+curl). Requests are localhost, so the secret provides defense-in-depth.
+
+- Example config: `docs/examples/Settings.forge-gitlab-simple.toml`
+- See [Script-based setup](#script-based--cronjob-setup) for curl examples
+
+## Authentication methods
+
+| Method | Headers | Verifies | Providers | Recommendation |
+|--------|---------|----------|-----------|----------------|
+| HMAC signing token | `webhook-signature`, `webhook-id`, `webhook-timestamp` | Identity + integrity | GitLab 19.0+ | Recommended |
+| HMAC secret | `X-Hub-Signature-256` | Identity + integrity | GitHub | Recommended |
+| Plain secret token | `X-Gitlab-Token` | Identity only | GitLab (all versions) | Acceptable with HTTPS |
+| None | — | Nothing | — | Development/localhost only |
+
+**Signing tokens and HMAC secrets** verify both that the sender holds the
+configured key and that the payload has not been modified in transit.
+
+**Plain secret tokens** verify that the sender holds the configured key but
+do not independently verify payload integrity. Combine with HTTPS for
+transport protection.
+
+## GitLab signing token setup
+
+> Requires GitLab 19.0 or later.
+
+1. In GitLab, go to your project's **Settings > Webhooks > Add new webhook**
+2. Enter the **URL**: `https://sashiko.example.com/api/webhook/gitlab`
+3. Select **Generate signing token** — copy the token now (it is shown only once)
+4. Under **Trigger**, check **Merge request events**
+5. Ensure **Enable SSL verification** is checked
+6. Select **Add webhook**
+
+In Sashiko's `Settings.toml`:
+
+```toml
+[forge]
+enabled = true
+provider = "gitlab"
+webhook_secret = "whsec_YOUR_COPIED_TOKEN_HERE"
+```
+
+Restart Sashiko. Open or update a merge request to trigger a test delivery.
+Check Sashiko's logs for `"GitLab merge_request: ..."` to confirm receipt.
+
+For more details, see
+[GitLab's signing token documentation](https://docs.gitlab.com/ee/user/project/integrations/webhooks/#signing-tokens).
+
+## GitLab legacy secret token setup
+
+For GitLab versions before 19.0, or for simpler setups where HMAC signing
+is not needed:
+
+1. In GitLab, go to your project's **Settings > Webhooks > Add new webhook**
+2. Enter the **URL**: `https://sashiko.example.com/api/webhook/gitlab`
+3. In the **Secret token** field, enter a strong random value
+4. Under **Trigger**, check **Merge request events**
+5. Select **Add webhook**
+
+In Sashiko's `Settings.toml`, use the same value:
+
+```toml
+[forge]
+enabled = true
+provider = "gitlab"
+webhook_secret = "YOUR_SECRET_TOKEN_HERE"
+```
+
+> **Note:** The plain secret token verifies the sender's identity but does
+> not independently verify payload integrity. Use HTTPS between GitLab and
+> Sashiko for transport protection.
+
+**Migrating to a signing token:** Configure both a signing token and a
+secret token on the same webhook during migration. Sashiko checks for the
+signing token headers first and falls back to the legacy token. Once
+verified, remove the secret token from the webhook settings.
+
+## GitHub HMAC setup
+
+1. Generate a strong random secret:
+
+   ```bash
+   openssl rand -hex 32
+   ```
+
+2. In GitHub, go to your repository's **Settings > Webhooks > Add webhook**
+3. Enter the **Payload URL**: `https://sashiko.example.com/api/webhook/github`
+4. Set **Content type** to `application/json`
+5. Paste the generated secret into the **Secret** field
+6. Under **Which events**, select **Let me select individual events** and
+   check **Pull requests**
+7. Select **Add webhook**
+
+In Sashiko's `Settings.toml`, use the same secret:
+
+```toml
+[forge]
+enabled = true
+provider = "github"
+webhook_secret = "YOUR_HEX_SECRET_HERE"
+```
+
+For more details, see
+[GitHub's webhook validation documentation](https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries).
+
+## Script-based / cronjob setup
+
+For setups that poll the GitLab API via a script and post results to Sashiko
+via curl, the plain secret token is the simplest approach:
+
+```bash
+curl -X POST http://localhost:8080/api/webhook/gitlab \
+  -H "Content-Type: application/json" \
+  -H "X-Gitlab-Event: Merge Request Hook" \
+  -H "X-Gitlab-Token: your-secret-here" \
+  -d @payload.json
+```
+
+For HMAC signing from a shell script (stronger, but more complex):
+
+```bash
+SECRET="your-signing-key"
+BODY=$(cat payload.json)
+MSG_ID=$(uuidgen)
+TIMESTAMP=$(date +%s)
+
+SIG=$(printf '%s.%s.%s' "$MSG_ID" "$TIMESTAMP" "$BODY" \
+  | openssl dgst -sha256 -hmac "$SECRET" -binary | base64)
+
+curl -X POST http://localhost:8080/api/webhook/gitlab \
+  -H "Content-Type: application/json" \
+  -H "X-Gitlab-Event: Merge Request Hook" \
+  -H "webhook-id: $MSG_ID" \
+  -H "webhook-timestamp: $TIMESTAMP" \
+  -H "webhook-signature: v1,$SIG" \
+  -d "$BODY"
+```
+
+> **Note:** When using a `whsec_`-prefixed token, you must strip the prefix
+> and base64-decode the remainder to get the raw HMAC key. For shell scripts,
+> using a plain secret string is simpler.
+
+## Production deployment checklist
+
+- [ ] HTTPS with a valid TLS certificate (Let's Encrypt or CA-issued)
+- [ ] Reverse proxy (nginx/Caddy/Traefik) terminates TLS and forwards to
+      Sashiko on localhost
+- [ ] `webhook_secret` configured in `Settings.toml`
+- [ ] Signing token (not plain secret) for GitLab 19.0+
+- [ ] `Settings.toml` file permissions restricted (`chmod 600`)
+- [ ] Rate limiting configured at the reverse proxy level
+- [ ] Log rotation configured (logrotate or journald)
+- [ ] Firewall rules: only allow inbound on the HTTPS port from expected
+      source IPs
+- [ ] For GitLab: enable **SSL verification** in webhook settings
+- [ ] For GitLab administrators: consider enabling **Block requests to the
+      local network from webhooks** in Admin > Settings > Network
+
+## Reverse proxy examples
+
+### nginx
+
+```nginx
+server {
+    listen 443 ssl http2;
+    server_name sashiko.example.com;
+
+    ssl_certificate /etc/letsencrypt/live/sashiko.example.com/fullchain.pem;
+    ssl_certificate_key /etc/letsencrypt/live/sashiko.example.com/privkey.pem;
+    ssl_protocols TLSv1.2 TLSv1.3;
+
+    # Security headers
+    add_header Strict-Transport-Security "max-age=31536000" always;
+    add_header X-Frame-Options DENY;
+    add_header X-Content-Type-Options nosniff;
+
+    # Rate limiting for webhook endpoint
+    limit_req_zone $binary_remote_addr zone=webhook:10m rate=10r/m;
+
+    location /api/webhook/ {
+        limit_req zone=webhook burst=5;
+        proxy_pass http://127.0.0.1:8080;
+        proxy_set_header Host $host;
+        proxy_set_header X-Real-IP $remote_addr;
+        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
+        proxy_set_header X-Forwarded-Proto $scheme;
+    }
+
+    location / {
+        proxy_pass http://127.0.0.1:8080;
+        proxy_set_header Host $host;
+        proxy_set_header X-Real-IP $remote_addr;
+    }
+}
+```
+
+> **Note:** Do not configure the proxy to modify the request body.
+> HMAC signatures are computed over the exact bytes sent by the forge.
+
+### Caddy
+
+```
+sashiko.example.com {
+    reverse_proxy localhost:8080
+}
+```
+
+Caddy automatically obtains and renews TLS certificates via Let's Encrypt.
+
+## Network security
+
+**GitLab.com IP ranges:** See
+[GitLab's IP range documentation](https://docs.gitlab.com/ee/user/gitlab_com/#ip-range)
+for the current list of IP addresses used by GitLab.com webhooks.
+
+**GitHub webhook IPs:** Query the
+[GitHub meta API](https://api.github.com/meta) and use the `hooks` array
+for the current webhook delivery IP ranges.
+
+**Self-hosted forges:** Configure your firewall to allow inbound traffic
+only from the forge server's IP address.
+
+**Private deployments:** Consider using a VPN or SSH tunnel instead of
+exposing Sashiko to the public internet. See
+[Tunnel-based topology](#tunnel-based-ngrok-cloudflare-tunnel-ssh) above.
+
+## Secret management
+
+**Environment variable override:** Set `SASHIKO__FORGE__WEBHOOK_SECRET` to
+override the value in `Settings.toml` without storing the secret on disk.
+
+**File permissions:** Restrict access to `Settings.toml` when it contains
+secrets:
+
+```bash
+chmod 600 Settings.toml
+```
+
+Run Sashiko as a dedicated service user with minimal privileges.
+
+**Token rotation:**
+
+1. Generate a new token in your forge's webhook settings
+2. Update `webhook_secret` in `Settings.toml` (or the environment variable)
+3. Restart Sashiko
+4. Verify a test webhook delivery succeeds
+5. Remove the old token from the forge webhook settings
+
+**Secret managers:** For production deployments, consider using a secret
+manager such as HashiCorp Vault, SOPS, or systemd's `LoadCredential` to
+inject the secret at runtime rather than storing it in a configuration file.
+
+## Troubleshooting
+
+### 401 Unauthorized
+
+The webhook secret is configured in Sashiko but the signature check failed.
+
+**Common causes:**
+
+- Secret mismatch — the value in `Settings.toml` does not match the value
+  configured in the forge's webhook settings
+- Wrong token type — using a `whsec_`-prefixed signing token with a forge
+  that sends a plain `X-Gitlab-Token` header, or vice versa
+- Trailing whitespace or newline in the TOML value
+- Base64 encoding error in a `whsec_` token (Sashiko falls back to using
+  the raw string as the key, which will not match)
+
+**To debug:** Run Sashiko with `RUST_LOG=debug` and check the logs for
+the specific verification failure. Also check the webhook delivery logs in
+the GitLab or GitHub UI for the request headers and response status.
+
+### 403 Forbidden
+
+The request was rejected because Sashiko is not configured to accept
+non-localhost requests.
+
+**Common causes:**
+
+- `webhook_secret` is not set in `Settings.toml`, and
+  `--enable-unsafe-all-submit` is not passed
+- The `[forge]` section is missing or `enabled` is `false`
+
+**Fix:** Configure `webhook_secret` (recommended) or pass
+`--enable-unsafe-all-submit` (not recommended for production).
+
+### 400 Bad Request
+
+The webhook payload failed validation.
+
+**Common causes:**
+
+- Invalid commit SHA format (must be 40 or 64 hex characters)
+- Repository URL uses an unrecognized scheme or targets a blocked address
+- PR/MR number is zero or negative
+- Wrong event type header (expected `pull_request` for GitHub or
+  `Merge Request Hook` for GitLab)
+
+### 413 Payload Too Large
+
+The request body exceeds the 2 MiB limit.
+
+## FAQ
+
+**Do I need webhook authentication if Sashiko only listens on localhost?**
+
+It is not required, but it provides defense-in-depth. Other processes
+running on the same host could send forged requests to the webhook endpoint.
+
+**Can I use the same secret for both GitHub and GitLab?**
+
+It is technically possible with a plain (non-`whsec_`-prefixed) secret, but
+using separate tokens per provider is recommended for isolation.
+
+**What happens if I configure a secret but the forge does not send one?**
+
+The request is rejected with 401 Unauthorized. Both sides must be configured
+with the same secret.
+
+**Will existing webhooks break when I add `webhook_secret` to Settings.toml?**
+
+Only if the forge is not configured to send the corresponding secret. Add
+the secret to both the forge settings and Sashiko's `Settings.toml` at the
+same time.
+
+**Do I still need `--enable-unsafe-all-submit`?**
+
+Not when `webhook_secret` is configured. The signature check authenticates
+non-localhost requests. The flag is only needed for unauthenticated setups.
+
+**What about replay attacks?**
+
+The current implementation does not validate the `webhook-timestamp` header
+for freshness. Use HTTPS to protect against network-level replay. Timestamp
+validation is planned as a future enhancement.
+
+**Port 8080 or 9080?**
+
+The default listening port is 8080 (`server.port` in Settings.toml).
+Configure it to any available port as needed.
+
+## See also
+
+- [Forge Setup Guide](FORGE_SETUP.md) — general forge integration architecture
+- [GitHub Setup Guide](GITHUB_SETUP.md) — GitHub-specific webhook configuration
+- [GitLab Setup Guide](GITLAB_SETUP.md) — GitLab-specific webhook configuration
+- [Configuration Reference](configuration.md) — all Settings.toml options
diff --git a/docs/configuration.md b/docs/configuration.md
index 5b5599574..b6022294c 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -15,6 +15,21 @@ the [LLM Provider Configuration Guide](llm-providers.md).
 
 ## Settings.toml sections
 
+### `[forge]`
+
+Optional. Controls forge (GitHub/GitLab) webhook integration.
+
+| Key | Type | Default | Description |
+|-----|------|---------|-------------|
+| `enabled` | bool | `false` | Enable forge webhook endpoint. |
+| `disable_nntp` | bool | `true` | Disable NNTP ingestion when forge is enabled. |
+| `provider` | string | -- | Forge provider: `"github"` or `"gitlab"`. |
+| `webhook_secret` | string | -- | Webhook signing token or shared secret for authenticating incoming requests. When configured, non-localhost requests are authenticated via signature verification. See the [Webhook Security Guide](WEBHOOK_SECURITY.md). |
+| `api_token` | string | -- | Forge API token (for future API-based features). |
+
+> **Security:** When `Settings.toml` contains secrets, restrict file
+> permissions: `chmod 600 Settings.toml`.
+
 ### `[database]`
 
 | Key | Type | Default | Description |
@@ -280,6 +295,7 @@ Downstream tools can parse this format with simple line splitting.
 | `CLOUD_ML_REGION` | GCP region for Vertex AI provider. |
 | `SASHIKO_SERVER` | Override daemon URL for CLI commands. |
 | `SASHIKO__*` | Override any Settings.toml value (e.g. `SASHIKO__AI__PROVIDER`). |
+| `SASHIKO__FORGE__WEBHOOK_SECRET` | Override webhook secret from Settings.toml. Avoids storing the secret on disk. |
 | `SASHIKO_PATCHWORK_TOKEN` | Patchwork API token. Fills in `patchwork.token` for enabled subsystems that have `api_url` set but no explicit token in TOML. |
 | `NO_COLOR` | Disable ANSI color output. |
 | `SASHIKO_LOG_PLAIN` | Use plain log format (no level/target/timestamp). |
diff --git a/docs/examples/Settings.forge-github.toml b/docs/examples/Settings.forge-github.toml
new file mode 100644
index 000000000..97de51ce9
--- /dev/null
+++ b/docs/examples/Settings.forge-github.toml
@@ -0,0 +1,15 @@
+# GitHub webhook integration with HMAC-SHA256 verification.
+# See docs/WEBHOOK_SECURITY.md for full setup guide.
+
+[forge]
+enabled = true
+provider = "github"
+# GitHub webhook secret for HMAC-SHA256 verification.
+# Generate: openssl rand -hex 32
+# Enter the same value in GitHub: repo Settings > Webhooks > Secret.
+# Verifies both sender identity and payload integrity.
+webhook_secret = "YOUR_HEX_SECRET_HERE"
+
+[server]
+host = "127.0.0.1"
+port = 8080
diff --git a/docs/examples/Settings.forge-gitlab-production.toml b/docs/examples/Settings.forge-gitlab-production.toml
new file mode 100644
index 000000000..3d78eb52b
--- /dev/null
+++ b/docs/examples/Settings.forge-gitlab-production.toml
@@ -0,0 +1,16 @@
+# GitLab webhook integration — production deployment with signing token.
+# Use with a reverse proxy (nginx/Caddy) that terminates TLS.
+# See docs/WEBHOOK_SECURITY.md for full setup guide.
+
+[forge]
+enabled = true
+provider = "gitlab"
+# GitLab 19.0+ signing token (recommended).
+# Generate in GitLab: Settings > Webhooks > Add new > Generate signing token.
+# Verifies both sender identity and payload integrity.
+webhook_secret = "whsec_YOUR_BASE64_SIGNING_TOKEN_HERE"
+# api_token = "glpat-..."  # Optional: for future API-based features
+
+[server]
+host = "127.0.0.1"   # Bind to localhost; reverse proxy handles public traffic
+port = 8080
diff --git a/docs/examples/Settings.forge-gitlab-simple.toml b/docs/examples/Settings.forge-gitlab-simple.toml
new file mode 100644
index 000000000..886f90f8a
--- /dev/null
+++ b/docs/examples/Settings.forge-gitlab-simple.toml
@@ -0,0 +1,15 @@
+# GitLab webhook integration — simple setup for scripts and cronjobs.
+# Use when polling GitLab API and posting results to Sashiko via curl.
+# See docs/WEBHOOK_SECURITY.md for full setup guide.
+
+[forge]
+enabled = true
+provider = "gitlab"
+# Plain secret token for script-based setups.
+# Use the same value in your curl script: -H "X-Gitlab-Token: YOUR_SECRET"
+# Verifies sender identity. Combine with HTTPS for transport protection.
+webhook_secret = "YOUR_SECRET_HERE"
+
+[server]
+host = "127.0.0.1"
+port = 8080
diff --git a/docs/examples/Settings.forge-selfhosted.toml b/docs/examples/Settings.forge-selfhosted.toml
new file mode 100644
index 000000000..07b5debb1
--- /dev/null
+++ b/docs/examples/Settings.forge-selfhosted.toml
@@ -0,0 +1,14 @@
+# Self-hosted GitLab on the same LAN as Sashiko.
+# No reverse proxy needed if both are on a trusted private network.
+# See docs/WEBHOOK_SECURITY.md for full setup guide.
+
+[forge]
+enabled = true
+provider = "gitlab"
+# When webhook_secret is configured, Sashiko authenticates requests
+# from any address. No --enable-unsafe-all-submit needed.
+webhook_secret = "whsec_YOUR_BASE64_SIGNING_TOKEN_HERE"
+
+[server]
+host = "::"      # Bind to all interfaces (accessible from LAN)
+port = 8080

From 2901b29fa5ccf3ec1e20ecf14cd62c7fd8d66b70 Mon Sep 17 00:00:00 2001
From: derekbarbosa <derekasobrab@gmail.com>
Date: Mon, 6 Jul 2026 21:13:19 -0400
Subject: [PATCH 4/5] docs: add webhook input hardening design document

Add designs/DESIGN_WEBHOOK_INPUT_HARDENING.md per repository
convention for auditability. Covers threat model, HMAC signature
verification architecture, access control revision, body size limits,
input validation, startup warnings, configuration, deployment
topologies, and known risks.

Assisted-by: claude-opus-4.6 <noreply@anthropic.com>
Signed-off-by: derekbarbosa <derekasobrab@gmail.com>
---
 designs/DESIGN_WEBHOOK_INPUT_HARDENING.md | 148 ++++++++++++++++++++++
 1 file changed, 148 insertions(+)
 create mode 100644 designs/DESIGN_WEBHOOK_INPUT_HARDENING.md

diff --git a/designs/DESIGN_WEBHOOK_INPUT_HARDENING.md b/designs/DESIGN_WEBHOOK_INPUT_HARDENING.md
new file mode 100644
index 000000000..2463414bb
--- /dev/null
+++ b/designs/DESIGN_WEBHOOK_INPUT_HARDENING.md
@@ -0,0 +1,148 @@
+# Design: Webhook & Ingestion Input Hardening
+
+## Goal
+Harden all HTTP ingestion endpoints against untrusted input. Forge
+webhook and submit API endpoints accept external data that flows into
+git commands, database queries, and the review pipeline. This design
+addresses four related security gaps that share the same attack
+surface.
+
+## Threat Model
+
+When Sashiko accepts unauthenticated webhook requests, an attacker
+who can reach the endpoint can:
+
+1. Forge webhooks to trigger expensive AI reviews (cost amplification)
+2. Inject arbitrary repository URLs for Sashiko to clone (SSRF risk)
+3. Exhaust server memory via oversized payloads or gzip bombs
+4. Pass malformed commit SHAs to git commands
+5. Pollute the review database with fake patchsets
+
+## Architecture Changes
+
+### 1. Webhook Signature Verification (`src/forge.rs`)
+
+The `ForgeProvider` trait gains body and secret parameters:
+
+```rust
+fn validate_event(
+    &self,
+    headers: &HeaderMap,
+    body: &Bytes,
+    secret: Option<&str>,
+) -> Result<(), StatusCode>;
+```
+
+Three verification methods, selected by header presence:
+
+- **Standard Webhooks HMAC-SHA256** (GitLab 19.0+ signing token):
+  verifies `webhook-signature` header over
+  `{webhook-id}.{webhook-timestamp}.{body}`.
+- **GitHub HMAC-SHA256**: verifies `X-Hub-Signature-256` header.
+- **Legacy secret token**: constant-time comparison of
+  `X-Gitlab-Token` header.
+
+Secret type auto-detection: `whsec_` prefix indicates a Standard
+Webhooks signing token (base64-decoded); plain strings are used as
+raw key bytes. A warning is logged if base64 decoding fails for a
+`whsec_`-prefixed token.
+
+All comparisons use the `subtle` crate for constant-time operations.
+
+### 2. Access Control Revision (`src/api.rs`)
+
+When `webhook_secret` is configured, non-localhost requests are
+permitted because the signature check is the access control. The
+`--enable-unsafe-all-submit` flag is only needed for unauthenticated
+setups.
+
+```rust
+let is_loopback = addr.ip().to_canonical().is_loopback();
+let has_secret = webhook_secret.is_some();
+if !is_loopback && !has_secret && !state.allow_all_submit {
+    return Err(StatusCode::FORBIDDEN);
+}
+```
+
+### 3. Body Size Limits (`src/api.rs`)
+
+- Explicit `DefaultBodyLimit::max(2 MiB)` on the axum router (makes
+  the implicit axum 0.8 default auditable).
+- HTTP download cap (10 MiB) on `fetch_and_inject_thread` for
+  lore.kernel.org mbox fetches.
+- Decompression cap (50 MiB) via `Read::take()` on the gzip decoder
+  to prevent decompression bombs.
+
+### 4. Input Validation (`src/forge.rs`)
+
+Validation functions applied in both forge `parse_payload` methods:
+
+- `is_valid_git_sha(s)`: accepts 40-char SHA-1 or 64-char SHA-256,
+  hex digits only.
+- `is_safe_repo_url(url)`: requires `https://`, `http://`, or `git@`
+  scheme; rejects known SSRF targets (cloud metadata endpoints,
+  loopback addresses).
+- `pr_number > 0` check on both forge providers.
+
+These validations are applied only in the forge webhook path, not in
+the CLI submit path, because the CLI legitimately sends git refs and
+local filesystem paths.
+
+Message-ID path separator sanitization (`/` and `\`) is applied in
+the Thread submit handler to prevent URL path manipulation when
+constructing lore.kernel.org fetch URLs.
+
+### 5. Startup Warnings (`src/main.rs`)
+
+Two warning conditions at startup:
+
+- Forge enabled without `webhook_secret` and without
+  `--enable-unsafe-all-submit`: warns that non-localhost requests
+  will be rejected.
+- Forge enabled without `webhook_secret` but with
+  `--enable-unsafe-all-submit`: warns about accepting unauthenticated
+  requests.
+
+## Configuration
+
+```toml
+[forge]
+enabled = true
+provider = "gitlab"
+webhook_secret = "whsec_..."  # signing token (recommended)
+# OR
+webhook_secret = "my-secret"  # plain secret token
+```
+
+No new required config fields. The existing `webhook_secret` field
+(previously dead code) is activated.
+
+## Dependencies
+
+New crate dependencies:
+- `hmac = "0.13"` (HMAC-SHA256 computation)
+- `base64 = "0.22"` (signing token key decoding)
+- `subtle = "2"` (constant-time comparison)
+
+## Deployment Topologies
+
+The design supports four deployment patterns, documented in
+`docs/WEBHOOK_SECURITY.md`:
+
+1. Public server with reverse proxy (nginx/Caddy terminates TLS)
+2. Tunnel-based (ngrok, Cloudflare Tunnel, SSH)
+3. Self-hosted forge on same LAN
+4. Script-based polling (cronjob + curl)
+
+When `webhook_secret` is configured, topologies 1-3 do not require
+`--enable-unsafe-all-submit`.
+
+## Risks
+
+- SSRF blocklist is best-effort (DNS rebinding can bypass string
+  checks). The primary access control is signature verification.
+- `subtle::ConstantTimeEq` reveals length differences between
+  compared strings (acceptable for webhook secrets with sufficient
+  entropy).
+- No replay attack prevention via timestamp validation in this
+  version (documented as future enhancement).

From 918ff152fed84c764905cf7ed23fbf5124df464c Mon Sep 17 00:00:00 2001
From: derekbarbosa <derekasobrab@gmail.com>
Date: Wed, 8 Jul 2026 16:13:31 -0400
Subject: [PATCH 5/5] fix: address review findings on input hardening

- Return an explicit error when decompressed mbox hits the 50 MiB
  limit instead of silently truncating the stream
- Raise DefaultBodyLimit from 2 MiB to 25 MiB to accommodate large
  CLI mbox submissions and GitHub webhook payloads (up to 25 MB)
- Read decompressed data into Vec<u8> before UTF-8 conversion to
  avoid InvalidData errors when the byte limit splits a multi-byte
  character
- Extend SSRF blocklist with decimal (2130706433) and hex
  (0x7f000001) representations of loopback addresses
- Replace message-ID slash/backslash blocklist with percent-encoding
  via the percent-encoding crate, preserving RFC 5322 message-IDs
  that contain path-significant characters
- Replace full-URL string matching in is_safe_repo_url with parsed
  host-only checks via the url crate, eliminating false positives
  from blocklist patterns appearing in usernames or paths
- Restructure forge_webhook access control to make the security
  model explicit: when webhook_secret is configured, signature
  verification is the sole access control for ALL requests including
  loopback. The is_loopback bypass only applies when no secret is
  configured. This is critical for reverse proxy deployments where
  all traffic arrives from loopback.

Assisted-by: gemini-3.1-pro
Signed-off-by: derekbarbosa <derekasobrab@gmail.com>
---
 Cargo.lock                                |  2 +
 Cargo.toml                                |  2 +
 designs/DESIGN_WEBHOOK_INPUT_HARDENING.md |  5 +-
 docs/WEBHOOK_SECURITY.md                  |  2 +-
 src/api.rs                                | 73 ++++++++++++++--------
 src/forge.rs                              | 74 ++++++++++++++++++-----
 6 files changed, 115 insertions(+), 43 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 7904c4c76..3b5b2f7f4 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3410,6 +3410,7 @@ dependencies = [
  "lettre",
  "libsql",
  "mail-parser",
+ "percent-encoding",
  "regex",
  "reqwest",
  "serde",
@@ -3428,6 +3429,7 @@ dependencies = [
  "tracing-subscriber",
  "tree-sitter",
  "tree-sitter-c",
+ "url",
 ]
 
 [[package]]
diff --git a/Cargo.toml b/Cargo.toml
index ad6d6fcc5..82fea3cc9 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -67,3 +67,5 @@ tree-sitter-c = "0.24.2"
 hmac = "0.13"
 base64 = "0.22"
 subtle = "2"
+url = "2.5.8"
+percent-encoding = "2.3.2"
diff --git a/designs/DESIGN_WEBHOOK_INPUT_HARDENING.md b/designs/DESIGN_WEBHOOK_INPUT_HARDENING.md
index 2463414bb..3602cd28d 100644
--- a/designs/DESIGN_WEBHOOK_INPUT_HARDENING.md
+++ b/designs/DESIGN_WEBHOOK_INPUT_HARDENING.md
@@ -66,8 +66,9 @@ if !is_loopback && !has_secret && !state.allow_all_submit {
 
 ### 3. Body Size Limits (`src/api.rs`)
 
-- Explicit `DefaultBodyLimit::max(2 MiB)` on the axum router (makes
-  the implicit axum 0.8 default auditable).
+- Explicit `DefaultBodyLimit::max(25 MiB)` on the axum router.
+  Sized to accommodate large CLI mbox submissions and GitHub
+  webhook payloads (up to 25 MB).
 - HTTP download cap (10 MiB) on `fetch_and_inject_thread` for
   lore.kernel.org mbox fetches.
 - Decompression cap (50 MiB) via `Read::take()` on the gzip decoder
diff --git a/docs/WEBHOOK_SECURITY.md b/docs/WEBHOOK_SECURITY.md
index 3fa1868f6..a60c47b8d 100644
--- a/docs/WEBHOOK_SECURITY.md
+++ b/docs/WEBHOOK_SECURITY.md
@@ -361,7 +361,7 @@ The webhook payload failed validation.
 
 ### 413 Payload Too Large
 
-The request body exceeds the 2 MiB limit.
+The request body exceeds the 25 MiB limit.
 
 ## FAQ
 
diff --git a/src/api.rs b/src/api.rs
index db29bb748..99d6620c4 100644
--- a/src/api.rs
+++ b/src/api.rs
@@ -309,7 +309,7 @@ pub fn build_router(
         .route("/", get_service(ServeFile::new("static/index.html")))
         .nest_service("/static", ServeDir::new("static"))
         .layer(middleware::from_fn(redirect_www))
-        .layer(axum::extract::DefaultBodyLimit::max(2 * 1024 * 1024))
+        .layer(axum::extract::DefaultBodyLimit::max(25 * 1024 * 1024))
         .with_state(state)
 }
 
@@ -472,14 +472,22 @@ async fn submit_patch(
         }
         SubmitRequest::Thread { msgid } => {
             let id = generate_synthetic_id("thread");
-            let clean_msgid = msgid.trim_matches(|c| c == '<' || c == '>').to_string();
-
-            // Reject message-IDs with path separators to prevent URL path traversal
-            // when constructing the lore.kernel.org fetch URL. Note: ".." is valid
-            // in RFC 5322 local-parts and is not rejected here.
-            if clean_msgid.contains('/') || clean_msgid.contains('\\') {
-                return Err(StatusCode::BAD_REQUEST);
-            }
+            // Percent-encode path-significant characters in the message-ID
+            // for safe inclusion in the lore.kernel.org fetch URL. This
+            // handles RFC 5322 message-IDs that contain `/` or other
+            // path-sensitive characters without rejecting them.
+            const PATH_SEGMENT_ENCODE: &percent_encoding::AsciiSet = &percent_encoding::CONTROLS
+                .add(b'/')
+                .add(b'\\')
+                .add(b'?')
+                .add(b'#')
+                .add(b' ')
+                .add(b'%');
+            let clean_msgid = percent_encoding::utf8_percent_encode(
+                msgid.trim_matches(|c| c == '<' || c == '>'),
+                PATH_SEGMENT_ENCODE,
+            )
+            .to_string();
             info!(
                 "Received thread fetch request: {} (msgid: {})",
                 id, clean_msgid
@@ -556,14 +564,23 @@ async fn fetch_and_inject_thread(
         .into());
     }
 
-    // Decompress the gzip data using a blocking task to avoid blocking the async runtime
+    // Decompress into bytes first, then convert to UTF-8. Reading directly
+    // into a String via read_to_string would produce an InvalidData error
+    // if the byte limit splits a multi-byte UTF-8 character.
     let raw = tokio::task::spawn_blocking(move || -> Result<String, std::io::Error> {
         use std::io::Read;
         let decoder = flate2::read::GzDecoder::new(&bytes[..]);
         let mut limited = decoder.take(MAX_MBOX_DECOMPRESSED);
-        let mut raw = String::new();
-        limited.read_to_string(&mut raw)?;
-        Ok(raw)
+        let mut raw_bytes = Vec::new();
+        limited.read_to_end(&mut raw_bytes)?;
+        if raw_bytes.len() as u64 >= MAX_MBOX_DECOMPRESSED {
+            return Err(std::io::Error::other(format!(
+                "Decompressed mbox exceeds {} byte limit",
+                MAX_MBOX_DECOMPRESSED
+            )));
+        }
+        String::from_utf8(raw_bytes)
+            .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))
     })
     .await??;
 
@@ -1118,20 +1135,28 @@ async fn forge_webhook(
         return Err(StatusCode::FORBIDDEN);
     }
 
-    let is_loopback = addr.ip().to_canonical().is_loopback();
     let webhook_secret = state.settings.forge.webhook_secret.as_deref();
     let has_secret = webhook_secret.is_some();
 
-    // When webhook_secret is configured, the signature check in
-    // validate_event is the access control — no need for
-    // --enable-unsafe-all-submit. Without a secret, fall back to the
-    // existing localhost-only or explicit flag behavior.
-    if !is_loopback && !has_secret && !state.allow_all_submit {
-        info!(
-            "Refused {} webhook from {}: configure webhook_secret or use --enable-unsafe-all-submit",
-            provider, addr
-        );
-        return Err(StatusCode::FORBIDDEN);
+    // Access control: when webhook_secret is configured, signature
+    // verification in validate_event is the sole access control for ALL
+    // requests. This is critical for reverse proxy deployments where all
+    // traffic arrives from loopback — the signature check cannot be
+    // bypassed by source IP.
+    //
+    // When no secret is configured, fall back to localhost-only or the
+    // explicit --enable-unsafe-all-submit flag. Note: this is insecure
+    // behind a reverse proxy; operators MUST configure webhook_secret
+    // for proxied deployments.
+    if !has_secret {
+        let is_loopback = addr.ip().to_canonical().is_loopback();
+        if !is_loopback && !state.allow_all_submit {
+            info!(
+                "Refused {} webhook from {}: configure webhook_secret or use --enable-unsafe-all-submit",
+                provider, addr
+            );
+            return Err(StatusCode::FORBIDDEN);
+        }
     }
 
     let forge = state.forge_registry.get(&provider).ok_or_else(|| {
diff --git a/src/forge.rs b/src/forge.rs
index 75027d506..68246ce19 100644
--- a/src/forge.rs
+++ b/src/forge.rs
@@ -35,23 +35,37 @@ pub fn is_valid_repo_url(url: &str) -> bool {
 }
 
 /// Check that a repository URL does not target known internal or metadata
-/// endpoints. Returns false for cloud metadata services, loopback addresses,
-/// and other destinations that should never be cloned.
+/// endpoints. Parses the URL and checks the host component only, avoiding
+/// false positives from blocklist patterns appearing in usernames or paths.
 ///
 /// This is a best-effort blocklist, not a complete SSRF mitigation. DNS
-/// rebinding and URL encoding can bypass string-based checks. The primary
-/// access control is webhook signature verification.
+/// rebinding can bypass host-based checks. The primary access control
+/// is webhook signature verification.
 pub fn is_safe_repo_url(url: &str) -> bool {
     if !is_valid_repo_url(url) {
         return false;
     }
-    let lower = url.to_lowercase();
-    !lower.contains("169.254.")
-        && !lower.contains("metadata.google")
-        && !lower.contains("localhost")
-        && !lower.contains("127.0.0.1")
-        && !lower.contains("[::1]")
-        && !lower.contains("0.0.0.0")
+    // For git@ URLs, extract the host between @ and :
+    let host = if let Some(rest) = url.strip_prefix("git@") {
+        rest.split(':').next().unwrap_or("").to_ascii_lowercase()
+    } else if let Ok(parsed) = url::Url::parse(url) {
+        parsed.host_str().unwrap_or("").to_ascii_lowercase()
+    } else {
+        return false;
+    };
+
+    // Blocklist applied to the host component only
+    !host.contains("169.254.")
+        && !host.contains("metadata.google")
+        && !host.starts_with("localhost")
+        && !host.starts_with("127.")
+        && host != "[::1]"
+        && host != "0.0.0.0"
+        // Decimal IP for 127.0.0.1 = 2130706433
+        && host != "2130706433"
+        // Hex and octal IP representations
+        && !host.starts_with("0x7f")
+        && !host.starts_with("0177")
 }
 
 /// Decode a webhook secret. If prefixed with "whsec_", strip the prefix
@@ -106,10 +120,12 @@ fn verify_standard_webhook_signature(
 }
 
 /// Verify a GitHub HMAC-SHA256 signature. The header value has the format
-/// "sha256={hex_digest}".
+/// "sha256={hex_digest}". GitHub sends lowercase hex per their documentation.
+/// The received hex is normalized to lowercase before comparison for
+/// robustness against forges that may use uppercase.
 fn verify_github_signature(secret: &str, body: &[u8], signature_header: &str) -> bool {
     let hex_sig = match signature_header.strip_prefix("sha256=") {
-        Some(s) => s,
+        Some(s) => s.to_ascii_lowercase(),
         None => return false,
     };
     let mut mac = match HmacSha256::new_from_slice(secret.as_bytes()) {
@@ -118,7 +134,11 @@ fn verify_github_signature(secret: &str, body: &[u8], signature_header: &str) ->
     };
     mac.update(body);
     let result = mac.finalize().into_bytes();
-    let computed: String = result.iter().map(|b| format!("{:02x}", b)).collect();
+    let mut computed = String::with_capacity(64);
+    for b in result {
+        use std::fmt::Write;
+        let _ = write!(computed, "{:02x}", b);
+    }
     computed.as_bytes().ct_eq(hex_sig.as_bytes()).into()
 }
 
@@ -146,8 +166,11 @@ pub trait ForgeProvider: Send + Sync {
     fn name(&self) -> &str;
 
     /// Validate webhook event type and verify signature when a secret is
-    /// configured. Returns `UNAUTHORIZED` if the signature is missing or
-    /// invalid, `BAD_REQUEST` if the event type is wrong.
+    /// configured. When `secret` is `None`, only event-type validation is
+    /// performed and the request is treated as unauthenticated — callers
+    /// must enforce their own access control before calling this method.
+    /// Returns `UNAUTHORIZED` if the signature is missing or invalid,
+    /// `BAD_REQUEST` if the event type is wrong.
     fn validate_event(
         &self,
         headers: &HeaderMap,
@@ -487,9 +510,28 @@ mod tests {
         ));
         assert!(!is_safe_repo_url("http://metadata.google.internal/"));
         assert!(!is_safe_repo_url("http://localhost:5432/"));
+        assert!(!is_safe_repo_url("http://localhost.localdomain/repo"));
         assert!(!is_safe_repo_url("http://127.0.0.1:8080/repo"));
+        assert!(!is_safe_repo_url("http://127.1/repo"));
         assert!(!is_safe_repo_url("http://[::1]:8080/repo"));
         assert!(!is_safe_repo_url("http://0.0.0.0/repo"));
+        // Decimal and hex IP representations of 127.0.0.1
+        assert!(!is_safe_repo_url("http://2130706433/repo"));
+        assert!(!is_safe_repo_url("http://0x7f000001/repo"));
+        // Octal representation
+        assert!(!is_safe_repo_url("http://0177.0.0.1/repo"));
+    }
+
+    #[test]
+    fn test_is_safe_repo_url_no_false_positives_on_path() {
+        // Blocklist patterns in username or path should NOT trigger rejection
+        assert!(is_safe_repo_url(
+            "https://github.com/user-127.0.0.1/repo.git"
+        ));
+        assert!(is_safe_repo_url(
+            "https://github.com/org/localhost-tools.git"
+        ));
+        assert!(is_safe_repo_url("git@github.com:0x7f-labs/project.git"));
     }
 
     #[test]