-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
8a918e6
9c0dc2e
4053922
1d63f2d
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 | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1411,6 +1411,27 @@ def _validate_catalog_url(self, url: str) -> None: | |||||||||||||||||||||||||||||
| if not parsed.netloc: | ||||||||||||||||||||||||||||||
| raise ValidationError("Catalog URL must be a valid URL with a host.") | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def _make_request(self, url: str) -> "urllib.request.Request": | ||||||||||||||||||||||||||||||
| """Build a urllib Request, adding a GitHub auth header when available. | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Reads GITHUB_TOKEN or GH_TOKEN from the environment and attaches an | ||||||||||||||||||||||||||||||
| ``Authorization: token <value>`` header for requests to GitHub-hosted | ||||||||||||||||||||||||||||||
| domains (``raw.githubusercontent.com``, ``github.com``, | ||||||||||||||||||||||||||||||
| ``api.github.com``). Non-GitHub URLs are returned as plain requests | ||||||||||||||||||||||||||||||
| so credentials are never leaked to third-party hosts. | ||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||
| import urllib.request | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| headers: Dict[str, str] = {} | ||||||||||||||||||||||||||||||
| token = os.environ.get("GITHUB_TOKEN") or os.environ.get("GH_TOKEN") | ||||||||||||||||||||||||||||||
| if token and any( | ||||||||||||||||||||||||||||||
| host in url | ||||||||||||||||||||||||||||||
| for host in ("raw.githubusercontent.com", "github.com", "api.github.com") | ||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| headers: Dict[str, str] = {} | |
| token = os.environ.get("GITHUB_TOKEN") or os.environ.get("GH_TOKEN") | |
| if token and any( | |
| host in url | |
| for host in ("raw.githubusercontent.com", "github.com", "api.github.com") | |
| ): | |
| from urllib.parse import urlparse | |
| headers: Dict[str, str] = {} | |
| token = os.environ.get("GITHUB_TOKEN") or os.environ.get("GH_TOKEN") | |
| hostname = (urlparse(url).hostname or "").lower() | |
| github_hosts = {"raw.githubusercontent.com", "github.com", "api.github.com"} | |
| if token and hostname in github_hosts: |
| 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 |
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.
Docs enumerate GitHub hosts as
raw.githubusercontent.com,github.com, andapi.github.com, but the implementation also treatscodeload.github.comas GitHub-owned (and tests rely on it). Please either includecodeload.github.comin this list or adjust the wording so the parenthetical isn’t interpreted as exhaustive.