Summary
The Bitbucket bridge has several defensive guards that exist because earlier reviews asked for
them, and none of them has a test. Add cases so a later simplification cannot remove them
silently.
Background
Three guards are currently unprotected:
1. Non-advancing pagination cursor (Data Center). get_pull_request_commits breaks out of
its while True loop when nextPageStart is missing, not an integer, or not greater than the
current start. That last check exists because a review on #766 pointed out the loop would hang
forever against a server that keeps returning the same cursor. The tests cover only the happy
path: nextPageStart: 25 followed by isLastPage: True.
2. Repeated pagination URL (Cloud). The Cloud fetchers thread a seen_urls set through
_validated_next_url so a next link pointing back at an already-fetched URL is rejected.
No test exercises that rejection.
3. Redirect scheme downgrade. SameHostRedirectHandler compares scheme, host and effective
port before following a redirect, so an https → http redirect on the same host is refused
and the Authorization header is never sent in clear text. Three tests exist for that handler
— same-origin accepted, different host rejected, different port rejected — but not the downgrade,
which is the security-relevant one.
Each guard is a couple of lines of code and each test is a couple of lines of fixture. The value
is that the next person to tidy these loops finds out immediately.
Where to look
tools/bitbucket/tests/test_bitbucket.py — all the tests live here.
test_datacenter_get_pull_request_commits_follows_next_page_start and
test_same_host_redirect_handler_rejects_different_port are the closest models to copy.
tools/bitbucket/src/magpie_bitbucket/datacenter.py — get_pull_request_commits, for the
three loop exits.
tools/bitbucket/src/magpie_bitbucket/cloud.py — the seen_urls set and
_validated_next_url.
tools/bitbucket/src/magpie_bitbucket/client.py — SameHostRedirectHandler, _origin and
_effective_port.
Acceptance criteria
Estimated effort
~2 hours for someone new to the codebase; the third test is the most interesting and the fixtures
for the first two are small.
Getting started
[How to contribute](https://github.com/apache/airflow-steward/blob/main/CONTRIBUTING.md#your-first-contribution)
This issue was drafted with the help of an AI-assisted tool and reviewed by a Magpie maintainer before posting. If anything here is unclear or looks wrong, say so on the issue: a real person is reading.
Summary
The Bitbucket bridge has several defensive guards that exist because earlier reviews asked for
them, and none of them has a test. Add cases so a later simplification cannot remove them
silently.
Background
Three guards are currently unprotected:
1. Non-advancing pagination cursor (Data Center).
get_pull_request_commitsbreaks out ofits
while Trueloop whennextPageStartis missing, not an integer, or not greater than thecurrent
start. That last check exists because a review on #766 pointed out the loop would hangforever against a server that keeps returning the same cursor. The tests cover only the happy
path:
nextPageStart: 25followed byisLastPage: True.2. Repeated pagination URL (Cloud). The Cloud fetchers thread a
seen_urlsset through_validated_next_urlso anextlink pointing back at an already-fetched URL is rejected.No test exercises that rejection.
3. Redirect scheme downgrade.
SameHostRedirectHandlercompares scheme, host and effectiveport before following a redirect, so an
https→httpredirect on the same host is refusedand the
Authorizationheader is never sent in clear text. Three tests exist for that handler— same-origin accepted, different host rejected, different port rejected — but not the downgrade,
which is the security-relevant one.
Each guard is a couple of lines of code and each test is a couple of lines of fixture. The value
is that the next person to tidy these loops finds out immediately.
Where to look
tools/bitbucket/tests/test_bitbucket.py— all the tests live here.test_datacenter_get_pull_request_commits_follows_next_page_startandtest_same_host_redirect_handler_rejects_different_portare the closest models to copy.tools/bitbucket/src/magpie_bitbucket/datacenter.py—get_pull_request_commits, for thethree loop exits.
tools/bitbucket/src/magpie_bitbucket/cloud.py— theseen_urlsset and_validated_next_url.tools/bitbucket/src/magpie_bitbucket/client.py—SameHostRedirectHandler,_originand_effective_port.Acceptance criteria
isLastPage: Falseand anextPageStartequal toor below the current
start, and asserts the loop terminates rather than hanging.nextURL is rejected rather than re-fetched.https→httpredirect on the same host raises rather than beingfollowed.
test, then restoring it.)
Estimated effort
~2 hours for someone new to the codebase; the third test is the most interesting and the fixtures
for the first two are small.
Getting started
[How to contribute](https://github.com/apache/airflow-steward/blob/main/CONTRIBUTING.md#your-first-contribution)
This issue was drafted with the help of an AI-assisted tool and reviewed by a Magpie maintainer before posting. If anything here is unclear or looks wrong, say so on the issue: a real person is reading.