diff --git a/bot_bottle/backend/macos_container/util.py b/bot_bottle/backend/macos_container/util.py index 0794d01e..d51a7e8a 100644 --- a/bot_bottle/backend/macos_container/util.py +++ b/bot_bottle/backend/macos_container/util.py @@ -689,6 +689,13 @@ def pinned_local_image_ref(ref: str) -> str: nested-containers layer. A content-derived tag prevents another concurrent build from moving the provider's ordinary ``:latest`` tag between those two builds. + + Unlike Docker, `container image tag` only accepts ``image-name[:tag]`` as + its source and rejects a bare image ID ("cannot specify 64 byte hex string + as reference"), so the mutable ``ref`` is what gets tagged here. The + post-tag inspection below is what keeps that fail-closed: if ``ref`` moved + between the two commands, the new tag will not resolve to the ID this call + derived its name from. """ image = image_id(ref) digest = image.removeprefix("sha256:") @@ -701,14 +708,14 @@ def pinned_local_image_ref(ref: str) -> str: repository = repository[:last_colon] pinned_ref = f"{repository}:sha256-{digest}" result = subprocess.run( - [_CONTAINER, "image", "tag", image, pinned_ref], + [_CONTAINER, "image", "tag", ref, pinned_ref], capture_output=True, text=True, check=False, ) if result.returncode != 0: die( - f"could not tag exact local image {image}: " + f"could not tag exact local image {image} via {ref!r}: " f"{(result.stderr or result.stdout or '').strip() or ''}" ) if image_id(pinned_ref) != image: diff --git a/tests/unit/test_macos_container_util.py b/tests/unit/test_macos_container_util.py index e1ee1004..6dd6a1aa 100644 --- a/tests/unit/test_macos_container_util.py +++ b/tests/unit/test_macos_container_util.py @@ -158,8 +158,11 @@ resolver #2 f"registry:5000/agent:sha256-{'a' * 64}", pinned, ) + # The tag source is the name:tag ref, not the image ID: Apple + # Container's `image tag` rejects a bare 64-byte hex string as a + # reference. The second image_id call is what keeps this fail-closed. run.assert_called_once_with( - ["container", "image", "tag", image, pinned], + ["container", "image", "tag", "registry:5000/agent:latest", pinned], capture_output=True, text=True, check=False, @@ -169,6 +172,31 @@ resolver #2 [call.args for call in inspect.call_args_list], ) + def test_pinned_local_image_ref_accepts_bare_hex_image_id(self): + # `container image inspect` reports "id" without the sha256: prefix. + image = "b" * 64 + completed = util.subprocess.CompletedProcess( + args=[], returncode=0, stdout="", stderr="", + ) + with patch.object(util, "image_id", return_value=image), \ + patch.object(util.subprocess, "run", return_value=completed) as run: + pinned = util.pinned_local_image_ref("agent:latest") + self.assertEqual(f"agent:sha256-{'b' * 64}", pinned) + self.assertEqual( + ["container", "image", "tag", "agent:latest", pinned], + run.call_args.args[0], + ) + + def test_pinned_local_image_ref_dies_when_tag_moved_mid_flight(self): + completed = util.subprocess.CompletedProcess( + args=[], returncode=0, stdout="", stderr="", + ) + with patch.object( + util, "image_id", side_effect=["c" * 64, "d" * 64], + ), patch.object(util.subprocess, "run", return_value=completed), \ + self.assertRaises(SystemExit): + util.pinned_local_image_ref("agent:latest") + def test_commit_container_execs_tar_and_builds_image(self): # stderr is bytes because subprocess.run uses stderr=PIPE without text=True completed = util.subprocess.CompletedProcess(