diff --git a/server/src/api/lifecycle.py b/server/src/api/lifecycle.py index 49e91c077..d507cc9ac 100644 --- a/server/src/api/lifecycle.py +++ b/server/src/api/lifecycle.py @@ -429,11 +429,6 @@ async def proxy_sandbox_endpoint_request(request: Request, sandbox_id: str, port target_host = endpoint.endpoint query_string = request.url.query - target_url = ( - f"http://{target_host}/{full_path}?{query_string}" - if query_string - else f"http://{target_host}/{full_path}" - ) client: httpx.AsyncClient = request.app.state.http_client @@ -463,9 +458,10 @@ async def proxy_sandbox_endpoint_request(request: Request, sandbox_id: str, port req = client.build_request( method=request.method, - url=target_url, + url=f"http://{target_host}/{full_path}", + params=query_string if query_string else None, headers=headers, - content=request.stream(), + content=request.stream() if request.method in ("POST", "PUT", "PATCH", "DELETE") else None, ) resp = await client.send(req, stream=True) diff --git a/server/tests/test_routes_proxy.py b/server/tests/test_routes_proxy.py index 8bd042b03..cc6c183fb 100644 --- a/server/tests/test_routes_proxy.py +++ b/server/tests/test_routes_proxy.py @@ -39,10 +39,18 @@ def __init__(self): self.raise_connect_error = False self.raise_generic_error = False - def build_request(self, method: str, url: str, headers: dict, content): + def build_request( + self, + method: str, + url: str, + headers: dict, + content, + params: str | None = None, + ): self.built = { "method": method, "url": url, + "params": params, "headers": headers, "content": content, } @@ -103,7 +111,8 @@ def get_endpoint(sandbox_id: str, port: int, resolve_internal: bool = False) -> assert fake_client.built is not None assert fake_client.built["method"] == "POST" - assert fake_client.built["url"] == "http://10.57.1.91:40109/api/run?q=search" + assert fake_client.built["url"] == "http://10.57.1.91:40109/api/run" + assert fake_client.built["params"] == "q=search" forwarded_headers = fake_client.built["headers"] lowered_headers = {k.lower(): v for k, v in forwarded_headers.items()} assert "host" not in lowered_headers @@ -116,6 +125,87 @@ def get_endpoint(sandbox_id: str, port: int, resolve_internal: bool = False) -> assert lowered_headers.get("x-trace") == "trace-1" +def test_proxy_forwards_get_request_with_query_params( + client: TestClient, + auth_headers: dict, + monkeypatch, +) -> None: + """Test that GET requests with query parameters are forwarded correctly. + + This test verifies the fix for issue #484 where GET requests with query + parameters were failing with 400 MISSING_QUERY when using use_server_proxy. + The query string should be passed via httpx params, not embedded in URL. + """ + class StubService: + @staticmethod + def get_endpoint(sandbox_id: str, port: int, resolve_internal: bool = False) -> Endpoint: + assert sandbox_id == "sbx-123" + assert port == 44772 + assert resolve_internal is True + return Endpoint(endpoint="10.57.1.91:40109") + + monkeypatch.setattr(lifecycle, "sandbox_service", StubService()) + + fake_client = _FakeAsyncClient() + fake_client.response = _FakeStreamingResponse( + status_code=200, + headers={"content-type": "application/json"}, + chunks=[b'[{"name":"file.txt","size":100}]'], + ) + client.app.state.http_client = fake_client + + response = client.get( + "/v1/sandboxes/sbx-123/proxy/44772/files/search", + params={"path": "/workspace"}, + headers=auth_headers, + ) + + assert response.status_code == 200 + assert fake_client.built is not None + assert fake_client.built["method"] == "GET" + assert fake_client.built["url"] == "http://10.57.1.91:40109/files/search" + assert fake_client.built["params"] == "path=%2Fworkspace" + assert fake_client.built["content"] is None + + +def test_proxy_forwards_delete_request_with_body( + client: TestClient, + auth_headers: dict, + monkeypatch, +) -> None: + """Test that DELETE requests with body payload are forwarded correctly. + + This verifies that DELETE requests with JSON/body payload are not + incorrectly stripped when proxying. + """ + class StubService: + @staticmethod + def get_endpoint(sandbox_id: str, port: int, resolve_internal: bool = False) -> Endpoint: + return Endpoint(endpoint="10.57.1.91:40109") + + monkeypatch.setattr(lifecycle, "sandbox_service", StubService()) + + fake_client = _FakeAsyncClient() + fake_client.response = _FakeStreamingResponse( + status_code=200, + headers={"content-type": "application/json"}, + chunks=[b'{"deleted":true}'], + ) + client.app.state.http_client = fake_client + + response = client.request( + "DELETE", + "/v1/sandboxes/sbx-123/proxy/44772/resources", + headers=auth_headers, + content=b'{"id": "resource-123"}', + ) + + assert response.status_code == 200 + assert fake_client.built is not None + assert fake_client.built["method"] == "DELETE" + assert fake_client.built["content"] is not None + + def test_proxy_filters_response_hop_by_hop_headers( client: TestClient, auth_headers: dict,