From f21902df9577bec54814fbc2332137d4bb7de3e4 Mon Sep 17 00:00:00 2001 From: Nick Sweeting Date: Sun, 16 Aug 2026 19:54:54 -0700 Subject: [PATCH] Restore binary and hook ownership boundaries --- archivebox/cli/archivebox_pluginmap.py | 2 +- archivebox/plugins/hooks.py | 26 +-- archivebox/services/__init__.py | 3 +- archivebox/services/binary_service.py | 172 ------------------ archivebox/services/runner.py | 7 +- .../tests/test_archive_result_service.py | 24 ++- archivebox/tests/test_binary_service.py | 6 +- archivebox/tests/test_hooks.py | 12 +- 8 files changed, 29 insertions(+), 223 deletions(-) diff --git a/archivebox/cli/archivebox_pluginmap.py b/archivebox/cli/archivebox_pluginmap.py index e63c1272..4330e6f9 100644 --- a/archivebox/cli/archivebox_pluginmap.py +++ b/archivebox/cli/archivebox_pluginmap.py @@ -18,7 +18,7 @@ EVENT_FLOW_DIAGRAM = """ │ └─ BinaryRequestEvent │ │ └─ abxpkg BinaryService builtin providers │ │ └─ BinaryEvent │ -│ └─ BinaryCacheService / project cache backend │ +│ └─ ArchiveBox DB history projection │ │ │ │ CrawlEvent │ │ └─ CrawlSetupEvent │ diff --git a/archivebox/plugins/hooks.py b/archivebox/plugins/hooks.py index fd03f58e..d01fb6a2 100644 --- a/archivebox/plugins/hooks.py +++ b/archivebox/plugins/hooks.py @@ -44,7 +44,6 @@ __package__ = "archivebox.plugins" import json import os -import sys from collections.abc import Mapping from pathlib import Path from typing import TYPE_CHECKING, Any, Optional, Protocol, TypeGuard, runtime_checkable @@ -327,28 +326,9 @@ def run_hook( if not script.is_file(): raise FileNotFoundError(f"Hook script not found: {script}") - # ArchiveBox resolves hook dependencies before execution and exports the - # resolved *_BINARY env values below. Run Python hooks in the current Python - # environment so per-snapshot hook execution does not repeatedly pay the - # abxpkg script bootstrap cost from the standalone shebang. - # Shell/JS hooks still dispatch through abxpkg-resolved interpreters. - ext = script.suffix.lower() - if ext == ".sh": - bash_projection = Path(hook_config["ABXPKG_LIB_DIR"]).expanduser() / "env" / "bin" / "bash" - if not bash_projection.is_symlink() or not os.access(bash_projection, os.X_OK): - raise RuntimeError(f"Bash must be resolved by abxpkg into {bash_projection}") - cmd = [str(bash_projection), str(script)] - elif ext == ".py": - cmd = [sys.executable, str(script)] - elif ext == ".js": - node_projection = Path(hook_config["ABXPKG_LIB_DIR"]).expanduser() / "env" / "bin" / "node" - if not node_projection.is_symlink() or not os.access(node_projection, os.X_OK): - raise RuntimeError(f"Node.js must be resolved by abxpkg into {node_projection}") - hook_config["NODE_BINARY"] = str(node_projection) - cmd = [str(node_projection), str(script)] - else: - # Try to execute directly (assumes shebang) - cmd = [str(script)] + # Hooks are opaque executables. Their shipped abxpkg shebang owns runtime + # and dependency resolution just as it does under abx-dl. + cmd = [str(script)] # Build CLI arguments from kwargs for key, value in kwargs.items(): diff --git a/archivebox/services/__init__.py b/archivebox/services/__init__.py index 7709e39a..0a6b0778 100644 --- a/archivebox/services/__init__.py +++ b/archivebox/services/__init__.py @@ -1,5 +1,5 @@ from .archive_result_service import ArchiveResultService -from .binary_service import ArchiveBoxBinaryService, ArchiveBoxDBBinaryCacheBackend +from .binary_service import ArchiveBoxBinaryService from .crawl_service import CrawlService from .machine_service import MachineService from .process_service import ProcessService @@ -10,7 +10,6 @@ from .tag_service import TagService __all__ = [ "ArchiveResultService", "ArchiveBoxBinaryService", - "ArchiveBoxDBBinaryCacheBackend", "CrawlService", "MachineService", "ProcessService", diff --git a/archivebox/services/binary_service.py b/archivebox/services/binary_service.py index f7127959..4b1e5dec 100644 --- a/archivebox/services/binary_service.py +++ b/archivebox/services/binary_service.py @@ -9,152 +9,12 @@ from typing import Any from asgiref.sync import sync_to_async from django.utils import timezone -from abxpkg import Binary as AbxBinary -from abxpkg import BinProvider, PROVIDER_CLASS_BY_NAME from abxpkg.binary_service import BinaryEvent, BinaryRequestEvent from abxpkg.config import load_derived_cache from abxbus import BaseEvent, EventBus from abx_dl.services.base import BaseService -class ArchiveBoxDBBinaryCacheBackend: - """ArchiveBox machine.Binary projection backend for abxpkg BinaryCacheService.""" - - async def get(self, request: BinaryRequestEvent) -> AbxBinary | None: - from archivebox.config.common import get_config - from archivebox.machine.models import Binary, Machine, _canonical_binary_name - - machine = await sync_to_async(Machine.current, thread_sensitive=True)() - binary_name = _canonical_binary_name(request.name) - if not binary_name: - return None - - persisted_overrides = _persisted_overrides_for_request(request) - native_overrides = request.overrides or {} - requested_provider_names = _provider_names(request.binproviders) - await sync_to_async(get_config, thread_sensitive=True)() - - installed_qs = ( - Binary.objects.filter(machine=machine, name=binary_name, status=Binary.StatusChoices.INSTALLED) - .exclude(abspath="") - .exclude(abspath__isnull=True) - .order_by("-modified_at") - ) - async for installed in installed_qs: - installed_path = Path(installed.abspath).expanduser().resolve(strict=False) - if not await sync_to_async(installed_path.exists, thread_sensitive=True)(): - await _mark_binary_queued(installed) - continue - if persisted_overrides and installed.overrides != persisted_overrides: - await _mark_binary_queued(installed) - continue - - provider_name = (installed.binprovider or installed.binproviders.split(",", 1)[0]).strip() - if provider_name and provider_name not in requested_provider_names: - await _mark_binary_queued(installed) - continue - - provider = _provider_for_name(provider_name, installed.name, native_overrides) - binary_env = BinProvider.build_exec_env(providers=[provider], base_env={}) if provider is not None else {} - provider_names = _provider_names(installed.binproviders or request.binproviders or "env") - return AbxBinary.model_validate( - { - "name": request.name, - "description": request.description, - "binproviders": _providers_for_names(provider_names), - "overrides": native_overrides, - "loaded_binprovider": provider, - "loaded_abspath": installed.abspath, - "loaded_version": installed.version or None, - "loaded_sha256": installed.sha256 or None, - "env": binary_env, - }, - ) - - existing = await Binary.objects.filter(machine=machine, name=binary_name).order_by("-modified_at").afirst() - if existing is None: - await Binary.objects.acreate( - machine=machine, - name=binary_name, - binproviders=_binproviders_to_str(request.binproviders), - overrides=persisted_overrides, - status=Binary.StatusChoices.QUEUED, - ) - else: - changed = False - requested_binproviders = _binproviders_to_str(request.binproviders) - if requested_binproviders and existing.binproviders != requested_binproviders: - existing.binproviders = requested_binproviders - changed = True - if persisted_overrides and existing.overrides != persisted_overrides: - existing.overrides = persisted_overrides - changed = True - if existing.status != Binary.StatusChoices.QUEUED: - existing.status = Binary.StatusChoices.QUEUED - existing.retry_at = None - changed = True - if changed: - await existing.asave(update_fields=["binproviders", "overrides", "status", "retry_at", "modified_at"]) - return None - - async def set(self, request: BinaryRequestEvent | None, binary: AbxBinary) -> None: - from archivebox.machine.models import Binary, Machine, _canonical_binary_name - - machine = await sync_to_async(Machine.current, thread_sensitive=True)() - binary_name = _canonical_binary_name(binary.name) - if not binary_name: - return - request_context = request.extra_context if request is not None else {} - binary_id = str(request_context.get("binary_id") or "") - if binary_id: - existing = await Binary.objects.filter(id=binary_id).afirst() - else: - existing = None - if existing is None: - existing, _created = await Binary.objects.aget_or_create( - machine=machine, - name=binary_name, - defaults={"status": Binary.StatusChoices.QUEUED}, - ) - - existing.abspath = str(binary.loaded_abspath or "") - if binary.loaded_version: - existing.version = str(binary.loaded_version) - if binary.loaded_sha256: - existing.sha256 = str(binary.loaded_sha256) - existing.binproviders = _binproviders_to_str( - request.binproviders if request is not None else [provider.name for provider in binary.binproviders], - ) - if binary.loaded_binprovider is not None: - existing.binprovider = binary.loaded_binprovider.name - existing.overrides = _persisted_overrides_for_request(request) if request is not None else binary.overrides - existing.status = Binary.StatusChoices.INSTALLED - existing.retry_at = None - await existing.asave( - update_fields=["abspath", "version", "sha256", "binproviders", "binprovider", "overrides", "status", "retry_at", "modified_at"], - ) - - async def invalidate(self, request: BinaryRequestEvent, binary: AbxBinary, reason: str) -> None: - from archivebox.machine.models import Binary, Machine, _canonical_binary_name - - machine = await sync_to_async(Machine.current, thread_sensitive=True)() - binary_name = _canonical_binary_name(request.name) - if not binary_name: - return - installed = ( - await Binary.objects.filter(machine=machine, name=binary_name, status=Binary.StatusChoices.INSTALLED) - .exclude(abspath="") - .exclude(abspath__isnull=True) - .order_by("-modified_at") - .afirst() - ) - if installed is None: - return - installed.status = Binary.StatusChoices.QUEUED - installed.retry_at = None - await installed.asave(update_fields=["status", "retry_at", "modified_at"]) - - class ArchiveBoxBinaryService(BaseService): """Preserve ArchiveBox's legacy Binary Process rows around abxpkg requests.""" @@ -503,38 +363,6 @@ def _binproviders_to_str(binproviders: str | list[str] | None) -> str: return ",".join(_provider_names(binproviders)) -def _providers_for_names(names: list[str]) -> list[BinProvider]: - providers: list[BinProvider] = [] - for name in names: - provider_class = PROVIDER_CLASS_BY_NAME.get(name) - if provider_class is not None: - providers.append(provider_class()) - return providers - - -def _provider_for_name(provider_name: str, binary_name: str, overrides: dict[str, Any] | None) -> BinProvider | None: - provider_class = PROVIDER_CLASS_BY_NAME.get(provider_name) - if provider_class is None: - return None - provider = provider_class() - provider_overrides = overrides.get(provider_name) if isinstance(overrides, dict) else None - if isinstance(provider_overrides, dict): - provider = provider.get_provider_with_overrides( - overrides={binary_name: provider_overrides}, - ) - return provider - - -async def _mark_binary_queued(binary) -> None: - from archivebox.machine.models import Binary - - if binary.status == Binary.StatusChoices.QUEUED: - return - binary.status = Binary.StatusChoices.QUEUED - binary.retry_at = None - await binary.asave(update_fields=["status", "retry_at", "modified_at"]) - - def _persisted_overrides_for_request(request: BinaryRequestEvent | None) -> dict[str, Any]: if request is None: return {} diff --git a/archivebox/services/runner.py b/archivebox/services/runner.py index 5dcd6d92..cd41079d 100644 --- a/archivebox/services/runner.py +++ b/archivebox/services/runner.py @@ -20,7 +20,7 @@ from django.utils import timezone from rich.console import Console from rich.text import Text -from abxpkg.binary_service import BinaryCacheService, BinaryRequestEvent, BinaryService +from abxpkg.binary_service import BinaryRequestEvent, BinaryService from abx_dl.config import GlobalConfig, RuntimeConfig from abx_dl.events import ( CrawlAbortEvent, @@ -64,7 +64,7 @@ from archivebox.workers.models import ACTIVE_STATE_LEASE_SECONDS from archivebox.crawls.locks import crawl_lifecycle_lock from .archive_result_service import ArchiveResultService -from .binary_service import ArchiveBoxBinaryService, ArchiveBoxDBBinaryCacheBackend, project_abxpkg_derived_cache_to_db +from .binary_service import ArchiveBoxBinaryService, project_abxpkg_derived_cache_to_db from .crawl_service import CrawlService from .machine_service import MachineService from .process_service import ProcessService as PersistedProcessService @@ -222,7 +222,6 @@ class CrawlRunner: register_sonic_daemon_event_handler(self.bus) PersistedProcessService(self.bus) ArchiveBoxBinaryService(self.bus) - BinaryCacheService(self.bus, backend=ArchiveBoxDBBinaryCacheBackend()) BinaryService(self.bus) TagService(self.bus) CrawlService(self.bus, crawl_id=str(crawl.id)) @@ -1333,7 +1332,6 @@ async def _run_binary(binary_id: str) -> None: bus = create_bus(name=_bus_name("ArchiveBox_binary", str(binary.id)), total_timeout=1800.0) process_service = PersistedProcessService(bus) binary_process_service = ArchiveBoxBinaryService(bus) - BinaryCacheService(bus, backend=ArchiveBoxDBBinaryCacheBackend()) BinaryService(bus, lib_dir=Path(config["ABXPKG_LIB_DIR"])) TagService(bus) ArchiveResultService(bus) @@ -1867,7 +1865,6 @@ async def _run_install(plugin_names: list[str] | None = None) -> None: bus = create_bus(name="ArchiveBox_install", total_timeout=3600.0) PersistedProcessService(bus) ArchiveBoxBinaryService(bus) - BinaryCacheService(bus, backend=ArchiveBoxDBBinaryCacheBackend()) BinaryService(bus) TagService(bus) ArchiveResultService(bus) diff --git a/archivebox/tests/test_archive_result_service.py b/archivebox/tests/test_archive_result_service.py index bb653434..c0f51dbe 100644 --- a/archivebox/tests/test_archive_result_service.py +++ b/archivebox/tests/test_archive_result_service.py @@ -2,6 +2,7 @@ from pathlib import Path from importlib.resources import files import json import os +import shutil import pytest @@ -698,20 +699,22 @@ def test_process_started_uses_node_binary_for_js_hooks_without_plugin_binary(tmp assert "chrome zombies. cpu usage:" in process.stdout -def test_binary_event_reuses_existing_installed_binary_row(): +def test_binary_event_updates_existing_row_from_native_abxpkg_resolution(): from archivebox.machine.models import Binary, Machine - from archivebox.services.binary_service import ArchiveBoxDBBinaryCacheBackend - from abxpkg.binary_service import BinaryCacheService, BinaryService + from archivebox.services.binary_service import ArchiveBoxBinaryService + from abxpkg.binary_service import BinaryService import asyncio machine = Machine.current() binary = install_real_binary("wget", machine=machine, binproviders="env,apt,brew") - installed_abspath = binary.abspath - installed_version = binary.version - installed_provider = binary.binprovider + native_wget = shutil.which("wget") + assert native_wget is not None + binary.abspath = "/bin/sh" + binary.save(update_fields=["abspath", "modified_at"]) + stale_abspath = binary.abspath bus = create_bus(name="test_binary_event_reuses_existing_installed_binary_row") - BinaryCacheService(bus, backend=ArchiveBoxDBBinaryCacheBackend()) + ArchiveBoxBinaryService(bus) BinaryService(bus) event = BinaryRequestEvent( name="wget", @@ -731,7 +734,8 @@ def test_binary_event_reuses_existing_installed_binary_row(): binary.refresh_from_db() assert Binary.objects.filter(machine=machine, name="wget").count() == 1 assert binary.status == Binary.StatusChoices.INSTALLED - assert binary.abspath == installed_abspath - assert binary.version == installed_version - assert binary.binprovider == installed_provider + assert Path(binary.abspath).resolve() == Path(native_wget).resolve() + assert binary.abspath != stale_abspath + assert binary.version + assert binary.binprovider == "env" assert binary.binproviders == "env,apt,brew" diff --git a/archivebox/tests/test_binary_service.py b/archivebox/tests/test_binary_service.py index 12fdb59e..4f196d0e 100644 --- a/archivebox/tests/test_binary_service.py +++ b/archivebox/tests/test_binary_service.py @@ -41,9 +41,9 @@ def _run_real_binary_state_machine(data_dir: Path, *, name: str, binproviders: s def test_binary_request_preserves_native_overrides_in_db(): - from abxpkg.binary_service import BinaryCacheService, BinaryEvent, BinaryRequestEvent, BinaryService + from abxpkg.binary_service import BinaryEvent, BinaryRequestEvent, BinaryService from abx_dl.orchestrator import create_bus - from archivebox.services.binary_service import ArchiveBoxDBBinaryCacheBackend + from archivebox.services.binary_service import ArchiveBoxBinaryService machine = Machine.current() overrides = { @@ -64,7 +64,7 @@ def test_binary_request_preserves_native_overrides_in_db(): assert binary.status == Binary.StatusChoices.INSTALLED assert Path(binary.abspath).resolve() == Path(sys.executable).resolve() bus = create_bus(name=f"test_binary_native_overrides_{uuid.uuid4().hex[:8]}") - BinaryCacheService(bus, backend=ArchiveBoxDBBinaryCacheBackend()) + ArchiveBoxBinaryService(bus) BinaryService(bus) binary_events: list[BinaryEvent] = [] diff --git a/archivebox/tests/test_hooks.py b/archivebox/tests/test_hooks.py index 1bb8ad19..063f706d 100755 --- a/archivebox/tests/test_hooks.py +++ b/archivebox/tests/test_hooks.py @@ -13,7 +13,6 @@ import json import hashlib import os import subprocess -import sys from importlib.resources import files from pathlib import Path @@ -417,14 +416,13 @@ class TestHookExecution: assert "chrome zombies" in result.stdout @pytest.mark.django_db(transaction=True) - def test_real_js_hook_runs_through_abxpkg_node_projection(self, tmp_path, hermetic_lib_dir): + def test_real_js_hook_runs_through_abxpkg_shebang(self, tmp_path, hermetic_lib_dir): from archivebox.plugins.hooks import run_hook from archivebox.services.runner import run_install lib_dir = hermetic_lib_dir run_install(plugin_names=["chrome"]) node_env = resolve_abxpkg_binary_env(lib_dir, deps_from=CHROME_CONFIG) - node_projection = lib_dir / "env" / "bin" / "node" crawl_dir = tmp_path / "crawl" snap_dir = crawl_dir / "snapshot" hook_path = Path(str(files("abx_plugins.plugins.chrome").joinpath("on_CrawlSetup__89_chrome_kill_zombies.js"))) @@ -443,7 +441,7 @@ class TestHookExecution: ) process.refresh_from_db() - assert process.cmd[0] == str(node_projection) + assert process.cmd == [str(hook_path)] assert process.exit_code == 0, process.stderr assert "chrome zombies" in process.stdout @@ -627,8 +625,8 @@ def test_run_hook_exports_singular_node_modules_dir_with_colon_node_path(tmp_pat @pytest.mark.django_db(transaction=True) -def test_run_hook_executes_python_hooks_with_resolved_runtime_env(tmp_path): - """ArchiveBox-run Python hooks use the active interpreter and resolved env.""" +def test_run_hook_executes_python_hooks_through_abxpkg_shebang(tmp_path): + """ArchiveBox treats Python hooks as opaque abxpkg-launched executables.""" from archivebox.plugins.hooks import run_hook snap_dir = tmp_path / "snapshot" @@ -648,7 +646,7 @@ def test_run_hook_executes_python_hooks_with_resolved_runtime_env(tmp_path): ) process.refresh_from_db() - assert process.cmd[:2] == [sys.executable, str(hook_path)] + assert process.cmd == [str(hook_path), "--url=https://example.com/runtime"] assert process.exit_code == 0, process.stderr assert process.env["SNAP_DIR"] == str(snap_dir) records = process.parse_records_from_text(process.stdout)