Refuse redirects on the signed download hop in every SDK

Merged
JE
2mo ago
+524-50

Closes #805.

The finding, corrected

DownloadURL's second hop — the unauthenticated GET of the presigned URL hop 1 named — followed redirects in four SDKs, each by a different stack default:

SDKMechanismCap
Gono CheckRedirect on the bare client → net/http default10
TypeScriptbare fetch(resolvedLocation) → redirect: "follow"20
Pythonfollow_redirects=True, written outhttpx default
Swifttransport.data(for:), the following entry pointURLSession default

The issue's table said Ruby had no download path. It does (Client#download_url), and it has refused hop-2 redirects since #178 introduced the feature, because Net::HTTP#request never follows. Kotlin has refused since #178 too — it runs hop 2 on hop 1's followRedirects = false client. So the fleet was 4 following / 2 refusing, and every other hop a response could steer (hop 1 here, §16 discovery, §23 polls) already refuses.

The product call, answered from bc3

The issue deferred the remedy pending "what the fleet's storage backends actually do". From bc3:

  • Production storage is S3WithDirectEndpointsService against a single Pure FlashBlade endpoint (config/storage.yml: https://storage.basecamp.com, path-style, no direct_download_endpoint, nothing redirecting in front of it).
  • Hop 1's 302 is minted by Downloading#respond_with_download_redirect from the blob's presigned service URL. A presigned GET on a path-style single endpoint is answered by that endpoint — the region/virtual-host redirects behind "S3 redirects" are an AWS multi-region artefact this store does not have.
  • Local and test environments send_file on hop 1 (respond_with_download_on_disk); hop 2 never occurs.
  • Strongest evidence: Kotlin and Ruby have shipped the refusal for the life of the feature with no broken download.

So: suppress, uniformly (the issue's option 1). Not a cap — a cap bounds loops and resource use, not destination; one redirect to an internal address is under every cap. Not per-hop validation — a signed URL is legitimately cross-origin to the API and the SDK holds no roster of storage hosts, so the only policy it can state is "the host the API named, and nothing that host names in turn". Refusal states exactly that. If the storage tier ever starts redirecting, every SDK fails loudly with the 3xx and a "not followed" message, and the remedy is a spec change argued from the new evidence — not a default that happened to work.

What changed

  • SPEC §13 / §14: hop 2 is redirect: manual; new ### Hop-2 Redirect Policy [conformance] records the rule, the rejected alternatives, the bc3 evidence, and the failure mode. Appendix D row added.
  • Conformance: downloads.json "DownloadURL refuses a redirect on the signed second hop" — three scripted responses, requestCount: 2, statusCode: 302, errorMessage: "not followed", requestPath /signed/logo.png at index -1. It discriminates: Go, TS and Python consumed the third response before this change.
  • Go: CheckRedirect: ErrUseLastResponse on the bare client; isRedirectStatus shared with hop 1's dispatch.
  • TypeScript: fetch(resolvedLocation, { redirect: "manual" }); a browser's opaqueredirect (status 0) falls into the existing !ok refusal.
  • Python: follow_redirects=False, plus a real bug fix — hop 2 checked >= 400, so a 3xx (or a 304) would have returned as a success with an empty body. Now "not 2xx". Pinned by a 304 test.
  • Swift: fetchSignedDownload uses transport.dataNoRedirect(for:). The test MockTransport now records which entry point carried each request, so the test pins the seam the mock cannot otherwise observe. URLSessionTransport.dataNoRedirect now blocks the redirect with a task-level delegate on the caller's own session (as data(for:) already does to sanitize credentials) instead of a one-shot session copy, so a caller's pinning/mTLS/auth-challenge delegate applies on both hops — it was silently dropped on hop 1 before.
  • Kotlin / Ruby: the explicit refusal and its message. Ruby matches the five statuses rather than Net::HTTPRedirection, which also covers 304 (caught in self-review, tested).
  • A unit test in every SDK whose third server must stay undialled; a MIGRATING entry.

Verified

Go (go test ./..., golangci-lint, auth-routable guard), TS (full vitest, oxlint, tsc), Python (ruff, mypy, pytest), Ruby (full suite, rubocop), Kotlin (jvmTest), doc-constants-check, conformance-fixtures-check, and the Go/TS/Ruby/Python/Kotlin conformance runners all passing the new case by name.

Not verified locally: Swift. The package does not build on Linux for pre-existing reasons (CFAbsoluteTimeGetCurrent in BaseService.swift); the Swift source and test changes are reviewed by eye only. CI is the check — please look at that leg first.

Filed separately from #804 by design; no dependency either way.

Summary by cubic

Refuses redirects on the signed download hop across all SDKs. Previously Go, TypeScript, Python, and Swift followed hop‑2 redirects; now hop 2 never follows, returns 301/302/303/307/308 as an API error with a “not followed” message, treats any other non‑2xx as failure, and never dials the Location.

  • Spec and conformance: adds SPEC §14 “Hop‑2 Redirect Policy” and a conformance case that asserts two requests, a 302 status, and the refusal message; prose now names the exact redirect set (301, 302, 303, 307, 308).
  • SDKs: Go sets net/http CheckRedirect to refuse; TypeScript uses fetch with redirect: "manual"; Python sets httpx follow_redirects=False and checks “not 2xx”; Swift routes hop 2 through URLSession’s no‑redirect path and now blocks redirects via a task‑level delegate on the caller’s session (preserving the session delegate); Kotlin and Ruby surface the explicit refusal message. Each adds a test pinning that the third host is never contacted.
  • Migration (required): if your storage tier redirects signed URLs, downloads now fail with that redirect status. There is no flag to re‑enable following; return the final storage URL from the API instead. MIGRATING documents the behavior change.