fix: fail closed for webhook routes without secrets
Reject unsigned webhook requests when a route has no effective HMAC secret, even if the request handler is reached without the normal connect-time validation. Add regression coverage for the direct-handler path.
This commit is contained in:
@@ -379,9 +379,21 @@ class WebhookAdapter(BasePlatformAdapter):
|
|||||||
logger.error("[webhook] Failed to read body: %s", e)
|
logger.error("[webhook] Failed to read body: %s", e)
|
||||||
return web.json_response({"error": "Bad request"}, status=400)
|
return web.json_response({"error": "Bad request"}, status=400)
|
||||||
|
|
||||||
# Validate HMAC signature FIRST (skip for INSECURE_NO_AUTH testing mode)
|
# Validate HMAC signature FIRST (skip only for the explicit local-test
|
||||||
|
# INSECURE_NO_AUTH mode). Missing/empty secrets must fail closed here,
|
||||||
|
# not only during connect(), so direct handler reuse cannot turn a
|
||||||
|
# network webhook route into an unauthenticated agent-dispatch surface.
|
||||||
secret = route_config.get("secret", self._global_secret)
|
secret = route_config.get("secret", self._global_secret)
|
||||||
if secret and secret != _INSECURE_NO_AUTH:
|
if not secret:
|
||||||
|
logger.error(
|
||||||
|
"[webhook] Route %s has no HMAC secret; refusing request",
|
||||||
|
route_name,
|
||||||
|
)
|
||||||
|
return web.json_response(
|
||||||
|
{"error": "Webhook route is missing an HMAC secret"},
|
||||||
|
status=500,
|
||||||
|
)
|
||||||
|
if secret != _INSECURE_NO_AUTH:
|
||||||
if not self._validate_signature(request, raw_body, secret):
|
if not self._validate_signature(request, raw_body, secret):
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"[webhook] Invalid signature for route %s", route_name
|
"[webhook] Invalid signature for route %s", route_name
|
||||||
|
|||||||
@@ -498,6 +498,22 @@ class TestHTTPHandling:
|
|||||||
assert data["status"] == "accepted"
|
assert data["status"] == "accepted"
|
||||||
assert data["route"] == "test"
|
assert data["route"] == "test"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_route_without_secret_rejects_unsigned_request(self):
|
||||||
|
"""Missing HMAC secret must fail closed even if connect() was bypassed."""
|
||||||
|
routes = {"test": {"prompt": "hi"}}
|
||||||
|
adapter = _make_adapter(routes=routes, secret="")
|
||||||
|
adapter.handle_message = AsyncMock()
|
||||||
|
|
||||||
|
app = _create_app(adapter)
|
||||||
|
async with TestClient(TestServer(app)) as cli:
|
||||||
|
resp = await cli.post("/webhooks/test", json={"data": "value"})
|
||||||
|
assert resp.status == 500
|
||||||
|
data = await resp.json()
|
||||||
|
assert data["error"] == "Webhook route is missing an HMAC secret"
|
||||||
|
|
||||||
|
adapter.handle_message.assert_not_called()
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_health_endpoint(self):
|
async def test_health_endpoint(self):
|
||||||
"""GET /health returns 200 with status=ok."""
|
"""GET /health returns 200 with status=ok."""
|
||||||
|
|||||||
Reference in New Issue
Block a user