feedback

Author: dmcilvaney

.github/workflows/check-rendered-specs.yml

@@ -74,25 +74,31 @@ jobs:
         run: |
           docker build \
             --build-arg UID=$(id -u) \
-            -t azldev-render \
+            -t localhost/azldev-render \
             -f .github/workflows/containers/render.Dockerfile \
             .github/workflows/containers/
 
-      # Render runs inside a privileged container because mock needs mount
-      # namespaces. Only the PR checkout is mounted (not the full workspace) —
-      # this prevents a malicious spec's %() macro from writing to the trusted
-      # base checkout where our scripts live. The azldev binary is mounted
-      # read-only from the host's GOPATH.
+      # Render runs inside a container with SYS_ADMIN capability because mock
+      # needs mount namespaces for chroot operations. We avoid --privileged to
+      # limit the blast radius if a malicious spec's %() macro achieves code
+      # execution inside the container. Only the PR checkout is mounted (not
+      # the full workspace) — the trusted base checkout where our scripts live
+      # is not accessible. The azldev binary is mounted read-only.
+      #
+      # NOTE: --security-opt no-new-privileges would be ideal but mock's
+      # userhelper requires setuid, which that flag blocks.
       - name: Render specs
         env:
           WORKSPACE: ${{ github.workspace }}
         run: |
           set -o pipefail
-          docker run --privileged --rm \
+          docker run --rm \
+            --cap-add=SYS_ADMIN \
+            --security-opt seccomp=unconfined \
             -v "$WORKSPACE/pr-head:/workdir" \
             -v "$(go env GOPATH)/bin:/gobin:ro" \
             -e PATH="/gobin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" \
-            azldev-render \
+            localhost/azldev-render \
             azldev component render -q -a --clean-stale -O json \
             | tee render-output.json
 

.github/workflows/scripts/check_rendered_specs.py

@@ -20,9 +20,11 @@
 import argparse
 import difflib
 import json
+import os
 import re
 import subprocess
 import sys
+import tempfile
 from pathlib import Path
 
 # ---------------------------------------------------------------------------
@@ -203,9 +205,10 @@ def _unique_components(items: list[dict]) -> list[str]:
     return out
 
 
+# NOTE: _unique_components and _render_command are duplicated in post_render_comment.py
 def _render_command(components: list[str], use_all: bool = False) -> str:
     if use_all or len(components) > 30:
-        return "azldev component render -a"
+        return "azldev component render -a --clean-stale"
     return f"azldev component render {' '.join(components)}"
 
 
@@ -225,51 +228,53 @@ def generate_patch(
     if not paths:
         return b""
 
-    # Write paths to a file to avoid ARG_MAX limits
-    pathspec_file = Path("render-check-pathspec.txt")
-    pathspec_file.write_text("\n".join(paths), encoding="utf-8")
+    # Write paths to temp files outside CWD (which may be an untrusted
+    # PR checkout where symlinks could redirect writes).
+    pathspec_fd, pathspec_path = tempfile.mkstemp(prefix="render-check-", suffix=".txt")
+    with os.fdopen(pathspec_fd, "w") as f:
+        f.write("\n".join(paths))
 
     # Mark untracked files as intent-to-add so git diff includes them
+    extra_pathspec_path = None
     if extra_files:
-        extra_pathspec = Path("render-check-extra-pathspec.txt")
-        extra_pathspec.write_text("\n".join(extra_files), encoding="utf-8")
+        extra_fd, extra_pathspec_path = tempfile.mkstemp(
+            prefix="render-check-extra-", suffix=".txt"
+        )
+        with os.fdopen(extra_fd, "w") as f:
+            f.write("\n".join(extra_files))
         try:
             subprocess.run(
-                ["git", "add", "-N", "--pathspec-from-file", str(extra_pathspec)],
+                ["git", "add", "-N", "--pathspec-from-file", extra_pathspec_path],
                 check=True,
                 capture_output=True,
             )
         except subprocess.CalledProcessError:
             pass
-        finally:
-            extra_pathspec.unlink(missing_ok=True)
 
     try:
         result = subprocess.run(
-            ["git", "diff", "--pathspec-from-file", str(pathspec_file)],
+            ["git", "diff", "--pathspec-from-file", pathspec_path],
             capture_output=True,
             check=True,
         )
         patch = result.stdout
     except subprocess.CalledProcessError:
         patch = b""
     finally:
-        pathspec_file.unlink(missing_ok=True)
+        Path(pathspec_path).unlink(missing_ok=True)
 
     # Undo the intent-to-add so we don't leave index dirty
-    if extra_files:
-        extra_pathspec = Path("render-check-extra-pathspec.txt")
-        extra_pathspec.write_text("\n".join(extra_files), encoding="utf-8")
+    if extra_pathspec_path:
         try:
             subprocess.run(
-                ["git", "reset", "--pathspec-from-file", str(extra_pathspec)],
+                ["git", "reset", "--pathspec-from-file", extra_pathspec_path],
                 check=True,
                 capture_output=True,
             )
         except subprocess.CalledProcessError:
             pass
         finally:
-            extra_pathspec.unlink(missing_ok=True)
+            Path(extra_pathspec_path).unlink(missing_ok=True)
 
     return patch
 

.github/workflows/scripts/post_render_comment.py

@@ -57,9 +57,10 @@ def _unique_components(items: list[dict]) -> list[str]:
     return out
 
 
+# NOTE: _unique_components and _render_command are duplicated in check_rendered_specs.py
 def _render_command(components: list[str], use_all: bool = False) -> str:
     if use_all or len(components) > 30:
-        return "azldev component render -a"
+        return "azldev component render -a --clean-stale"
     return f"azldev component render {' '.join(components)}"
 
 
@@ -84,7 +85,7 @@ def format_comment(
     all_comps: list[str] = sorted(
         set(_unique_components(content_diffs) + _unique_components(missing_files))
     )
-    use_all = bool(extra_files)
+    use_all = bool(extra_files) or bool(missing_files)
     remediation_cmd = _render_command([] if use_all else all_comps, use_all=use_all)
 
     lines: list[str] = [