Address PR review comments and rename target 'both' to 'combined':

- Remove unused fuse-overlayfs from Dockerfile.release (vfs driver)
- Drop tar from release dependency check (no longer used)
- Raise error on missing layer blob instead of warn+continue
- Replace _safe_tar_extract with safe_extractall (rejects symlinks/devices)
- Make checksum writing idempotent (skip write if content unchanged)
- Rename --target 'both' to 'combined' across CLI, OCI, CI, and release

Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>

Author: jacobweinstock

.github/workflows/ci.yml

@@ -422,7 +422,7 @@ jobs:
     needs: [publish-per-arch]
     env:
       ARCH: amd64
-      TARGET: both
+      TARGET: combined
     steps:
       - name: Checkout code
         uses: actions/checkout@v6

.github/workflows/release.yml

@@ -30,17 +30,17 @@ jobs:
       - name: Install Python dependencies
         run: pip install -r requirements.txt
 
-      - name: Pull release artifacts (both)
+      - name: Pull release artifacts (combined)
         env:
           VERSION_EXCLUDE: ${{ github.ref_name }}
-        run: ./build.py release pull --target both --pull-output artifacts/both
+        run: ./build.py release pull --target combined --pull-output artifacts/combined
 
       - name: Create GitHub Release
         env:
           GH_TOKEN: ${{ github.token }}
         run: |
           gh release create "${{ github.ref_name }}" \
