From 88b0484898446cb1707f3a6413297a17b48238c2 Mon Sep 17 00:00:00 2001 From: Daoyuan Li <94409450+DaoyuanLi2816@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:54:03 -0700 Subject: [PATCH] fix(channels): validate channel provider before resolving its config (#4100) `_provider_config` resolved a request-supplied provider name with an unallowlisted `getattr(config, provider, None)`. `ChannelConnectionsConfig` carries non-provider attributes -- the `enabled` and `require_bound_identity` bool fields plus the `provider_status` method -- so a name matching one of them returned that attribute instead of falling through to the intended 404. Callers (e.g. `POST /api/channels/{provider}/connect`, reachable by any authenticated user) then dereferenced the bool/method as a provider config, crashing with `AttributeError` -> HTTP 500. Validate `provider` against the `_PROVIDER_META` allowlist before the lookup, matching how `_credential_fields` / `_connect_instruction` / `_connect_url` already gate provider names, so unknown providers get a clean 404. Add a parametrized router regression test covering `enabled`, `require_bound_identity`, `provider_status`, and an unknown name. --- .../gateway/routers/channel_connections.py | 7 ++++++ .../tests/test_channel_connections_router.py | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/backend/app/gateway/routers/channel_connections.py b/backend/app/gateway/routers/channel_connections.py index ca26058e3..5fbb62b7d 100644 --- a/backend/app/gateway/routers/channel_connections.py +++ b/backend/app/gateway/routers/channel_connections.py @@ -201,6 +201,13 @@ def _get_repository(request: Request, config: ChannelConnectionsConfig) -> Chann def _provider_config(config: ChannelConnectionsConfig, provider: str): + # Resolve provider configs only for known providers. An unrestricted + # getattr would let a request-supplied name that happens to match another + # config attribute (e.g. the "enabled" / "require_bound_identity" bool + # fields) slip past the 404 and return a non-provider value, which callers + # then dereference as a provider config (AttributeError -> HTTP 500). + if provider not in _PROVIDER_META: + raise HTTPException(status_code=404, detail="Unknown channel provider") provider_config = getattr(config, provider, None) if provider_config is None: raise HTTPException(status_code=404, detail="Unknown channel provider") diff --git a/backend/tests/test_channel_connections_router.py b/backend/tests/test_channel_connections_router.py index 046568f10..7cc3f71b9 100644 --- a/backend/tests/test_channel_connections_router.py +++ b/backend/tests/test_channel_connections_router.py @@ -598,6 +598,29 @@ def test_connect_unconfigured_runtime_channel_returns_400(tmp_path): anyio.run(repo.close) +@pytest.mark.parametrize("provider", ["enabled", "require_bound_identity", "provider_status", "unknown_provider"]) +def test_connect_rejects_non_provider_config_attribute_with_404(tmp_path, provider): + import anyio + + # A request-supplied provider name that collides with a real (non-provider) + # ChannelConnectionsConfig attribute -- e.g. the "enabled" / + # "require_bound_identity" bool fields, or the "provider_status" method -- + # must resolve to the intended 404. Before the allowlist check, an + # unrestricted getattr returned that attribute instead of falling through to + # the 404, and the connect handler then dereferenced it as a provider config + # (AttributeError -> HTTP 500) for any authenticated user. + repo = anyio.run(_make_repo, tmp_path) + app = _make_app(_enabled_connections_config(), repo, _channels_config()) + + with TestClient(app, raise_server_exceptions=False) as client: + response = client.post(f"/api/channels/{provider}/connect") + + assert response.status_code == 404 + assert response.json()["detail"] == "Unknown channel provider" + + anyio.run(repo.close) + + def test_configure_provider_runtime_credentials_enables_connect_without_file_edits(tmp_path): import anyio