Skip to content

Commit 6945193

Browse files
chkp-ronizclaudedanielmeppiel
authored
fix: enforce ARTIFACTORY_ONLY for virtual package types (#418)
Virtual file, collection, and subdirectory packages now respect ARTIFACTORY_ONLY=1. Previously only the primary zip-archive download path enforced the proxy-only guard; virtual packages could bypass it and reach GitHub directly in air-gapped or proxy-only environments. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Daniel Meppiel <51440732+danielmeppiel@users.noreply.github.com>
1 parent 28da431 commit 6945193

File tree

3 files changed

+63
-1
lines changed

3 files changed

+63
-1
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
88

99
## [Unreleased]
1010

11+
### Fixed
12+
13+
- Virtual package types (files, collections, subdirectories) now respect `ARTIFACTORY_ONLY=1`, matching the primary zip-archive proxy-only behavior (#418)
14+
1115
### Added
1216

1317
- `ci-runtime.yml` workflow for nightly + manual runtime inference tests, decoupled from release pipeline (#371)

src/apm_cli/deps/github_downloader.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1848,13 +1848,18 @@ def download_package(
18481848

18491849
# Handle virtual packages differently
18501850
if dep_ref.is_virtual:
1851+
art_proxy = self._parse_artifactory_base_url()
1852+
if self._is_artifactory_only() and not dep_ref.is_artifactory() and not art_proxy:
1853+
raise RuntimeError(
1854+
f"ARTIFACTORY_ONLY is set but no Artifactory proxy is configured for '{repo_ref}'. "
1855+
"Set ARTIFACTORY_BASE_URL or use explicit Artifactory FQDN syntax."
1856+
)
18511857
if dep_ref.is_virtual_file():
18521858
return self.download_virtual_file_package(dep_ref, target_path, progress_task_id, progress_obj)
18531859
elif dep_ref.is_virtual_collection():
18541860
return self.download_collection_package(dep_ref, target_path, progress_task_id, progress_obj)
18551861
elif dep_ref.is_virtual_subdirectory():
18561862
# When ARTIFACTORY_ONLY is set, download full archive and extract subdir
1857-
art_proxy = self._parse_artifactory_base_url()
18581863
if self._is_artifactory_only() and art_proxy:
18591864
return self._download_subdirectory_from_artifactory(
18601865
dep_ref, target_path, art_proxy, progress_task_id, progress_obj

tests/unit/test_artifactory_support.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,3 +789,56 @@ def test_download_package_errors_without_base_url(self):
789789
dl = GitHubPackageDownloader()
790790
with pytest.raises(RuntimeError, match="ARTIFACTORY_ONLY is set"):
791791
dl.download_package("microsoft/some-package", Path("/tmp/test-pkg"))
792+
793+
def test_virtual_file_errors_without_base_url(self):
794+
"""ARTIFACTORY_ONLY without ARTIFACTORY_BASE_URL raises for virtual file packages."""
795+
with patch.dict(os.environ, {"ARTIFACTORY_ONLY": "1"}, clear=True):
796+
dl = GitHubPackageDownloader()
797+
with pytest.raises(RuntimeError, match="ARTIFACTORY_ONLY is set"):
798+
dl.download_package(
799+
"owner/repo/prompts/deploy.prompt.md", Path("/tmp/test-pkg")
800+
)
801+
802+
def test_virtual_collection_errors_without_base_url(self):
803+
"""ARTIFACTORY_ONLY without ARTIFACTORY_BASE_URL raises for virtual collection packages."""
804+
with patch.dict(os.environ, {"ARTIFACTORY_ONLY": "1"}, clear=True):
805+
dl = GitHubPackageDownloader()
806+
with pytest.raises(RuntimeError, match="ARTIFACTORY_ONLY is set"):
807+
dl.download_package(
808+
"owner/repo/collections/my-collection", Path("/tmp/test-pkg")
809+
)
810+
811+
def test_virtual_subdirectory_errors_without_base_url(self):
812+
"""ARTIFACTORY_ONLY without ARTIFACTORY_BASE_URL raises for virtual subdirectory packages."""
813+
with patch.dict(os.environ, {"ARTIFACTORY_ONLY": "1"}, clear=True):
814+
dl = GitHubPackageDownloader()
815+
with pytest.raises(RuntimeError, match="ARTIFACTORY_ONLY is set"):
816+
dl.download_package(
817+
"owner/repo/skills/my-skill", Path("/tmp/test-pkg")
818+
)
819+
820+
def test_explicit_artifactory_fqdn_virtual_file_passes(self):
821+
"""Explicit Artifactory FQDN on virtual file dep is NOT blocked by ARTIFACTORY_ONLY."""
822+
with patch.dict(os.environ, {"ARTIFACTORY_ONLY": "1"}, clear=True):
823+
dl = GitHubPackageDownloader()
824+
dep = DependencyReference.parse(
825+
"art.example.com/artifactory/github/owner/repo/prompts/deploy.prompt.md"
826+
)
827+
assert dep.is_artifactory()
828+
assert dep.is_virtual_file()
829+
# Should not raise - explicit Artifactory FQDN bypasses the guard
830+
with patch.object(dl, 'download_virtual_file_package', return_value=MagicMock()):
831+
dl.download_package(dep, Path("/tmp/test-pkg"))
832+
833+
def test_explicit_artifactory_fqdn_virtual_collection_passes(self):
834+
"""Explicit Artifactory FQDN on virtual collection dep is NOT blocked by ARTIFACTORY_ONLY."""
835+
with patch.dict(os.environ, {"ARTIFACTORY_ONLY": "1"}, clear=True):
836+
dl = GitHubPackageDownloader()
837+
dep = DependencyReference.parse(
838+
"art.example.com/artifactory/github/owner/repo/collections/my-collection"
839+
)
840+
assert dep.is_artifactory()
841+
assert dep.is_virtual_collection()
842+
# Should not raise - explicit Artifactory FQDN bypasses the guard
843+
with patch.object(dl, 'download_collection_package', return_value=MagicMock()):
844+
dl.download_package(dep, Path("/tmp/test-pkg"))

0 commit comments

Comments
 (0)