diff --git a/CHANGELOG.md b/CHANGELOG.md index 77612f86..1d0a37d3 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. - **Retrieval no longer consults the query cache.** Its key names no workspace, knowledge tier or document filter, so a hit would have served one workspace's results to another. The path was never reached (`_get_app_cache()` returns `None`), which is why it is 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."""