diff --git a/Cargo.lock b/Cargo.lock index 5263fbbbb..3b5b2f7f4 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,14 +3406,17 @@ dependencies = [ "flate2", "futures", "google-cloud-auth", + "hmac 0.13.0", "lettre", "libsql", "mail-parser", + "percent-encoding", "regex", "reqwest", "serde", "serde_json", "sha2 0.11.0", + "subtle", "tempfile", "termcolor", "thiserror 2.0.18", @@ -3425,6 +3429,7 @@ dependencies = [ "tracing-subscriber", "tree-sitter", "tree-sitter-c", + "url", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 8803d9178..82fea3cc9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,3 +64,8 @@ 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" +url = "2.5.8" +percent-encoding = "2.3.2" 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/designs/DESIGN_WEBHOOK_INPUT_HARDENING.md b/designs/DESIGN_WEBHOOK_INPUT_HARDENING.md new file mode 100644 index 000000000..3602cd28d --- /dev/null +++ b/designs/DESIGN_WEBHOOK_INPUT_HARDENING.md @@ -0,0 +1,149 @@ +# 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(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 + 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). 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..a60c47b8d --- /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 25 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 diff --git a/src/api.rs b/src/api.rs index a1944cfe0..99d6620c4 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(25 * 1024 * 1024)) .with_state(state) } @@ -471,7 +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(); + // 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 @@ -535,15 +551,36 @@ 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 + // 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 { use std::io::Read; - let mut decoder = flate2::read::GzDecoder::new(&bytes[..]); - let mut raw = String::new(); - decoder.read_to_string(&mut raw)?; - Ok(raw) + let decoder = flate2::read::GzDecoder::new(&bytes[..]); + let mut limited = decoder.take(MAX_MBOX_DECOMPRESSED); + 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??; @@ -1097,9 +1134,29 @@ 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); - return Err(StatusCode::FORBIDDEN); + + let webhook_secret = state.settings.forge.webhook_secret.as_deref(); + let has_secret = webhook_secret.is_some(); + + // 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(|| { @@ -1107,7 +1164,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 2d082a97e..68246ce19 100644 --- a/src/forge.rs +++ b/src/forge.rs @@ -14,9 +14,140 @@ 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; + +type HmacSha256 = Hmac; + +/// 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()) +} + +/// 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. 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 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; + } + // 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 +/// and base64-decode the remainder (Standard Webhooks convention). Otherwise +/// return the raw string bytes. +fn decode_webhook_secret(secret: &str) -> Vec { + 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}". 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.to_ascii_lowercase(), + 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 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() +} + +/// 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)] @@ -34,8 +165,18 @@ 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. 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, + body: &Bytes, + secret: Option<&str>, + ) -> Result<(), StatusCode>; /// Parse webhook payload and extract metadata fn parse_payload(&self, body: &Bytes) -> Result<(String, ForgeMetadata), StatusCode>; @@ -49,7 +190,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()) @@ -59,6 +205,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(()) } @@ -87,7 +243,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 +259,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, @@ -117,7 +286,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()) @@ -127,6 +301,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(()) } @@ -155,7 +358,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 +374,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 +452,434 @@ 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://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] + 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); + } + + // --- 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> { ); }); + // 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(