From 11af2103f1cd7c99dd612485345867d15524a9fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Marques?= Date: Thu, 30 Jul 2026 21:43:09 -0300 Subject: [PATCH 1/2] fix: keep signed URLs out of downloader logs --- maestro_worker_python/download_file.py | 5 +-- maestro_worker_python/sanitize_url.py | 46 ++++++++++++++++++++++++++ tests/test_download_file.py | 37 +++++++++++++++++++++ tests/test_sanitize_url.py | 37 +++++++++++++++++++++ 4 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 maestro_worker_python/sanitize_url.py create mode 100644 tests/test_sanitize_url.py diff --git a/maestro_worker_python/download_file.py b/maestro_worker_python/download_file.py index 2b3dfc6..cb4bb6b 100644 --- a/maestro_worker_python/download_file.py +++ b/maestro_worker_python/download_file.py @@ -9,10 +9,11 @@ import requests from .response import ValidationError +from .sanitize_url import sanitize_url def download_file(url: str, filename: str | None = None) -> str: - logging.info(f"Downloading input: {url}") + logging.info("Downloading input: %s", sanitize_url(url)) response = requests.get(url, allow_redirects=True, timeout=300) try: @@ -39,7 +40,7 @@ def download_files_manager(*urls: str) -> Iterator[None | str | list[str]]: list_objects = [] for url in urls: filename = tempfile.NamedTemporaryFile() - logging.info("Downloading file from url -> %s, filename -> %s", url, filename.name) + logging.info("Downloading file from url -> %s, filename -> %s", sanitize_url(url), filename.name) with ThreadPoolExecutor(max_workers=20) as exe: thread_list.append(exe.submit(download_file, url, filename.name)) list_objects.append(filename) diff --git a/maestro_worker_python/sanitize_url.py b/maestro_worker_python/sanitize_url.py new file mode 100644 index 0000000..08e0e55 --- /dev/null +++ b/maestro_worker_python/sanitize_url.py @@ -0,0 +1,46 @@ +from __future__ import annotations + +from urllib.parse import urlsplit, urlunsplit + +# Ports implied by their scheme carry no information, and keeping them would make +# ``https://host:443/x`` and ``https://host/x`` read as two different locations. +_DEFAULT_PORTS = { + "ftp": 21, + "http": 80, + "https": 443, + "ws": 80, + "wss": 443, +} + +# Substituted for a URL that cannot be parsed. An unparseable string is the one +# case where there is no way to show it holds no credential, so none of it is kept. +UNPARSEABLE_URL = "" + + +def sanitize_url(url: str) -> str: + """Strip credentials and signing material from a URL while keeping it identifiable. + + Removes userinfo, query, and fragment, lowercases scheme and host, and drops + the scheme's default port. The path is passed through byte-for-byte: decoding + it would let percent-encoded data be reinterpreted as structure, and + re-encoding it would change which object the URL names. + + The result is safe to log but is not anonymous — it still names an object. + """ + try: + parts = urlsplit(url) + port = parts.port + except ValueError: + return UNPARSEABLE_URL + + # hostname drops userinfo and lowercases in one step, unlike netloc. + host = (parts.hostname or "").lower() + + if ":" in host: + # An IPv6 literal loses the brackets it needs to round-trip. + host = f"[{host}]" + + if port is not None and port != _DEFAULT_PORTS.get(parts.scheme.lower()): + host = f"{host}:{port}" + + return urlunsplit((parts.scheme.lower(), host, parts.path, "", "")) diff --git a/tests/test_download_file.py b/tests/test_download_file.py index 6be7dcc..74f6ff2 100644 --- a/tests/test_download_file.py +++ b/tests/test_download_file.py @@ -1,3 +1,4 @@ +import logging import os import pytest @@ -5,6 +6,22 @@ from maestro_worker_python.download_file import download_file, download_files_manager from maestro_worker_python.response import ValidationError +SIGNED_QUERY = "X-Goog-Signature=deadbeefcafe" + + +def _assert_no_signing_material_logged(caplog, httpserver): + # The in-process test server logs the request line it received, query included. + # That is the fixture's own access log, not the library under test. + records = [record for record in caplog.records if record.name != "werkzeug"] + assert records, "nothing was logged, so this proves nothing" + for record in records: + message = record.getMessage() + assert "deadbeefcafe" not in message + assert "X-Goog-Signature" not in message + # The sanitized location must still be logged, or redaction has cost the + # operator the ability to tell which input a download refers to. + assert any(httpserver.url_for("/test") in record.getMessage() for record in records) + def test_download_file(httpserver): httpserver.expect_request("/test").respond_with_data("hello") @@ -38,6 +55,26 @@ def test_download_files_manager(httpserver): assert all(files_content) +def test_download_file_keeps_signing_material_out_of_the_logs(httpserver, caplog): + httpserver.expect_request("/test").respond_with_data("hello") + url = httpserver.url_for(f"/test?{SIGNED_QUERY}") + + with caplog.at_level(logging.INFO): + download_file(url) + + _assert_no_signing_material_logged(caplog, httpserver) + + +def test_download_files_manager_keeps_signing_material_out_of_the_logs(httpserver, caplog): + httpserver.expect_request("/test").respond_with_data("hello") + url = httpserver.url_for(f"/test?{SIGNED_QUERY}") + + with caplog.at_level(logging.INFO), download_files_manager(url) as downloaded: + assert downloaded is not None + + _assert_no_signing_material_logged(caplog, httpserver) + + def test_download_files_manager_delete(httpserver): httpserver.expect_request("/test").respond_with_data("hello") url = httpserver.url_for("/test?foo=bar") diff --git a/tests/test_sanitize_url.py b/tests/test_sanitize_url.py new file mode 100644 index 0000000..3817fc1 --- /dev/null +++ b/tests/test_sanitize_url.py @@ -0,0 +1,37 @@ +import pytest + +from maestro_worker_python.sanitize_url import UNPARSEABLE_URL, sanitize_url + + +@pytest.mark.parametrize( + ("url", "expected"), + [ + # A signed GCS read: the signature is the credential. + ( + "https://storage.googleapis.com/bucket/song.mp3" + "?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Signature=deadbeef", + "https://storage.googleapis.com/bucket/song.mp3", + ), + ("https://user:hunter2@host/path", "https://host/path"), + ("https://host/path#fragment", "https://host/path"), + ("HTTPS://Storage.GoogleAPIs.COM/Bucket/Song.mp3", "https://storage.googleapis.com/Bucket/Song.mp3"), + ("https://host:443/path", "https://host/path"), + ("http://host:80/path", "http://host/path"), + ("https://host:8443/path", "https://host:8443/path"), + ("gs://My-Bucket/path/to/obj.wav?x=1", "gs://my-bucket/path/to/obj.wav"), + ("hf://meta-llama/Llama-3/model.safetensors", "hf://meta-llama/Llama-3/model.safetensors"), + ("file:///tmp/local.wav", "file:///tmp/local.wav"), + ("http://[::1]:8080/path", "http://[::1]:8080/path"), + ], +) +def test_sanitize_url_removes_credentials_and_normalizes_the_location(url, expected): + assert sanitize_url(url) == expected + + +def test_sanitize_url_passes_the_path_through_without_decoding_it(): + """Decoding would let %2F be reread as a separator, naming a different object.""" + assert sanitize_url("https://host/a%2Fb/My%20Song%20(1).mp3?sig=x") == "https://host/a%2Fb/My%20Song%20(1).mp3" + + +def test_sanitize_url_drops_a_url_it_cannot_parse(): + assert sanitize_url("https://host:not-a-port/path") == UNPARSEABLE_URL From 9f10dbf5c9cee21a7fd7141449234d7696a59081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Marques?= Date: Thu, 30 Jul 2026 21:49:07 -0300 Subject: [PATCH 2/2] feat: add an opaque internal response field --- README.md | 9 +++- maestro_worker_python/response.py | 2 + pyproject.toml | 1 + tests/test_serve.py | 68 +++++++++++++++++++++++++++++++ uv.lock | 40 ++++++++++++++++++ 5 files changed, 119 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b1bda3e..c579a03 100644 --- a/README.md +++ b/README.md @@ -74,13 +74,20 @@ curl --request POST --url http://localhost:8000/inference --header 'Content-Typ --data '{"input_1": "Hello"}' ``` -Workers return a `WorkerResponse` with three fields: +Workers return a `WorkerResponse`: - `billable_seconds`: the billable duration as a number of seconds, including fractional seconds when available, or `null` when it cannot be determined. - `stats`: numeric worker measurements. Use `duration` for total worker processing time; additional keys may report individual phases. - `result`: the worker-specific JSON object. +- `internal`: an optional object passed through without inspection, for data the + deployment's own response consumer reads and then removes before answering its + callers. Its contents are entirely up to that deployment; this library gives it + no meaning and does not validate it. Leave it unset if nothing consumes it. + +Any other field a worker returns is dropped. `internal` is the one exception, so +a worker cannot reach a caller with a field nobody agreed to. The `/health` endpoint reports the worker artifact version supplied through `WORKER_VERSION` and available GPU metadata in addition to `ok`. Deployments diff --git a/maestro_worker_python/response.py b/maestro_worker_python/response.py index d65a0ec..0f64dd9 100644 --- a/maestro_worker_python/response.py +++ b/maestro_worker_python/response.py @@ -7,6 +7,8 @@ class WorkerResponse(BaseModel): billable_seconds: float | None stats: dict[str, float] result: dict[str, Any] + # Opaque passthrough for the deployment; other undeclared fields are dropped. + internal: dict[str, Any] | None = None class ValidationError(Exception): diff --git a/pyproject.toml b/pyproject.toml index d53fa53..9b50d25 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,6 +26,7 @@ maestro-upload-server = "maestro_worker_python.run_upload_server:main" [dependency-groups] dev = [ + "httpx2>=2.9.1", "pytest>=9.0.3,<10", "pytest-httpserver>=1.0.6,<2", "ruff>=0.14.9,<0.15", diff --git a/tests/test_serve.py b/tests/test_serve.py index 32002e5..c600f5e 100644 --- a/tests/test_serve.py +++ b/tests/test_serve.py @@ -4,9 +4,30 @@ from pathlib import Path import pytest +from fastapi.testclient import TestClient from maestro_worker_python.config import settings +# Deliberately mixed value types: `internal` is typed only as an object, so +# coercion of anything inside it would be a silent corruption of a channel this +# library is supposed to be agnostic about. +INTERNAL_PAYLOAD = { + "file_transfers": [ + {"direction": "input", "field": "input_url", "url": "https://host/in.mp3", "bytes": 0}, + {"direction": "output", "field": "vocals", "url": "https://host/out%2Fa.wav", "bytes": 1234}, + ], + "nested": {"flag": True, "count": 7, "ratio": 0.5, "absent": None}, +} + + +def _write_worker(worker_path: Path, returned: str) -> None: + worker_path.write_text( + f"INTERNAL = {INTERNAL_PAYLOAD!r}\n" + "class MoisesWorker:\n" + " def inference(self, input_data):\n" + f" return {returned}\n" + ) + def _import_serve(monkeypatch: pytest.MonkeyPatch, worker_path: Path): monkeypatch.setattr(settings, "model_path", str(worker_path)) @@ -55,3 +76,50 @@ async def run(): asyncio.run(run()) assert not ready_path.exists() + + +def test_inference_passes_internal_content_through_uninspected(tmp_path, monkeypatch): + worker_path = tmp_path / "worker.py" + _write_worker( + worker_path, + '{"billable_seconds": 1.0, "stats": {"duration": 0.5}, "result": {"ok": True}, "internal": INTERNAL}', + ) + serve_module = _import_serve(monkeypatch, worker_path) + + body = TestClient(serve_module.app).post("/inference", json={}).json() + + assert body["internal"] == INTERNAL_PAYLOAD + nested = body["internal"]["nested"] + # Equality alone would accept 7 becoming 7.0 or True becoming 1. + assert isinstance(nested["count"], int) and not isinstance(nested["count"], bool) + assert isinstance(nested["flag"], bool) + assert body["result"] == {"ok": True} + + +def test_inference_drops_a_field_the_response_model_does_not_declare(tmp_path, monkeypatch): + """The single declared passthrough is what keeps the envelope closed.""" + worker_path = tmp_path / "worker.py" + _write_worker( + worker_path, + '{"billable_seconds": 1.0, "stats": {}, "result": {}, "internal": {}, "leaked": "not for callers"}', + ) + serve_module = _import_serve(monkeypatch, worker_path) + + body = TestClient(serve_module.app).post("/inference", json={}).json() + + assert "leaked" not in body + + +def test_inference_distinguishes_an_empty_internal_object_from_no_internal_object(tmp_path, monkeypatch): + """Producers signal support with the object itself, so the two must not collapse.""" + worker_path = tmp_path / "worker.py" + _write_worker(worker_path, '{"billable_seconds": 1.0, "stats": {}, "result": {}, "internal": {}}') + serve_module = _import_serve(monkeypatch, worker_path) + empty = TestClient(serve_module.app).post("/inference", json={}).json() + + _write_worker(worker_path, '{"billable_seconds": 1.0, "stats": {}, "result": {}}') + serve_module = _import_serve(monkeypatch, worker_path) + unset = TestClient(serve_module.app).post("/inference", json={}).json() + + assert empty["internal"] == {} + assert unset["internal"] is None diff --git a/uv.lock b/uv.lock index 1ca3c9c..7780ff5 100644 --- a/uv.lock +++ b/uv.lock @@ -162,6 +162,35 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/04/4b/29cac41a4d98d144bf5f6d33995617b185d14b22401f75ca86f384e87ff1/h11-0.16.0-py3-none-any.whl", hash = "sha256:63cf8bbe7522de3bf65932fda1d9c2772064ffb3dae62d55932da54b31cb6c86", size = 37515, upload-time = "2025-04-24T03:35:24.344Z" }, ] +[[package]] +name = "httpcore2" +version = "2.9.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "h11" }, + { name = "truststore" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/39/a8/20ed1ed79cbc2ecdf5301c0968ab7c85547212e2a7bd126ddd2d986e206e/httpcore2-2.9.1.tar.gz", hash = "sha256:4d8acbf8b306f48c9d6046591fd5ba4037d1b1b1000d140fc2c3eab1e9a0c0e2", size = 67089, upload-time = "2026-07-24T09:21:03.867Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/9f/fb/46c52b781975c335a2bcf1072c7bbc007cbdc8d674217f5ee1daba2c848b/httpcore2-2.9.1-py3-none-any.whl", hash = "sha256:6182472379e855fe4221246a2bb7ecede403bc61c6798062ae1787d051ccde26", size = 82809, upload-time = "2026-07-24T09:21:01.178Z" }, +] + +[[package]] +name = "httpx2" +version = "2.9.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "anyio" }, + { name = "httpcore2" }, + { name = "idna" }, + { name = "truststore" }, + { name = "typing-extensions", marker = "python_full_version < '3.13'" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/21/14/38128fbafd7e0ed41d874df6c9a653d47c2d111cfe59e2b4ac95161b4abd/httpx2-2.9.1.tar.gz", hash = "sha256:1932a768737e3666291582833da748cc4e563c337cf96706fccc04fa6e58764a", size = 95458, upload-time = "2026-07-24T09:21:04.972Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/13/b8/cfd91c4ab9134d386d48f0b6ac662ff3d4be6efdee59ee1c67ebc3c0487c/httpx2-2.9.1-py3-none-any.whl", hash = "sha256:1820fe14a9ab1107bfeff39259987429450b070ec0ff38cc87eb0d8c97fdc71a", size = 91191, upload-time = "2026-07-24T09:21:02.6Z" }, +] + [[package]] name = "idna" version = "3.18" @@ -207,6 +236,7 @@ dependencies = [ [package.dev-dependencies] dev = [ + { name = "httpx2" }, { name = "pytest" }, { name = "pytest-httpserver" }, { name = "ruff" }, @@ -228,6 +258,7 @@ requires-dist = [ [package.metadata.requires-dev] dev = [ + { name = "httpx2", specifier = ">=2.9.1" }, { name = "pytest", specifier = ">=9.0.3,<10" }, { name = "pytest-httpserver", specifier = ">=1.0.6,<2" }, { name = "ruff", specifier = ">=0.14.9,<0.15" }, @@ -610,6 +641,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/7b/61/cceae43728b7de99d9b847560c262873a1f6c98202171fd5ed62640b494b/tomli-2.4.1-py3-none-any.whl", hash = "sha256:0d85819802132122da43cb86656f8d1f8c6587d54ae7dcaf30e90533028b49fe", size = 14583, upload-time = "2026-03-25T20:22:03.012Z" }, ] +[[package]] +name = "truststore" +version = "0.10.4" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/53/a3/1585216310e344e8102c22482f6060c7a6ea0322b63e026372e6dcefcfd6/truststore-0.10.4.tar.gz", hash = "sha256:9d91bd436463ad5e4ee4aba766628dd6cd7010cf3e2461756b3303710eebc301", size = 26169, upload-time = "2025-08-12T18:49:02.73Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/19/97/56608b2249fe206a67cd573bc93cd9896e1efb9e98bce9c163bcdc704b88/truststore-0.10.4-py3-none-any.whl", hash = "sha256:adaeaecf1cbb5f4de3b1959b42d41f6fab57b2b1666adb59e89cb0b53361d981", size = 18660, upload-time = "2025-08-12T18:49:01.46Z" }, +] + [[package]] name = "ty" version = "0.0.14"