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

[rocklambros]

Closes #138. Follow-up to #124.

What changed

The token now goes out as Authorization: Bearer. deposit() previously built params = {"access_token": token} and passed it to all six request calls. Query strings get copied into access logs, egress proxies and Referer headers, so the credential outlived the request in places nobody audits.

The bucket host is verified before the token is attached. The upload targeted draft["links"]["bucket"], a URL taken from the API response. The API base is hardcoded, so this rested on trusting Zenodo to name its own host rather than on an open redirect, but a token carrying deposit:write and deposit:actions should not follow a URL the script never checked. upload_url() compares the bucket host against the API host and fails closed.

Ordering matters here: the target resolves before bundle.open(), so a rejected host ends the run with nothing sent and no file handle opened.

Neither is urgent while the script runs by hand from a laptop. Both matter once the deposit runs from CI against a repository secret, which is the direction documentation/ZENODO.md sets out.

Tests

Three assertions were added, and each was confirmed failing against the old code before the fix went in:

Assertion	Failure it catches
token sent as an Authorization Bearer header	Reverting to params=
token never reaches a query string	A stray access_token on any call
refuses to upload / sends nothing to the foreign host / does not publish	Dropping the host check

The stub previously returned a bucket on example.invalid while the API base was zenodo.org. That no longer reflects Zenodo, which serves the bucket from the API host, so draft_payload() takes a bucket_base and the happy-path runs pass the matching host. The foreign-host case now has its own run.

Full suite: 26 assertions, all passing. --check passes. semgrep p/python, p/security-audit, p/secrets on .github/scripts/: 0 findings.

One tradeoff worth naming

The host check fails closed. If Zenodo ever moves file storage to a separate host, the deposit stops rather than uploading. That is the safer default for a credential, but it is a release-blocking failure mode, so documentation/ZENODO.md now shows the exact error and says what to do about it: confirm the new host really is Zenodo's, then widen the comparison in upload_url().

Not changed

The script is still wired to nothing. No workflow runs it, and no Zenodo token belongs in repository secrets until record ownership is settled, per the reasoning in #124.

[rocklambros]

@stefanoamorelli tagging you since this is your script from #124 and it changes a test you wrote.

Two things worth your judgment rather than a rubber stamp:

1. The fail-closed host check. If Zenodo moves file storage off the API host, this stops the deposit instead of uploading. I think that is right for a deposit:write token, but it turns a security property into a release-blocking one on a path that runs about once a year. You know the Zenodo side better than I do. If their bucket has ever resolved somewhere other than the API host, say so and I will make the comparison a host allowlist instead of an equality check.

2. The test stub change. The old draft_payload() returned a bucket on example.invalid while the API base was zenodo.org, with the note on line 126 that the bucket URL comes from Zenodo so only self-built URLs get checked. That mismatch would have hidden the new check completely, so draft_payload() now takes a bucket_base and the happy paths use the matching host. Flag it if you had a reason for the foreign host that I missed.

No rush on this one. Nothing runs the script automatically, so it is a hardening pass ahead of any CI wiring rather than a live exposure.

[stefanoamorelli]

thanks @rocklambros! both changes LGTM.

1. you can keep the equality check. Zenodo's docs return the bucket on the API host, and our record (22109015) has every file link on zenodo.org with no redirects.
2. example.invalid was reserved-domain hygiene.

Two things to fix, inline comments:

1. Token assertions run after run 1 only, so the concept branch is unasserted. (I reverted the records/{concept} GET to params= and the suite still printed all checks passed.)
2. upload_url checks the host but not the scheme, so http://zenodo.org/api/files/... sends the token cleartext.

nits:

• line 154's comment no longer holds, the bucket is inside ours now
• check("refuses to upload", refused) passes on any SystemExit

[stefanoamorelli]

These two only run against run 1, and CALLS resets at the top of every run(). Run 1 passes concept=None, so the records/{concept} GET, the newversion POST, the draft GET and the inherited-file DELETE never get asserted, which is 4 of the 8 call sites you moved onto headers=. I reverted the concept GET to params= and the full suite still printed all checks passed.

Hoisting into a helper covers it:

Suggested change

	# The token belongs in a header. Query strings reach access logs, egress proxies
	# and Referer headers, all of which outlive the request.
	check(
	"token sent as an Authorization Bearer header",
	all((k.get("headers") or {}).get("Authorization") == "Bearer fake-token" for _, _, k in CALLS),
	)
	check(
	"token never reaches a query string",
	not any(
	"access_token" in u or "access_token" in (k.get("params") or {})
	for _, u, k in CALLS
	),
	)
	# The token belongs in a header. Query strings reach access logs, egress proxies
	# and Referer headers, all of which outlive the request.
	def check_token(label: str) -> None:
	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
	),
	)
	check_token("first deposit")

then check_token("new version") after line 200. I ran it, suite stays green and that same revert now fails both.

[rocklambros]

Hoisted into check_token(label), called after both runs. 69f2937

Confirmed your repro before and after: reverting the concept GET to params= now fails both assertions on the new-version run, where it previously printed all checks passed.

FAIL token sent as an Authorization Bearer header (new version)
FAIL token never reaches a query string (new version)

[stefanoamorelli]

netloc only, so http://zenodo.org/api/files/bucket-1 passes and the token goes out cleartext. The redirect stripping doesn't help here because the script builds the URL itself, no redirect involved. A separate check keeps the two failure modes legible, which matters now that ZENODO.md documents the exact error text:

Suggested change

	expected = urlparse(base).netloc
	if urlparse(bucket).scheme != "https":
	sys.exit(f"error: bucket URL is not https: {bucket!r}")
	expected = urlparse(base).netloc

Everything else I threw at it refused correctly, including userinfo (https://zenodo.org@evil.invalid), protocol-relative, explicit :443, trailing dot and subdomains.

[rocklambros]

Added as a separate exit ahead of the host comparison, as suggested. 69f2937

Confirmed failing first. With the check absent, a bucket on http://zenodo.org/api uploaded the bundle and published, token attached:

bucket URL on the API host over http
uploaded t.zip (2205823 bytes)
version DOI: 10.5281/zenodo.222
  FAIL refuses on a plaintext bucket URL no SystemExit
  FAIL sends the token over no cleartext URL
  FAIL does not publish over http

Keeping the two exits separate earns its keep in the docs: ZENODO.md now carries both strings and says to widen the host comparison for one and report the other, since there is no version of a cleartext bucket worth allowing.

[rocklambros]

@stefanoamorelli all four addressed in 69f2937. Both fixes and both nits, with replies in the threads.

Nits

• The comment above the API base check is rewritten. Since the stub now serves the bucket from the API host, ours was filtering nothing and would have silently dropped a foreign URL from the very check meant to catch one. It reads every call in the run now.
• refuses to upload asserted on a bare SystemExit. It now asserts on the reason. Verified by making upload_url() exit with the missing-bucket message instead: the old form passed, the new one fails with error: Zenodo returned no bucket link for the draft printed as the detail.

One thing I added beyond the review

The scheme check had no test of its own, which is the same failure your review is about, so the http case got its own run. That is what produced the RED output in the thread above.

Kept as-is

Equality check stays, on your read of the docs and record 22109015. example.invalid stays on html and latest_draft where the reserved-domain hygiene still applies. Only the bucket moved to the API host, and only because the check needs a realistic happy path.

Verification

31 assertions, all passing. Each new one confirmed failing against the code it guards, including your params= revert. --check passes. semgrep p/python, p/security-audit, p/secrets on .github/scripts/: 236 rules, 0 findings.

Still wired to nothing, so no live exposure either way. Back to you when you have a moment.