-            artifacts/both/* \
+            artifacts/combined/* \
             --generate-notes \
             --title "${{ github.ref_name }}"
 

Dockerfile.release

@@ -6,14 +6,12 @@
 #   docker run --rm -v $(pwd):/work captainos-release release publish
 FROM python:3.12-slim
 
-# Install buildah, skopeo, git, and tar
+# Install buildah, skopeo, and git
 RUN apt-get update && apt-get install -y --no-install-recommends \
     buildah \
     skopeo \
-    fuse-overlayfs \
     git \
     ca-certificates \
-    tar \
     && rm -rf /var/lib/apt/lists/* \
     && git config --global --add safe.directory /work
 

README.md

@@ -130,7 +130,7 @@ When a `v*` tag is pushed, the release workflow:
 ./build.py release publish --target amd64
 
 # Pull and extract artifacts
-./build.py release pull --target both --pull-output ./out/release/
+./build.py release pull --target combined --pull-output ./out/release/
 
 # Tag all artifact images with a release version
 ./build.py release tag v1.0.0

captain/artifacts.py

@@ -96,9 +96,13 @@ def collect_checksums(
         digest = _sha256(path)
         lines.append(f"{digest}  {path.name}")
     if lines:
+        content = "\n".join(lines) + "\n"
         output.parent.mkdir(parents=True, exist_ok=True)
-        output.write_text("\n".join(lines) + "\n")
-        _log.log(f"Wrote checksums to {output}")
+        if output.is_file() and output.read_text() == content:
+            _log.log(f"Checksums unchanged: {output}")
+        else:
+            output.write_text(content)
+            _log.log(f"Wrote checksums to {output}")
         for line in lines:
             _log.log(f"  {line}")
     else:

captain/cli.py

@@ -153,7 +153,7 @@ def _build_parser(command: str) -> configargparse.ArgParser:
         desc = "OCI release workflow: pull (or build) → publish → tag"
         release_cmds = {
             "publish": "Publish artifacts as a multi-arch OCI image",
-            "pull": "Pull and extract artifacts (amd64, arm64, or both)",
+            "pull": "Pull and extract artifacts (amd64, arm64, or combined)",
             "tag": "Tag all artifact images with a version",
         }
         commands_list = "\n".join(f"  {name:14s} {d}" for name, d in release_cmds.items())
@@ -402,8 +402,8 @@ def _add_release_target_flag(parser: configargparse.ArgParser) -> None:
         "--target",
         env_var="TARGET",
         default=None,
-        choices=["amd64", "arm64", "both"],
-        help="artifact target (amd64, arm64, or both; default: --arch value)",
+        choices=["amd64", "arm64", "combined"],
+        help="artifact target (amd64, arm64, or combined; default: --arch value)",
     )
     g.add_argument(
         "--git-sha",
@@ -985,7 +985,7 @@ def _cmd_checksums(cfg: Config, _extra_args: list[str], args: object = None) ->
         [_add_common_flags, _add_release_base_flags, _add_release_target_flag],
     ),
     "pull": (
-        "Pull and extract artifacts (amd64, arm64, or both)",
+        "Pull and extract artifacts (amd64, arm64, or combined)",
         [
             _add_common_flags,
             _add_release_base_flags,

captain/oci.py

@@ -3,7 +3,7 @@
 Each artifact file is pushed as its own layer so that OCI registries
 can deduplicate blobs between per-arch and combined images.
 
-Combined image (``target="both"``, no tag suffix):
+Combined image (``target="combined"``, no tag suffix):
   A multi-arch index where each platform manifest has the native
   arch's layers first, then the other arch's layers (8 layers total).
   ``linux/amd64`` → ``[A1‥A4, B1‥B4]``,
@@ -262,7 +262,7 @@ def _publish_combined(
     digests match exactly between the per-arch and combined images.
 
     If the per-arch images don't exist in the registry yet (e.g.
-    running ``--target both`` locally with no prior per-arch publish),
+    running ``--target combined`` locally with no prior per-arch publish),
     they are built and pushed first as a fallback.
 
     Skips the combined image if it already exists (unless *force*).
@@ -340,20 +340,20 @@ def publish(
     so OCI registries deduplicate blobs automatically.
 
     *target* selects which artifacts to include: ``"amd64"``,
-    ``"arm64"``, or ``"both"``.
+    ``"arm64"``, or ``"combined"``.
 
     Images are skipped if they already exist in the registry
     (unless *force* is ``True``).  For per-arch targets this prevents
     overwriting images that the combined image depends on.
     """
     _log = logger or _default_log
-    arches = list(_ARCHES) if target == "both" else [target]
-    tag_suffix = "" if target == "both" else f"-{target}"
+    arches = list(_ARCHES) if target == "combined" else [target]
+    tag_suffix = "" if target == "combined" else f"-{target}"
     full_tag = f"{tag}{tag_suffix}"
     final_ref = _image_ref(registry, repository, artifact_name, full_tag)
 
     # For per-arch targets, skip if the image already exists.
-    if target != "both" and not force and skopeo.image_exists(final_ref, logger=_log):
+    if target != "combined" and not force and skopeo.image_exists(final_ref, logger=_log):
         _log.log(f"{final_ref} already exists — skipping (use --force to overwrite)")
         return
 
@@ -376,7 +376,7 @@ def publish(
         arch_layer_tars[arch] = [_deterministic_tar(f, out) for f in files]
 
     try:
-        if target == "both":
+        if target == "combined":
             _publish_combined(
                 arch_layer_tars=arch_layer_tars,
                 registry=registry,
@@ -433,12 +433,12 @@ def pull(
 ) -> None:
     """Pull and extract OCI artifacts.
 
-    *target* may be ``"amd64"``, ``"arm64"``, or ``"both"``.  The tag
+    *target* may be ``"amd64"``, ``"arm64"``, or ``"combined"``.  The tag
     suffix is ``-{target}`` for single architectures, or bare ``{tag}``
-    for ``"both"``.
+    for ``"combined"``.
     """
     _log = logger or _default_log
-    tag_suffix = "" if target == "both" else f"-{target}"
+    tag_suffix = "" if target == "combined" else f"-{target}"
     ref = _image_ref(registry, repository, artifact_name, f"{tag}{tag_suffix}")
     skopeo.export_image(ref, output_dir, logger=_log)
 

captain/skopeo.py

@@ -8,12 +8,11 @@
 from __future__ import annotations
 
 import json
-import os
 import tarfile
 from pathlib import Path
 
 from captain.log import StageLogger, for_stage
-from captain.util import run
+from captain.util import run, safe_extractall
 
 _default_log = for_stage("skopeo")
 
@@ -98,24 +97,6 @@ def copy_to_dir(
     return output_dir
 
 
-def _safe_tar_extract(tar: tarfile.TarFile, output_dir: Path) -> None:
-    """Extract *tar* members into *output_dir*, rejecting unsafe paths.
-
-    Prevents path-traversal attacks where a malicious image could contain
-    entries with ``../`` or absolute paths that write outside the target
-    directory.
-    """
-    resolved_base = output_dir.resolve()
-    for member in tar:
-        member_path = os.path.normpath(member.name)
-        if os.path.isabs(member_path) or member_path.startswith(".."):
-            raise ValueError(f"Refusing to extract tar member with unsafe path: {member.name!r}")
-        dest = (resolved_base / member_path).resolve()
-        if not str(dest).startswith(str(resolved_base) + os.sep) and dest != resolved_base:
-            raise ValueError(f"Tar member escapes output directory: {member.name!r}")
-        tar.extract(member, path=output_dir)
-
-
 def export_image(
     image_ref: str,
     output_dir: Path,
@@ -150,9 +131,8 @@ def export_image(
             if not blob_file.exists():
                 blob_file = tmp_dir / digest_str.split(":")[-1]
             if not blob_file.exists():
-                _log.warn(f"Layer blob not found: {digest_str}")
-                continue
+                raise FileNotFoundError(f"Layer blob not found: {digest_str}")
 
             _log.log(f"Extracting layer {digest_str[:20]}…")
             with tarfile.open(blob_file, "r:*") as tf:
-                _safe_tar_extract(tf, output_dir)
+                safe_extractall(tf, output_dir)

captain/util.py

@@ -167,7 +167,7 @@ def check_release_dependencies() -> list[str]:
 
     Returns a list of missing command names (empty if all found).
     """
-    return _missing(["buildah", "skopeo", "git", "tar"])
+    return _missing(["buildah", "skopeo", "git"])
 
 
 def check_dependencies(arch: str) -> list[str]: