diff --git a/docs/source/reference/cli.md b/docs/source/reference/cli.md index 16c42947f..6a422da90 100644 --- a/docs/source/reference/cli.md +++ b/docs/source/reference/cli.md @@ -108,6 +108,13 @@ reward is zero; the validator never converts missing or null step rewards to zer Cleanup removes run-owned containers. Built images remain in Docker's local cache for reuse; the report records their immutable image IDs. Remove an unwanted image with `docker image rm ` after its validation runs have finished. +Use `--cleanup-images` to remove the current run's image automatically after all +original and replay containers finish, including failure or interruption. Opt-in +builds receive a unique ownership tag and label; cleanup never forces removal or +prunes parent images or build caches. If another tag refers to the image, that +image is preserved. The artifact bundle's `cleanup.json` records image cleanup +separately, including whether the image itself was removed. This option applies +only to local Docker validation and has no effect when no build is attempted. Runtime reports use schema version 2 and severity policy v2. Static reports for v1 manifests retain schema version 1 and policy v1. An explicit v1 policy with a diff --git a/src/openenv/cli/commands/validate.py b/src/openenv/cli/commands/validate.py index 036cad3fe..673d39c6e 100644 --- a/src/openenv/cli/commands/validate.py +++ b/src/openenv/cli/commands/validate.py @@ -139,6 +139,13 @@ def validate( help="Skip the image build; build-dependent checks are SKIPped with a reason", ), ] = False, + cleanup_images: Annotated[ + bool, + typer.Option( + "--cleanup-images", + help="Remove this run's Docker images after validation; preserve shared caches", + ), + ] = False, local: Annotated[ bool, typer.Option( @@ -218,6 +225,11 @@ def validate( runtime_target = target if runtime_target is not None: + if cleanup_images: + typer.echo( + "Error: --cleanup-images requires local package validation", err=True + ) + raise typer.Exit(EXIT_FAIL) if local: typer.echo("Error: --local cannot be combined with a running URL", err=True) raise typer.Exit(EXIT_FAIL) @@ -261,6 +273,7 @@ def validate( package_root, max_level=_LEVELS[level], skip_build=skip_build, + cleanup_images=cleanup_images, policy=load_policy(policy_version) if policy_version else None, artifacts_dir=( output.parent / (output.stem + ".artifacts") if output else None diff --git a/src/openenv/validation/providers/docker.py b/src/openenv/validation/providers/docker.py index 81dc3883a..d0fdda6f0 100644 --- a/src/openenv/validation/providers/docker.py +++ b/src/openenv/validation/providers/docker.py @@ -35,6 +35,7 @@ _LABEL = "org.openenv.validation.run" +_IMAGE_LABEL = "org.openenv.validation.image-owner" _MAX_OUTPUT = 65536 _EXCLUDED = { ".git", @@ -76,6 +77,31 @@ def _safe_text(value: str, secrets: tuple[str, ...] = ()) -> str: ) +def _image_tag(owner: str) -> str: + if not re.fullmatch(r"validation-[a-f0-9]{32}", owner): + raise ProviderError("Invalid validation image owner") + return f"openenv-validation:{owner}" + + +def _image_ids(reference: str | None = None, *, owner: str | None = None) -> set[str]: + """List immutable IDs without inferring absence from engine-specific errors.""" + argv = ["docker", "image", "ls", "--all", "--no-trunc", "--quiet"] + if reference is not None: + argv.append(reference) + if owner is not None: + argv.extend(["--filter", f"label={_IMAGE_LABEL}={owner}"]) + code, output, _ = _command(argv, 10) + if code or len(output.encode("utf-8")) >= _MAX_OUTPUT: + raise ProviderError("Could not obtain a complete image inventory") + images = set() + for line in output.splitlines(): + digest = line.removeprefix("sha256:") + if not re.fullmatch(r"[a-f0-9]{64}", digest): + raise ProviderError("Could not verify image inventory") + images.add("sha256:" + digest) + return images + + def _command( argv: list[str], timeout_s: float, max_bytes: int = _MAX_OUTPUT ) -> tuple[int, str, str]: @@ -273,7 +299,23 @@ def __init__( self.max_context_bytes = max_context_bytes self.helper_image = helper_image - def build(self, root: Path, execution: ExecutionDeclaration) -> str: + def build( + self, + root: Path, + execution: ExecutionDeclaration, + *, + image_owner: str | None = None, + ) -> str: + image_args = ( + [ + "--tag", + _image_tag(image_owner), + "--label", + f"{_IMAGE_LABEL}={image_owner}", + ] + if image_owner is not None + else [] + ) deadline = time.monotonic() + self.build_timeout_s try: root = root.resolve(strict=True) @@ -302,6 +344,7 @@ def build(self, root: Path, execution: ExecutionDeclaration) -> str: str(iidfile), "--file", str(dockerfile), + *image_args, str(context), ], deadline - time.monotonic(), @@ -309,10 +352,72 @@ def build(self, root: Path, execution: ExecutionDeclaration) -> str: if code != 0: raise StartupError("Image build failed: " + _safe_text(stderr[-4096:])) image = iidfile.read_text().strip() if iidfile.exists() else "" + if image_owner is not None: + # A tagged BuildKit export's iidfile can differ from the immutable + # image ID addressable by the Docker daemon. + code, image, _ = _command( + [ + "docker", + "image", + "inspect", + "--format", + "{{.Id}}", + _image_tag(image_owner), + ], + min(10, deadline - time.monotonic()), + ) + if code: + raise ProviderError("Docker did not publish the owned image") + image = image.strip() if not re.fullmatch(r"sha256:[a-f0-9]{64}", image): raise ProviderError("Docker did not return an immutable image ID") return image + def cleanup_image(self, owner: str, expected_id: str | None = None) -> dict: + """Release only this build's tag, preserving other tags and parent images.""" + tag = _image_tag(owner) + evidence = {"owner": owner, "tag": tag, "required": True, "completed": False} + code, output, _ = _command( + [ + "docker", + "image", + "inspect", + "--format", + f'{{{{.Id}}}} {{{{index .Config.Labels "{_IMAGE_LABEL}"}}}}', + tag, + ], + 10, + ) + if code: + if not _image_ids(tag): + return {**evidence, "completed": True, "reason": "owned tag absent"} + raise ProviderError("Could not verify image ownership for cleanup") + fields = output.split() + if ( + len(fields) != 2 + or not re.fullmatch(r"sha256:[a-f0-9]{64}", fields[0]) + or fields[1] != owner + or expected_id is not None + and fields[0] != expected_id + ): + raise ProviderError( + "Refusing cleanup of an image with another identity or owner" + ) + image_id = fields[0] + code, _, _ = _command(["docker", "image", "rm", "--no-prune", tag], 10) + if _image_ids(tag): + if code: + raise ProviderError("Could not remove validation image tag") + raise ProviderError("Image tag removal could not be independently verified") + return { + **evidence, + "image_id": image_id, + "completed": True, + # Retagging preserves this immutable label; unrelated caches cannot + # exhaust the inventory budget used to verify this run's image. + "image_removed": image_id not in _image_ids(owner=owner), + } + def start(self, spec: LaunchSpec) -> "DockerRunningSubject": if spec.network.mode not in self.supported_network_modes: raise UnsupportedCapability( diff --git a/src/openenv/validation/runner.py b/src/openenv/validation/runner.py index 6870be7d5..0b9e4315d 100644 --- a/src/openenv/validation/runner.py +++ b/src/openenv/validation/runner.py @@ -140,12 +140,13 @@ def _applicable(check_id, manifest): return True -def _runtime(subject, graders, *, skip_build, provider): +def _runtime(subject, graders, *, skip_build, provider, cleanup_images=False): """Own build/start/collection/stop and retain failure evidence through teardown.""" manifest = subject.manifest plan = None evidence = None running = None + image_owner = image_ref = None inspection = {} cleanup = {"required": False, "completed": True} started = time.monotonic() @@ -166,10 +167,15 @@ def _runtime(subject, graders, *, skip_build, provider): raise UnsupportedCapability( "credential_delivery is deferred for this release" ) - if provider is None: + if provider is None or cleanup_images: from .providers.docker import DockerValidationProvider - provider = DockerValidationProvider() + if provider is None: + provider = DockerValidationProvider() + if cleanup_images and not isinstance(provider, DockerValidationProvider): + raise UnsupportedCapability( + "--cleanup-images requires the Docker provider" + ) if manifest.network.mode not in provider.supported_network_modes: raise UnsupportedCapability( f"provider cannot enforce network mode {manifest.network.mode}" @@ -190,7 +196,16 @@ def _runtime(subject, graders, *, skip_build, provider): env_vars={"OPENENV_VALIDATION_TOKEN": secrets.token_urlsafe(32)}, ) attempted = True - image_ref = provider.build(subject.root, manifest.execution) + if cleanup_images: + # Register ownership before build: cancellation can occur after Docker + # publishes a tag but before the client returns its immutable ID. + image_owner = spec.run_id + cleanup["required"] = True + image_ref = provider.build( + subject.root, manifest.execution, image_owner=image_owner + ) + else: + image_ref = provider.build(subject.root, manifest.execution) spec = LaunchSpec.model_validate({**spec.model_dump(), "image_ref": image_ref}) running = provider.start(spec) cleanup = {"required": True, "completed": False} @@ -320,6 +335,26 @@ def _runtime(subject, graders, *, skip_build, provider): result.status = CheckStatus.ERROR result.evidence.append("subject teardown failed") result.duration_s = time.monotonic() - started + if image_owner is not None: + try: + cleanup["image"] = provider.cleanup_image(image_owner, image_ref) + except (Exception, KeyboardInterrupt) as exc: + reason = f"image cleanup failed ({type(exc).__name__})" + cleanup["image"] = { + "required": True, + "completed": False, + "owner": image_owner, + "reason": reason, + } + cleanup["completed"] = False + if result is None: + result = _outcome( + "runtime.startup", CheckStatus.ERROR, reason, started=started + ) + else: + result.status = CheckStatus.ERROR + result.evidence.append(reason) + result.duration_s = time.monotonic() - started return [result, *checks], attempted, plan, evidence, inspection, cleanup @@ -328,6 +363,7 @@ def run_validation( *, max_level: Level = Level.SEMANTIC, skip_build: bool = False, + cleanup_images: bool = False, policy: SeverityPolicy | None = None, provider=None, artifacts_dir: Path | None = None, @@ -349,6 +385,9 @@ def run_validation( Level ceiling; graders above it are not selected. skip_build (`bool`, *optional*, defaults to `False`): Skip the image build; build-dependent checks SKIP with a reason. + cleanup_images (`bool`, *optional*, defaults to `False`): + Remove this run's Docker image after its original and replay containers. + Other image tags, parent images and build caches are preserved. policy ([`~openenv.validation.policy.SeverityPolicy`], *optional*): `None` chooses v1 for static and v2 for runtime/semantic ceilings. provider ([`~openenv.validation.providers.ValidationProvider`], *optional*): @@ -438,6 +477,7 @@ def run_validation( ], skip_build=skip_build, provider=provider, + cleanup_images=cleanup_images, ) results.extend(runtime_results) if attempted: diff --git a/tests/test_cli/test_validate.py b/tests/test_cli/test_validate.py index 4ca5ce166..1ca618d10 100644 --- a/tests/test_cli/test_validate.py +++ b/tests/test_cli/test_validate.py @@ -486,6 +486,18 @@ def test_validate_command_rejects_mixed_path_and_url(tmp_path: Path) -> None: assert "Cannot combine a local path argument with --url" in result.output +def test_cleanup_images_rejects_legacy_url_without_connecting(): + with patch( + "openenv.cli.commands.validate.validate_running_environment" + ) as validate_url: + result = runner.invoke( + app, ["validate", "--url", "http://localhost:8000", "--cleanup-images"] + ) + assert result.exit_code == 1 + assert "requires local package" in result.output + validate_url.assert_not_called() + + def test_write_report_validation_report_only() -> None: assert write_report.__annotations__["report"] is ValidationReport with pytest.raises(AttributeError): diff --git a/tests/test_validation/integration/test_docker_lifecycle.py b/tests/test_validation/integration/test_docker_lifecycle.py index a937f9e6c..95aa7f1bd 100644 --- a/tests/test_validation/integration/test_docker_lifecycle.py +++ b/tests/test_validation/integration/test_docker_lifecycle.py @@ -7,7 +7,11 @@ from pathlib import Path import pytest -from openenv.validation.manifest import NetworkPolicy, ResourceDeclaration +from openenv.validation.manifest import ( + ExecutionDeclaration, + NetworkPolicy, + ResourceDeclaration, +) from openenv.validation.providers import ProviderError, StartupError from openenv.validation.providers.docker import DockerValidationProvider from openenv.validation.runtime.contracts import LaunchSpec @@ -175,3 +179,61 @@ def test_docker_exec_timeout_removes_process_tree(spec): finally: running.stop() record_cleanup("exec_timeout", spec.run_id) + + +@pytest.mark.parametrize("shared", [False, True]) +def test_docker_owned_image_cleanup_preserves_shared_images(spec, tmp_path, shared): + """Only a unique build is disposable, after both image consumers release it.""" + owner = "validation-" + uuid.uuid4().hex + sentinel = "openenv-validation-test:" + uuid.uuid4().hex + (tmp_path / "Dockerfile").write_text('FROM scratch\nCMD ["/unused"]\n') + provider = DockerValidationProvider() + image = provider.build(tmp_path, ExecutionDeclaration(), image_owner=owner) + assert image != spec.image_ref + containers = [] + try: + for _ in range(2): + created = subprocess.run( + [ + "docker", + "create", + "--label", + f"org.openenv.validation.run={owner}", + image, + ], + check=True, + capture_output=True, + text=True, + timeout=10, + ) + containers.append(created.stdout.strip()) + assert containers[0] != containers[1] + with pytest.raises(ProviderError, match="remove validation image"): + provider.cleanup_image(owner, image) + if shared: + subprocess.run(["docker", "tag", image, sentinel], check=True, timeout=10) + finally: + for container in containers: + subprocess.run( + ["docker", "rm", container], check=True, capture_output=True, timeout=10 + ) + cleaned = provider.cleanup_image(owner, image) + try: + assert cleaned["completed"] is True + assert cleaned["image_removed"] is not shared + assert provider.cleanup_image(owner, image)["completed"] is True + for preserved in [spec.image_ref, *([sentinel] if shared else [])]: + subprocess.run( + ["docker", "image", "inspect", "--format", "{{.Id}}", preserved], + check=True, + capture_output=True, + timeout=10, + ) + finally: + if shared: + subprocess.run( + ["docker", "image", "rm", "--no-prune", sentinel], + check=True, + capture_output=True, + timeout=10, + ) diff --git a/tests/test_validation/test_docker_provider.py b/tests/test_validation/test_docker_provider.py index 41603c90a..f753df2fd 100644 --- a/tests/test_validation/test_docker_provider.py +++ b/tests/test_validation/test_docker_provider.py @@ -412,6 +412,7 @@ def test_build_uses_filtered_snapshot_and_returns_immutable_image( def build(argv, timeout_s): assert timeout_s <= 600 + assert "--tag" not in argv and "--label" not in argv context = Path(argv[-1]) assert set(path.name for path in context.iterdir()) == { "Dockerfile", @@ -434,6 +435,208 @@ def build(argv, timeout_s): ) +def test_opt_in_image_build_has_supervisor_owned_tag_and_label(tmp_path, monkeypatch): + (tmp_path / "Dockerfile").write_text("FROM scratch\n") + owner = "validation-" + "b" * 32 + + def build(argv, timeout_s): + if argv[1:3] == ["image", "inspect"]: + assert 0 < timeout_s <= 3 + assert argv[-1] == f"openenv-validation:{owner}" + return 0, IMAGE, "" + assert argv[argv.index("--tag") + 1] == f"openenv-validation:{owner}" + assert argv[argv.index("--label") + 1] == f"{docker._IMAGE_LABEL}={owner}" + Path(argv[argv.index("--iidfile") + 1]).write_text("sha256:" + "d" * 64) + return 0, "", "" + + monkeypatch.setattr(docker, "_command", build) + assert ( + docker.DockerValidationProvider(build_timeout_s=3).build( + tmp_path, ExecutionDeclaration(), image_owner=owner + ) + == IMAGE + ) + + +@pytest.mark.parametrize("shared", [False, True]) +@pytest.mark.parametrize("missing_error", ["No such image", "image not known"]) +@pytest.mark.parametrize("prefixed_id", [False, True]) +def test_image_cleanup_releases_only_owned_tag_and_is_idempotent( + monkeypatch, shared, missing_error, prefixed_id +): + owner = "validation-" + "b" * 32 + tag = f"openenv-validation:{owner}" + listed_id = IMAGE if prefixed_id else IMAGE.removeprefix("sha256:") + calls = [] + removed = False + + def command(argv, timeout_s): + nonlocal removed + calls.append(argv) + if argv[2] == "ls": + if not removed: + return 0, listed_id + "\n", "" + if argv[-1] == tag: + return 0, "", "" + return (0, listed_id + "\n", "") if shared else (0, "", "") + if argv[2] == "rm": + assert argv == ["docker", "image", "rm", "--no-prune", tag] + removed = True + return 0, "", "" + if argv[-1] == IMAGE and shared: + return 0, IMAGE, "" + if removed: + return 1, "", missing_error + return 0, f"{IMAGE} {owner}", "" + + monkeypatch.setattr(docker, "_command", command) + provider = docker.DockerValidationProvider() + result = provider.cleanup_image(owner, IMAGE) + assert result["completed"] and result["image_removed"] is not shared + assert result["image_id"] == IMAGE + assert provider.cleanup_image(owner, IMAGE)["completed"] + assert sum(argv[2] == "rm" for argv in calls) == 1 + + +@pytest.mark.parametrize("present", [False, True]) +def test_image_cleanup_absent_tag_uses_inventory_not_inspect_error( + monkeypatch, present +): + owner = "validation-" + "b" * 32 + calls = [] + + def command(argv, timeout_s): + calls.append(argv) + if argv[2] == "inspect": + return 125, "", "image not known" + assert argv[2] == "ls" + assert argv[-1] == f"openenv-validation:{owner}" + assert "--filter" not in argv + return 0, IMAGE + "\n" if present else "", "" + + monkeypatch.setattr(docker, "_command", command) + if present: + with pytest.raises(ProviderError, match="ownership"): + docker.DockerValidationProvider().cleanup_image(owner, IMAGE) + else: + result = docker.DockerValidationProvider().cleanup_image(owner, IMAGE) + assert result["completed"] and result["reason"] == "owned tag absent" + assert not any(argv[2] == "rm" for argv in calls) + + +@pytest.mark.parametrize("phase", ["before", "after", "image"]) +@pytest.mark.parametrize( + "inventory", + [ + (1, "", "engine unavailable"), + (0, "x" * docker._MAX_OUTPUT, ""), + (0, "bad row\n", ""), + ], +) +def test_image_cleanup_cannot_verify_absence_from_failed_or_incomplete_inventory( + monkeypatch, phase, inventory +): + owner = "validation-" + "b" * 32 + calls = [] + removed = False + + def command(argv, timeout_s): + nonlocal removed + calls.append(argv) + if argv[2] == "ls": + if phase == "image" and argv[-1] == f"openenv-validation:{owner}": + return 0, "", "" + return inventory + if argv[2] == "rm": + removed = True + return 0, "", "" + if phase == "before" or removed: + return 1, "", "image not known" + return 0, f"{IMAGE} {owner}", "" + + monkeypatch.setattr(docker, "_command", command) + with pytest.raises(ProviderError): + docker.DockerValidationProvider().cleanup_image(owner, IMAGE) + assert any(argv[2] == "rm" for argv in calls) is (phase != "before") + if phase == "image": + assert calls[-1][-2:] == ["--filter", f"label={docker._IMAGE_LABEL}={owner}"] + + +@pytest.mark.parametrize("shared", [False, True]) +def test_image_cleanup_ignores_unrelated_image_cache(monkeypatch, shared): + owner = "validation-" + "b" * 32 + tag = f"openenv-validation:{owner}" + queries = [] + + def command(argv, timeout_s): + if argv[2] == "inspect": + return 0, f"{IMAGE} {owner}", "" + if argv[2] == "rm": + return 0, "", "" + assert argv[2] == "ls" + queries.append(argv) + if argv[-1] == tag: + # Tag presence must remain visible even if another owner retags it. + assert "--filter" not in argv + return 0, "", "" + if argv[-2:] == ["--filter", f"label={docker._IMAGE_LABEL}={owner}"]: + return 0, IMAGE + "\n" if shared else "", "" + # An unfiltered query would overflow with unrelated cache entries. + return 0, ("sha256:" + "c" * 64 + "\n") * 2000, "" + + monkeypatch.setattr(docker, "_command", command) + result = docker.DockerValidationProvider().cleanup_image(owner, IMAGE) + assert result["completed"] and result["image_removed"] is not shared + assert len(queries) == 2 + assert queries[-1][-2:] == ["--filter", f"label={docker._IMAGE_LABEL}={owner}"] + + +@pytest.mark.parametrize("remove_code", [0, 1]) +@pytest.mark.parametrize("tag_remains", [False, True]) +def test_image_cleanup_verifies_removal_independently_of_command_exit( + monkeypatch, remove_code, tag_remains +): + owner = "validation-" + "b" * 32 + removed = False + + def command(argv, timeout_s): + nonlocal removed + if argv[2] == "rm": + removed = True + return remove_code, "", "concurrent removal" + if argv[2] == "ls": + # Podman can report full image IDs without the sha256 prefix. + output = IMAGE.removeprefix("sha256:") + "\n" if tag_remains else "" + return 0, output, "" + if removed and not tag_remains: + return 1, "", "image not known" + return 0, f"{IMAGE} {owner}", "" + + monkeypatch.setattr(docker, "_command", command) + if tag_remains: + with pytest.raises(ProviderError): + docker.DockerValidationProvider().cleanup_image(owner, IMAGE) + else: + result = docker.DockerValidationProvider().cleanup_image(owner, IMAGE) + assert result["completed"] and result["image_removed"] + + +@pytest.mark.parametrize("wrong_field", ["owner", "identity"]) +def test_image_cleanup_refuses_changed_identity_or_owner(monkeypatch, wrong_field): + owner = "validation-" + "b" * 32 + calls = [] + + def command(argv, timeout_s): + calls.append(argv) + return 0, f"{IMAGE} {'other' if wrong_field == 'owner' else owner}", "" + + monkeypatch.setattr(docker, "_command", command) + expected = "sha256:" + "c" * 64 if wrong_field == "identity" else IMAGE + with pytest.raises(ProviderError, match="another identity or owner"): + docker.DockerValidationProvider().cleanup_image(owner, expected) + assert len(calls) == 1 and calls[0][2] == "inspect" + + def test_build_rejects_symlink_escape_without_invoking_docker(tmp_path, commands): (tmp_path / "Dockerfile").write_text("FROM scratch\n") (tmp_path / "escape").symlink_to("/etc/passwd") diff --git a/tests/test_validation/test_runtime_execution.py b/tests/test_validation/test_runtime_execution.py index 77b37e4a1..4d833d48e 100644 --- a/tests/test_validation/test_runtime_execution.py +++ b/tests/test_validation/test_runtime_execution.py @@ -10,6 +10,7 @@ import yaml from openenv.validation.policy import load_policy, PolicyError from openenv.validation.providers import ProviderError, StartupError +from openenv.validation.providers.docker import DockerValidationProvider from openenv.validation.report import CheckResult from openenv.validation.runner import run_validation, source_digest from openenv.validation.runtime.artifacts import write_runtime_bundle @@ -228,6 +229,153 @@ def test_skip_build_has_no_provider_side_effects(package): ) +@pytest.mark.parametrize( + "phase", + [ + "success", + "build", + "build_interrupt", + "start", + "collection", + "interrupt", + "teardown", + ], +) +def test_image_cleanup_runs_after_containers_even_when_execution_fails( + package, monkeypatch, tmp_path, phase, baseline_only +): + provider = DockerValidationProvider() + subject = FakeRuntimeProvider().subject + owner = None + events = [] + + def build(root, execution, *, image_owner): + nonlocal owner + owner = image_owner + events.append("build") + if phase == "build": + raise StartupError("build failed after image export") + if phase == "build_interrupt": + raise KeyboardInterrupt + return "sha256:" + "a" * 64 + + def start(spec): + events.append("start") + if phase == "start": + raise StartupError("startup failed") + return subject + + def collect(*args, **kwargs): + if phase == "interrupt": + raise KeyboardInterrupt + result = measured_episode() + if phase == "collection": + result = replace(result, failure_reason="step failed (TimeoutError)") + return result + + def cleanup(image_owner, expected_id): + events.append("image cleanup") + assert image_owner == owner + assert (expected_id is None) == (phase in {"build", "build_interrupt"}) + assert subject.stopped or phase in { + "build", + "build_interrupt", + "start", + "teardown", + } + return {"required": True, "completed": True, "image_removed": True} + + if phase == "teardown": + + def failed_stop(): + events.append("stop") + raise ProviderError("container teardown failed") + + monkeypatch.setattr(subject, "stop", failed_stop) + + monkeypatch.setattr(provider, "build", build) + monkeypatch.setattr(provider, "start", start) + monkeypatch.setattr(provider, "cleanup_image", cleanup) + monkeypatch.setattr("openenv.validation.runner.collect_runtime_evidence", collect) + bundle = tmp_path / "bundle" + report = run_validation( + package, + max_level=Level.RUNTIME, + provider=provider, + cleanup_images=True, + artifacts_dir=bundle, + ) + startup = next(r for r in report.results if r.check_id == "runtime.startup") + expected = ( + CheckStatus.PASS + if phase == "success" + else CheckStatus.ERROR + if phase in {"interrupt", "build_interrupt", "teardown"} + else CheckStatus.FAIL + ) + assert startup.status is expected + assert events[-1] == "image cleanup" + recorded = json.loads((bundle / "cleanup.json").read_text()) + assert recorded["image"]["completed"] + if phase == "teardown": + assert events[-2:] == ["stop", "image cleanup"] + assert recorded["completed"] is False + if phase in {"interrupt", "build_interrupt"}: + assert startup.evidence == ["validation interrupted"] + + +@pytest.mark.parametrize("phase", ["build", "start"]) +def test_image_cleanup_error_preserves_primary_failure( + package, monkeypatch, tmp_path, phase +): + provider = DockerValidationProvider() + + def build(*args, **kwargs): + if phase == "build": + raise StartupError("original build failure") + return "sha256:" + "a" * 64 + + def start(*args): + raise StartupError("original start failure") + + def cleanup(*args): + raise ProviderError("private cleanup detail") + + monkeypatch.setattr(provider, "build", build) + monkeypatch.setattr(provider, "start", start) + monkeypatch.setattr(provider, "cleanup_image", cleanup) + bundle = tmp_path / "bundle" + report = run_validation( + package, + max_level=Level.RUNTIME, + provider=provider, + cleanup_images=True, + artifacts_dir=bundle, + ) + startup = next(r for r in report.results if r.check_id == "runtime.startup") + assert startup.status is CheckStatus.ERROR + assert startup.evidence == [ + f"original {phase} failure", + "image cleanup failed (ProviderError)", + ] + cleanup = json.loads((bundle / "cleanup.json").read_text()) + assert cleanup["completed"] is cleanup["image"]["completed"] is False + assert ( + "private cleanup detail" not in json.dumps(cleanup) + report.model_dump_json() + ) + + +def test_image_cleanup_rejects_non_docker_provider_before_build(package): + provider = FakeRuntimeProvider() + report = run_validation( + package, max_level=Level.RUNTIME, provider=provider, cleanup_images=True + ) + assert not provider.builds + startup = next(r for r in report.results if r.check_id == "runtime.startup") + assert startup.status is CheckStatus.SKIP + assert "Docker provider" in startup.evidence[0] + + @pytest.mark.parametrize("change", ["network", "gpu", "build"]) def test_unsupported_capability_is_refused_before_build(package, change): import yaml diff --git a/tests/validation_runtime/README.md b/tests/validation_runtime/README.md index 918d51546..0996bd271 100644 --- a/tests/validation_runtime/README.md +++ b/tests/validation_runtime/README.md @@ -55,7 +55,7 @@ telemetry, container identity and cleanup outcome. A process-only provider expli skips fresh-container determinism. Subject-emitted records are compared with the independently collected wire trace. -The Docker suite contains 34 required cases: three provider lifecycle tests, +The Docker suite contains 36 required cases: five provider lifecycle tests, six network-policy tests, 24 CLI fault/control cases, and one real `echo_env` canary. The network tests run `public` and `no-network` subjects against a validator-owned sink and verify the isolated namespace and helper. The slow-step @@ -64,7 +64,9 @@ episode budget. The hung-step case checks the episode deadline; the interruption case sends SIGINT only after a container log confirms the second step has begun. Both must retain the completed reset/state/step/state prefix and remove their own containers. Each CLI case uses -a unique image label for independent cleanup verification. +a unique image label for independent cleanup verification. Image lifecycle cases +also verify opt-in removal after image consumers exit, idempotent cleanup, +and preservation of preexisting base images and independently added user tags. The Echo canary copies the actual `envs/echo_env` sources unchanged and records their hashes. A test overlay adds only the execution declaration, replay plan and diff --git a/tests/validation_runtime/acceptance.json b/tests/validation_runtime/acceptance.json index 46d52b087..85c3fff26 100644 --- a/tests/validation_runtime/acceptance.json +++ b/tests/validation_runtime/acceptance.json @@ -48,6 +48,8 @@ "tests.test_validation.integration.test_docker_lifecycle::test_docker_lifecycle_effective_limits_and_owned_cleanup", "tests.test_validation.integration.test_docker_lifecycle::test_docker_unhealthy_startup_removes_container", "tests.test_validation.integration.test_docker_lifecycle::test_docker_exec_timeout_removes_process_tree", + "tests.test_validation.integration.test_docker_lifecycle::test_docker_owned_image_cleanup_preserves_shared_images[False]", + "tests.test_validation.integration.test_docker_lifecycle::test_docker_owned_image_cleanup_preserves_shared_images[True]", "tests.test_validation.integration.test_docker_network::test_no_network_subject_is_served_only_through_the_helper", "tests.test_validation.integration.test_docker_network::test_good_network_policy[public]", "tests.test_validation.integration.test_docker_network::test_good_network_policy[no-network]",