Skip to content

Commit 2320d54

Browse files
userhas404dCopilot
andcommitted
fix(auth): address review feedback from #2087
- Fix redirect handler to preserve Authorization on GitHub-to-GitHub redirects (e.g. github.com → codeload.github.com). The previous implementation relied on super().redirect_request() which strips auth on cross-host redirects, breaking private repo archive downloads. - Add codeload.github.com to documented host lists in both EXTENSION-USER-GUIDE.md and presets/README.md - Add redirect auth-preservation and auth-stripping tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d6e9023 commit 2320d54

4 files changed

Lines changed: 43 additions & 3 deletions

File tree

extensions/EXTENSION-USER-GUIDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ In addition to extension-specific environment variables (`SPECKIT_{EXT_ID}_*`),
423423
| Variable | Description | Default |
424424
|----------|-------------|---------|
425425
| `SPECKIT_CATALOG_URL` | Override the full catalog stack with a single URL (backward compat) | Built-in default stack |
426-
| `GH_TOKEN` / `GITHUB_TOKEN` | GitHub token for authenticated requests to GitHub-hosted URLs (`raw.githubusercontent.com`, `github.com`, `api.github.com`). Required when your catalog JSON or extension ZIPs are hosted in a private GitHub repository. | None |
426+
| `GH_TOKEN` / `GITHUB_TOKEN` | GitHub token for authenticated requests to GitHub-hosted URLs (`raw.githubusercontent.com`, `github.com`, `api.github.com`, `codeload.github.com`). Required when your catalog JSON or extension ZIPs are hosted in a private GitHub repository. | None |
427427

428428
#### Example: Using a custom catalog for testing
429429

presets/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ See [scaffold/](scaffold/) for a scaffold you can copy to create your own preset
9696
| Variable | Description | Default |
9797
|----------|-------------|---------|
9898
| `SPECKIT_PRESET_CATALOG_URL` | Override the full catalog stack with a single URL (replaces all defaults) | Built-in default stack |
99-
| `GH_TOKEN` / `GITHUB_TOKEN` | GitHub token for authenticated requests to GitHub-hosted URLs (`raw.githubusercontent.com`, `github.com`, `api.github.com`). Required when your catalog JSON or preset ZIPs are hosted in a private GitHub repository. | None |
99+
| `GH_TOKEN` / `GITHUB_TOKEN` | GitHub token for authenticated requests to GitHub-hosted URLs (`raw.githubusercontent.com`, `github.com`, `api.github.com`, `codeload.github.com`). Required when your catalog JSON or preset ZIPs are hosted in a private GitHub repository. | None |
100100

101101
#### Example: Using a private GitHub-hosted catalog
102102

src/specify_cli/_github_http.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,16 @@ class _StripAuthOnRedirect(urllib.request.HTTPRedirectHandler):
5050
"""
5151

5252
def redirect_request(self, req, fp, code, msg, headers, newurl):
53+
original_auth = req.get_header("Authorization")
5354
new_req = super().redirect_request(req, fp, code, msg, headers, newurl)
5455
if new_req is not None:
5556
hostname = (urlparse(newurl).hostname or "").lower()
56-
if hostname not in GITHUB_HOSTS:
57+
if hostname in GITHUB_HOSTS:
58+
if original_auth:
59+
new_req.add_unredirected_header("Authorization", original_auth)
60+
else:
5761
new_req.headers.pop("Authorization", None)
62+
new_req.unredirected_hdrs.pop("Authorization", None)
5863
return new_req
5964

6065

tests/test_extensions.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2514,6 +2514,41 @@ def test_make_request_token_added_for_codeload_github_com(self, temp_dir, monkey
25142514
req = catalog._make_request("https://codeload.github.com/org/repo/zip/refs/tags/v1.0.0")
25152515
assert req.get_header("Authorization") == "token ghp_testtoken"
25162516

2517+
def test_redirect_preserves_auth_for_github_to_codeload(self, temp_dir, monkeypatch):
2518+
"""Auth header is preserved when GitHub redirects to codeload.github.com."""
2519+
from specify_cli._github_http import _StripAuthOnRedirect, GITHUB_HOSTS
2520+
from urllib.request import Request
2521+
from unittest.mock import MagicMock
2522+
import io
2523+
2524+
handler = _StripAuthOnRedirect()
2525+
original_url = "https://github.com/org/repo/archive/refs/tags/v1.zip"
2526+
redirect_url = "https://codeload.github.com/org/repo/zip/refs/tags/v1"
2527+
req = Request(original_url, headers={"Authorization": "token ghp_test"})
2528+
fp = io.BytesIO(b"")
2529+
new_req = handler.redirect_request(req, fp, 302, "Found", {}, redirect_url)
2530+
assert new_req is not None
2531+
auth = new_req.get_header("Authorization") or new_req.unredirected_hdrs.get("Authorization")
2532+
assert auth == "token ghp_test"
2533+
2534+
def test_redirect_strips_auth_for_github_to_external(self, temp_dir, monkeypatch):
2535+
"""Auth header is stripped when GitHub redirects to a non-GitHub host."""
2536+
from specify_cli._github_http import _StripAuthOnRedirect
2537+
from urllib.request import Request
2538+
import io
2539+
2540+
handler = _StripAuthOnRedirect()
2541+
original_url = "https://github.com/org/repo/releases/download/v1/asset.zip"
2542+
redirect_url = "https://objects.githubusercontent.com/github-production-release-asset/12345"
2543+
req = Request(original_url, headers={"Authorization": "token ghp_test"})
2544+
fp = io.BytesIO(b"")
2545+
new_req = handler.redirect_request(req, fp, 302, "Found", {}, redirect_url)
2546+
assert new_req is not None
2547+
auth_header = new_req.headers.get("Authorization")
2548+
auth_unredirected = new_req.unredirected_hdrs.get("Authorization")
2549+
assert auth_header is None
2550+
assert auth_unredirected is None
2551+
25172552
def test_fetch_single_catalog_sends_auth_header(self, temp_dir, monkeypatch):
25182553
"""_fetch_single_catalog passes Authorization header via opener for GitHub URLs."""
25192554
from unittest.mock import patch, MagicMock

0 commit comments

Comments
 (0)