From a1eaf2c8982f48184f03fe3c349a929fbaf1563e Mon Sep 17 00:00:00 2001 From: Micheal Kinney Date: Sat, 10 Oct 2026 23:08:44 -0500 Subject: [PATCH 1/2] fix: pin get_binary_url safe-path behavior (OVH-05) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The safe runtime (default, legacy=False) used to return a fabricated cache-relative path that did not correspond to any real download — acquisition writes to cache_dir/ while get_binary_url returned cache_dir/, and the method also produced a path rather than a URL despite its name. Latent: nothing internal called it on the safe path. Raise NotImplementedError on the safe path with a message directing callers to ensure_binary() (or a pre-verified binary_path= constructor argument). Acquisition stays explicit — no network at construction, no implicit URL fabrication. Legacy branch byte-identical: supported platforms still return the URL string and unsupported platforms still raise the typed error pinned by tests/test_characterization_platform.py. Adds tests/test_default_api.py::test_get_binary_url_raises_on_safe_runtime pinning the new behavior. Drops the now-unused ArtifactCatalog import from py7zip.py. OVH-05 per docs/handoffs/2026-10-11-py7zip-overhaul.md; addresses F2 in docs/handoffs/PLATFORM_ARCHITECTURE_PY7ZIP_2026-10-10.md. Validated: python3 -m pytest tests/test_default_api.py -q # 4 passed python3 -m pytest tests/test_characterization_platform.py \ tests/test_characterization_urls.py -q # 34 passed (legacy byte-identical) python3 -m pytest tests -p no:cacheprovider -q # 129 passed (was 128) Co-Authored-By: Claude Code --- py7zip/py7zip.py | 24 ++++++++++++++++++++---- tests/test_default_api.py | 15 +++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/py7zip/py7zip.py b/py7zip/py7zip.py index dda88d9..d4b5ee9 100644 --- a/py7zip/py7zip.py +++ b/py7zip/py7zip.py @@ -8,7 +8,6 @@ import requests -from .platforms import ArtifactCatalog from .safe import SafePy7zip @@ -145,10 +144,27 @@ def download_binary(self): os.chmod(self.binary_path, 0o755) def get_binary_url(self): - """Dynamically gets the URL of the required system binary.""" + """Return the source location for the platform's 7-Zip binary. + + On the safe runtime (default), binary acquisition is explicit: + callers must invoke :meth:`ensure_binary` (or supply a + pre-verified ``binary_path``) before using the wrapper. This + method raises :class:`NotImplementedError` rather than returning + a fabricated cache-relative path, because the safe runtime does + not construct download URLs at construction time. + + On the legacy runtime (``legacy=True``), returns the URL where + the historical in-package binary would have been served from, + or raises :class:`NotImplementedError` for unsupported host + platforms. + """ if not self.legacy: - spec = ArtifactCatalog.resolve(self.platform_info) - return f"{self._safe.cache_dir}/{spec.relative_path}" + raise NotImplementedError( + "get_binary_url() is not supported on the safe runtime; " + "call ensure_binary() to acquire a verified binary into " + "the cache directory, or supply a pre-verified " + "binary_path=... to the constructor." + ) if self.platform not in self.supported: raise NotImplementedError(f"Platform '{self.platform}' is not supported.") return self.url diff --git a/tests/test_default_api.py b/tests/test_default_api.py index 81a911d..ad4fc98 100644 --- a/tests/test_default_api.py +++ b/tests/test_default_api.py @@ -30,6 +30,21 @@ def test_default_aliases_require_individual_option_arguments(tmp_path): wrapper.compress("source", "destination", options="-y") +def test_get_binary_url_raises_on_safe_runtime(tmp_path): + """The safe runtime exposes acquisition, not download URLs. + + Per the F2 finding in the platform-architecture audit, the safe + runtime previously returned a fabricated cache-relative path that + did not correspond to any real download. Acquisition is now + explicit via ``ensure_binary()``; this method raises a typed + error pointing callers at the correct API. + """ + wrapper = Py7zip(cache_dir=tmp_path) + + with pytest.raises(NotImplementedError, match="ensure_binary"): + wrapper.get_binary_url() + + def test_legacy_mode_is_explicit(): assert "legacy" in Py7zip.__init__.__code__.co_varnames assert ArchiveRunner is not None From c5927c86b94c0346365901c731d80bec242e3247 Mon Sep 17 00:00:00 2001 From: Micheal Kinney Date: Sat, 10 Oct 2026 23:23:02 -0500 Subject: [PATCH 2/2] fix: point get_binary_url error at public API PR #22 review follow-up: the safe-runtime NotImplementedError message and get_binary_url docstring directed callers to ensure_binary(), which is internal to SafePy7zip (reachable only via the private _safe attribute). The public Py7zip entry point for acquisition is download_binary(), which delegates to _safe.ensure_binary(); point the message and docstring there instead. test_get_binary_url_raises_on_safe_runtime now asserts the message names download_binary() and never mentions the internal ensure_binary name. Adds the AGENTS.md-required Unreleased entry to .github/CHANGELOG.md for the OVH-05 user-visible behavior change. Validated: python3 -m pytest tests/test_default_api.py \ tests/test_characterization_platform.py \ tests/test_characterization_urls.py -p no:cacheprovider -q # 38 passed ruff check py7zip/py7zip.py tests/test_default_api.py # passed ruff format --check py7zip/py7zip.py tests/test_default_api.py # clean python3 -I -m py_compile py7zip/py7zip.py tests/test_default_api.py Co-Authored-By: Claude Code --- .github/CHANGELOG.md | 4 ++++ py7zip/py7zip.py | 4 ++-- tests/test_default_api.py | 11 ++++++++--- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/.github/CHANGELOG.md b/.github/CHANGELOG.md index 8d992c0..0920053 100644 --- a/.github/CHANGELOG.md +++ b/.github/CHANGELOG.md @@ -9,6 +9,10 @@ the package version; publishing uses PyPI trusted publishing. - Made the safe runtime the default while retaining the legacy runtime only behind an explicit migration flag. +- Changed the safe runtime's `Py7zip.get_binary_url()` to raise + `NotImplementedError` directing callers to the public `download_binary()` + entry point instead of returning a fabricated cache-relative path + (OVH-05). ## 0.7.3 diff --git a/py7zip/py7zip.py b/py7zip/py7zip.py index d4b5ee9..00a6056 100644 --- a/py7zip/py7zip.py +++ b/py7zip/py7zip.py @@ -147,7 +147,7 @@ def get_binary_url(self): """Return the source location for the platform's 7-Zip binary. On the safe runtime (default), binary acquisition is explicit: - callers must invoke :meth:`ensure_binary` (or supply a + callers must invoke :meth:`download_binary` (or supply a pre-verified ``binary_path``) before using the wrapper. This method raises :class:`NotImplementedError` rather than returning a fabricated cache-relative path, because the safe runtime does @@ -161,7 +161,7 @@ def get_binary_url(self): if not self.legacy: raise NotImplementedError( "get_binary_url() is not supported on the safe runtime; " - "call ensure_binary() to acquire a verified binary into " + "call download_binary() to acquire a verified binary into " "the cache directory, or supply a pre-verified " "binary_path=... to the constructor." ) diff --git a/tests/test_default_api.py b/tests/test_default_api.py index ad4fc98..e1b1ccc 100644 --- a/tests/test_default_api.py +++ b/tests/test_default_api.py @@ -36,14 +36,19 @@ def test_get_binary_url_raises_on_safe_runtime(tmp_path): Per the F2 finding in the platform-architecture audit, the safe runtime previously returned a fabricated cache-relative path that did not correspond to any real download. Acquisition is now - explicit via ``ensure_binary()``; this method raises a typed - error pointing callers at the correct API. + explicit via the public ``download_binary()`` entry point; this + method raises a typed error pointing callers at that API rather + than the internal ``SafePy7zip.ensure_binary()``. """ wrapper = Py7zip(cache_dir=tmp_path) - with pytest.raises(NotImplementedError, match="ensure_binary"): + with pytest.raises(NotImplementedError) as excinfo: wrapper.get_binary_url() + message = str(excinfo.value) + assert "download_binary()" in message + assert "ensure_binary" not in message + def test_legacy_mode_is_explicit(): assert "legacy" in Py7zip.__init__.__code__.co_varnames