diff --git a/machine-docs/BACKLOG-drone.md b/machine-docs/BACKLOG-drone.md index 3cd040e..8b9a73e 100644 --- a/machine-docs/BACKLOG-drone.md +++ b/machine-docs/BACKLOG-drone.md @@ -108,4 +108,4 @@ minimum the integration test must use this pattern. **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 +- [x] CLOSED — Builder fixed 2026-06-11: `_CaptureOneRedirect` no-follow pattern; unit tests updated; 10/10 pass diff --git a/machine-docs/BUILDER-INBOX.md b/machine-docs/BUILDER-INBOX.md deleted file mode 100644 index dd2bbdb..0000000 --- a/machine-docs/BUILDER-INBOX.md +++ /dev/null @@ -1,24 +0,0 @@ -# BUILDER-INBOX - -**From:** Adversary -**Date:** 2026-06-11T21:45Z -**Re:** ADV-drone-01 CRITICAL — fix before claiming M1 - -Filed ADV-drone-01 in BACKLOG-drone.md. Summary: - -`tests/drone/functional/test_scm_configured.py::test_login_redirects_to_gitea_dep` follows -ALL redirects via `urllib.request.urlopen`. The redirect chain is: - - drone /login → 303 → gitea /login/oauth/authorize → 302 → gitea /user/login (unauthenticated) - -Final URL is `/user/login`. The assertion `parsed.path == "/login/oauth/authorize"` is ALWAYS -False — the test fails even for a correctly wired drone. - -**Verified against live drone.ci.commoninternet.net:** final_url = `https://git.autonomic.zone/user/login`. - -**Fix required:** Stop following redirects after drone's first 303; capture the Location header -from that response. See the exact fix pattern in BACKLOG-drone.md ADV-drone-01. - -Do NOT claim M1 until this is fixed. If claimed without fix, I will VETO. - -— Adversary diff --git a/tests/drone/functional/test_scm_configured.py b/tests/drone/functional/test_scm_configured.py index 4c77fe5..82ea900 100644 --- a/tests/drone/functional/test_scm_configured.py +++ b/tests/drone/functional/test_scm_configured.py @@ -6,30 +6,47 @@ The negative control: a drone deployed WITHOUT DRONE_GITEA_CLIENT_ID + DRONE_GIT authorize endpoint — it would error or redirect elsewhere. This test is therefore falsified by a misconfigured drone. -Test: GET https:///login (following redirects) must land at the per-run gitea dep's -/login/oauth/authorize URL, and the client_id query param must match the OAuth2 app the -harness created in the gitea dep (recorded in deps["gitea"]["client_id"]). +Test: GET https:///login must issue a 303 redirect whose Location header points to +the per-run gitea dep's /login/oauth/authorize URL. We capture ONLY drone's first redirect +(not gitea's subsequent redirect to /user/login for unauthenticated users). -Per the Adversary's pre-probe (REVIEW-drone.md): this redirect mechanism is the correct -SCM-configured tooth — verified against the live drone.ci.commoninternet.net instance. +Per ADV-drone-01: following all redirects causes the assertion to land on gitea's /user/login +(200 OK after gitea redirects unauthenticated users away from /login/oauth/authorize), which +means the path assertion always fails. The fix is a no-follow handler that captures the +Location header from drone's 303 directly. """ from __future__ import annotations import ssl +import urllib.error import urllib.parse import urllib.request import pytest +class _CaptureOneRedirect(urllib.request.HTTPRedirectHandler): + """Stop redirect-following after the FIRST hop; raise HTTPError so the caller can inspect + the Location header from drone's 303 without following gitea's subsequent redirects.""" + + 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 + + @pytest.mark.requires_deps def test_login_redirects_to_gitea_dep(live_app, deps): - """Drone's /login must redirect to the per-run gitea dep's OAuth2 authorize endpoint. + """Drone's /login must issue a 303 redirect to the per-run gitea dep's OAuth2 authorize + endpoint. Proves: (a) gitea is the SCM backend (not github or unconfigured); (b) the OAuth2 - client_id matches the app the harness created in the dep gitea instance; (c) the - redirect targets the TEST-RUN gitea, not any hardcoded external provider. + client_id in the Location header matches the app the harness created in the dep gitea + instance; (c) the redirect targets the TEST-RUN gitea, not any hardcoded external provider. + + ADV-drone-01 fix: only drone's first 303 is captured; gitea's own redirects (unauthenticated + user → /user/login) are not followed, so the path assertion is against the correct URL. """ assert "gitea" in deps, ( f"gitea dep not in deps — dep provisioning should have populated this. " @@ -43,19 +60,41 @@ def test_login_redirects_to_gitea_dep(live_app, deps): ctx.check_hostname = False ctx.verify_mode = ssl.CERT_NONE - # Follow all redirects from /login — the final URL must be gitea's OAuth2 authorize page. - req = urllib.request.Request(f"https://{live_app}/login", method="GET") - with urllib.request.urlopen(req, timeout=30, context=ctx) as resp: - final_url = resp.geturl() + opener = urllib.request.build_opener( + _CaptureOneRedirect(), + urllib.request.HTTPSHandler(context=ctx), + ) - parsed = urllib.parse.urlparse(final_url) - assert parsed.scheme == "https", f"Unexpected scheme in final URL: {final_url!r}" + redirect_url = None + try: + opener.open(f"https://{live_app}/login", timeout=30) + pytest.fail( + f"Expected a 302/303 redirect from https://{live_app}/login but got 200 OK — " + f"drone may not have gitea SCM configured (check COMPOSE_FILE + GITEA_DOMAIN)" + ) + except urllib.error.HTTPError as e: + if e.code not in (302, 303): + raise AssertionError( + f"Expected 302/303 from /login, got {e.code} — " + f"drone may not have gitea SCM configured" + ) from e + redirect_url = e.headers.get("Location") or e.headers.get("location", "") + + assert redirect_url, ( + f"Drone /login returned {e.code} but Location header is empty — " + f"check drone gitea SCM configuration" + ) + + parsed = urllib.parse.urlparse(redirect_url) + assert parsed.scheme == "https", ( + f"Redirect Location has unexpected scheme: {redirect_url!r}" + ) assert parsed.netloc == gitea_domain, ( f"Drone /login did not redirect to the gitea dep ({gitea_domain!r}); " - f"final URL: {final_url!r} — check GITEA_DOMAIN + COMPOSE_FILE in drone's .env" + f"Location: {redirect_url!r} — check GITEA_DOMAIN + COMPOSE_FILE in drone's .env" ) assert parsed.path == "/login/oauth/authorize", ( - f"Final URL path is {parsed.path!r}, expected /login/oauth/authorize — " + f"Redirect path is {parsed.path!r}, expected /login/oauth/authorize — " f"drone may not have gitea SCM configured" ) params = urllib.parse.parse_qs(parsed.query) diff --git a/tests/unit/test_gitea_dep.py b/tests/unit/test_gitea_dep.py index 29e45cf..9c7e466 100644 --- a/tests/unit/test_gitea_dep.py +++ b/tests/unit/test_gitea_dep.py @@ -118,22 +118,22 @@ def test_enrich_deps_gitea_does_not_call_keycloak_path(monkeypatch): # --------------------------------------------------------------------------- -@pytest.mark.parametrize("final_url,gitea_domain,client_id,expect_pass", [ - # Correct redirect: final URL is gitea dep's authorize endpoint with matching client_id +@pytest.mark.parametrize("location_url,gitea_domain,client_id,expect_pass", [ + # Correct redirect: Location header points to gitea dep's authorize endpoint with matching client_id ( "https://gite-aabbcc.ci.commoninternet.net/login/oauth/authorize?client_id=abc-123&redirect_uri=x", "gite-aabbcc.ci.commoninternet.net", "abc-123", True, ), - # Wrong domain: redirected to production gitea, not the dep + # Wrong domain: drone redirected to production gitea, not the dep ( "https://git.autonomic.zone/login/oauth/authorize?client_id=abc-123", "gite-aabbcc.ci.commoninternet.net", "abc-123", False, ), - # Wrong path: not the OAuth authorize endpoint + # Wrong path: not the OAuth authorize endpoint (e.g. gitea's /user/login after full-redirect-follow) ( "https://gite-aabbcc.ci.commoninternet.net/user/login?client_id=abc-123", "gite-aabbcc.ci.commoninternet.net", @@ -148,11 +148,15 @@ def test_enrich_deps_gitea_does_not_call_keycloak_path(monkeypatch): False, ), ]) -def test_scm_redirect_assertions(final_url, gitea_domain, client_id, expect_pass): - """Parametrized verification of the SCM-configured test assertion logic (no HTTP calls).""" +def test_scm_redirect_assertions(location_url, gitea_domain, client_id, expect_pass): + """Parametrized verification of the SCM-configured test assertion logic (no HTTP calls). + + Tests the URL assertions against the Location header from drone's first 303 redirect + (per ADV-drone-01 fix: _CaptureOneRedirect stops after drone's hop, not gitea's). + """ import urllib.parse - parsed = urllib.parse.urlparse(final_url) + parsed = urllib.parse.urlparse(location_url) params = urllib.parse.parse_qs(parsed.query) checks = [ @@ -163,6 +167,6 @@ def test_scm_redirect_assertions(final_url, gitea_domain, client_id, expect_pass ] all_pass = all(checks) assert all_pass == expect_pass, ( - f"Expected {'pass' if expect_pass else 'fail'} for URL {final_url!r}; " + f"Expected {'pass' if expect_pass else 'fail'} for URL {location_url!r}; " f"checks: {checks}" )