From e76ef652c59390e5ee74448d24769f9f454baee5 Mon Sep 17 00:00:00 2001 From: "Joseph T. French" Date: Sat, 7 Nov 2026 19:27:55 -0600 Subject: [PATCH] fix(files): keep the signed Host header when s3_endpoint_url rewrites the upload URL With s3_endpoint_url set, FileClient.upload rewrote the presigned URL's host and sent the request with the new one. SigV4 binds the Host header, so the S3 server answered 403 SignatureDoesNotMatch; LocalStack's skipped signature checks had hidden it. The connection now goes to the override while the request carries the Host the URL was signed for. --- robosystems_client/clients/README.md | 2 +- robosystems_client/clients/facade.py | 3 +- robosystems_client/clients/file_client.py | 11 ++++-- tests/test_file_client.py | 48 ++++++++++++++++++++--- 4 files changed, 53 insertions(+), 11 deletions(-) diff --git a/robosystems_client/clients/README.md b/robosystems_client/clients/README.md index 4e0e21b9..8cf3745b 100644 --- a/robosystems_client/clients/README.md +++ b/robosystems_client/clients/README.md @@ -68,7 +68,7 @@ The package reads no environment variables on its own. `extract_token_from_envir | `max_retries` | `5` | Replays of a `429` response; reconnect attempts for `create_sse_client()` | | `retry_delay` | `1000` | Base backoff in milliseconds for both of the above | | `timeout` | `30` | Seconds, for ledger, investor and library GraphQL reads and `create_sse_client()`. Other REST calls set no client timeout | -| `s3_endpoint_url` | `None` | Rewrites the host of file upload URLs, for example for LocalStack | +| `s3_endpoint_url` | `None` | Where the client connects for file uploads when it reaches S3 by a different name than the API signed for (a container reaching a dev stack through `host.docker.internal`). The signed `Host` header is kept, as SigV4 requires | | `token_provider` | `None` | Callable returning the current credential | ## Cypher queries diff --git a/robosystems_client/clients/facade.py b/robosystems_client/clients/facade.py index 598b1254..7a50c5a0 100644 --- a/robosystems_client/clients/facade.py +++ b/robosystems_client/clients/facade.py @@ -28,7 +28,8 @@ class RoboSystemsClientConfig: max_retries: int = 5 retry_delay: int = 1000 timeout: int = 30 - s3_endpoint_url: Optional[str] = None # Override S3 endpoint (e.g., for LocalStack) + # Where the client reaches S3 when that is not the host the API signed for. + s3_endpoint_url: Optional[str] = None # Zero-arg callable returning the current auth credential, consulted # on every request by the GraphQL-backed facades (ledger / investor / # library). Wins over the static token when set — use it when the diff --git a/robosystems_client/clients/file_client.py b/robosystems_client/clients/file_client.py index 5c2f3e37..85b7e8ab 100644 --- a/robosystems_client/clients/file_client.py +++ b/robosystems_client/clients/file_client.py @@ -109,7 +109,7 @@ def upload( graph_id: Graph database identifier table_name: Table to associate file with file_or_buffer: File path, Path object, BytesIO, or file-like object - options: Upload options (progress callback, LocalStack URL fix, auto-ingest) + options: Upload options (progress callback, auto-ingest) Returns: FileUploadResult with file metadata and status @@ -184,13 +184,15 @@ def upload( upload_url = upload_data.get("upload_url") file_id = upload_data.get("file_id") - # Override S3 endpoint if configured (e.g., for LocalStack) + # The client may reach S3 by a different name than the API signed for + # (a container reaching a dev stack through host.docker.internal). SigV4 + # binds the Host header, so the request keeps the signed one. + upload_headers = {"Content-Type": "application/x-parquet"} if self.s3_endpoint_url: from urllib.parse import urlparse, urlunparse parsed_url = urlparse(upload_url) override_parsed = urlparse(self.s3_endpoint_url) - # Replace scheme, host, and port with the override endpoint upload_url = urlunparse( ( override_parsed.scheme or parsed_url.scheme, @@ -201,6 +203,7 @@ def upload( parsed_url.fragment, ) ) + upload_headers["Host"] = parsed_url.netloc # Step 2: Upload file to S3 if options.on_progress: @@ -209,7 +212,7 @@ def upload( s3_response = self._http_client.put( upload_url, content=file_content, - headers={"Content-Type": "application/x-parquet"}, + headers=upload_headers, ) if s3_response.status_code not in [200, 204]: diff --git a/tests/test_file_client.py b/tests/test_file_client.py index 5f97259d..bf2a0e18 100644 --- a/tests/test_file_client.py +++ b/tests/test_file_client.py @@ -294,14 +294,15 @@ def test_upload_with_progress_callback( def test_upload_with_s3_endpoint_override( self, mock_create, mock_update, mock_config, graph_id ): - """Test upload rewrites S3 URL when s3_endpoint_url is set.""" - mock_config["s3_endpoint_url"] = "http://localhost:4566" + """The connection goes to the override; the Host header stays the signed + one, since SigV4 binds it and the server checks the signature against it.""" + mock_config["s3_endpoint_url"] = "http://host.docker.internal:8333" mock_create_resp = Mock() mock_create_resp.status_code = 200 mock_create_resp.parsed = Mock( result={ - "upload_url": "https://s3.amazonaws.com/bucket/file.parquet?sig=abc", + "upload_url": "http://localhost:8333/bucket/file.parquet?X-Amz-Signature=abc", "file_id": "file-s3-override", } ) @@ -318,9 +319,46 @@ def test_upload_with_s3_endpoint_override( client.upload(graph_id, "Entity", BytesIO(b"data")) - # Verify S3 PUT was called with overridden URL put_url = client._http_client.put.call_args[0][0] - assert "localhost:4566" in put_url + put_headers = client._http_client.put.call_args.kwargs["headers"] + assert put_url == ( + "http://host.docker.internal:8333/bucket/file.parquet?X-Amz-Signature=abc" + ) + assert put_headers["Host"] == "localhost:8333" + assert put_headers["Content-Type"] == "application/x-parquet" + + @patch("robosystems_client.clients.file_client.ingest_file") + @patch("robosystems_client.clients.file_client.create_file_upload") + def test_upload_without_override_sends_no_host_header( + self, mock_create, mock_update, mock_config, graph_id + ): + """Without an override the presigned URL is used as signed and the + client sets no Host header of its own.""" + mock_create_resp = Mock() + mock_create_resp.status_code = 200 + mock_create_resp.parsed = Mock( + result={ + "upload_url": "https://bucket.s3.amazonaws.com/file.parquet?X-Amz-Signature=abc", + "file_id": "file-plain", + } + ) + mock_create.return_value = mock_create_resp + + mock_update_resp = Mock() + mock_update_resp.status_code = 200 + mock_update_resp.parsed = Mock(result={"file_size_bytes": 100, "row_count": 5}) + mock_update.return_value = mock_update_resp + + client = FileClient(mock_config) + client._http_client = Mock() + client._http_client.put.return_value = Mock(status_code=200) + + client.upload(graph_id, "Entity", BytesIO(b"data")) + + put_url = client._http_client.put.call_args[0][0] + put_headers = client._http_client.put.call_args.kwargs["headers"] + assert put_url == "https://bucket.s3.amazonaws.com/file.parquet?X-Amz-Signature=abc" + assert "Host" not in put_headers @pytest.mark.unit