-
Notifications
You must be signed in to change notification settings - Fork 7.4k
feat(extensions): authenticate GitHub-hosted catalog and download requests with GITHUB_TOKEN/GH_TOKEN #2087
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
147a0af
c4ef1d1
eb4f894
f32d059
2ccd213
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2142,6 +2142,132 @@ def test_clear_cache(self, temp_dir): | |||||||||||||||||||||||||||||||||||||||||||||
| assert not catalog.cache_file.exists() | ||||||||||||||||||||||||||||||||||||||||||||||
| assert not catalog.cache_metadata_file.exists() | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # --- _make_request / GitHub auth --- | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def _make_catalog(self, temp_dir): | ||||||||||||||||||||||||||||||||||||||||||||||
| project_dir = temp_dir / "project" | ||||||||||||||||||||||||||||||||||||||||||||||
| project_dir.mkdir() | ||||||||||||||||||||||||||||||||||||||||||||||
| (project_dir / ".specify").mkdir() | ||||||||||||||||||||||||||||||||||||||||||||||
| return ExtensionCatalog(project_dir) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_make_request_no_token_no_auth_header(self, temp_dir, monkeypatch): | ||||||||||||||||||||||||||||||||||||||||||||||
| """Without a token, requests carry no Authorization header.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| monkeypatch.delenv("GITHUB_TOKEN", raising=False) | ||||||||||||||||||||||||||||||||||||||||||||||
| monkeypatch.delenv("GH_TOKEN", raising=False) | ||||||||||||||||||||||||||||||||||||||||||||||
| catalog = self._make_catalog(temp_dir) | ||||||||||||||||||||||||||||||||||||||||||||||
| req = catalog._make_request("https://raw.githubusercontent.com/org/repo/main/catalog.json") | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "Authorization" not in req.headers | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_make_request_github_token_added_for_raw_githubusercontent(self, temp_dir, monkeypatch): | ||||||||||||||||||||||||||||||||||||||||||||||
| """GITHUB_TOKEN is attached for raw.githubusercontent.com URLs.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| monkeypatch.setenv("GITHUB_TOKEN", "ghp_testtoken") | ||||||||||||||||||||||||||||||||||||||||||||||
| monkeypatch.delenv("GH_TOKEN", raising=False) | ||||||||||||||||||||||||||||||||||||||||||||||
| catalog = self._make_catalog(temp_dir) | ||||||||||||||||||||||||||||||||||||||||||||||
| req = catalog._make_request("https://raw.githubusercontent.com/org/repo/main/catalog.json") | ||||||||||||||||||||||||||||||||||||||||||||||
| assert req.get_header("Authorization") == "token ghp_testtoken" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_make_request_gh_token_fallback(self, temp_dir, monkeypatch): | ||||||||||||||||||||||||||||||||||||||||||||||
| """GH_TOKEN is used when GITHUB_TOKEN is absent.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| monkeypatch.delenv("GITHUB_TOKEN", raising=False) | ||||||||||||||||||||||||||||||||||||||||||||||
| monkeypatch.setenv("GH_TOKEN", "ghp_ghtoken") | ||||||||||||||||||||||||||||||||||||||||||||||
| catalog = self._make_catalog(temp_dir) | ||||||||||||||||||||||||||||||||||||||||||||||
| req = catalog._make_request("https://github.com/org/repo/releases/download/v1/ext.zip") | ||||||||||||||||||||||||||||||||||||||||||||||
| assert req.get_header("Authorization") == "token ghp_ghtoken" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_make_request_github_token_takes_precedence_over_gh_token(self, temp_dir, monkeypatch): | ||||||||||||||||||||||||||||||||||||||||||||||
| """GITHUB_TOKEN takes precedence over GH_TOKEN when both are set.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| monkeypatch.setenv("GITHUB_TOKEN", "ghp_primary") | ||||||||||||||||||||||||||||||||||||||||||||||
| monkeypatch.setenv("GH_TOKEN", "ghp_secondary") | ||||||||||||||||||||||||||||||||||||||||||||||
| catalog = self._make_catalog(temp_dir) | ||||||||||||||||||||||||||||||||||||||||||||||
| req = catalog._make_request("https://api.github.com/repos/org/repo") | ||||||||||||||||||||||||||||||||||||||||||||||
| assert req.get_header("Authorization") == "token ghp_primary" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_make_request_token_not_added_for_non_github_url(self, temp_dir, monkeypatch): | ||||||||||||||||||||||||||||||||||||||||||||||
| """Auth header is never attached to non-GitHub URLs to prevent credential leakage.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| monkeypatch.setenv("GITHUB_TOKEN", "ghp_testtoken") | ||||||||||||||||||||||||||||||||||||||||||||||
| catalog = self._make_catalog(temp_dir) | ||||||||||||||||||||||||||||||||||||||||||||||
| req = catalog._make_request("https://internal.example.com/catalog.json") | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "Authorization" not in req.headers | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_make_request_token_not_added_for_github_lookalike_host(self, temp_dir, monkeypatch): | |
| """Auth header is not attached to non-GitHub hosts that only contain github.com in the hostname.""" | |
| monkeypatch.setenv("GITHUB_TOKEN", "ghp_testtoken") | |
| catalog = self._make_catalog(temp_dir) | |
| req = catalog._make_request("https://github.com.evil.com/org/repo/releases/download/v1/ext.zip") | |
| assert "Authorization" not in req.headers | |
| def test_make_request_token_not_added_for_non_github_host_with_github_in_path(self, temp_dir, monkeypatch): | |
| """Auth header is not attached when a non-GitHub host includes github.com only in the URL path.""" | |
| monkeypatch.setenv("GITHUB_TOKEN", "ghp_testtoken") | |
| catalog = self._make_catalog(temp_dir) | |
| req = catalog._make_request("https://evil.example.com/github.com/org/repo/releases/download/v1/ext.zip") | |
| assert "Authorization" not in req.headers | |
| def test_make_request_token_not_added_for_non_github_host_with_github_in_query(self, temp_dir, monkeypatch): | |
| """Auth header is not attached when a non-GitHub host includes github.com only in the query string.""" | |
| monkeypatch.setenv("GITHUB_TOKEN", "ghp_testtoken") | |
| catalog = self._make_catalog(temp_dir) | |
| req = catalog._make_request("https://evil.example.com/download?source=https://github.com/org/repo/releases/download/v1/ext.zip") | |
| assert "Authorization" not in req.headers |
Copilot
AI
Apr 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test docstring says the auth header is passed to urlopen, but the implementation path with Authorization uses urllib.request.build_opener(...).open(). Adjust the docstring to match the behavior being asserted.
| """download_extension passes Authorization header to urlopen for GitHub URLs.""" | |
| """download_extension passes Authorization header via opener for GitHub URLs.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GitHub-hosted URL check uses substring matching (
host in url), which can incorrectly attach the token to non-GitHub hosts (e.g.,https://github.com.evil.com/...orhttps://internal.example.com/path/github.com/...) and violates the stated goal of preventing credential leakage. Parse the URL and compareurlparse(url).hostname(lowercased) against an allowlist of exact hostnames instead of scanning the full URL string.