From 5f480110c95e189c1804d1b97ab31fff70d68273 Mon Sep 17 00:00:00 2001 From: Mohammed Alkindi Date: Fri, 23 Oct 2026 10:49:09 -0700 Subject: [PATCH 1/3] fix(deps): keep the proxy archive rejection instead of a later HTTP 404 When a registry proxy URL returns 200 but the archive is rejected (unsafe member such as a symlink, or a body that is not a zip), the loop moved on to the remaining fallback URLs and their 404s overwrote the cause, so the error ended in "Last error: HTTP 404". Track the first archive rejection separately and report it when no URL succeeds. A later valid archive still wins. Refs #3110 --- .../content/docs/enterprise/registry-proxy.md | 2 + src/apm_cli/deps/download_strategies.py | 8 ++- tests/unit/test_artifactory_support.py | 72 +++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/docs/src/content/docs/enterprise/registry-proxy.md b/docs/src/content/docs/enterprise/registry-proxy.md index fe0c51aac5..0003c624a7 100644 --- a/docs/src/content/docs/enterprise/registry-proxy.md +++ b/docs/src/content/docs/enterprise/registry-proxy.md @@ -286,6 +286,8 @@ and `apm cache clean`. | HTTP 404 on every ref of an existing GitLab project | Default branch is `master`, APM defaults to `main` | Pin the ref: `#master` in `apm.yml` | | Upstream URL in `X-Artifactory-Origin-Remote-Path` has a duplicated group name | The remote's "Repository Path Prefix" is prepending a segment that's also in the request | Clear the prefix field on the remote | +When the proxy serves an archive that APM rejects, for example because a member is a symlink or the body is not a zip, the error reports that archive's URL and reason after `Archive error:`, even if later fallback URLs return HTTP 404. + For fully disconnected CI (no proxy reach at all), build a bundle on a connected host with `apm pack` and restore offline. See [Pack and distribute](../../producer/pack-a-bundle/). diff --git a/src/apm_cli/deps/download_strategies.py b/src/apm_cli/deps/download_strategies.py index bdbd22d5d7..12c4aad984 100644 --- a/src/apm_cli/deps/download_strategies.py +++ b/src/apm_cli/deps/download_strategies.py @@ -437,6 +437,9 @@ def download_artifactory_archive( max_archive_bytes = int(os.environ.get("ARTIFACTORY_MAX_ARCHIVE_MB", "500")) * 1024 * 1024 last_error = None + # A 200 archive that fails validation is a more useful cause than a + # later 404 from a fallback URL, so report it unless a later URL succeeds. + archive_error = None for url in archive_urls: _debug(f"Trying Artifactory archive: {url}") resp = None @@ -467,9 +470,11 @@ def download_artifactory_archive( _debug(last_error) except ArchiveError as e: last_error = f"Unsafe zip archive from {url}: {e}" + archive_error = archive_error or last_error _debug(last_error) except zipfile.BadZipFile: last_error = f"Invalid zip archive from {url}" + archive_error = archive_error or last_error _debug(last_error) except requests.RequestException as e: last_error = str(e) @@ -478,9 +483,10 @@ def download_artifactory_archive( if resp is not None: _close_response(resp, "artifactory archive") + detail = f"Archive error: {archive_error}" if archive_error else f"Last error: {last_error}" raise RuntimeError( f"Failed to download package {owner}/{repo}#{ref} from Artifactory " - f"({host}/{prefix}). Last error: {last_error}" + f"({host}/{prefix}). {detail}" ) def download_file_from_artifactory( diff --git a/tests/unit/test_artifactory_support.py b/tests/unit/test_artifactory_support.py index c206b18eef..eaacc7c192 100644 --- a/tests/unit/test_artifactory_support.py +++ b/tests/unit/test_artifactory_support.py @@ -1107,6 +1107,78 @@ def test_raises_on_empty_archive(self): target, ) + def _make_symlink_zip_bytes(self): + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr("repo-main/", "") + zf.writestr("repo-main/apm.yml", b"name: test\nversion: 1.0.0\n") + link = zipfile.ZipInfo("repo-main/link") + link.create_system = 3 + link.external_attr = 0o120777 << 16 + zf.writestr(link, "apm.yml") + return buf.getvalue() + + @pytest.mark.parametrize( + ("body", "expected"), + [ + pytest.param( + "symlink", + "Unsafe zip archive from https://art.example.com/artifactory/github/owner/" + "repo/archive/refs/heads/main.zip: Refusing to extract symlink: repo-main/link", + id="unsafe-member", + ), + pytest.param( + b"", + "Invalid zip archive from https://art.example.com/artifactory/github/owner/" + "repo/archive/refs/heads/main.zip", + id="invalid-zip", + ), + ], + ) + def test_archive_rejection_not_masked_by_later_404(self, body, expected): + """A rejected 200 archive stays the reported cause when later URLs 404 (#3110).""" + content = self._make_symlink_zip_bytes() if body == "symlink" else body + responses = [_mock_stream_response(200, content)] + [ + _mock_stream_response(404) for _ in range(4) + ] + + target = self.temp_dir / "pkg" + with patch.object(self.downloader, "_resilient_get", side_effect=responses): + with pytest.raises(RuntimeError) as exc_info: + self.downloader._download_artifactory_archive( + "art.example.com", + "artifactory/github", + "owner", + "repo", + "main", + target, + ) + + message = str(exc_info.value) + assert message.endswith(expected) + assert "HTTP 404" not in message + + def test_rejected_archive_then_valid_archive_succeeds(self): + """A later valid archive still wins after an earlier archive is rejected.""" + responses = [ + _mock_stream_response(200, self._make_symlink_zip_bytes()), + _mock_stream_response(404), + _mock_stream_response(200, self._make_zip_bytes()), + ] + + target = self.temp_dir / "pkg" + with patch.object(self.downloader, "_resilient_get", side_effect=responses): + self.downloader._download_artifactory_archive( + "art.example.com", + "artifactory/github", + "owner", + "repo", + "main", + target, + ) + + assert (target / "README.md").exists() + def test_nested_directories_extracted(self): """Nested directories within the archive are properly extracted.""" files = { From 8217fc625eb713bea58ab0666bd5abce29423218 Mon Sep 17 00:00:00 2001 From: Mohammed Alkindi Date: Fri, 23 Oct 2026 11:41:16 -0700 Subject: [PATCH 2/3] docs(changelog): note the proxy archive diagnostic fix Refs #3110 --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 82960dc779..cff361bbcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- When a registry proxy serves an archive that APM rejects (an unsafe member such as a symlink, or a body that is not a zip), `apm install` now reports that rejection instead of a later fallback URL's HTTP 404. Archive containment checks are unchanged and symlinks are still refused. + ## [0.35.0] - 2026-10-22 ### Added From 7f3f57315260044b5747c073d361071b5d94582e Mon Sep 17 00:00:00 2001 From: Mohammed Alkindi Date: Fri, 23 Oct 2026 11:42:57 -0700 Subject: [PATCH 3/3] docs(changelog): add the PR number to the proxy archive entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cff361bbcc..75e2a1ed45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- When a registry proxy serves an archive that APM rejects (an unsafe member such as a symlink, or a body that is not a zip), `apm install` now reports that rejection instead of a later fallback URL's HTTP 404. Archive containment checks are unchanged and symlinks are still refused. +- When a registry proxy serves an archive that APM rejects (an unsafe member such as a symlink, or a body that is not a zip), `apm install` now reports that rejection instead of a later fallback URL's HTTP 404. Archive containment checks are unchanged and symlinks are still refused. (#3375) ## [0.35.0] - 2026-10-22