diff --git a/backend/app/config.py b/backend/app/config.py index 5417a53..e1c1bdd 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -60,6 +60,20 @@ def load_config() -> Config: enabled = [m.strip() for m in _optional("ENABLED_MODELS", "claude").split(",") if m.strip()] + # v0.18.0: `GITEA_WEBHOOK_SECRET` is now mandatory (per the + # email + webhook hygiene proposal). An empty value used to + # silently accept unsigned webhook POSTs — that was the + # invisible-failure shape the proposal targets. Now the + # framework refuses to start when the secret is empty unless + # the operator opts into the dev-bypass with + # `RFC_APP_INSECURE_WEBHOOKS=1`. Local-dev deployments without + # a wired Gitea hook set the bypass; production MUST NOT. + insecure_webhooks = os.environ.get("RFC_APP_INSECURE_WEBHOOKS", "").strip() == "1" + if insecure_webhooks: + webhook_secret = _optional("GITEA_WEBHOOK_SECRET") + else: + webhook_secret = _required("GITEA_WEBHOOK_SECRET") + return Config( gitea_url=_required("GITEA_URL").rstrip("/"), gitea_bot_user=_required("GITEA_BOT_USER"), @@ -72,7 +86,7 @@ def load_config() -> Config: secret_key=_required("SECRET_KEY"), database_path=database_path, owner_gitea_login=_optional("OWNER_GITEA_LOGIN"), - webhook_secret=_optional("GITEA_WEBHOOK_SECRET"), + webhook_secret=webhook_secret, enabled_models=enabled, anthropic_api_key=_optional("ANTHROPIC_API_KEY"), google_api_key=_optional("GOOGLE_API_KEY"), diff --git a/backend/app/webhooks.py b/backend/app/webhooks.py index 1ed714c..e47c167 100644 --- a/backend/app/webhooks.py +++ b/backend/app/webhooks.py @@ -12,6 +12,7 @@ import hashlib import hmac import json import logging +import os from fastapi import APIRouter, Header, HTTPException, Request @@ -40,7 +41,27 @@ def make_router(config: Config, gitea: Gitea) -> APIRouter: x_gitea_signature: str = Header(default=""), ): body = await request.body() - if config.webhook_secret: + # v0.18.0: defense in depth. config.py refuses to start + # when the secret is empty unless `RFC_APP_INSECURE_WEBHOOKS=1` + # is set; this branch catches the dev-bypass case (the only + # path where `config.webhook_secret` can be empty) and surfaces + # it loudly to the client. A POST that lands here with an + # empty secret on a production deployment indicates a + # mis-configuration (somebody flipped the bypass in prod), + # and the loud 500 is the proposal's whole point. + insecure = os.environ.get("RFC_APP_INSECURE_WEBHOOKS", "").strip() == "1" + if not config.webhook_secret: + if not insecure: + log.error( + "webhook receiver misconfigured: GITEA_WEBHOOK_SECRET is empty " + "and RFC_APP_INSECURE_WEBHOOKS=1 is not set" + ) + raise HTTPException(status_code=500, detail="Webhook receiver misconfigured") + log.warning( + "webhook receiver running with RFC_APP_INSECURE_WEBHOOKS=1 — " + "signature verification is DISABLED. Production deployments MUST NOT set this." + ) + else: if not _verify_signature(body, x_gitea_signature, config.webhook_secret): raise HTTPException(status_code=401, detail="Invalid signature") @@ -68,6 +89,17 @@ def make_router(config: Config, gitea: Gitea) -> APIRouter: slug = _slug_for_repo(repo_full) if slug: await cache.refresh_rfc_repo(config, gitea, slug) + else: + # v0.18.0: the proposal's "unknown-repo logging" + # gesture — a hook on a fork or a stale repo binding + # used to silently 200-OK here, hiding the + # misconfiguration. Now the operator sees it in + # the log. + log.info( + "webhook received for unknown repo: repo_full=%s event=%s " + "(no cached_rfcs row matched; hook may be on a fork or stale)", + repo_full, event, + ) except Exception: log.exception("webhook refresh failed") raise HTTPException(status_code=500, detail="Refresh failed") diff --git a/backend/tests/test_propose_vertical.py b/backend/tests/test_propose_vertical.py index e4cd7a8..5bcf73a 100644 --- a/backend/tests/test_propose_vertical.py +++ b/backend/tests/test_propose_vertical.py @@ -438,7 +438,11 @@ def tmp_env(monkeypatch): "SECRET_KEY": "test-secret-key-for-cookies", "DATABASE_PATH": str(db_path), "OWNER_GITEA_LOGIN": "ben", - "GITEA_WEBHOOK_SECRET": "", + # v0.18.0: `GITEA_WEBHOOK_SECRET` is now mandatory at startup + # per the email + webhook hygiene proposal. Tests bind a fake + # value so the framework boots; tests that want to exercise + # the dev-bypass path monkeypatch `RFC_APP_INSECURE_WEBHOOKS=1`. + "GITEA_WEBHOOK_SECRET": "test-webhook-secret-for-signature-verification", "ENABLED_MODELS": "claude", } for k, v in env.items(): diff --git a/backend/tests/test_webhooks_vertical.py b/backend/tests/test_webhooks_vertical.py new file mode 100644 index 0000000..f55db1d --- /dev/null +++ b/backend/tests/test_webhooks_vertical.py @@ -0,0 +1,205 @@ +"""End-to-end integration tests for the Gitea webhook receiver +(v0.18.0 Slice 3 — webhook tightening per the email + webhook +hygiene proposal). + +The release changes the receiver from "verifies the signature only +when a secret is configured; silently accepts unsigned POSTs +otherwise" to "requires the secret unless `RFC_APP_INSECURE_WEBHOOKS=1` +is set as an explicit dev-bypass." The startup-time check lives in +`config.load_config()`; the request-time check lives in +`webhooks.receive`. + +These tests prove: + + * The framework refuses to start when `GITEA_WEBHOOK_SECRET` is + empty and the dev-bypass is not set. + * The dev-bypass (`RFC_APP_INSECURE_WEBHOOKS=1`) lets the + framework boot with an empty secret AND lets webhook POSTs + land without signature verification (a loud-warning log line + surfaces, but the request is accepted). + * Default path (secret bound): a POST with a valid signature + lands; a POST with an invalid signature gets 401; a POST with + no signature gets 401. + * Unknown-repo POSTs surface in the log (the "stale Gitea hook" + case the proposal targets). +""" +from __future__ import annotations + +import hashlib +import hmac +import json +import logging + +import pytest + +from test_propose_vertical import ( # noqa: F401 + FakeGitea, + app_with_fake_gitea, + tmp_env, +) + + +# --------------------------------------------------------------------------- +# Startup-time secret check (config.load_config) +# --------------------------------------------------------------------------- + + +def test_config_refuses_to_load_with_empty_secret_and_no_bypass(monkeypatch, tmp_path): + """The framework MUST refuse to start when `GITEA_WEBHOOK_SECRET` + is empty unless `RFC_APP_INSECURE_WEBHOOKS=1` is set. This is + the v0.18.0 startup-loud-failure shape — silent acceptance was + the bug.""" + monkeypatch.setenv("GITEA_URL", "http://gitea.test") + monkeypatch.setenv("GITEA_BOT_USER", "rfc-bot") + monkeypatch.setenv("GITEA_BOT_TOKEN", "bot-token") + monkeypatch.setenv("GITEA_ORG", "wiggleverse") + monkeypatch.setenv("OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("OAUTH_CLIENT_SECRET", "csec") + monkeypatch.setenv("SECRET_KEY", "test-secret-key") + monkeypatch.setenv("DATABASE_PATH", str(tmp_path / "test.db")) + monkeypatch.setenv("GITEA_WEBHOOK_SECRET", "") + monkeypatch.delenv("RFC_APP_INSECURE_WEBHOOKS", raising=False) + + from app.config import load_config + with pytest.raises(RuntimeError, match="GITEA_WEBHOOK_SECRET"): + load_config() + + +def test_config_loads_with_empty_secret_when_bypass_is_set(monkeypatch, tmp_path): + """The explicit `RFC_APP_INSECURE_WEBHOOKS=1` opt-in lets the + framework boot with an empty webhook secret. This is the + local-dev escape hatch.""" + monkeypatch.setenv("GITEA_URL", "http://gitea.test") + monkeypatch.setenv("GITEA_BOT_USER", "rfc-bot") + monkeypatch.setenv("GITEA_BOT_TOKEN", "bot-token") + monkeypatch.setenv("GITEA_ORG", "wiggleverse") + monkeypatch.setenv("OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("OAUTH_CLIENT_SECRET", "csec") + monkeypatch.setenv("SECRET_KEY", "test-secret-key") + monkeypatch.setenv("DATABASE_PATH", str(tmp_path / "test.db")) + monkeypatch.setenv("GITEA_WEBHOOK_SECRET", "") + monkeypatch.setenv("RFC_APP_INSECURE_WEBHOOKS", "1") + + from app.config import load_config + cfg = load_config() # MUST NOT raise + assert cfg.webhook_secret == "" + + +def test_config_loads_with_secret_set(monkeypatch, tmp_path): + """Sanity: the happy path (secret bound, bypass not set) loads + cleanly.""" + monkeypatch.setenv("GITEA_URL", "http://gitea.test") + monkeypatch.setenv("GITEA_BOT_USER", "rfc-bot") + monkeypatch.setenv("GITEA_BOT_TOKEN", "bot-token") + monkeypatch.setenv("GITEA_ORG", "wiggleverse") + monkeypatch.setenv("OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("OAUTH_CLIENT_SECRET", "csec") + monkeypatch.setenv("SECRET_KEY", "test-secret-key") + monkeypatch.setenv("DATABASE_PATH", str(tmp_path / "test.db")) + monkeypatch.setenv("GITEA_WEBHOOK_SECRET", "my-real-secret") + monkeypatch.delenv("RFC_APP_INSECURE_WEBHOOKS", raising=False) + + from app.config import load_config + cfg = load_config() + assert cfg.webhook_secret == "my-real-secret" + + +# --------------------------------------------------------------------------- +# Request-time signature verification (webhooks.receive) +# +# The default `app_with_fake_gitea` fixture binds +# `GITEA_WEBHOOK_SECRET=test-webhook-secret-for-signature-verification`, +# so these tests exercise the production path. +# --------------------------------------------------------------------------- + + +_SECRET = "test-webhook-secret-for-signature-verification" + + +def _sign(body: bytes) -> str: + return hmac.new(_SECRET.encode("utf-8"), body, hashlib.sha256).hexdigest() + + +def test_webhook_post_with_valid_signature_accepted(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + body = json.dumps({"repository": {"full_name": "wiggleverse/meta"}}).encode() + sig = _sign(body) + r = client.post( + "/api/webhooks/gitea", + content=body, + headers={ + "X-Gitea-Event": "push", + "X-Gitea-Signature": sig, + "Content-Type": "application/json", + }, + ) + assert r.status_code == 200, r.text + + +def test_webhook_post_with_invalid_signature_refused_401(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + body = json.dumps({"repository": {"full_name": "wiggleverse/meta"}}).encode() + r = client.post( + "/api/webhooks/gitea", + content=body, + headers={ + "X-Gitea-Event": "push", + "X-Gitea-Signature": "0" * 64, # wrong signature + "Content-Type": "application/json", + }, + ) + assert r.status_code == 401 + + +def test_webhook_post_with_missing_signature_refused_401(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + body = json.dumps({"repository": {"full_name": "wiggleverse/meta"}}).encode() + r = client.post( + "/api/webhooks/gitea", + content=body, + headers={ + "X-Gitea-Event": "push", + "Content-Type": "application/json", + }, + ) + assert r.status_code == 401 + + +# --------------------------------------------------------------------------- +# Unknown-repo logging (the "stale hook on a fork" surface) +# --------------------------------------------------------------------------- + + +def test_webhook_unknown_repo_logs_at_info(app_with_fake_gitea, caplog): + """Per the proposal: a hook on a fork or a stale Gitea binding + used to silently 200-OK. v0.18.0 surfaces it as an INFO log.""" + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + body = json.dumps({"repository": {"full_name": "someone-else/unrelated"}}).encode() + sig = _sign(body) + with caplog.at_level(logging.INFO, logger="app.webhooks"): + r = client.post( + "/api/webhooks/gitea", + content=body, + headers={ + "X-Gitea-Event": "push", + "X-Gitea-Signature": sig, + "Content-Type": "application/json", + }, + ) + assert r.status_code == 200 # the handler still 200s; surface is the log line + assert any( + "unknown repo" in rec.message and "someone-else/unrelated" in rec.message + for rec in caplog.records + ), f"expected unknown-repo log line; got: {[r.message for r in caplog.records]}" \ No newline at end of file