diff --git a/CHANGELOG.md b/CHANGELOG.md index 2afdae4..8fa4c4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,22 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed +- GitHub: a versioned PURL is answered with an archive of the ref asked for + rather than the repository clone URL. The clone URL resolves whatever ref you + name, so a tag that does not exist came back `validated=True`, and a consumer + handing it to `tarfile.open` extracted zero files without an error (#33). +- GitHub: the archive URL is built as `/archive/{ref}.tar.gz` rather than + `/archive/refs/tags/{ref}.tar.gz`. The old form only resolves tags, so it + 404'd for a branch that exists, and it cannot resolve a commit sha at all. + +### Changed +- A versioned `pkg:github/*` PURL now returns a `.tar.gz` archive URL where it + previously returned a `.git` clone URL. A versionless PURL still returns the + clone URL, which is the honest answer when there is no ref to archive. + ## [1.3.0] - 2026-08-31 ### Fixed diff --git a/src/purl2src/handlers/github.py b/src/purl2src/handlers/github.py index 340a5e9..b7d704e 100644 --- a/src/purl2src/handlers/github.py +++ b/src/purl2src/handlers/github.py @@ -10,18 +10,27 @@ class GitHubHandler(BaseHandler): """Handler for GitHub repositories.""" + def _archive_url(self, purl: Purl) -> str: + """Build the source archive URL for the requested ref. + + The plain ``/archive/{ref}.tar.gz`` form resolves a tag, a branch or a + commit sha, and 404s for a ref that does not exist. The ``refs/tags/`` + form only resolves tags, so it 404s for a branch that is really there. + """ + return f"https://github.com/{purl.namespace}/{purl.name}/" f"archive/{purl.version}.tar.gz" + def build_download_url(self, purl: Purl) -> Optional[str]: """ - Build GitHub repository URL. + Build GitHub download URL. - Returns git URL for cloning. + A ref that was asked for is answered with an archive of that ref. The + clone URL resolves whatever ref you name, so returning it for a + versioned PURL reports a ref that may not exist as validated. With no + ref to archive, the repository itself is the honest answer. """ if not purl.namespace: return None - # Base repository URL - repo_url = f"https://github.com/{purl.namespace}/{purl.name}.git" - # If subpath is specified, we need the specific file URL if purl.subpath: # For files, use raw content URL @@ -31,7 +40,10 @@ def build_download_url(self, purl: Purl) -> Optional[str]: f"{purl.namespace}/{purl.name}/{branch}/{purl.subpath}" ) - return repo_url + if purl.version: + return self._archive_url(purl) + + return f"https://github.com/{purl.namespace}/{purl.name}.git" def get_download_url_from_api(self, purl: Purl) -> Optional[str]: """Query GitHub API for download URL.""" @@ -57,10 +69,7 @@ def get_download_url_from_api(self, purl: Purl) -> Optional[str]: # For branches/tags, return archive URL if purl.version: - return ( - f"https://github.com/{purl.namespace}/{purl.name}/" - f"archive/refs/tags/{purl.version}.tar.gz" - ) + return self._archive_url(purl) return None diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 71f5851..f0822bd 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -74,8 +74,8 @@ def test_github_package(self): result = get_download_url("pkg:github/facebook/react@v18.2.0") assert result.download_url is not None assert "github.com/facebook/react" in result.download_url - # GitHub handler returns git URL - assert result.download_url == "https://github.com/facebook/react.git" + # A ref that was asked for is answered with an archive of that ref + assert result.download_url == "https://github.com/facebook/react/archive/v18.2.0.tar.gz" def test_conda_package(self): """Verify Conda package resolves correctly (if qualifiers provided).""" diff --git a/tests/test_handlers/test_github.py b/tests/test_handlers/test_github.py index 96e7eeb..f9750bb 100644 --- a/tests/test_handlers/test_github.py +++ b/tests/test_handlers/test_github.py @@ -17,8 +17,12 @@ def setup_method(self): self.handler = GitHubHandler(self.http_client) def test_build_download_url_simple_repo(self): - """Test building download URL for simple repository.""" - purl = Purl(ecosystem="github", namespace="rails", name="rails", version="v7.0.0") + """Test building download URL for simple repository. + + Versionless: a versioned PURL is answered with an archive of that ref, + not the clone URL, which resolves whatever ref you name. + """ + purl = Purl(ecosystem="github", namespace="rails", name="rails") url = self.handler.build_download_url(purl) assert url == "https://github.com/rails/rails.git" @@ -93,15 +97,19 @@ def test_get_download_url_from_api_release_failure(self): url = self.handler.get_download_url_from_api(purl) # Should fallback to archive URL - assert url == "https://github.com/rails/rails/archive/refs/tags/v7.0.0.tar.gz" + assert url == "https://github.com/rails/rails/archive/v7.0.0.tar.gz" def test_get_download_url_from_api_branch_main(self): - """Test API method for main branch.""" + """Test API method for main branch. + + A branch has no entry under refs/tags, so the form this used to assert + 404s for a branch that is really there. + """ purl = Purl(ecosystem="github", namespace="rails", name="rails", version="main") url = self.handler.get_download_url_from_api(purl) # Should not try releases API for main/master - assert url == "https://github.com/rails/rails/archive/refs/tags/main.tar.gz" + assert url == "https://github.com/rails/rails/archive/main.tar.gz" # Verify no API call was made self.http_client.get_json.assert_not_called() @@ -112,7 +120,7 @@ def test_get_download_url_from_api_branch_master(self): url = self.handler.get_download_url_from_api(purl) # Should not try releases API for main/master - assert url == "https://github.com/rails/rails/archive/refs/tags/master.tar.gz" + assert url == "https://github.com/rails/rails/archive/master.tar.gz" # Verify no API call was made self.http_client.get_json.assert_not_called() @@ -193,11 +201,57 @@ def test_get_download_url_from_api_release_no_tarball(self): url = self.handler.get_download_url_from_api(purl) # Should fallback to archive URL - assert url == "https://github.com/rails/rails/archive/refs/tags/v7.0.0.tar.gz" + assert url == "https://github.com/rails/rails/archive/v7.0.0.tar.gz" def test_archive_url_format(self): """Test that archive URL format is correct.""" purl = Purl(ecosystem="github", namespace="user", name="repo", version="v1.0.0") url = self.handler.get_download_url_from_api(purl) - assert url == "https://github.com/user/repo/archive/refs/tags/v1.0.0.tar.gz" + assert url == "https://github.com/user/repo/archive/v1.0.0.tar.gz" + + def test_versioned_purl_builds_an_archive_not_a_clone_url(self): + """A version asked for is answered with an artifact for that version. + + Regression test for #33: the clone URL resolves whatever ref was asked + for, so a tag that does not exist was reported as validated. + """ + purl = Purl(ecosystem="github", namespace="rails", name="rails", version="v7.0.0") + url = self.handler.build_download_url(purl) + assert url == "https://github.com/rails/rails/archive/v7.0.0.tar.gz" + + def test_versionless_purl_still_builds_the_clone_url(self): + """With no ref to archive, the repository itself is the honest answer.""" + purl = Purl(ecosystem="github", namespace="rails", name="rails") + url = self.handler.build_download_url(purl) + assert url == "https://github.com/rails/rails.git" + + def test_branch_archive_is_not_built_under_refs_tags(self): + """A branch is not a tag, and the refs/tags form 404s for one.""" + purl = Purl(ecosystem="github", namespace="rails", name="rails", version="main") + assert self.handler.build_download_url(purl) == ( + "https://github.com/rails/rails/archive/main.tar.gz" + ) + + def test_commit_sha_archive_is_built(self): + """The plain archive form resolves a commit sha, which refs/tags cannot.""" + sha = "023767fe9872d0e0e4ba4e1b1b7b2b53f8e2c1a9" + purl = Purl(ecosystem="github", namespace="rails", name="rails", version=sha) + assert self.handler.build_download_url(purl) == ( + f"https://github.com/rails/rails/archive/{sha}.tar.gz" + ) + + def test_missing_tag_fails_rather_than_returning_the_repository(self): + """A tag that does not exist reports failure, not a validated clone URL.""" + purl = Purl(ecosystem="github", namespace="expressjs", name="express", version="v99.99.99") + + self.http_client.get_json.side_effect = Exception("404") + # Nothing named v99.99.99 resolves; the bare repo URL would have. + self.http_client.validate_url.side_effect = lambda url: "99.99.99" not in url + + with patch("shutil.which", return_value=None): + result = self.handler.get_download_url(purl, validate=True) + + assert result.status == "failed" + assert result.download_url is None + assert result.validated is False