From 857227c7c34ea9331664de96b726a68f0df0d9df Mon Sep 17 00:00:00 2001 From: Oscar V Date: Tue, 1 Sep 2026 00:04:47 -0700 Subject: [PATCH 1/2] Answer a github ref with an archive of that ref A versioned github PURL was answered with the repository clone URL. That URL resolves whatever ref you name, so a tag that does not exist came back validated=True and status=success, and the git checkout in fallback_command would then fail. A consumer handing the URL to tarfile.open extracts zero files without an error, which is why suphm already drops any download URL ending in .git. A ref that was asked for is now answered with an archive of that ref. With no ref to archive, the clone URL remains the honest answer, so a versionless PURL is unchanged. The archive URL is built as /archive/{ref}.tar.gz rather than /archive/refs/tags/{ref}.tar.gz. The refs/tags form resolves tags only, so it 404d for main, a branch that is really there, and it cannot resolve a commit sha. The plain form resolves a tag, a branch or a sha, and 404s for a ref that does not exist, which is what lets a missing ref report failure. A tag and a branch sharing a name leaves the resolution order to GitHub. That ambiguity exists in the coordinate rather than in the lookup, and naming the ref class would need it spelled in the PURL, so it is left alone here. Six existing tests changed. One asserted the clone URL for a versioned PURL, which is the behaviour being removed. Five asserted the refs/tags form, and two of those asserted archive/refs/tags/main.tar.gz, a URL that 404s. Closes #33 --- CHANGELOG.md | 16 +++++++ src/purl2src/handlers/github.py | 29 ++++++++----- tests/test_handlers/test_github.py | 70 ++++++++++++++++++++++++++---- 3 files changed, 97 insertions(+), 18 deletions(-) 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_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 From ebcd31da27c60dd4816d29e5a2a2fa4acf8942cf Mon Sep 17 00:00:00 2001 From: Oscar V Date: Tue, 1 Sep 2026 00:07:16 -0700 Subject: [PATCH 2/2] Update the github e2e expectation to the archive URL The e2e test asserted the clone URL for pkg:github/facebook/react@v18.2.0, which is the behaviour this branch removes. It passed locally off a cached pre-change answer and failed in CI, which has no cache. --- tests/test_e2e.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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)."""