Summary
tools/sourcehut/src/magpie_sourcehut/client.py issues its GraphQL POST
with a bare urllib.request.urlopen(req) — no HTTPS assertion and no
redirect handler. The request carries Authorization: Bearer <token>, so a
redirect response would cause urllib's default handler to follow it and
re-send the credential to the redirect target.
Surfaced while reviewing #1078, which added the first write to the Bitbucket
bridge. That bridge now demonstrates the safer pattern, so this is a
consistency gap rather than a new discovery.
Current state
req = urllib.request.Request(
...,
headers={..., "Authorization": f"Bearer {token}"},
method="POST",
)
try:
with urllib.request.urlopen(req) as resp:No _require_https(url) equivalent, and no build_opener(...), so
HTTPRedirectHandler applies by default.
What the Bitbucket bridge does instead
tools/bitbucket/src/magpie_bitbucket/client.py:
_require_https(url) before the request is built, so a plaintext URL
cannot carry the credential at all.- Two explicit redirect policies rather than the default:
NoAuthRedirectHandler — raises on any redirect. Used by get_json and
by the new post_json.SameHostRedirectHandler — follows only same-origin HTTPS redirects,
rejecting host changes, port changes and scheme downgrades. Used by
get_text.
- Writes deliberately take the strict handler: replaying a mutation at a
redirected location is worse than failing and making the caller retry.
Suggested change
- Add an HTTPS assertion before building the request.
- Build an opener with an explicit redirect policy instead of relying on the
default. For a mutation, rejecting redirects outright is the appropriate
choice, matching post_json.
- Add tests for the redirect policy. Note
tools/bitbucket has four tests
for SameHostRedirectHandler but none for NoAuthRedirectHandler — worth
covering both handlers wherever they guard a write.
Notes
No evidence this has been exploited or that sourcehut.org issues such
redirects; this is defence in depth on a credential-bearing request, and
alignment with the pattern the Bitbucket bridge now sets.
Summary
tools/sourcehut/src/magpie_sourcehut/client.pyissues its GraphQLPOSTwith a bare
urllib.request.urlopen(req)— no HTTPS assertion and noredirect handler. The request carries
Authorization: Bearer <token>, so aredirect response would cause urllib's default handler to follow it and
re-send the credential to the redirect target.
Surfaced while reviewing #1078, which added the first write to the Bitbucket
bridge. That bridge now demonstrates the safer pattern, so this is a
consistency gap rather than a new discovery.
Current state
No
_require_https(url)equivalent, and nobuild_opener(...), soHTTPRedirectHandlerapplies by default.What the Bitbucket bridge does instead
tools/bitbucket/src/magpie_bitbucket/client.py:_require_https(url)before the request is built, so a plaintext URLcannot carry the credential at all.
NoAuthRedirectHandler— raises on any redirect. Used byget_jsonandby the new
post_json.SameHostRedirectHandler— follows only same-origin HTTPS redirects,rejecting host changes, port changes and scheme downgrades. Used by
get_text.redirected location is worse than failing and making the caller retry.
Suggested change
default. For a mutation, rejecting redirects outright is the appropriate
choice, matching
post_json.tools/bitbuckethas four testsfor
SameHostRedirectHandlerbut none forNoAuthRedirectHandler— worthcovering both handlers wherever they guard a write.
Notes
No evidence this has been exploited or that sourcehut.org issues such
redirects; this is defence in depth on a credential-bearing request, and
alignment with the pattern the Bitbucket bridge now sets.