diff --git a/hack/gitlab-runner-vm/create-vm.sh b/hack/gitlab-runner-vm/create-vm.sh index 1ea52da1c1..7162f1558a 100755 --- 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,6 +408,14 @@ 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}" @@ -404,18 +423,13 @@ done 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 index 6b4e77b693..06af02530c 100755 --- 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 index 2f66aae3e4..183275fd36 100755 --- 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 index c63cc970a0..94dc686252 100755 --- 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 index 055812bc74..2c59cee637 100755 --- 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 +# 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 index 3bd18b2644..13164bc208 100755 --- 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 : 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 index b009ce63f8..a3552501f0 100755 --- 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 \ | 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 index a680074ada..b33077b107 100644 --- 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: