zenodo: send the API token as a Bearer header and verify the bucket host

.github/scripts/test_zenodo_release.py

@@ -34,19 +34,29 @@ def json(self) -> dict:
         return self._payload
 
 
-def draft_payload(deposit_id: int = 111, files: list | None = None) -> dict:
+def draft_payload(
+    deposit_id: int = 111,
+    files: list | None = None,
+    bucket_base: str = "https://zenodo.org/api",
+) -> dict:
+    # Zenodo returns the bucket on its own host. `bucket_base` lets a test hand
+    # back a foreign host instead, which the script must refuse to upload to.
     return {
         "id": deposit_id,
         "files": files or [],
         "links": {
-            "bucket": f"https://example.invalid/api/files/bucket-{deposit_id}",
+            "bucket": f"{bucket_base}/files/bucket-{deposit_id}",
             "html": f"https://example.invalid/deposit/{deposit_id}",
             "latest_draft": f"https://example.invalid/api/deposit/depositions/{deposit_id}",
         },
     }
 
 
-def install_fake_requests(concept_exists: bool, inherited: list | None = None) -> None:
+def install_fake_requests(
+    concept_exists: bool,
+    inherited: list | None = None,
+    bucket_base: str = "https://zenodo.org/api",
+) -> None:
     """Swap in a `requests` module that records calls and returns canned data."""
 
     def record(method):
@@ -55,9 +65,9 @@ def call(url, **kwargs):
             if method == "GET" and "/records/" in url:
                 return FakeResponse({"id": 999})
             if method == "GET" and "/deposit/depositions/" in url:
-                return FakeResponse(draft_payload(111, inherited))
+                return FakeResponse(draft_payload(111, inherited, bucket_base))
             if method == "POST" and url.endswith("/actions/newversion"):
-                return FakeResponse(draft_payload(111, inherited))
+                return FakeResponse(draft_payload(111, inherited, bucket_base))
             if method == "POST" and url.endswith("/actions/publish"):
                 return FakeResponse(
                     {
@@ -67,10 +77,10 @@ def call(url, **kwargs):
                     }
                 )
             if method == "POST" and url.endswith("/deposit/depositions"):
-                return FakeResponse(draft_payload(111))
+                return FakeResponse(draft_payload(111, None, bucket_base))
             if method == "DELETE":
                 return FakeResponse({})
-            return FakeResponse(draft_payload(111, inherited))
+            return FakeResponse(draft_payload(111, inherited, bucket_base))
 
         return call
 
@@ -92,10 +102,39 @@ def sequence() -> list[tuple[str, str]]:
     return [(m, u.split("/api", 1)[-1] if "/api" in u else u) for m, u, _ in CALLS]
 
 
-def run(name: str, *, concept: str | None, publish: bool, sandbox: bool, inherited=None):
-    global CALLS
-    CALLS = []
-    install_fake_requests(bool(concept), inherited)
+def check_token(label: str) -> None:
+    """Assert the token reached every call site of one run as a header.
+
+    `CALLS` resets at the top of `run()`, so this has to be called after each
+    run. The concept-record path carries four call sites the first run never
+    reaches, and they went unasserted while this lived inline.
+    """
+    # The token belongs in a header. Query strings reach access logs, egress
+    # proxies and Referer headers, all of which outlive the request.
+    check(
+        f"token sent as an Authorization Bearer header ({label})",
+        all((k.get("headers") or {}).get("Authorization") == "Bearer fake-token" for _, _, k in CALLS),
+    )
+    check(
+        f"token never reaches a query string ({label})",
+        not any(
+            "access_token" in u or "access_token" in (k.get("params") or {})
+            for _, u, k in CALLS
+        ),
+    )
+
+
+def build_inputs():
+    """Bundle and metadata, rebuilt from the repository each time."""
+    config = z.load_config()
+    cff = z.load_citation()
+    spec = config["deposits"]["top10"]
+    files = z.collect_files(spec, "2026")
+    bundle = z.write_bundle(files, Path(sys.argv[1] if len(sys.argv) > 1 else "/tmp") / "t.zip")
+    return bundle, z.build_metadata(cff, spec, "2026", "2026-06-15")
+
+
+def set_env(concept: str | None) -> None:
     import os
 
     os.environ["ZENODO_TOKEN"] = "fake-token"
@@ -104,12 +143,23 @@ def run(name: str, *, concept: str | None, publish: bool, sandbox: bool, inherit
     else:
         os.environ.pop("TEST_CONCEPT", None)
 
-    config = z.load_config()
-    cff = z.load_citation()
-    spec = config["deposits"]["top10"]
-    files = z.collect_files(spec, "2026")
-    bundle = z.write_bundle(files, Path(sys.argv[1] if len(sys.argv) > 1 else "/tmp") / "t.zip")
-    metadata = z.build_metadata(cff, spec, "2026", "2026-06-15")
+
+def run(
+    name: str,
+    *,
+    concept: str | None,
+    publish: bool,
+    sandbox: bool,
+    inherited=None,
+    bucket_base: str | None = None,
+):
+    global CALLS
+    CALLS = []
+    base = "https://sandbox.zenodo.org/api" if sandbox else "https://zenodo.org/api"
+    install_fake_requests(bool(concept), inherited, bucket_base or base)
+    set_env(concept)
+
+    bundle, metadata = build_inputs()
     print(f"\n{name}")
     z.deposit(bundle, metadata, concept_env="TEST_CONCEPT", sandbox=sandbox, publish=publish)
     return metadata
@@ -123,9 +173,10 @@ def run(name: str, *, concept: str | None, publish: bool, sandbox: bool, inherit
 check("does not call newversion", not any("newversion" in u for _, u in seq))
 check("uploads to the bucket URL", any(m == "PUT" and "files/bucket" in u for m, u in seq))
 check("publishes", any(u.endswith("/actions/publish") for _, u in seq))
-# The bucket URL comes from Zenodo's response, so only check URLs we build.
-ours = [u for _, u, _ in CALLS if "zenodo.org" in u]
-check("uses the production API base", ours and all(u.startswith("https://zenodo.org/api") for u in ours))
+# Every request now goes to the API host, the bucket upload included, so this
+# covers the whole run. A foreign URL fails here rather than being filtered out.
+urls = [u for _, u, _ in CALLS]
+check("uses the production API base", urls and all(u.startswith("https://zenodo.org/api") for u in urls))
 put_meta = [k for m, u, k in CALLS if m == "PUT" and "depositions" in u]
 check("sends metadata under a `metadata` key", bool(put_meta) and "metadata" in put_meta[0]["json"])
 sent = put_meta[0]["json"]["metadata"]
@@ -140,7 +191,7 @@ def run(name: str, *, concept: str | None, publish: bool, sandbox: bool, inherit
 check("every author is deposited", len(sent["creators"]) == len(people), str(len(sent["creators"])))
 check("author order survives", sent["creators"][-1]["name"] == "Klondike, Gavin")
 check("contributor type is valid", sent["contributors"][0]["type"] == "HostingInstitution")
-check("token passed as access_token", all(k.get("params", {}).get("access_token") == "fake-token" for _, _, k in CALLS))
+check_token("first deposit")
 
 run(
     "new version with inherited files, sandbox, draft only",
@@ -155,9 +206,45 @@ def run(name: str, *, concept: str | None, publish: bool, sandbox: bool, inherit
 check("deletes the inherited file", any(m == "DELETE" and u.endswith("/files/f1") for m, u in seq))
 check("uploads the new bundle", any(m == "PUT" and "files/bucket" in u for m, u in seq))
 check("does NOT publish in draft mode", not any(u.endswith("/actions/publish") for _, u in seq))
-ours = [u for _, u, _ in CALLS if "zenodo.org" in u]
-check("uses the sandbox API base", ours and all(u.startswith("https://sandbox.zenodo.org/api") for u in ours))
-check("never touches production", not any(u.startswith("https://zenodo.org/api") for u in ours))
+urls = [u for _, u, _ in CALLS]
+check("uses the sandbox API base", urls and all(u.startswith("https://sandbox.zenodo.org/api") for u in urls))
+check("never touches production", not any(u.startswith("https://zenodo.org/api") for u in urls))
+# The concept path adds four call sites the first run never reaches: the record
+# GET, the newversion POST, the draft GET and the inherited-file DELETE.
+check_token("new version")
+
+# The bucket URL arrives in Zenodo's response. Attaching the token to whatever
+# host it names would hand the credential to anyone who can shape that response.
+def refuse(name: str, bucket_base: str) -> str:
+    """Run a deposit that must fail closed, and return why it exited.
+
+    The reason matters: a bare `except SystemExit` also catches the missing
+    bucket link and every API error, so it would pass with the host check gone.
+    """
+    global CALLS
+    CALLS = []
+    install_fake_requests(False, None, bucket_base)
+    set_env(None)
+    bundle, metadata = build_inputs()
+    print(f"\n{name}")
+    try:
+        z.deposit(bundle, metadata, concept_env="TEST_CONCEPT", sandbox=False, publish=True)
+        return ""
+    except SystemExit as exc:
+        return str(exc)
+
+
+why = refuse("bucket URL on a host the script did not verify", "https://evil.invalid/api")
+check("refuses on a host mismatch", "does not match the API host" in why, why or "no SystemExit")
+check("sends nothing to the foreign host", not any("evil.invalid" in u for _, u, _ in CALLS))
+check("does not publish", not any(u.endswith("/actions/publish") for _, u, _ in CALLS))
+
+# Right host, wrong scheme. Comparing netloc alone lets this through, and the
+# token then crosses the network in cleartext.
+why = refuse("bucket URL on the API host over http", "http://zenodo.org/api")
+check("refuses on a plaintext bucket URL", "is not https" in why, why or "no SystemExit")
+check("sends the token over no cleartext URL", not any(u.startswith("http://") for _, u, _ in CALLS))
+check("does not publish over http", not any(u.endswith("/actions/publish") for _, u, _ in CALLS))
 
 print()
 if FAILURES:

.github/scripts/zenodo_release.py

@@ -258,6 +258,30 @@ def api_call(response, what: str) -> dict:
     return response.json() if response.content else {}
 
 
+def upload_url(draft: dict, base: str, filename: str) -> str:
+    """The upload target for the bundle, once its host has been checked.
+
+    Zenodo names the bucket in its own response. The token is about to be
+    attached to that URL, so anything able to shape the response could otherwise
+    redirect a `deposit:write` credential to a host of its choosing, or downgrade
+    it to cleartext. Fail closed unless the bucket sits on the API host the
+    script chose and arrives over https.
+
+    Scheme and host exit separately. ZENODO.md documents the error text an
+    operator has to act on, and the two failure modes call for different action.
+    """
+    bucket = (draft.get("links") or {}).get("bucket")
+    if not bucket:
+        sys.exit("error: Zenodo returned no bucket link for the draft")
+    if urlparse(bucket).scheme != "https":
+        sys.exit(f"error: bucket URL is not https: {bucket!r}")
+    expected = urlparse(base).netloc
+    found = urlparse(bucket).netloc
+    if found != expected:
+        sys.exit(f"error: bucket host {found!r} does not match the API host {expected!r}")
+    return f"{bucket}/{filename}"
+
+
 def deposit(
     bundle: Path,
     metadata: dict,
@@ -271,27 +295,29 @@ def deposit(
     if not token:
         sys.exit("error: ZENODO_TOKEN is not set")
     base = "https://sandbox.zenodo.org/api" if sandbox else "https://zenodo.org/api"
-    params = {"access_token": token}
+    # A header rather than an `access_token` query parameter: query strings are
+    # recorded by access logs, egress proxies and Referer headers.
+    headers = {"Authorization": f"Bearer {token}"}
     concept = os.environ.get(concept_env)
 
     if concept:
         print(f"creating a new version of concept record {concept}")
         latest = api_call(
-            requests.get(f"{base}/records/{concept}", params=params, timeout=TIMEOUT),
+            requests.get(f"{base}/records/{concept}", headers=headers, timeout=TIMEOUT),
             "resolving concept record",
         )
         versioned = api_call(
             requests.post(
                 f"{base}/deposit/depositions/{latest['id']}/actions/newversion",
-                params=params,
+                headers=headers,
                 timeout=TIMEOUT,
             ),
             "creating new version",
         )
         draft_id = urlparse(versioned["links"]["latest_draft"]).path.rstrip("/").rsplit("/", 1)[-1]
         draft = api_call(
             requests.get(
-                f"{base}/deposit/depositions/{draft_id}", params=params, timeout=TIMEOUT
+                f"{base}/deposit/depositions/{draft_id}", headers=headers, timeout=TIMEOUT
             ),
             "fetching draft",
         )
@@ -300,7 +326,7 @@ def deposit(
             api_call(
                 requests.delete(
                     f"{base}/deposit/depositions/{draft['id']}/files/{stale['id']}",
-                    params=params,
+                    headers=headers,
                     timeout=TIMEOUT,
                 ),
                 f"removing inherited file {stale.get('filename')}",
@@ -309,27 +335,25 @@ def deposit(
         print(f"{concept_env} is unset, creating the first deposition")
         draft = api_call(
             requests.post(
-                f"{base}/deposit/depositions", params=params, json={}, timeout=TIMEOUT
+                f"{base}/deposit/depositions", headers=headers, json={}, timeout=TIMEOUT
             ),
             "creating deposition",
         )
 
+    # Resolved before the file is opened, so a rejected host stops the run
+    # without the token ever leaving the process.
+    target = upload_url(draft, base, bundle.name)
     with bundle.open("rb") as handle:
         api_call(
-            requests.put(
-                f"{draft['links']['bucket']}/{bundle.name}",
-                data=handle,
-                params=params,
-                timeout=TIMEOUT,
-            ),
+            requests.put(target, data=handle, headers=headers, timeout=TIMEOUT),
             f"uploading {bundle.name}",
         )
     print(f"uploaded {bundle.name} ({bundle.stat().st_size} bytes)")
 
     draft = api_call(
         requests.put(
             f"{base}/deposit/depositions/{draft['id']}",
-            params=params,
+            headers=headers,
             json={"metadata": metadata},
             timeout=TIMEOUT,
         ),
@@ -343,7 +367,7 @@ def deposit(
     published = api_call(
         requests.post(
             f"{base}/deposit/depositions/{draft['id']}/actions/publish",
-            params=params,
+            headers=headers,
             timeout=TIMEOUT,
         ),
         "publishing",

documentation/ZENODO.md

@@ -96,6 +96,37 @@ use it. Decide who owns the record before wiring that up.
    the existing record. Without it the script creates an unrelated record and
    splits the citation history.
 
+## How the token is handled
+
+The token goes out as an `Authorization: Bearer` header, never as a query
+parameter. URLs carrying a credential get copied into access logs, egress
+proxies and `Referer` headers, all of which outlive the request.
+
+Zenodo names the upload bucket in its own response, and the script checks that
+bucket before attaching the token. Two things have to hold: the bucket sits on
+the same host as the API, and it arrives over https.
+
+If Zenodo ever moves file storage to a different host, the deposit stops with:
+
+```text
+error: bucket host 'files.example.org' does not match the API host 'zenodo.org'
+```
+
+That is the check doing its job, not a broken bundle. Confirm the new host is
+really Zenodo's, then widen the comparison in `upload_url()`. Failing closed is
+deliberate: the alternative sends a `deposit:write` token wherever the response
+points.
+
+A bucket URL on the right host but the wrong scheme stops with:
+
+```text
+error: bucket URL is not https: 'http://zenodo.org/api/files/bucket-1'
+```
+
+Treat that one as a problem to report, not a comparison to widen. Uploading
+would put the token on the wire in cleartext, so there is no version of this
+worth allowing.
+
 ## Releasing
 
 Add `<edition>/final/`, update `CITATION.cff` with that edition's authors, then: