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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
4fc2b10945
commit
5b8167f9a4
@@ -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)
|
send_stream, receive_stream = anyio.create_memory_object_stream(max_buffer_size=4)
|
||||||
app = _make_app(send_stream=send_stream)
|
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 <secret>`` compare (no scheme) — the rejection is the
|
||||||
|
# point regardless of which secret value is configured.
|
||||||
with _client(app) as client:
|
with _client(app) as client:
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"/webhooks/nextcloud",
|
"/webhooks/nextcloud",
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ from starlette.testclient import TestClient
|
|||||||
|
|
||||||
from nextcloud_mcp_server.auth import webhook_routes
|
from nextcloud_mcp_server.auth import webhook_routes
|
||||||
from nextcloud_mcp_server.auth.webhook_routes import (
|
from nextcloud_mcp_server.auth.webhook_routes import (
|
||||||
|
WebhookSecretNotConfigured,
|
||||||
disable_webhook_preset,
|
disable_webhook_preset,
|
||||||
enable_webhook_preset,
|
enable_webhook_preset,
|
||||||
)
|
)
|
||||||
@@ -131,3 +132,26 @@ def test_disable_exception_message_is_html_escaped(monkeypatch):
|
|||||||
assert response.status_code == 500
|
assert response.status_code == 500
|
||||||
assert "</p><script>y</script>" in response.text
|
assert "</p><script>y</script>" in response.text
|
||||||
assert "<script>y</script>" not in response.text
|
assert "<script>y</script>" 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
|
||||||
|
|||||||
Reference in New Issue
Block a user