ggallen · waynesun09 · Aug 17, 2026 · Aug 17, 2026
diff --git a/hack/gitlab-runner-vm/create-vm.sh b/hack/gitlab-runner-vm/create-vm.sh
@@ -143,6 +143,15 @@ if [ ! -f "${TEMPLATE}" ]; then
   exit 1
 fi
 
+if ! [[ "${VM_USER}" =~ ^[a-z_][a-z0-9_-]*$ ]]; then
+  echo "ERROR: VM_USER must be a plain Unix user name (got: ${VM_USER})" >&2
+  exit 1
+fi
+if [[ "${RUNNER_ACCESS_LEVEL}" != "not_protected" && "${RUNNER_ACCESS_LEVEL}" != "ref_protected" ]]; then
+  echo "ERROR: RUNNER_ACCESS_LEVEL must be not_protected or ref_protected (got: ${RUNNER_ACCESS_LEVEL})" >&2
+  exit 1
+fi
+
 # Preflight — every tool and file this run depends on. Without this, a missing
 # executor script or an absent `timeout` is discovered only after the VM has
 # booted and a runner has been registered, so the failure costs a rollback.
@@ -234,8 +243,8 @@ fi
 python3 -c "
 import sys
 template = sys.stdin.read()
-print(template.replace('__VM_NAME__', sys.argv[1]).replace('__SSH_PUBLIC_KEY__', sys.argv[2]), end='')
-" "${vm_name}" "${SSH_PUBLIC_KEY}" < "${TEMPLATE}" \
+print(template.replace('__VM_NAME__', sys.argv[1]).replace('__SSH_PUBLIC_KEY__', sys.argv[2]).replace('__VM_USER__', sys.argv[3]), end='')
+" "${vm_name}" "${SSH_PUBLIC_KEY}" "${VM_USER}" < "${TEMPLATE}" \
   | oc create -n "${NAMESPACE}" -f -
 
 cleanup_vm() {
@@ -245,7 +254,8 @@ cleanup_vm() {
 trap cleanup_vm ERR
 # ERR does not fire on Ctrl-C; the boot and cloud-init waits below can take
 # up to 20 minutes, so print the cleanup hint on interrupt as well.
-trap 'cleanup_vm; exit 130' INT TERM
+trap 'cleanup_vm; exit 130' INT
+trap 'cleanup_vm; exit 143' TERM
 
 # ----------------------------------------------------------------------
 # 3. Wait for the VM to boot and accept SSH
@@ -324,7 +334,8 @@ trap cleanup_runner ERR
 # ERR does not fire on Ctrl-C, and the window below spans a ~20-minute setup
 # run — without this, an interrupt leaves the runner registered with nobody
 # tracking it.
-trap 'cleanup_runner; exit 130' INT TERM
+trap 'cleanup_runner; exit 130' INT
+trap 'cleanup_runner; exit 143' TERM
 
 REGISTRATION_TOKEN=$(echo "${runner_json}" | python3 -c "import sys,json; print(json.load(sys.stdin)['token'])")
 runner_id=$(echo "${runner_json}" | python3 -c "import sys,json; print(json.load(sys.stdin)['id'])")
@@ -397,25 +408,28 @@ for val in "${REGISTRATION_TOKEN}" "${GITLAB_URL}" "${RUNNER_TAG}" "${RUNNER_IMA
     exit 1
   fi
 done
+# One remote session: install the .env removal trap first, receive the env
+# file on stdin, then run setup.sh. Doing this in one session means there is
+# no window where the token-bearing file exists without a trap covering it.
+# The signal handlers must terminate the shell (which then fires EXIT): a
+# handler that merely returns would swallow the SIGHUP from a dropped SSH
+# connection and let setup.sh keep running while the local side deregisters
+# the runner. Bounded at 20 minutes (image pulls and binary downloads are the
+# bottleneck).
 {
   printf "REGISTRATION_TOKEN='%s'\n" "${REGISTRATION_TOKEN}"
   printf "GITLAB_URL='%s'\n" "${GITLAB_URL}"
   printf "RUNNER_TAG='%s'\n" "${RUNNER_TAG}"
   printf "RUNNER_IMAGE='%s'\n" "${RUNNER_IMAGE}"
   printf "OPENSHELL_VERSION='%s'\n" "${OPENSHELL_VERSION}"
   printf "GITLAB_RUNNER_VERSION='%s'\n" "${GITLAB_RUNNER_VERSION}"
-} | virtctl -n "${NAMESPACE}" ssh "${VM_USER}"@vm/"${vm_name}" \
-  -t "-o StrictHostKeyChecking=no" -t "-o UserKnownHostsFile=/dev/null" \
-  -c "umask 077 && cat > ~/gitlab-runner-vm/.env"
-
-# Run setup — the ERR trap handles runner deregistration on failure.
-# Bounded at 20 minutes (image pulls and binary downloads are the bottleneck).
-timeout 1200 virtctl -n "${NAMESPACE}" ssh "${VM_USER}"@vm/"${vm_name}" \
+} | timeout 1200 virtctl -n "${NAMESPACE}" ssh "${VM_USER}"@vm/"${vm_name}" \
   -t "-o StrictHostKeyChecking=no" -t "-o UserKnownHostsFile=/dev/null" \
-  -c "trap 'rm -f ~/gitlab-runner-vm/.env' EXIT INT TERM HUP; set -a && . ~/gitlab-runner-vm/.env && set +a && bash ~/gitlab-runner-vm/setup.sh"
+  -c "trap 'rm -f ~/gitlab-runner-vm/.env' EXIT; trap 'exit 129' HUP; trap 'exit 130' INT; trap 'exit 143' TERM; umask 077 && cat > ~/gitlab-runner-vm/.env && set -a && . ~/gitlab-runner-vm/.env && set +a && bash ~/gitlab-runner-vm/setup.sh"
 
-# Setup succeeded — clear the rollback trap.
-trap - ERR
+# Setup succeeded — clear every rollback trap so a stray signal during the
+# final output cannot deregister a healthy runner.
+trap - ERR INT TERM
 
 echo ""
 echo "Done. Runner ${vm_name} (ID ${runner_id}) is ready."

diff --git a/hack/gitlab-runner-vm/delete-vm.sh b/hack/gitlab-runner-vm/delete-vm.sh
@@ -169,7 +169,7 @@ for r in runners:
   # been deregistered above, leaving nothing to find it by.
   if delete_err=$(oc -n "${NAMESPACE}" delete vm --wait=false -- "${vm_name}" 2>&1); then
     echo "  OK: VM ${vm_name} deletion initiated"
-  elif printf '%s' "${delete_err}" | grep -q 'NotFound'; then
+  elif printf '%s' "${delete_err}" | grep -q '(NotFound)'; then
     echo "  WARN: VM ${vm_name} not found — nothing to delete"
   else
     echo "  ERROR: failed to delete VM ${vm_name}: ${delete_err}" >&2
@@ -180,7 +180,7 @@ for r in runners:
   # NotFound-vs-error split as the VM above.
   if delete_err=$(oc -n "${NAMESPACE}" delete dv --wait=false -- "${vm_name}" 2>&1); then
     echo "  OK: DataVolume ${vm_name} deletion initiated"
-  elif printf '%s' "${delete_err}" | grep -q 'NotFound'; then
+  elif printf '%s' "${delete_err}" | grep -q '(NotFound)'; then
     : # no DataVolume — nothing to delete
   else
     echo "  ERROR: failed to delete DataVolume ${vm_name}: ${delete_err}" >&2

diff --git a/hack/gitlab-runner-vm/executor/cleanup.sh b/hack/gitlab-runner-vm/executor/cleanup.sh
@@ -29,14 +29,15 @@ STATE_FILE="${STATE_DIR}/container-${JOB_ID}"
 
 if [ -f "${STATE_FILE}" ]; then
   CONTAINER_NAME=$(cat "${STATE_FILE}")
-  if ! [[ "${CONTAINER_NAME}" =~ ^runner-[0-9]+$ ]]; then
-    echo "WARN: state file holds an unexpected container name (${CONTAINER_NAME}) — skipping"
-    rm -f "${STATE_FILE}"
-    exit 0
+  if [[ "${CONTAINER_NAME}" =~ ^runner-[0-9]+$ ]]; then
+    echo "Cleaning up container: ${CONTAINER_NAME}"
+    podman stop --time 10 "${CONTAINER_NAME}" 2>/dev/null || true
+    podman rm -f "${CONTAINER_NAME}" 2>/dev/null || true
+  else
+    # Skip only the podman calls — the staging copy of the gateway mTLS
+    # material below must still be removed.
+    echo "WARN: state file holds an unexpected container name (${CONTAINER_NAME}) — not touching podman"
   fi
-  echo "Cleaning up container: ${CONTAINER_NAME}"
-  podman stop --time 10 "${CONTAINER_NAME}" 2>/dev/null || true
-  podman rm -f "${CONTAINER_NAME}" 2>/dev/null || true
   rm -f "${STATE_FILE}"
 fi
 

diff --git a/hack/gitlab-runner-vm/executor/prepare.sh b/hack/gitlab-runner-vm/executor/prepare.sh
@@ -62,8 +62,8 @@ require_under_root() {
   fi
   # ':' and ',' are field separators in podman's -v spec; a path containing
   # them would produce an opaque volume-parse error after the image pull.
-  if [[ "${resolved}" == *[:,]* ]]; then
-    echo "ERROR: ${name} must be under ${resolved_root} and must not contain ':' or ',' (got: ${resolved})" >&2
+  if [[ "${resolved}" == *[:,]* || "${resolved}" == *[[:cntrl:]]* ]]; then
+    echo "ERROR: ${name} must not contain ':' ',' or control characters (got: ${resolved})" >&2
     exit 1
   fi
   printf '%s' "${resolved}"
@@ -92,8 +92,12 @@ OPENSHELL_CONFIG="${HOME}/.config/openshell"
 # stopped leftover from an earlier failed stage. Use plain `podman rm` (no -f):
 # it refuses a running container atomically, with no inspect/rm window.
 if podman container exists "${CONTAINER_NAME}" 2>/dev/null; then
-  if ! podman rm "${CONTAINER_NAME}" >/dev/null 2>&1; then
-    echo "ERROR: container ${CONTAINER_NAME} exists and is running — refusing to reuse it" >&2
+  if ! rm_err=$(podman rm "${CONTAINER_NAME}" 2>&1); then
+    if podman inspect --format '{{.State.Running}}' "${CONTAINER_NAME}" 2>/dev/null | grep -q true; then
+      echo "ERROR: container ${CONTAINER_NAME} exists and is running — refusing to reuse it" >&2
+    else
+      echo "ERROR: could not remove stale container ${CONTAINER_NAME}: ${rm_err}" >&2
+    fi
     exit 1
   fi
 fi

diff --git a/hack/gitlab-runner-vm/executor/prepare_validation_test.sh b/hack/gitlab-runner-vm/executor/prepare_validation_test.sh
@@ -45,13 +45,14 @@ ln -s /etc "${FAKE_HOME}/builds/escape"
 failures=0
 
 # run_case <expected: accept|reject> <var> <value>
+# JOB_RESPONSE_OVERRIDE, when set, replaces the JOB_RESPONSE_FILE path.
 run_case() {
   local expected="$1" var="$2" value="$3" rc=0 output
   output=$(
     cd "${FAKE_HOME}" && \
     PATH="${SHIM_DIR}:${PATH}" \
     HOME="${FAKE_HOME}" \
-    JOB_RESPONSE_FILE="${JOB_RESPONSE}" \
+    JOB_RESPONSE_FILE="${JOB_RESPONSE_OVERRIDE-${JOB_RESPONSE}}" \
     CUSTOM_ENV_CI_JOB_IMAGE="registry.example.com/img:latest" \
     env "${var}=${value}" bash "${PREPARE}" 2>&1
   ) || rc=$?
@@ -62,8 +63,10 @@ run_case() {
   local got
   if [ "${rc}" -eq 0 ]; then
     got="accept"
-  elif printf '%s' "${output}" | grep -q "ERROR: ${short} must be under"; then
+  elif printf '%s' "${output}" | grep -Eq "ERROR: ${short} must (be under|not contain)"; then
     got="reject"
+  elif printf '%s' "${output}" | grep -q "ERROR: could not read job id from JOB_RESPONSE_FILE"; then
+    got="reject-identity"
   else
     got="error(rc=${rc}): $(printf '%s' "${output}" | tail -1)"
   fi
@@ -89,12 +92,29 @@ run_case reject CUSTOM_ENV_CI_BUILDS_DIR "${FAKE_HOME}/builds/escape/pki"
 run_case accept CUSTOM_ENV_CI_BUILDS_DIR "builds/relative"
 run_case reject CUSTOM_ENV_CI_BUILDS_DIR "builds/../etc"
 run_case reject CUSTOM_ENV_CI_BUILDS_DIR "${FAKE_HOME}/builds/a:b"
+run_case reject CUSTOM_ENV_CI_BUILDS_DIR "${FAKE_HOME}/builds/a"$'\n'"b"
 
 echo "== CACHE_DIR =="
 run_case accept CUSTOM_ENV_CI_CACHE_DIR "${FAKE_HOME}/cache"
 run_case reject CUSTOM_ENV_CI_CACHE_DIR "${FAKE_HOME}/cache-evil"
 run_case reject CUSTOM_ENV_CI_CACHE_DIR "${FAKE_HOME}/cache/../../etc"
 
+echo "== job identity =="
+# A spoofed CUSTOM_ENV_CI_JOB_ID must be ignored: identity comes from the
+# runner-written JOB_RESPONSE_FILE, so this still accepts under the real id.
+run_case accept CUSTOM_ENV_CI_JOB_ID "999999"
+# Missing, unreadable, or malformed response files must fail before podman.
+JOB_RESPONSE_OVERRIDE="${FAKE_HOME}/does-not-exist.json" \
+  run_case reject-identity CUSTOM_ENV_CI_BUILDS_DIR "${FAKE_HOME}/builds"
+printf '{"id": "not-an-int"}\n' > "${FAKE_HOME}/bad-id.json"
+JOB_RESPONSE_OVERRIDE="${FAKE_HOME}/bad-id.json" \
+  run_case reject-identity CUSTOM_ENV_CI_BUILDS_DIR "${FAKE_HOME}/builds"
+printf 'not json' > "${FAKE_HOME}/bad-json.json"
+JOB_RESPONSE_OVERRIDE="${FAKE_HOME}/bad-json.json" \
+  run_case reject-identity CUSTOM_ENV_CI_BUILDS_DIR "${FAKE_HOME}/builds"
+JOB_RESPONSE_OVERRIDE="" \
+  run_case reject-identity CUSTOM_ENV_CI_BUILDS_DIR "${FAKE_HOME}/builds"
+
 if [ "${failures}" -ne 0 ]; then
   echo "${failures} case(s) failed" >&2
   exit 1

diff --git a/hack/gitlab-runner-vm/executor/run.sh b/hack/gitlab-runner-vm/executor/run.sh
@@ -42,18 +42,55 @@ fi
 
 # Forward CI environment variables into the container.
 # GitLab Runner exposes job variables as CUSTOM_ENV_* — strip the prefix and
-# pass each one as a separate --env argument. A line-delimited --env-file
-# cannot carry these safely: file-type CI/CD variables (PEM material, keys)
-# contain newlines, which an `env`-parsing loop truncates to the first line,
-# and a continuation line beginning with CUSTOM_ENV_ would be re-parsed as an
-# attacker-chosen assignment. argv preserves values verbatim, and keeps
-# secrets out of a temp file on disk.
+# forward each one with `--env NAME` (no value). Podman then copies the value
+# from its own environment, so secrets never appear in argv or on disk. A
+# line-delimited --env-file cannot carry these safely: file-type CI/CD
+# variables (PEM material, keys) contain newlines, which an `env`-parsing loop
+# truncates to the first line, and a continuation line beginning with
+# CUSTOM_ENV_ would be re-parsed as an attacker-chosen assignment.
+# The exports happen in a subshell that execs podman, so a job variable
+# named PATH or HOME cannot alter this script's own environment. Names that
+# would change how the podman *process itself* behaves (locating conmon and
+# the OCI runtime, its storage/config, its home) are never secrets, so those
+# few are passed inline as NAME=VALUE instead of through podman's environ.
+PODMAN_BIN=$(command -v podman) || exit "${SYSTEM_FAILURE_EXIT_CODE}"
+is_process_critical() {
+  case "$1" in
+    PATH|HOME|TMPDIR|XDG_RUNTIME_DIR|XDG_CONFIG_HOME|XDG_DATA_HOME|\
+    LD_PRELOAD|LD_LIBRARY_PATH|CONTAINERS_CONF|CONTAINERS_CONF_OVERRIDE|\
+    CONTAINERS_STORAGE_CONF|CONTAINERS_REGISTRIES_CONF|CONTAINER_HOST|\
+    CONTAINER_CONNECTION|CONTAINER_SSHKEY|DOCKER_HOST) return 0 ;;
+    *) return 1 ;;
+  esac
+}
 ENV_ARGS=()
 while IFS= read -r name; do
   [ -n "${name}" ] || continue
-  ENV_ARGS+=(--env "${name#CUSTOM_ENV_}=${!name}")
+  short="${name#CUSTOM_ENV_}"
+  if is_process_critical "${short}"; then
+    ENV_ARGS+=(--env "${short}=${!name}")
+  else
+    ENV_ARGS+=(--env "${short}")
+  fi
 done < <(compgen -v | grep '^CUSTOM_ENV_')
 
+# run_in_container <podman exec args...>: exec podman with the job's variables
+# exported under their short names.
+run_in_container() {
+  (
+    while IFS= read -r name; do
+      [ -n "${name}" ] || continue
+      short="${name#CUSTOM_ENV_}"
+      is_process_critical "${short}" && continue
+      # Readonly shell names (UID, EUID, SHELLOPTS, ...) cannot be exported;
+      # a CI variable colliding with one is dropped rather than aborting.
+      export "${short}=${!name}" 2>/dev/null \
+        || echo "WARN: cannot forward job variable ${short}" >&2
+    done < <(compgen -v | grep '^CUSTOM_ENV_')
+    exec "${PODMAN_BIN}" exec "$@"
+  )
+}
+
 # The script lives on the host — copy it into the container before executing.
 BUILD_SCRIPT_DIR=$(dirname "${SCRIPT_PATH}")
 podman exec "${CONTAINER_NAME}" mkdir -p "${BUILD_SCRIPT_DIR}" || exit "${SYSTEM_FAILURE_EXIT_CODE}"
@@ -65,7 +102,7 @@ podman cp "${SCRIPT_PATH}" "${CONTAINER_NAME}:${SCRIPT_PATH}" || exit "${SYSTEM_
 # it cannot execute inside the job script, and podman propagates that verbatim,
 # so those stay build failures. 125 is ambiguous too (the script may exit
 # 125), so fall back to container liveness there.
-podman exec \
+run_in_container \
   "${ENV_ARGS[@]}" \
   "${CONTAINER_NAME}" \
   bash -- "${SCRIPT_PATH}"; rc=$?

diff --git a/hack/gitlab-runner-vm/setup.sh b/hack/gitlab-runner-vm/setup.sh
@@ -122,7 +122,11 @@ install_ca_certs() {
   timeout 15 openssl s_client -connect "${host}:${port}" -servername "${host}" -showcerts </dev/null 2>/dev/null \
     | awk '/BEGIN CERTIFICATE/,/END CERTIFICATE/' > "${staged}" || true
 
-  if [ ! -s "${staged}" ] || ! openssl x509 -noout -in "${staged}" 2>/dev/null; then
+  # crl2pkcs7 | pkcs7 -print_certs parses every certificate in the bundle,
+  # not just the first — a transfer that truncated mid-chain must not install.
+  if [ ! -s "${staged}" ] \
+    || ! openssl crl2pkcs7 -nocrl -certfile "${staged}" 2>/dev/null \
+      | openssl pkcs7 -print_certs -noout >/dev/null 2>&1; then
     rm -f "${staged}"
     fail "failed to retrieve a valid CA chain from ${host}:${port}"
   fi
@@ -591,7 +595,7 @@ start_gateway() {
 
   # Register the gateway with the CLI so openshell commands can find it.
   # Check for an active gateway (line starting with *).
-  if ! openshell gateway list 2>/dev/null | grep -q '^\*'; then
+  if ! openshell gateway list 2>/dev/null | grep -Eq '^[[:space:]]*\*'; then
     # `gateway add` is not idempotent — it refuses when metadata for the
     # canonical "openshell" loopback name already exists — so fall back to
     # selecting that name. Both failing must fail setup: every job's agent
@@ -601,7 +605,7 @@ start_gateway() {
       && ! openshell gateway select openshell >/dev/null 2>&1; then
       fail "could not register or select the OpenShell gateway: ${add_err}"
     fi
-    if ! openshell gateway list 2>/dev/null | grep -q '^\*'; then
+    if ! openshell gateway list 2>/dev/null | grep -Eq '^[[:space:]]*\*'; then
       fail "no active OpenShell gateway after add/select"
     fi
     ok "gateway registered and selected"
@@ -736,7 +740,7 @@ verify() {
 
   # The unit being active says nothing about CLI registration, which is what
   # the agent inside job containers actually resolves the gateway through.
-  if openshell gateway list 2>/dev/null | grep -q '^\*'; then
+  if openshell gateway list 2>/dev/null | grep -Eq '^[[:space:]]*\*'; then
     ok "gateway registered with the CLI"
   else
     echo "  WARN: no active gateway in 'openshell gateway list'"; errors=$((errors + 1))

diff --git a/hack/gitlab-runner-vm/vm.yaml b/hack/gitlab-runner-vm/vm.yaml
@@ -85,7 +85,7 @@ spec:
         - cloudInitNoCloud:
             userData: |
               #cloud-config
-              user: fedora
+              user: __VM_USER__
               ssh_authorized_keys:
                 - __SSH_PUBLIC_KEY__
               packages: