mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix: resolve GitHub release asset API URL for private repo preset and workflow downloads (#2855)
* fix: resolve GitHub release asset API URL for private repo preset and workflow downloads - Add shared `resolve_github_release_asset_api_url` utility to `_github_http.py` for reuse across preset and workflow download paths - Apply the same private-repo fix from PR #2792 (extensions) to: - `PresetCatalog.download_pack` — ZIP downloads via catalog `download_url` - `preset add --from <url>` — ZIP downloads from a direct URL - `workflow add <url>` — workflow YAML downloads from a direct URL - `workflow add <id>` (catalog) — workflow YAML downloads via catalog `url` - For browser release URLs (`github.com/…/releases/download/…`), the asset is resolved via the GitHub REST API and downloaded with `Accept: application/octet-stream` - Direct REST API asset URLs (`api.github.com/…/releases/assets/<id>`) are downloaded directly with `Accept: application/octet-stream` - Auth is preserved end-to-end through the existing `open_url` infrastructure - Update `test_download_pack_sends_auth_header` and add `test_download_pack_accepts_direct_github_rest_asset_url` to cover both paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: URL-encode tag in release API URL to handle special characters Encode the tag as a path segment (using quote with safe='') when building the releases/tags/<tag> API URL. This prevents malformed URLs when tags contain reserved characters like '/' or '#'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: add CLI-level tests for preset add --from GitHub release URL resolution Adds regression tests covering: - resolve_github_release_asset_api_url unit tests (passthrough, resolution, network error, URL encoding of special chars in tags) - CLI-level 'preset add --from <github-release-url>' end-to-end flow - CLI-level 'preset add --from <api-asset-url>' direct passthrough Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: deduplicate release URL resolution; fix test issues - ExtensionCatalog._resolve_github_release_asset_api_url now delegates to the shared helper in _github_http.py (also gains URL-encoding fix) - Remove unused 'io' import from test_github_http.py - Remove duplicate 'provides' dict keys accidentally added to test_presets.py Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: align resolver timeout with download timeout; add workflow CLI tests - Pass timeout=30 to resolve_github_release_asset_api_url in both workflow add paths so worst-case latency matches the download timeout - Add CLI-level regression tests for 'workflow add <url>' covering browser URL resolution and direct API asset URL passthrough Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: remove unused urllib.request import; add catalog workflow test - Remove unused 'import urllib.request' in preset add --from path - Add CLI test for catalog-based 'workflow add <id>' with GitHub release URL resolution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * style: remove unused MagicMock imports from tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Manfred Riem <mnriem@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
lselvar
@@ -1,12 +1,15 @@
|
||||
"""Tests for GitHub-authenticated HTTP request helpers."""
|
||||
|
||||
import json
|
||||
import os
|
||||
from unittest.mock import patch
|
||||
from contextlib import contextmanager
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from specify_cli._github_http import (
|
||||
build_github_request,
|
||||
resolve_github_release_asset_api_url,
|
||||
)
|
||||
|
||||
|
||||
@@ -76,4 +79,112 @@ class TestBuildGitHubRequest:
|
||||
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://")
|
||||
build_github_request("http://")
|
||||
|
||||
|
||||
class TestResolveGitHubReleaseAssetApiUrl:
|
||||
"""Tests for resolve_github_release_asset_api_url()."""
|
||||
|
||||
def _make_open_url_fn(self, release_json):
|
||||
"""Create a fake open_url_fn that returns release JSON."""
|
||||
@contextmanager
|
||||
def fake_open(url, timeout=None, extra_headers=None):
|
||||
resp = MagicMock()
|
||||
resp.read.return_value = json.dumps(release_json).encode()
|
||||
yield resp
|
||||
return fake_open
|
||||
|
||||
def test_returns_none_for_non_github_url(self):
|
||||
"""Non-GitHub URLs should return None."""
|
||||
result = resolve_github_release_asset_api_url(
|
||||
"https://example.com/file.zip", lambda *a, **kw: None
|
||||
)
|
||||
assert result is None
|
||||
|
||||
def test_returns_none_for_non_release_github_url(self):
|
||||
"""GitHub URLs that aren't release downloads return None."""
|
||||
result = resolve_github_release_asset_api_url(
|
||||
"https://github.com/org/repo/archive/refs/tags/v1.zip",
|
||||
lambda *a, **kw: None,
|
||||
)
|
||||
assert result is None
|
||||
|
||||
def test_passthrough_for_existing_api_asset_url(self):
|
||||
"""Already-resolved REST API asset URLs are returned as-is."""
|
||||
url = "https://api.github.com/repos/org/repo/releases/assets/12345"
|
||||
result = resolve_github_release_asset_api_url(url, lambda *a, **kw: None)
|
||||
assert result == url
|
||||
|
||||
def test_resolves_browser_url_to_api_url(self):
|
||||
"""Browser release URL resolves to REST API asset URL."""
|
||||
release_json = {
|
||||
"assets": [
|
||||
{"name": "pack.zip", "url": "https://api.github.com/repos/org/repo/releases/assets/99"}
|
||||
]
|
||||
}
|
||||
result = resolve_github_release_asset_api_url(
|
||||
"https://github.com/org/repo/releases/download/v1.0/pack.zip",
|
||||
self._make_open_url_fn(release_json),
|
||||
)
|
||||
assert result == "https://api.github.com/repos/org/repo/releases/assets/99"
|
||||
|
||||
def test_returns_none_when_asset_not_found(self):
|
||||
"""Returns None when the release exists but asset name doesn't match."""
|
||||
release_json = {"assets": [{"name": "other.zip", "url": "https://api.github.com/repos/org/repo/releases/assets/1"}]}
|
||||
result = resolve_github_release_asset_api_url(
|
||||
"https://github.com/org/repo/releases/download/v1/missing.zip",
|
||||
self._make_open_url_fn(release_json),
|
||||
)
|
||||
assert result is None
|
||||
|
||||
def test_returns_none_on_network_error(self):
|
||||
"""Returns None when the API request fails."""
|
||||
import urllib.error
|
||||
|
||||
@contextmanager
|
||||
def failing_open(url, timeout=None, extra_headers=None):
|
||||
raise urllib.error.URLError("network error")
|
||||
yield # noqa: unreachable
|
||||
|
||||
result = resolve_github_release_asset_api_url(
|
||||
"https://github.com/org/repo/releases/download/v1/pack.zip",
|
||||
failing_open,
|
||||
)
|
||||
assert result is None
|
||||
|
||||
def test_tag_with_special_characters_is_url_encoded(self):
|
||||
"""Tags with reserved characters (e.g. '/') are encoded in the API URL."""
|
||||
captured_urls = []
|
||||
|
||||
@contextmanager
|
||||
def capturing_open(url, timeout=None, extra_headers=None):
|
||||
captured_urls.append(url)
|
||||
resp = MagicMock()
|
||||
resp.read.return_value = json.dumps({"assets": []}).encode()
|
||||
yield resp
|
||||
|
||||
resolve_github_release_asset_api_url(
|
||||
"https://github.com/org/repo/releases/download/feature%2Fv1/pack.zip",
|
||||
capturing_open,
|
||||
)
|
||||
# The tag "feature/v1" (decoded from %2F) must be re-encoded as "feature%2Fv1"
|
||||
assert len(captured_urls) == 1
|
||||
assert "releases/tags/feature%2Fv1" in captured_urls[0]
|
||||
|
||||
def test_tag_with_hash_is_url_encoded(self):
|
||||
"""Tags with '#' character are properly encoded."""
|
||||
captured_urls = []
|
||||
|
||||
@contextmanager
|
||||
def capturing_open(url, timeout=None, extra_headers=None):
|
||||
captured_urls.append(url)
|
||||
resp = MagicMock()
|
||||
resp.read.return_value = json.dumps({"assets": []}).encode()
|
||||
yield resp
|
||||
|
||||
resolve_github_release_asset_api_url(
|
||||
"https://github.com/org/repo/releases/download/v1%23beta/pack.zip",
|
||||
capturing_open,
|
||||
)
|
||||
assert len(captured_urls) == 1
|
||||
assert "releases/tags/v1%23beta" in captured_urls[0]
|
||||
Reference in New Issue
Block a user