From 9d2a48bce1b4ac65d5a8d4eea3ece28fdba0f49d Mon Sep 17 00:00:00 2001 From: ayesha-aziz123 Date: Mon, 4 May 2026 23:32:15 +0500 Subject: [PATCH 1/5] fix: validate URL scheme in build_github_request --- src/specify_cli/_github_http.py | 4 ++ tests/test_github_http.py | 73 +++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 tests/test_github_http.py diff --git a/src/specify_cli/_github_http.py b/src/specify_cli/_github_http.py index ee68a8325c..35e34dfef6 100644 --- a/src/specify_cli/_github_http.py +++ b/src/specify_cli/_github_http.py @@ -32,6 +32,10 @@ def build_github_request(url: str) -> urllib.request.Request: requests so credentials are never leaked to third-party hosts. """ headers: Dict[str, str] = {} + if not url or not url.strip(): + raise ValueError("url must not be empty") + if not url.startswith(("http://", "https://")): + raise ValueError(f"url must start with http:// or https://, got: {url!r}") github_token = (os.environ.get("GITHUB_TOKEN") or "").strip() gh_token = (os.environ.get("GH_TOKEN") or "").strip() token = github_token or gh_token or None diff --git a/tests/test_github_http.py b/tests/test_github_http.py new file mode 100644 index 0000000000..a24e29ee95 --- /dev/null +++ b/tests/test_github_http.py @@ -0,0 +1,73 @@ +"""Tests for GitHub-authenticated HTTP request helpers.""" + +import os +from unittest.mock import patch +import pytest + +from specify_cli._github_http import ( + build_github_request, +) + + +class TestBuildGithubRequest: + """Tests for build_github_request() URL validation and auth handling.""" + + # --- URL Validation Tests --- + + def test_empty_url_raises_value_error(self): + """build_github_request() must reject an empty string URL.""" + with pytest.raises(ValueError, match="url must not be empty"): + build_github_request("") + + def test_whitespace_url_raises_value_error(self): + """build_github_request() must reject a whitespace-only URL.""" + with pytest.raises(ValueError, match="url must not be empty"): + build_github_request(" ") + + def test_non_http_url_raises_value_error(self): + """build_github_request() must reject URLs without http/https scheme.""" + with pytest.raises(ValueError, match="url must start with http"): + build_github_request("not-a-url") + + def test_ftp_url_raises_value_error(self): + """build_github_request() must reject ftp:// URLs.""" + with pytest.raises(ValueError, match="url must start with http"): + build_github_request("ftp://github.com/file.zip") + + # --- Valid URL Tests --- + + def test_valid_https_url_returns_request(self): + """build_github_request() must return a Request for a valid https URL.""" + req = build_github_request("https://github.com/github/spec-kit") + assert req.full_url == "https://github.com/github/spec-kit" + + def test_valid_http_url_returns_request(self): + """build_github_request() must accept http:// URLs.""" + req = build_github_request("http://example.com/file") + assert req.full_url == "http://example.com/file" + + # --- Auth Header Tests --- + + def test_github_token_added_for_github_host(self): + """Authorization header is set when GITHUB_TOKEN is present.""" + with patch.dict(os.environ, {"GITHUB_TOKEN": "test-token", "GH_TOKEN": ""}): + req = build_github_request("https://github.com/github/spec-kit") + assert req.get_header("Authorization") == "Bearer test-token" + + def test_gh_token_used_as_fallback(self): + """GH_TOKEN is used when GITHUB_TOKEN is absent.""" + with patch.dict(os.environ, {"GITHUB_TOKEN": "", "GH_TOKEN": "fallback-token"}): + req = build_github_request("https://github.com/github/spec-kit") + assert req.get_header("Authorization") == "Bearer fallback-token" + + def test_no_auth_header_for_non_github_host(self): + """Authorization header must NOT be set for non-GitHub URLs.""" + with patch.dict(os.environ, {"GITHUB_TOKEN": "test-token"}): + req = build_github_request("https://example.com/file") + assert req.get_header("Authorization") is None + + def test_no_auth_header_when_no_token(self): + """No Authorization header when no token is set in environment.""" + with patch.dict(os.environ, {}, clear=True): + req = build_github_request("https://github.com/github/spec-kit") + assert req.get_header("Authorization") is None \ No newline at end of file From 4266f1881b27c88c64ff3636471cac40dcd4007c Mon Sep 17 00:00:00 2001 From: Ayesha Aziz <163914368+ayesha-aziz123@users.noreply.github.com> Date: Tue, 5 May 2026 01:49:27 +0500 Subject: [PATCH 2/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/specify_cli/_github_http.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/_github_http.py b/src/specify_cli/_github_http.py index 35e34dfef6..b70d66a7c4 100644 --- a/src/specify_cli/_github_http.py +++ b/src/specify_cli/_github_http.py @@ -34,12 +34,16 @@ def build_github_request(url: str) -> urllib.request.Request: headers: Dict[str, str] = {} if not url or not url.strip(): raise ValueError("url must not be empty") - if not url.startswith(("http://", "https://")): + url = url.strip() + parsed = urlparse(url) + if parsed.scheme not in {"http", "https"}: raise ValueError(f"url must start with http:// or https://, got: {url!r}") + if not parsed.hostname: + raise ValueError(f"url must include a hostname, got: {url!r}") github_token = (os.environ.get("GITHUB_TOKEN") or "").strip() gh_token = (os.environ.get("GH_TOKEN") or "").strip() token = github_token or gh_token or None - hostname = (urlparse(url).hostname or "").lower() + hostname = parsed.hostname.lower() if token and hostname in GITHUB_HOSTS: headers["Authorization"] = f"Bearer {token}" return urllib.request.Request(url, headers=headers) From ee9d58e0c51f88747e90b7bf97ed58cba146f059 Mon Sep 17 00:00:00 2001 From: ayesha-aziz123 Date: Tue, 5 May 2026 02:27:14 +0500 Subject: [PATCH 3/5] test: add missing hostname validation test for build_github_request --- src/specify_cli/_github_http.py | 9 +++++++-- tests/test_github_http.py | 7 ++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/_github_http.py b/src/specify_cli/_github_http.py index 35e34dfef6..b63f414d99 100644 --- a/src/specify_cli/_github_http.py +++ b/src/specify_cli/_github_http.py @@ -34,12 +34,17 @@ def build_github_request(url: str) -> urllib.request.Request: headers: Dict[str, str] = {} if not url or not url.strip(): raise ValueError("url must not be empty") - if not url.startswith(("http://", "https://")): - raise ValueError(f"url must start with http:// or https://, got: {url!r}") + url = url.strip() + parsed = urlparse(url) + if parsed.scheme not in {"http", "https"}: + raise ValueError(f"url must start with http:// or https://, got: {url!r}") + if not parsed.hostname: + raise ValueError(f"url must include a hostname, got: {url!r}") github_token = (os.environ.get("GITHUB_TOKEN") or "").strip() gh_token = (os.environ.get("GH_TOKEN") or "").strip() token = github_token or gh_token or None hostname = (urlparse(url).hostname or "").lower() + hostname = parsed.hostname.lower() if token and hostname in GITHUB_HOSTS: headers["Authorization"] = f"Bearer {token}" return urllib.request.Request(url, headers=headers) diff --git a/tests/test_github_http.py b/tests/test_github_http.py index a24e29ee95..64b9ef9db3 100644 --- a/tests/test_github_http.py +++ b/tests/test_github_http.py @@ -70,4 +70,9 @@ def test_no_auth_header_when_no_token(self): """No Authorization header when no token is set in environment.""" with patch.dict(os.environ, {}, clear=True): req = build_github_request("https://github.com/github/spec-kit") - assert req.get_header("Authorization") is None \ No newline at end of file + assert req.get_header("Authorization") is None + + def test_missing_hostname_raises_value_error(self): + """build_github_request() must reject URLs with valid scheme but no hostname.""" + with pytest.raises(ValueError, match="url must include a hostname"): + build_github_request("http://") \ No newline at end of file From 64d26245f902e0567b7f81807929f82053eb2257 Mon Sep 17 00:00:00 2001 From: ayesha-aziz123 Date: Tue, 5 May 2026 03:00:24 +0500 Subject: [PATCH 4/5] fix: update docstring and fix import grouping per Copilot feedback --- src/specify_cli/_github_http.py | 9 +++++++-- tests/test_github_http.py | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/_github_http.py b/src/specify_cli/_github_http.py index 3da1fd9e2f..274725d1ee 100644 --- a/src/specify_cli/_github_http.py +++ b/src/specify_cli/_github_http.py @@ -30,6 +30,11 @@ def build_github_request(url: str) -> urllib.request.Request: ``Authorization: Bearer `` header when the target hostname is one of the known GitHub-owned domains. Non-GitHub URLs are returned as plain requests so credentials are never leaked to third-party hosts. + + Raises: + ValueError: If ``url`` is empty or whitespace-only. + ValueError: If ``url`` does not use the ``http`` or ``https`` scheme. + ValueError: If ``url`` does not include a hostname. """ headers: Dict[str, str] = {} if not url or not url.strip(): @@ -37,9 +42,9 @@ def build_github_request(url: str) -> urllib.request.Request: url = url.strip() parsed = urlparse(url) if parsed.scheme not in {"http", "https"}: - raise ValueError(f"url must start with http:// or https://, got: {url!r}") + raise ValueError(f"url must start with http:// or https://, got: {url!r}") if not parsed.hostname: - raise ValueError(f"url must include a hostname, got: {url!r}") + raise ValueError(f"url must include a hostname, got: {url!r}") github_token = (os.environ.get("GITHUB_TOKEN") or "").strip() gh_token = (os.environ.get("GH_TOKEN") or "").strip() token = github_token or gh_token or None diff --git a/tests/test_github_http.py b/tests/test_github_http.py index 64b9ef9db3..cbee5b2afb 100644 --- a/tests/test_github_http.py +++ b/tests/test_github_http.py @@ -2,6 +2,7 @@ import os from unittest.mock import patch + import pytest from specify_cli._github_http import ( @@ -71,7 +72,7 @@ def test_no_auth_header_when_no_token(self): with patch.dict(os.environ, {}, clear=True): req = build_github_request("https://github.com/github/spec-kit") assert req.get_header("Authorization") is None - + def test_missing_hostname_raises_value_error(self): """build_github_request() must reject URLs with valid scheme but no hostname.""" with pytest.raises(ValueError, match="url must include a hostname"): From 33cb636509f05a16e3537788b344d5304e76122b Mon Sep 17 00:00:00 2001 From: ayesha-aziz123 Date: Tue, 5 May 2026 12:53:23 +0500 Subject: [PATCH 5/5] fix: sort imports and simplify url validation in build_github_request --- src/specify_cli/_github_http.py | 6 +++--- tests/test_github_http.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/_github_http.py b/src/specify_cli/_github_http.py index 274725d1ee..ffa804dbb7 100644 --- a/src/specify_cli/_github_http.py +++ b/src/specify_cli/_github_http.py @@ -8,8 +8,8 @@ import os import urllib.request -from urllib.parse import urlparse from typing import Dict +from urllib.parse import urlparse # GitHub-owned hostnames that should receive the Authorization header. # Includes codeload.github.com because GitHub archive URL downloads @@ -37,9 +37,9 @@ def build_github_request(url: str) -> urllib.request.Request: ValueError: If ``url`` does not include a hostname. """ headers: Dict[str, str] = {} - if not url or not url.strip(): - raise ValueError("url must not be empty") url = url.strip() + if not url: + raise ValueError("url must not be empty") parsed = urlparse(url) if parsed.scheme not in {"http", "https"}: raise ValueError(f"url must start with http:// or https://, got: {url!r}") diff --git a/tests/test_github_http.py b/tests/test_github_http.py index cbee5b2afb..f414aeeb2b 100644 --- a/tests/test_github_http.py +++ b/tests/test_github_http.py @@ -10,7 +10,7 @@ ) -class TestBuildGithubRequest: +class TestBuildGitHubRequest: """Tests for build_github_request() URL validation and auth handling.""" # --- URL Validation Tests ---