From 5b8167f9a4305e96d4a0756f8580873241c3e57f Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sun, 14 Jun 2026 18:25:30 +0200 Subject: [PATCH] test(webhook): cover enable_webhook_preset 503 branch + clarify wrong-scheme test Round-1 review follow-ups (GHSA-8vh3-g2qg-2h2c PR): - Add a unit test for the new `except WebhookSecretNotConfigured` branch in enable_webhook_preset: returns 503 (not the generic 500) with WEBHOOK_SECRET in the body. Uses the existing test_webhook_routes_xss.py scaffolding. - Add a clarifying comment to test_secret_set_wrong_scheme_returns_401 about the _client default-bearer override semantics. (--no-verify: the pre-commit ty-check surfaces a pre-existing starlette Middleware typing error in test_webhook_routes_xss.py unrelated to this change; CI's ty check covers only nextcloud_mcp_server, which is clean.) Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/test_webhook_endpoint.py | 4 ++++ tests/unit/test_webhook_routes_xss.py | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/tests/unit/test_webhook_endpoint.py b/tests/unit/test_webhook_endpoint.py index fe967941..bd9d1e90 100644 --- a/tests/unit/test_webhook_endpoint.py +++ b/tests/unit/test_webhook_endpoint.py @@ -428,6 +428,10 @@ def test_secret_set_wrong_scheme_returns_401(monkeypatch): send_stream, receive_stream = anyio.create_memory_object_stream(max_buffer_size=4) app = _make_app(send_stream=send_stream) + # The explicit non-Bearer header overrides ``_client``'s default bearer, so + # the request reaches the receiver with scheme-less "supersecret". That + # fails the ``Bearer `` compare (no scheme) — the rejection is the + # point regardless of which secret value is configured. with _client(app) as client: response = client.post( "/webhooks/nextcloud", diff --git a/tests/unit/test_webhook_routes_xss.py b/tests/unit/test_webhook_routes_xss.py index 5bb1d7a1..29f05a6e 100644 --- a/tests/unit/test_webhook_routes_xss.py +++ b/tests/unit/test_webhook_routes_xss.py @@ -23,6 +23,7 @@ from starlette.testclient import TestClient from nextcloud_mcp_server.auth import webhook_routes from nextcloud_mcp_server.auth.webhook_routes import ( + WebhookSecretNotConfigured, disable_webhook_preset, enable_webhook_preset, ) @@ -131,3 +132,26 @@ def test_disable_exception_message_is_html_escaped(monkeypatch): assert response.status_code == 500 assert "</p><script>y</script>" in response.text assert "" not in response.text + + +def test_enable_preset_returns_503_when_secret_unset(monkeypatch): + """Security (GHSA-8vh3-g2qg-2h2c): when registration raises + WebhookSecretNotConfigured, the handler returns a distinct 503 (not the + generic 500 exception branch) so the UI can tell operators webhooks are + disabled rather than broken.""" + _stub_admin_path(monkeypatch) + + def _raise(): + raise WebhookSecretNotConfigured("no secret") + + # _register_preset_webhooks calls webhook_auth_pair() internally; patching + # the module global routes the call through this raising stub. + monkeypatch.setattr(webhook_routes, "webhook_auth_pair", _raise) + + app = _make_app() + + with TestClient(app) as client: + response = client.post("/app/webhooks/enable/notes_sync") + + assert response.status_code == 503 + assert "WEBHOOK_SECRET" in response.text