diff --git a/changelog.d/pgw1138.md b/changelog.d/pgw1138.md new file mode 100644 index 00000000..01e60649 --- /dev/null +++ b/changelog.d/pgw1138.md @@ -0,0 +1 @@ +- **pgw#1138 (th#1722 §C): media uploads stop reconstructing the org.** tensorhub `de30113d` mounts the media-upload family org-less (`POST /api/v1/media/uploads`) because an upload addresses the CALLER'S OWN namespace — the org comes from the credential, never from the path — and keeps the org-addressed shape only as a transitional alias for wheels ≤0.106.0 (th#1799 deletes it once the fleet relocks onto a wheel carrying this change). So the client stops rebuilding a segment the hub derives itself: `RequestContext._media_upload_owner()` (which decoded the capability JWT's `tenant` claim for no other purpose) is **deleted**, the `X-Cozy-Owner` header is **deleted** (nothing in tensorhub reads it, in any version — the only `Cozy-Owner`-ish header in that tree is the orchestrator-internal `X-Cozy-Owner-Forwarded`), and with them goes the `RuntimeError("file save failed (missing owner)")` refusal and the whole J19-run34 403 class it was written against: a client that does not supply the org cannot supply the wrong one. `_decode_unverified_jwt_claims` stays — ten other call sites (transport, lifecycle, worker_identity, fleet_cells, procsplit, …) use it. The `upload.create/put/complete` phase split (th#1795) and the sha256-declare direct PUT are untouched; `complete_url` is derived from the create URL, so it follows automatically. **Test:** `tests/harness/upload_sink.py` now ROUTES like the hub — a pattern table mirroring `registerMediaUploadRoutes(v1, "/media")`, 404 for anything else including the alias — so `tests/test_media_upload_orgless_pgw1138.py` proves the URL by driving the real `ctx.save_bytes` codepath against a server that serves only tensorhub's canonical routes, rather than asserting a path string a stub pinned to itself. Seven tests across four files go RED on the pre-change source (four new, plus pgw#767, P9 and th#1111, which fail at the create leg with `status=404`). diff --git a/src/gen_worker/presigned_upload.py b/src/gen_worker/presigned_upload.py index 599df989..89b59a50 100644 --- a/src/gen_worker/presigned_upload.py +++ b/src/gen_worker/presigned_upload.py @@ -19,7 +19,8 @@ used at different route prefixes for datasets (/api/v1/datasets/:dataset_id/upload-sessions/:session_id/uploads), endpoint source (/api/v1/endpoints/:owner/:endpoint/releases/uploads), -and user media (/api/v1/media/:owner/uploads). Repo checkpoints do NOT use +and user media (/api/v1/media/uploads — org-less, the hub derives the org +from the credential; th#1722 §C). Repo checkpoints do NOT use this client anymore — they publish via the /commits API (gw#471, gen_worker.convert.hub). @@ -263,8 +264,8 @@ def presigned_upload_file( Args: file_path: Local path to the file. base_url: TensorHub base URL. - endpoint_path: e.g. "/api/v1/media/:owner/uploads" or "/api/v1/repos/.../uploads". - headers: Auth headers (Authorization, X-Cozy-Owner). + endpoint_path: e.g. "/api/v1/media/uploads" or "/api/v1/repos/.../uploads". + headers: Auth headers (Authorization). create_payload: Additional fields for the create POST (ref, path, request_id, etc.). blake3_hex: Pre-computed BLAKE3 hash of the file. size_bytes: File size in bytes. diff --git a/src/gen_worker/request_context/__init__.py b/src/gen_worker/request_context/__init__.py index 1cb685d7..6c6e9014 100644 --- a/src/gen_worker/request_context/__init__.py +++ b/src/gen_worker/request_context/__init__.py @@ -645,21 +645,6 @@ def _get_worker_capability_token(self) -> str: return self._worker_capability_token return _require_worker_capability_token() - def _media_upload_owner(self) -> str: - """Owner segment for /api/v1/media/:owner/uploads calls. - - The capability token's `tenant` claim is the canonical org uuid its - upload_media grant is bound to — the only owner tensorhub authorizes - media writes for. Falls back to ctx.owner for dev/local paths where - the token is absent or not a JWT. - """ - token = self._worker_capability_token or "" - if token: - claim = str(_decode_unverified_jwt_claims(token).get("tenant") or "").strip() - if claim: - return claim - return (self._owner or "").strip() - def _resolve_local_output_path(self, ref: str) -> Optional[str]: """ Dev-only local output backend. diff --git a/src/gen_worker/request_context/_stream.py b/src/gen_worker/request_context/_stream.py index 4e7f8c6b..91993ca7 100644 --- a/src/gen_worker/request_context/_stream.py +++ b/src/gen_worker/request_context/_stream.py @@ -14,7 +14,6 @@ import threading import time import tempfile -import urllib.parse from pathlib import Path from typing import TYPE_CHECKING, Any, Dict, Optional @@ -425,25 +424,17 @@ def _finalize_presigned_upload(self) -> Any: # /complete measured at 1060 ms server-side per image. create_payload["sha256"] = self._sha.hexdigest() - # Media upload. The URL owner segment MUST be the owner the - # capability token's upload_media grant is bound to (the token's - # `tenant` claim: the canonical invoking-org uuid). The - # dispatch-stamped ctx.owner can be a slug or a destination-repo - # owner resolving to a DIFFERENT org — tensorhub then finds no - # matching grant and 403s (J19 run34 sample images). Inference - # outputs work exactly because URL owner == token-bound owner. + # Media upload. th#1722 §C / pgw#1138: an upload addresses the + # CALLER'S OWN namespace, so the hub derives the org from the + # credential and the path never names one. The client cannot get the + # org wrong because it no longer supplies it — which retires the J19 + # run34 403 class (dispatch-stamped ctx.owner was a slug resolving to + # a different org than the capability grant's). create_payload["ref"] = self._ref job_id = str(self._ctx._job_id or "").strip() if job_id: create_payload["job_id"] = job_id - owner = self._ctx._media_upload_owner() - if not owner: - raise RuntimeError( - "file save failed (missing owner): media uploads require ctx.owner" - ) - headers["X-Cozy-Owner"] = owner - owner_seg = urllib.parse.quote(owner, safe="") - endpoint_path = f"/api/v1/media/{owner_seg}/uploads" + endpoint_path = "/api/v1/media/uploads" def _progress_cb(parts_done: int, total_parts: int, bytes_up: int) -> None: with self._progress_lock: diff --git a/tests/harness/upload_sink.py b/tests/harness/upload_sink.py index 8d8add9d..d33fe3b5 100644 --- a/tests/harness/upload_sink.py +++ b/tests/harness/upload_sink.py @@ -1,4 +1,4 @@ -"""A real local stand-in for tensorhub's `/api/v1/media/:owner/uploads`. +"""A real local stand-in for tensorhub's media-upload route family. Answers a dedup create, so a test needs no S3 part-PUT scripting to prove that an upload really happened. `requests_seen` being non-empty is the observable: @@ -6,6 +6,15 @@ process", which is the pgw#767 defect class and cannot be told apart from the result envelope alone. +**The sink ROUTES; it does not accept everything.** `_ORG_LESS_ROUTES` mirrors +`registerMediaUploadRoutes(v1, "/media")` in tensorhub `internal/api/files.go` +(th#1722 §C, `de30113d`): the org is derived from the credential and is never a +path segment. Anything else 404s exactly as gin does — including the +transitional org-addressed alias `/api/v1/media//uploads`, which th#1799 +deletes. That is what makes a client's URL construction testable end to end: a +test that let the server answer any path could not tell the two shapes apart, +which is the defect class pgw#1138 was filed against. + The v2 suite has its own richer `UploadSink` fixture (`tests_v2/conftest.py`, with a `status=` knob for refusal rows). This is the v1 suite's minimal twin — kept separate deliberately, because a cross-suite import would make the v2 @@ -15,37 +24,82 @@ from __future__ import annotations import json +import re import threading from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from typing import Any, ClassVar, Dict, List, Tuple +#: The canonical create route. Clients build this and nothing else. +MEDIA_UPLOADS_PATH = "/api/v1/media/uploads" + +#: The org-less media-upload family, one entry per tensorhub route. Kept as +#: patterns rather than prefixes so `/api/v1/media//uploads` — one extra +#: segment — cannot match by accident. +_ORG_LESS_ROUTES: Tuple[re.Pattern[str], ...] = tuple( + re.compile(p) + for p in ( + r"^/api/v1/media/uploads$", + r"^/api/v1/media/uploads/batch$", + r"^/api/v1/media/uploads/batch/complete$", + r"^/api/v1/media/uploads/[^/]+$", + r"^/api/v1/media/uploads/[^/]+/parts$", + r"^/api/v1/media/uploads/[^/]+/complete$", + ) +) + + +def is_media_upload_route(path: str) -> bool: + """True when `path` is served by tensorhub's org-less upload family.""" + bare = str(path or "").split("?", 1)[0] + return any(rx.match(bare) for rx in _ORG_LESS_ROUTES) + class DedupUploadSink(BaseHTTPRequestHandler): requests_seen: ClassVar[List[Tuple[str, Dict[str, Any]]]] = [] + headers_seen: ClassVar[List[Dict[str, str]]] = [] + rejected: ClassVar[List[str]] = [] def log_message(self, *_args: Any) -> None: pass def do_POST(self) -> None: # noqa: N802 length = int(self.headers.get("Content-Length", "0")) - body = json.loads(self.rfile.read(length) or b"{}") + raw = self.rfile.read(length) + if not is_media_upload_route(self.path): + type(self).rejected.append(self.path) + self._send(404, { + "error": "not_found", + "message": f"no route for POST {self.path}", + }) + return + body = json.loads(raw or b"{}") type(self).requests_seen.append((self.path, body)) - resp = json.dumps({ + type(self).headers_seen.append({k: v for k, v in self.headers.items()}) + self._send(200, { "dedup": True, "ref": body.get("ref") or "", "filename": "out.bin", "blake3": body.get("blake3") or "", "size_bytes": body.get("size_bytes") or 0, "mime_type": "application/octet-stream", "media_id": "m1", - }).encode("utf-8") - self.send_response(200) + }) + + def _send(self, code: int, payload: Dict[str, Any]) -> None: + resp = json.dumps(payload).encode("utf-8") + self.send_response(code) self.send_header("Content-Type", "application/json") self.send_header("Content-Length", str(len(resp))) self.end_headers() self.wfile.write(resp) -def serve_upload_sink() -> Tuple[ThreadingHTTPServer, str]: - """Start the sink on an ephemeral port. Caller shuts it down and resets - `DedupUploadSink.requests_seen`.""" +def reset_upload_sink() -> None: DedupUploadSink.requests_seen = [] + DedupUploadSink.headers_seen = [] + DedupUploadSink.rejected = [] + + +def serve_upload_sink() -> Tuple[ThreadingHTTPServer, str]: + """Start the sink on an ephemeral port. Caller shuts it down and calls + `reset_upload_sink()`.""" + reset_upload_sink() httpd = ThreadingHTTPServer(("127.0.0.1", 0), DedupUploadSink) threading.Thread(target=httpd.serve_forever, daemon=True).start() return httpd, f"http://127.0.0.1:{httpd.server_address[1]}" diff --git a/tests/test_inline_envelope_pgw767.py b/tests/test_inline_envelope_pgw767.py index cfe731a2..1f722235 100644 --- a/tests/test_inline_envelope_pgw767.py +++ b/tests/test_inline_envelope_pgw767.py @@ -22,7 +22,12 @@ from harness.hub_double import hub_double, is_ready, is_result_for from harness.toy_endpoints import EchoIn -from harness.upload_sink import DedupUploadSink, serve_upload_sink +from harness.upload_sink import ( + MEDIA_UPLOADS_PATH, + DedupUploadSink, + reset_upload_sink, + serve_upload_sink, +) def test_inline_dispatch_over_the_envelope_ceiling_still_really_uploads() -> None: @@ -49,11 +54,11 @@ def test_inline_dispatch_over_the_envelope_ceiling_still_really_uploads() -> Non "the inline media hint must not reach the result envelope" ) path, body = DedupUploadSink.requests_seen[-1] - assert path.startswith(f"/api/v1/media/{org_id}/uploads") + assert path == MEDIA_UPLOADS_PATH assert body["size_bytes"] > 64 * 1024 finally: httpd.shutdown() - DedupUploadSink.requests_seen = [] + reset_upload_sink() def test_save_bytes_still_inlines_media_for_the_client() -> None: diff --git a/tests/test_media_upload_orgless_pgw1138.py b/tests/test_media_upload_orgless_pgw1138.py new file mode 100644 index 00000000..0079dec8 --- /dev/null +++ b/tests/test_media_upload_orgless_pgw1138.py @@ -0,0 +1,167 @@ +"""pgw#1138 / th#1722 §C: a media upload addresses the CALLER'S OWN namespace. + +tensorhub `de30113d` mounts the upload family org-less +(`registerMediaUploadRoutes(v1, "/media")`, `internal/api/files.go`) and derives +the org from the credential; the org-addressed shape survives only as a +transitional alias for wheels <=0.106.0, which th#1799 deletes. So the client +stops reconstructing the org: no `tenant`-claim decode, no `:owner` segment, no +`X-Cozy-Owner` header (nothing in tensorhub reads it, in any version). + +These run against `harness.upload_sink`, which ROUTES like the hub does — +404 for anything outside the org-less family, including the alias. The +assertion that matters is therefore "the upload SUCCEEDED against a server +that serves only the hub's canonical routes", not a path string the test +pinned to itself. Against master every test here fails at the create leg with +`ArtifactTransferError`, because the client posts `/api/v1/media//uploads`. +""" + +from __future__ import annotations + +import base64 +import json +from typing import Any, Dict + +import pytest + +from gen_worker import RequestContext +from gen_worker.api.errors import ArtifactTransferError + +from harness.upload_sink import ( + MEDIA_UPLOADS_PATH, + DedupUploadSink, + is_media_upload_route, + reset_upload_sink, + serve_upload_sink, +) + + +def _unsigned_jwt(claims: Dict[str, Any]) -> str: + def seg(obj: Dict[str, Any]) -> str: + raw = json.dumps(obj).encode("utf-8") + return base64.urlsafe_b64encode(raw).decode("ascii").rstrip("=") + + return f"{seg({'alg': 'none', 'typ': 'JWT'})}.{seg(claims)}.sig" + + +def test_save_bytes_uploads_against_the_org_less_route_family() -> None: + """The real `ctx.save_bytes` codepath, against a server that serves only + tensorhub's org-less routes. RED on master: the create POST goes to + `/api/v1/media//uploads`, the sink 404s it, and the save raises.""" + httpd, base_url = serve_upload_sink() + try: + token = _unsigned_jwt( + {"tenant": "019f4c33-f3a5-705b-9848-0b3b0863c416", "request_id": "req-1138"} + ) + ctx = RequestContext( + request_id="req-1138", + owner="tensorhub", # the dispatch-stamped SLUG (the J19 run34 shape) + file_api_base_url=base_url, + worker_capability_token=token, + ) + asset = ctx.save_bytes("samples/pair-000.bin", b"payload") + + assert asset.ref == "samples/pair-000.bin" + assert not DedupUploadSink.rejected, ( + "the client addressed a route tensorhub does not serve: " + f"{DedupUploadSink.rejected}" + ) + assert DedupUploadSink.requests_seen, "the real upload sink must have been hit" + path, body = DedupUploadSink.requests_seen[-1] + assert path == MEDIA_UPLOADS_PATH, ( + "the org comes from the credential, never from the path" + ) + assert body["ref"] == "samples/pair-000.bin" + finally: + httpd.shutdown() + reset_upload_sink() + + +def test_the_upload_carries_no_owner_header() -> None: + """`X-Cozy-Owner` was write-only: no tensorhub version reads it.""" + httpd, base_url = serve_upload_sink() + try: + ctx = RequestContext( + request_id="req-1138-hdr", + owner="org", + file_api_base_url=base_url, + worker_capability_token=_unsigned_jwt({"tenant": "org-uuid"}), + ) + ctx.save_bytes("samples/hdr.bin", b"payload") + assert DedupUploadSink.headers_seen + sent = {k.lower() for k in DedupUploadSink.headers_seen[-1]} + assert "authorization" in sent, "the credential is what carries the org" + assert "x-cozy-owner" not in sent + finally: + httpd.shutdown() + reset_upload_sink() + + +def test_an_upload_with_no_derivable_owner_is_no_longer_a_failure_mode() -> None: + """`RuntimeError("file save failed (missing owner)")` is gone with the + reconstruction: a capability token without a `tenant` claim and an empty + `ctx.owner` used to refuse client-side. The hub knows the org.""" + httpd, base_url = serve_upload_sink() + try: + ctx = RequestContext( + request_id="req-1138-noowner", + owner="", + file_api_base_url=base_url, + worker_capability_token=_unsigned_jwt({"request_id": "req-1138-noowner"}), + ) + asset = ctx.save_bytes("samples/no-owner.bin", b"payload") + assert asset.ref == "samples/no-owner.bin" + assert DedupUploadSink.requests_seen + assert DedupUploadSink.requests_seen[-1][0] == MEDIA_UPLOADS_PATH + finally: + httpd.shutdown() + reset_upload_sink() + + +def test_the_org_reconstruction_helper_is_gone() -> None: + assert not hasattr(RequestContext, "_media_upload_owner") + + +def test_the_sink_would_have_caught_the_old_shape() -> None: + """The harness is only evidence if it can go red. The alias th#1799 + deletes must not match the org-less family.""" + assert is_media_upload_route(MEDIA_UPLOADS_PATH) + assert is_media_upload_route(MEDIA_UPLOADS_PATH + "/u1/complete") + assert not is_media_upload_route("/api/v1/media/some-org-uuid/uploads") + assert not is_media_upload_route("/api/v1/media/some-org-uuid/uploads/u1/complete") + + +def test_the_sink_404s_a_route_tensorhub_does_not_serve() -> None: + """The 404 the client would hit against a post-th#1799 hub surfaces as a + typed transfer error, not a silent success.""" + import requests + + httpd, base_url = serve_upload_sink() + try: + resp = requests.post( + f"{base_url}/api/v1/media/some-org/uploads", json={"ref": "x"}, timeout=5 + ) + assert resp.status_code == 404 + assert DedupUploadSink.rejected == ["/api/v1/media/some-org/uploads"] + assert not DedupUploadSink.requests_seen + finally: + httpd.shutdown() + reset_upload_sink() + + +def test_a_create_404_raises_a_typed_transfer_error() -> None: + """Guards the RED signal itself: had the sink answered every path, the + tests above would pass on master. Against a base URL with no upload + routes at all the client must fail loudly.""" + httpd, base_url = serve_upload_sink() + try: + ctx = RequestContext( + request_id="req-1138-404", + owner="org", + file_api_base_url=base_url + "/nope", + worker_capability_token=_unsigned_jwt({"tenant": "org-uuid"}), + ) + with pytest.raises(ArtifactTransferError): + ctx.save_bytes("samples/dead.bin", b"payload") + finally: + httpd.shutdown() + reset_upload_sink() diff --git a/tests/test_p9_result_upload_metrics.py b/tests/test_p9_result_upload_metrics.py index 275ae197..60d04d1f 100644 --- a/tests/test_p9_result_upload_metrics.py +++ b/tests/test_p9_result_upload_metrics.py @@ -1,17 +1,19 @@ """P9 (th#960/pgw#609 design table): inline <64KB vs blob_ref presigned PUT -by size alone, over a real hub-double + a real local media-upload HTTP sink -(dedup response — no S3 multipart scripting needed, matching -tests/test_media_upload_owner.py's real-codepath pattern). JobMetrics' -typed usage propagates regardless of which wire form the result took -(billing never scavenges the payload — pgw#512/#513 class). +by size alone, over a real hub-double + the shared media-upload HTTP sink +(`harness.upload_sink` — a dedup response over tensorhub's real route table, +so no S3 multipart scripting is needed). JobMetrics' typed usage propagates +regardless of which wire form the result took (billing never scavenges the +payload — pgw#512/#513 class). + +The owner-segment test this file used to carry (J19 run34: the capability +token's `tenant` claim, not the dispatch slug) is absorbed into +`test_media_upload_orgless_pgw1138.py`: th#1722 §C removed the segment +entirely, so there is no owner left to get wrong. """ from __future__ import annotations -import json -import threading -from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer -from typing import Any, ClassVar, Dict, List, Tuple +from typing import Tuple import msgspec @@ -19,38 +21,16 @@ from harness.hub_double import hub_double, is_ready, is_result_for from harness.toy_endpoints import EchoIn +from harness.upload_sink import ( + MEDIA_UPLOADS_PATH, + DedupUploadSink, + reset_upload_sink, + serve_upload_sink, +) -class _DedupUploadSink(BaseHTTPRequestHandler): - """Real local stand-in for tensorhub's /api/v1/media/:owner/uploads — - answers a dedup create so the test needs no S3 part PUT scripting, same - approach as test_media_upload_owner.py.""" - - requests_seen: ClassVar[List[Tuple[str, Dict[str, Any]]]] = [] - - def log_message(self, *_args: Any) -> None: - pass - - def do_POST(self) -> None: # noqa: N802 - length = int(self.headers.get("Content-Length", "0")) - body = json.loads(self.rfile.read(length) or b"{}") - type(self).requests_seen.append((self.path, body)) - resp = json.dumps({ - "dedup": True, "ref": body.get("ref") or "", "filename": "out.msgpack", - "blake3": body.get("blake3") or "", "size_bytes": body.get("size_bytes") or 0, - "mime_type": "application/octet-stream", "media_id": "m1", - }).encode("utf-8") - self.send_response(200) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(resp))) - self.end_headers() - self.wfile.write(resp) - - -def _serve() -> Tuple[ThreadingHTTPServer, str]: - httpd = ThreadingHTTPServer(("127.0.0.1", 0), _DedupUploadSink) - threading.Thread(target=httpd.serve_forever, daemon=True).start() - return httpd, f"http://127.0.0.1:{httpd.server_address[1]}" +def _serve() -> Tuple[object, str]: + return serve_upload_sink() def _payload() -> bytes: @@ -101,51 +81,10 @@ def test_large_output_ships_blob_ref_with_typed_usage_intact() -> None: assert res.metrics.input_tokens == 4000 assert res.metrics.input_cached_tokens == 100 assert res.metrics.output_tokens == 9000 - assert _DedupUploadSink.requests_seen, "the real upload sink must have been hit" - path, body = _DedupUploadSink.requests_seen[-1] - assert path.startswith(f"/api/v1/media/{org_id}/uploads") + assert DedupUploadSink.requests_seen, "the real upload sink must have been hit" + path, body = DedupUploadSink.requests_seen[-1] + assert path == MEDIA_UPLOADS_PATH assert body["size_bytes"] > 64 * 1024 finally: httpd.shutdown() - _DedupUploadSink.requests_seen = [] - - -def test_save_bytes_targets_token_bound_owner_not_dispatch_slug() -> None: - """Absorbed from test_media_upload_owner.py (J19 run34): the capability - token's `tenant` claim — NOT the dispatch-stamped ctx.owner (which can - be a slug) — is the owner segment tensorhub's upload_media grant - authorizes. A worker that used ctx.owner directly 403'd every upload - whose owner resolved to a different org.""" - import base64 - import json as json_mod - - from gen_worker import RequestContext - - def _unsigned_jwt(claims: Dict[str, Any]) -> str: - def seg(obj: Dict[str, Any]) -> str: - raw = json_mod.dumps(obj).encode("utf-8") - return base64.urlsafe_b64encode(raw).decode("ascii").rstrip("=") - - return f"{seg({'alg': 'none', 'typ': 'JWT'})}.{seg(claims)}.sig" - - owner_uuid = "019f4c33-f3a5-705b-9848-0b3b0863c416" - httpd, base_url = _serve() - try: - token = _unsigned_jwt({"tenant": owner_uuid, "request_id": "req-run34"}) - # ctx.owner is the dispatch-stamped SLUG — the run34 failure mode. - ctx = RequestContext( - request_id="req-run34", owner="tensorhub", - file_api_base_url=base_url, worker_capability_token=token, - ) - asset = ctx.save_bytes("samples/pair-000.bin", b"payload") - - assert asset.ref - assert _DedupUploadSink.requests_seen - path, body = _DedupUploadSink.requests_seen[-1] - assert path == f"/api/v1/media/{owner_uuid}/uploads", ( - "must ride the TOKEN-bound owner, never the dispatch slug" - ) - assert body["ref"] == "samples/pair-000.bin" - finally: - httpd.shutdown() - _DedupUploadSink.requests_seen = [] + reset_upload_sink() diff --git a/tests/test_upload_phase_split_pgw1125.py b/tests/test_upload_phase_split_pgw1125.py index 8a4dec31..a5b74f96 100644 --- a/tests/test_upload_phase_split_pgw1125.py +++ b/tests/test_upload_phase_split_pgw1125.py @@ -91,7 +91,7 @@ def test_the_three_legs_are_attributed_separately(tmp_path) -> None: result = presigned_upload_file( file_path=str(src), base_url=base, - endpoint_path="/api/v1/media/o/uploads", + endpoint_path="/api/v1/media/uploads", headers={"Authorization": "Bearer t"}, create_payload={"ref": "out.webp"}, blake3_hex="0" * 64, @@ -136,7 +136,7 @@ def do_POST(self) -> None: # noqa: N802 presigned_upload_file( file_path=str(src), base_url=base, - endpoint_path="/api/v1/media/o/uploads", + endpoint_path="/api/v1/media/uploads", headers={"Authorization": "Bearer t"}, create_payload={"ref": "out.webp"}, blake3_hex="0" * 64, @@ -256,7 +256,7 @@ def test_a_direct_final_grant_is_put_once_with_the_signed_headers_verbatim(tmp_p result = presigned_upload_file( file_path=str(src), base_url=base, - endpoint_path="/api/v1/media/o/uploads", + endpoint_path="/api/v1/media/uploads", headers={"Authorization": "Bearer t"}, create_payload={"ref": "out.webp", "sha256": "a" * 64}, blake3_hex="0" * 64, diff --git a/tests_v2/conftest.py b/tests_v2/conftest.py index 2edd2e87..659e2ea2 100644 --- a/tests_v2/conftest.py +++ b/tests_v2/conftest.py @@ -62,6 +62,7 @@ import json import os +import re import subprocess import sys import tempfile @@ -268,16 +269,44 @@ def blob_host(tmp_path: Path) -> Iterator[BlobHost]: # --------------------------------------------------------------------------- +#: The canonical org-less create route (th#1722 §C / pgw#1138). The org is +#: derived from the CREDENTIAL and is never a path segment. +MEDIA_UPLOADS_PATH = "/api/v1/media/uploads" + +#: tensorhub's org-less media-upload family, mirroring +#: `registerMediaUploadRoutes(v1, "/media")` in `internal/api/files.go`. +#: Patterns, not prefixes, so `/api/v1/media//uploads` — the transitional +#: alias th#1799 deletes — cannot match. Deliberately duplicated from +#: `tests/harness/upload_sink.py` rather than imported: a cross-suite import +#: would make the v1 harness load-bearing for v2. +_ORG_LESS_UPLOAD_ROUTES = tuple( + re.compile(p) + for p in ( + r"^/api/v1/media/uploads$", + r"^/api/v1/media/uploads/batch$", + r"^/api/v1/media/uploads/batch/complete$", + r"^/api/v1/media/uploads/[^/]+$", + r"^/api/v1/media/uploads/[^/]+/parts$", + r"^/api/v1/media/uploads/[^/]+/complete$", + ) +) + + class UploadSink: """Real HTTP sink for the worker's result-blob upload path. ``status=200`` answers a dedup create (no S3 part scripting needed); any other status is returned verbatim — the refusal rows. Records every POST as ``(path, decoded_json_body)`` in ``self.requests``. + + It ROUTES: a path outside tensorhub's org-less upload family 404s exactly + as gin does, so a client's URL construction is testable end to end rather + than assumed by an assertion the sink itself could not contradict. """ def __init__(self, status: int = 200) -> None: self.requests: List[Tuple[str, Dict[str, Any]]] = [] + self.rejected: List[str] = [] sink = self class _Handler(BaseHTTPRequestHandler): @@ -286,7 +315,18 @@ def log_message(self, *_a: Any) -> None: def do_POST(self) -> None: # noqa: N802 length = int(self.headers.get("Content-Length", "0")) - body = json.loads(self.rfile.read(length) or b"{}") + raw = self.rfile.read(length) + bare = self.path.split("?", 1)[0] + if not any(rx.match(bare) for rx in _ORG_LESS_UPLOAD_ROUTES): + sink.rejected.append(self.path) + payload = json.dumps({"error": "not_found"}).encode() + self.send_response(404) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + return + body = json.loads(raw or b"{}") sink.requests.append((self.path, body)) if sink.status != 200: payload = json.dumps({"error": "refused by test sink"}).encode() diff --git a/tests_v2/test_dispatch.py b/tests_v2/test_dispatch.py index bd7dbf1f..e174f0c2 100644 --- a/tests_v2/test_dispatch.py +++ b/tests_v2/test_dispatch.py @@ -23,6 +23,7 @@ from harness.hub_double import is_accept_for, is_model_event, is_ready, is_result_for from tests_v2 import catalog +from tests_v2.conftest import MEDIA_UPLOADS_PATH ORG = "00000000-0000-0000-0000-000000000042" @@ -161,9 +162,16 @@ def test_dispatch_load_serve_and_upload_walk(hub, blob_host, upload_sink) -> Non assert res.blob_ref and not res.inline assert (res.metrics.input_tokens, res.metrics.input_cached_tokens, res.metrics.output_tokens) == (4000, 100, 9000) + assert not upload_sink.rejected, ( + "the client addressed a route tensorhub does not serve: " + f"{upload_sink.rejected}" + ) assert upload_sink.requests, "the real upload sink was never hit" path, body = upload_sink.requests[-1] - assert path.startswith(f"/api/v1/media/{ORG}/uploads") + # th#1722 §C / pgw#1138: the org rides the CREDENTIAL, not the path — + # ORG is dispatched on the job and must not appear in the URL. + assert path == MEDIA_UPLOADS_PATH + assert ORG not in path assert body["size_bytes"] > 64 * 1024 # Retransmitted live attempt: re-acked, never re-executed — the