From eb58f9f05323411762923ba4182f79ef5d03ee18 Mon Sep 17 00:00:00 2001 From: autonomic-bot Date: Thu, 11 Jun 2026 21:42:42 +0000 Subject: [PATCH] =?UTF-8?q?review(drone):=20ADV-drone-01=20CRITICAL=20?= =?UTF-8?q?=E2=80=94=20test=5Fscm=5Fconfigured=20follows=20all=20redirects?= =?UTF-8?q?;=20assertion=20always=20fails=20even=20when=20wired=20correctl?= =?UTF-8?q?y?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- machine-docs/BACKLOG-drone.md | 73 ++++++++++++++++++++++++++++++++++- machine-docs/REVIEW-drone.md | 17 ++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/machine-docs/BACKLOG-drone.md b/machine-docs/BACKLOG-drone.md index e9c5a5a..3cd040e 100644 --- a/machine-docs/BACKLOG-drone.md +++ b/machine-docs/BACKLOG-drone.md @@ -37,4 +37,75 @@ _(Builder's section — Adversary read-only)_ ## Adversary findings -_(no findings yet — phase in initial state)_ +### ADV-drone-01 [adversary] test_scm_configured follows all redirects — assertion always fails + +**Filed:** 2026-06-11T21:37Z +**Severity:** CRITICAL — SCM-configured test is always failing, even for a correctly wired drone + +**Defect:** `tests/drone/functional/test_scm_configured.py::test_login_redirects_to_gitea_dep` +uses `urllib.request.urlopen(req, context=ctx)` which follows ALL redirect hops. The redirect +chain for a correctly-wired drone is: + +1. `GET /login` → 303 → `https:///login/oauth/authorize?client_id=...&...` +2. Gitea (unauthenticated user) → 302 → `https:///user/login?redirect_to=...` +3. Final: `https:///user/login` (200 OK) + +The test asserts `parsed.path == "/login/oauth/authorize"` but `final_url` is `/user/login`. +**The assertion ALWAYS fails even when drone is correctly wired.** + +**Verified:** reproduced against the live drone.ci.commoninternet.net: +``` +python3 -c " +import ssl, urllib.request, urllib.parse +ctx = ssl.create_default_context(); ctx.check_hostname = False; ctx.verify_mode = ssl.CERT_NONE +req = urllib.request.Request('https://drone.ci.commoninternet.net/login', method='GET') +with urllib.request.urlopen(req, timeout=30, context=ctx) as resp: + print(resp.geturl()) +# → https://git.autonomic.zone/user/login (NOT /login/oauth/authorize) +" +``` + +**Root cause:** The test was designed around the first-redirect check (per REVIEW-drone.md +pre-probe) but implemented as a follow-all check. The pre-probe used `curl --max-redirs 0` to +capture the Location header — the test must replicate this, not `urlopen(follow=True)`. + +**Required fix:** Capture ONLY drone's first redirect (the 303 → gitea OAuth authorize), stop +before gitea's own redirects. One correct pattern: + +```python +class _CaptureOneRedirect(urllib.request.HTTPRedirectHandler): + def http_error_302(self, req, fp, code, msg, headers): + raise urllib.error.HTTPError(req.full_url, code, msg, headers, fp) + http_error_303 = http_error_302 + +opener = urllib.request.build_opener( + _CaptureOneRedirect(), + urllib.request.HTTPSHandler(context=ctx), +) +try: + opener.open(f"https://{live_app}/login", timeout=30) + pytest.fail("Expected redirect from /login but got 200") +except urllib.error.HTTPError as e: + if e.code not in (302, 303): + raise AssertionError(f"Expected 302/303 from /login, got {e.code}") + redirect_url = e.headers.get("Location") or e.headers.get("location", "") + +parsed = urllib.parse.urlparse(redirect_url) +# now check parsed.netloc == gitea_domain and parsed.path == "/login/oauth/authorize" +``` + +**Also note:** The unit test `test_scm_redirect_assertions` tests the URL assertion logic +correctly (with pre-supplied URLs), but does NOT test the redirect-capture mechanism. A unit +test for `_CaptureOneRedirect` behavior against a mock HTTP server would be ideal, but at +minimum the integration test must use this pattern. + +**Repro steps:** +1. Deploy a correctly-wired drone (with gitea dep, compose.gitea.yml, DRONE_GITEA_CLIENT_ID set) +2. Run `test_login_redirects_to_gitea_dep` +3. It will FAIL with `AssertionError: Final URL path is '/user/login', expected '/login/oauth/authorize'` +4. This is a false failure — the assertion is about the URL AFTER gitea's own redirect, not drone's redirect + +**Resolution:** Builder fixes test to use no-follow-first-redirect pattern. Adversary re-verifies +by running the test against a live wired drone after fix. + +- [ ] OPEN — awaiting Builder fix diff --git a/machine-docs/REVIEW-drone.md b/machine-docs/REVIEW-drone.md index 5c35536..48c2852 100644 --- a/machine-docs/REVIEW-drone.md +++ b/machine-docs/REVIEW-drone.md @@ -105,6 +105,23 @@ Both need to be mirrored before `!testme` can be used. Builder must follow the r --- +## Pre-claim findings (before M1 is claimed) + +### ADV-drone-01 — test_scm_configured redirect bug (CRITICAL) + +**Filed:** 2026-06-11T21:37Z — see BACKLOG-drone.md for full details. + +`test_login_redirects_to_gitea_dep` uses `urllib.request.urlopen` (follow-all-redirects). The +chain is: drone /login → 303 → gitea OAuth authorize → 302 → gitea /user/login (unauthenticated). +`final_url` is `/user/login`, so `parsed.path == "/login/oauth/authorize"` is always False. +**The test always fails, even for a correctly wired drone.** + +Fix: capture only drone's first redirect (no-follow pattern; capture Location header from 303). + +This must be fixed before M1 can be claimed. If M1 is claimed without this fix, I will VETO. + +--- + ## Standing break-it probes - [ ] Verify drone WITHOUT gitea wiring fails SCM-configured test (negative control)