From 62ebaaea62b2aba06ba8d25bf7292ad3d3004771 Mon Sep 17 00:00:00 2001 From: jwvanderstam Date: Sun, 8 Nov 2026 22:56:01 +0100 Subject: [PATCH] fix(security): the webhook receiver answers every refusal with the same 403 POST /api/connectors/{id}/webhook is public, and answered 404 for an unknown connector and 400 for one of another type before checking the secret, so an anonymous caller could learn which connector ids exist and which are webhooks. A missing connector, another type, a connector without a secret and a wrong secret now all return the same 403. Found by the separate review of #528. The parametrised test fails on the old code for the missing and wrong-type cases. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KkWPqAcWbxfqz1aCP6Qsdx --- CHANGELOG.md | 4 ++++ src/routes_fastapi/connector_routes.py | 14 +++++++------ tests/integration/test_connector_routes.py | 21 +++++++++++-------- ...test_object_authorization_over_the_wire.py | 5 +++-- 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1840adf3..91263bbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ OpenAI, `https://api.openai.com/v1`) and drop any `provider/` prefix from `CLOUD - **`litellm` is gone from the image (QP-11, ADR-4).** It carried CVE-2026-114612, in a proxy server LocalChat never ran, and every fixed version pulls `boto3` into the image. The `pip-audit` suppression for it is removed; `SECURITY.md` item 12 is resolved. +- **The public webhook receiver no longer reveals which connector ids exist.** It answered + 404 for an unknown id and 400 for a connector of another type before checking the secret, + so an anonymous caller could probe for ids and for which were webhooks. Every refusal + before the secret is proven is now the same 403. Found by the separate review of #528. ### Changed diff --git a/src/routes_fastapi/connector_routes.py b/src/routes_fastapi/connector_routes.py index e6a9ed61..62f65559 100644 --- a/src/routes_fastapi/connector_routes.py +++ b/src/routes_fastapi/connector_routes.py @@ -237,11 +237,13 @@ async def receive_webhook(connector_id: str, request: Request, connectors: Conne # No user session here: this is a public receiver, and the connector id plus its # secret is the whole credential. ALL_WORKSPACES says that rather than relying on # an omitted argument to mean it. + # One answer for every refusal before the secret is proven: a missing connector, one + # of another type, and a wrong secret all read as 403, so an anonymous caller cannot + # learn which ids exist or which of them are webhooks. + forbidden = JSONResponse({"success": False, "message": "Forbidden"}, status_code=403) connector = connectors.get(connector_id, ALL_WORKSPACES) - if connector is None: - return JSONResponse({"success": False, "message": _NOT_FOUND}, status_code=404) - if connector.get("connector_type") != "webhook": - return JSONResponse({"success": False, "message": "Not a webhook connector"}, status_code=400) + if connector is None or connector.get("connector_type") != "webhook": + return forbidden # Mandatory, not "checked when present". This endpoint is public — it has to be, # since the caller is an external system with no session — so the secret is the @@ -254,11 +256,11 @@ async def receive_webhook(connector_id: str, request: Request, connectors: Conne "[Webhook] Connector has no secret configured; refusing the delivery. " "Set one on the connector." ) - return JSONResponse({"success": False, "message": "Forbidden"}, status_code=403) + return forbidden provided = request.headers.get("X-LocalChat-Secret", "") if not hmac.compare_digest(provided, secret): logger.warning("[Webhook] Bad secret for connector") - return JSONResponse({"success": False, "message": "Forbidden"}, status_code=403) + return forbidden instance = request.app.state.connector_registry.get(connector_id) if instance is None: diff --git a/tests/integration/test_connector_routes.py b/tests/integration/test_connector_routes.py index 247bbb5e..57fe0010 100644 --- a/tests/integration/test_connector_routes.py +++ b/tests/integration/test_connector_routes.py @@ -315,15 +315,18 @@ def test_a_connector_without_a_secret_accepts_nothing(self, client, app): assert resp.status_code == 403 assert not app.state.connector_registry.get.called - def test_wrong_connector_type_returns_400(self, client, app): - app.state.db.connectors.get = MagicMock(return_value=_connector(connector_type="local_folder")) - resp = client.post("/api/connectors/conn-1/webhook", json={}) - assert resp.status_code == 400 - - def test_connector_not_found_returns_404(self, client, app): - app.state.db.connectors.get = MagicMock(return_value=None) - resp = client.post("/api/connectors/missing/webhook", json={}) - assert resp.status_code == 404 + @pytest.mark.parametrize("found", [None, "local_folder", "wrong-secret"]) + def test_every_refusal_before_the_secret_reads_the_same(self, client, app, found): + """A missing connector, another type, and a wrong secret are indistinguishable, so an + anonymous caller learns neither which ids exist nor which are webhooks.""" + connector = None if found is None else _connector( + connector_type="webhook" if found == "wrong-secret" else found) + if connector is not None: + connector["config"] = {"secret": "correct-secret"} + app.state.db.connectors.get = MagicMock(return_value=connector) + resp = client.post("/api/connectors/conn-1/webhook", json={}, + headers={"X-LocalChat-Secret": "wrong-secret"}) + assert (resp.status_code, resp.json()) == (403, {"success": False, "message": "Forbidden"}) def test_invalid_payload_returns_400(self, client, app): app.state.db.connectors.get = MagicMock(return_value=_webhook_connector()) diff --git a/tests/integration/test_object_authorization_over_the_wire.py b/tests/integration/test_object_authorization_over_the_wire.py index 99d25c73..5bcbf24b 100644 --- a/tests/integration/test_object_authorization_over_the_wire.py +++ b/tests/integration/test_object_authorization_over_the_wire.py @@ -633,11 +633,12 @@ class TestThePublicWebhookReceiver: _PATH = "/api/connectors/{connector_id}/webhook" - def test_an_unknown_connector_is_not_found(self, provisioned: dict[str, Any]) -> None: + def test_an_unknown_connector_reads_like_a_wrong_secret(self, provisioned: dict[str, Any]) -> None: + """A 404 here told an anonymous caller which connector ids exist.""" response = provisioned["caller"].post( self._PATH.replace("{connector_id}", _DUMMY_GLOBAL_ID), json={}, timeout=15.0 ) - assert response.status_code == 404, response.text[:300] + assert response.status_code == 403, response.text[:300] def test_a_delivery_without_the_secret_is_refused(self, provisioned: dict[str, Any]) -> None: """No `X-LocalChat-Secret`, so `compare_digest` fails and the delivery is refused."""