Skip to content

chore: add reproducible GitLab Runner VM setup with OCI CA hook - #5951

Merged
ggallen merged 1 commit into
fullsend-ai:mainfrom
ggallen:gitlab-runner-vm-setup
Aug 18, 2026
Merged

chore: add reproducible GitLab Runner VM setup with OCI CA hook#5951
ggallen merged 1 commit into
fullsend-ai:mainfrom
ggallen:gitlab-runner-vm-setup

Conversation

@ggallen

@ggallen ggallen commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds hack/gitlab-runner-vm/ with scripts to provision GitLab Runner VMs on OpenShift Virtualization: vm.yaml manifest, setup.sh for end-to-end configuration, create-vm.sh/delete-vm.sh lifecycle helpers, and Podman custom executor scripts (prepare.sh, run.sh, cleanup.sh).
  • Installs an OCI createRuntime hook that injects the VM's host CA trust bundle into every container's /etc/ssl/certs/ca-certificates.crt before PID 1 starts, so the OpenShell supervisor trusts internal CAs (e.g. corporate GitLab) without requiring a custom sandbox image.
  • Usage: oc apply -f vm.yaml, then REGISTRATION_TOKEN=glrt-xxx ./setup.sh — two steps to a fully working runner.

Test plan

  • Deployed runner-01 (ID 65846) and runner-02 (ID 65847) in fullsend--runtime-int namespace
  • Verified OCI hook injects 148 CAs (including Red Hat internal CA) into sandbox containers
  • Verified curl https://gitlab.cee.redhat.com/api/v4/projects returns HTTP 200 from inside sandboxes on both runners
  • All pre-commit hooks pass (shellcheck, etc.)

🤖 Generated with Claude Code

@ggallen
ggallen requested a review from a team as a code owner August 5, 2026 19:35
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add reproducible GitLab Runner VM provisioning with OCI CA injection hook

✨ Enhancement ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add VM manifest and scripts to provision GitLab Runner VMs on OpenShift Virtualization.
• Configure rootless Podman custom executor and OpenShell gateway for fullsend jobs.
• Install an OCI createRuntime hook to inject host CA trust into sandbox containers.
Diagram

graph TD
  U["Operator"] --> CVM["create-vm.sh"] --> OCA["oc apply vm.yaml"] --> VM["Runner VM"] --> SETUP["setup.sh"] --> GR["gitlab-runner (custom executor)"] --> EX["executor/*.sh (podman)"]
  SETUP --> HOOK["OCI CA hook"]
  CVM --> GL["GitLab API"]
  GR --> GL
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Run GitLab Runner with Kubernetes executor on OpenShift (no VM)
  • ➕ Eliminates VM lifecycle/SSH automation
  • ➕ Native scaling and scheduling via Kubernetes primitives
  • ➕ Simpler patching/updates with Deployment + ConfigMaps/Secrets
  • ➖ Harder to reproduce the OpenShell + rootless Podman environment exactly
  • ➖ May require privileged features or different sandboxing model
  • ➖ CA trust injection still needs a container-level strategy (image/config/webhook)
2. Bake a custom sandbox/runner image that includes internal CAs
  • ➕ No runtime rootfs mutation; simpler than OCI hooks
  • ➕ More predictable behavior across container engines
  • ➖ Requires image build/publish pipeline and version management
  • ➖ CA rotation requires rebuilds and redeploys
  • ➖ Less flexible for multi-environment CA bundles
3. Use Ansible (or ignition/cloud-init-only) for VM configuration
  • ➕ Idempotent provisioning with clearer state management
  • ➕ Better structure for long-term maintenance as steps grow
  • ➖ Adds tooling/dependency overhead for operators
  • ➖ Harder to keep “single script” ergonomics
  • ➖ Cloud-init-only may become unwieldy for complex logic and verification

Recommendation: The PR’s approach is a good fit for the stated goal: a two-step, reproducible runner VM that works with internal TLS without rebuilding images. The OCI createRuntime hook is a pragmatic solution for OpenShell supervisor CA trust, though it should be treated as a security-sensitive component (host-to-container trust propagation). If this setup becomes widely used or needs fleet management, consider migrating the provisioning logic to an idempotent tool (e.g., Ansible) while keeping the same architecture (VM + custom executor + CA injection strategy).

Files changed (7) +1005 / -0

Enhancement (2) +81 / -0
prepare.shCustom executor prepare stage: create container and inject host config +47/-0

Custom executor prepare stage: create container and inject host config

• Implements the prepare phase by pulling the requested job image, creating and starting a long-running container, copying the host CA bundle into the container, and copying OpenShell gateway configuration so in-container tools can discover the host gateway.

hack/gitlab-runner-vm/executor/prepare.sh

run.shCustom executor run stage: forward CUSTOM_ENV_* and execute scripts in container +34/-0

Custom executor run stage: forward CUSTOM_ENV_* and execute scripts in container

• Implements the run phase by translating GitLab Runner’s CUSTOM_ENV_* variables into a Podman --env-file, copying the build script into the container, and executing it inside the running container.

hack/gitlab-runner-vm/executor/run.sh

Other (5) +924 / -0
create-vm.shOne-command VM creation + runner registration + remote setup execution +191/-0

One-command VM creation + runner registration + remote setup execution

• Adds an end-to-end provisioning script that auto-numbers runner VMs, applies the KubeVirt VM manifest, waits for SSH readiness, registers a project runner via the GitLab API, copies setup/executor assets to the VM, and runs setup with the derived registration token.

hack/gitlab-runner-vm/create-vm.sh

delete-vm.shVM teardown with best-effort GitLab runner deregistration +110/-0

VM teardown with best-effort GitLab runner deregistration

• Adds a cleanup script that can list runner VMs, attempts to determine the corresponding GitLab runner ID, deregisters it via the GitLab API, and deletes the VirtualMachine and its DataVolume from OpenShift.

hack/gitlab-runner-vm/delete-vm.sh

cleanup.shCustom executor cleanup stage: stop and remove job container +14/-0

Custom executor cleanup stage: stop and remove job container

• Implements the GitLab Runner custom executor cleanup phase by reading the container name from a per-job state file, stopping/removing it via Podman, and deleting the state file; failures are ignored to avoid masking job results.

hack/gitlab-runner-vm/executor/cleanup.sh

setup.shReproducible VM bootstrap: runner, Podman, OpenShell, CA hook, and verification +514/-0

Reproducible VM bootstrap: runner, Podman, OpenShell, CA hook, and verification

• Adds a comprehensive setup script to install/validate CA trust, install and register gitlab-runner, configure it to run as the current user, enable rootless Podman, install and start OpenShell gateway, install an OCI createRuntime hook to inject CA trust into containers, configure the custom executor, pre-pull required images, and verify the resulting system state.

hack/gitlab-runner-vm/setup.sh

vm.yamlKubeVirt VirtualMachine template with cloud-init prerequisites +95/-0

KubeVirt VirtualMachine template with cloud-init prerequisites

• Adds a Fedora-based VirtualMachine manifest for OpenShift Virtualization, including a DataVolume, basic sizing, and cloud-init that sets up the fedora user, authorized SSH key, and installs baseline packages (podman/curl/git) to support subsequent setup.

hack/gitlab-runner-vm/vm.yaml

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 5, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:37 PM UTC · Completed 7:53 PM UTC
Commit: e7dbc31 · View workflow run →

@qodo-code-review

qodo-code-review Bot commented Aug 5, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Unverified runner binary download ✓ Resolved 🐞 Bug ⛨ Security
Description
setup.sh downloads and installs the GitLab Runner binary from a mutable "latest" URL without
checksum/signature verification. This installs an unverified executable with elevated privileges and
creates a supply-chain risk.
Code

hack/gitlab-runner-vm/setup.sh[R84-87]

+  sudo curl -fsSL -o /usr/local/bin/gitlab-runner \
+    "https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-linux-${arch}"
+  sudo chmod +x /usr/local/bin/gitlab-runner
+  sudo gitlab-runner install --user "${USER}" --working-directory "${HOME}"
Relevance

●●● Strong

They’ve accepted adding checksum verification for downloaded executables; runner “latest” binary
without verification is risky.

PR-#5743
PR-#662

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script installs gitlab-runner via curl directly to /usr/local/bin without any integrity
check, while it performs sha256 verification for OpenShell downloads elsewhere in the same file.

hack/gitlab-runner-vm/setup.sh[68-91]
hack/gitlab-runner-vm/setup.sh[200-213]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`install_gitlab_runner()` downloads a privileged binary from a `latest` URL and installs it without validating integrity.

## Issue Context
The same script already verifies OpenShell artifacts using a sha256 checksums file, so adding similar integrity verification is consistent with existing approach.

## Fix Focus Areas
- hack/gitlab-runner-vm/setup.sh[68-91]
- hack/gitlab-runner-vm/setup.sh[200-213]

## Suggested fix
- Pin a specific GitLab Runner version (e.g., `GITLAB_RUNNER_VERSION`, defaulting to a known-good version).
- Download the corresponding checksum/signature from an independently trusted source and verify (e.g., sha256sum check).
- Fail fast if verification fails; do not install the binary.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Prepare assumes CA path ✓ Resolved 🐞 Bug ☼ Reliability
Description
executor/prepare.sh unconditionally copies the host CA bundle into a Fedora/RHEL-specific path
inside the job container and will fail the job on images that don’t have that trust store layout. It
also unconditionally copies OpenShell config files from the host, which will abort the prepare stage
if those files are missing.
Code

hack/gitlab-runner-vm/executor/prepare.sh[R35-38]

+podman cp /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem \
+  "${CONTAINER_NAME}:/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem"
+
+# Copy openshell gateway config so the fullsend CLI inside the container
Relevance

●●● Strong

Repo frequently hardens set -euo pipefail scripts with existence checks to avoid brittle failures.

PR-#390
PR-#1714

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script runs under set -euo pipefail and performs podman cp into a fixed trust-store path and
copies two host config files without any existence checks, making these operations fail-fast.

hack/gitlab-runner-vm/executor/prepare.sh[1-4]
hack/gitlab-runner-vm/executor/prepare.sh[34-44]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The prepare stage assumes container filesystem layout and host-side config existence; with `set -e`, any missing path/file causes job failure.

## Issue Context
GitLab jobs can specify arbitrary images; many common images (e.g., Debian/Alpine) do not use `/etc/pki/ca-trust/...`.

## Fix Focus Areas
- hack/gitlab-runner-vm/executor/prepare.sh[34-44]

## Suggested fix
- Before copying, probe for destination paths inside the container, e.g.:
 - `podman exec "$CONTAINER" test -d /etc/pki/ca-trust/extracted/pem` and/or `test -f /etc/ssl/certs/ca-certificates.crt`.
- Support multiple common trust-store destinations or mount a single known CA file to a known path and configure tooling to use it.
- Check host config file existence (`$HOME/.config/openshell/gateways`, `active_gateway`) and either:
 - emit a clear error with remediation steps, or
 - skip copying when absent (if acceptable) rather than failing cryptically.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Hardcoded SSH key in vm.yaml ✓ Resolved 📘 Rule violation ⛨ Security
Description
The VM manifest hardcodes a specific SSH public key in ssh_authorized_keys, baking
environment-specific access into the template so every VM created from it authorizes the same key.
Operators should provide their own key at deploy time (or use an explicit fake placeholder via
templating/variables) to avoid shared, non-rotatable access and increased blast radius if the key is
leaked.
Code

hack/gitlab-runner-vm/vm.yaml[R86-88]

+              user: fedora
+              ssh_authorized_keys:
+                - ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDBBP84b2Xkj1wdldtjoOfMGm0XWUi7/2iN6oIfxGBwLeJHBW9VKc27JOYSujFTSM/KFK5I0nhe8lVHcfr6bOd3NeVoiBXoEGtkf/Lpo73Kn1axFAWl2j1TkZGHiuBoyYkhA7puZcRembuWrPF+XY0MzUxcw3+H8ZXPKOcdQ9tyvxBE3z87GN9iJRfAcBmX+kMXddZOe8rUuOtruvhXYrNCBx6RnInWU0zWCGSEp4qdgrilC3aDkZdwyFJ4xW6eTy3vLwjFxpyO24RJ4pzi0dJtf7wtG3vtojifIc4ciijo/44OhV5ClEgElLY3sfUdpxwHBWc1mxCoJ41bcgrjZcfbtPP0k7Z7VabbxkwxlaBMZGM5wjmrbgFS1tnVbO5JuDAPWTIcy84N7qq7S1+QP5mToq4A7RX0vGAM/tdrSF7oblp02XRdUtILGmr9ldbfsEGg8ZimhDn2NLuZLAcEHSUICanYljf655NF46Y2ClPOS9arqTtqVRIhw5OYb8SqkYk=
Relevance

●●● Strong

Hardcoded SSH authorized key is environment-specific access data; likely expected to be
templated/configured instead.

PR-#761

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062040 requires avoiding hardcoded sensitive, environment-specific identifiers in
configs and scripts; the cited cloud-init userData in the VM manifest includes a concrete
ssh_authorized_keys public key, which directly ties SSH access for provisioned VMs to a single
embedded keypair rather than requiring the operator to supply an appropriate key at runtime.

Rule 1062040: Disallow hardcoded secrets and sensitive environment-specific identifiers in source code
hack/gitlab-runner-vm/vm.yaml[86-88]
hack/gitlab-runner-vm/vm.yaml[84-89]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`hack/gitlab-runner-vm/vm.yaml` includes a concrete `ssh_authorized_keys` entry in cloud-init `userData`, hardcoding who can SSH into newly created VMs and causing every VM created from the template to authorize the same key.

## Issue Context
Even though the value is a public key (not a private key), it is still an environment-specific access identifier and should not be committed as-is; embedding one repo-wide key increases the blast radius if it is leaked or widely distributed, and makes per-VM revocation/rotation and incident response difficult for automation intended to provision multiple runner VMs. Update the template so the operator supplies the key at deploy time (or the manifest contains a clearly fake placeholder), for example by substituting a placeholder variable from `create-vm.sh`, referencing an operator-managed Kubernetes Secret containing the public key, or using platform/dynamic credential mechanisms with documentation on how credentials are retrieved.

## Fix Focus Areas
- hack/gitlab-runner-vm/vm.yaml[84-89]
- hack/gitlab-runner-vm/create-vm.sh[109-110]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Hardcoded gitlab.cee.redhat.com default ✓ Resolved 📘 Rule violation ⛨ Security
Description
The scripts hardcode an internal GitLab hostname and an environment-specific namespace as defaults,
which can leak internal infrastructure identifiers and makes the code non-portable. These values
should be provided via environment variables/configuration rather than committed as real internal
defaults.
Code

hack/gitlab-runner-vm/create-vm.sh[R43-44]

+GITLAB_URL="${GITLAB_URL:-https://gitlab.cee.redhat.com}"
+NAMESPACE="${NAMESPACE:-fullsend--runtime-int}"
Relevance

●●● Strong

Team often removes env-specific identifiers from committed artifacts; hardcoded internal
host/namespace likely flagged.

PR-#761

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062040 disallows hardcoded sensitive or environment-specific identifiers such as
internal hostnames/domains. The added scripts default GITLAB_URL to
https://gitlab.cee.redhat.com and default NAMESPACE to fullsend--runtime-int, embedding
internal deployment details directly in source.

Rule 1062040: Disallow hardcoded secrets and sensitive environment-specific identifiers in source code
hack/gitlab-runner-vm/create-vm.sh[43-44]
hack/gitlab-runner-vm/setup.sh[20-20]
hack/gitlab-runner-vm/delete-vm.sh[27-28]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Internal/environment-specific identifiers are hardcoded as defaults (e.g., `https://gitlab.cee.redhat.com`, `fullsend--runtime-int`). This violates the requirement to avoid committing sensitive environment-specific identifiers.

## Issue Context
These scripts are intended to be reproducible/setup tooling; committing real internal endpoints/namespaces makes the repo less portable and may leak internal infrastructure details.

## Fix Focus Areas
- hack/gitlab-runner-vm/create-vm.sh[43-47]
- hack/gitlab-runner-vm/setup.sh[20-23]
- hack/gitlab-runner-vm/delete-vm.sh[27-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (3)
5. Runner deregistration tag mismatch ✓ Resolved 🐞 Bug ≡ Correctness
Description
delete-vm.sh filters GitLab runners by a hardcoded tag (PREFIX) but create-vm.sh registers runners
using the user-overridable RUNNER_TAG. If RUNNER_TAG is customized, delete-vm.sh may fail to
find the runner ID and will skip deregistration, leaving stale runners in GitLab.
Code

hack/gitlab-runner-vm/delete-vm.sh[R72-74]

+    runner_id=$(curl -sf --header "PRIVATE-TOKEN: ${GL_TOKEN}" \
+      "${GITLAB_URL}/api/v4/runners/all?tag_list=${PREFIX}" 2>/dev/null \
+      | python3 -c "
Relevance

●●● Strong

Clear correctness bug: delete filters by PREFIX while create registers RUNNER_TAG; they usually
accept such script consistency fixes.

PR-#433

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Creation registers a runner with tag_list=${RUNNER_TAG}, but deletion searches
/runners/all?tag_list=${PREFIX} where PREFIX is constant, so custom tag registrations won’t be
returned by the delete query.

hack/gitlab-runner-vm/create-vm.sh[43-47]
hack/gitlab-runner-vm/create-vm.sh[135-138]
hack/gitlab-runner-vm/delete-vm.sh[27-30]
hack/gitlab-runner-vm/delete-vm.sh[71-74]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Runner registration and deregistration don’t use the same tag value: creation uses `RUNNER_TAG`, deletion queries by hardcoded `PREFIX`.

## Issue Context
- `create-vm.sh` explicitly supports overriding `RUNNER_TAG`.
- `delete-vm.sh` currently may not find/deregister runners created with non-default tags.

## Fix Focus Areas
- hack/gitlab-runner-vm/create-vm.sh[43-47]
- hack/gitlab-runner-vm/create-vm.sh[135-138]
- hack/gitlab-runner-vm/delete-vm.sh[27-30]
- hack/gitlab-runner-vm/delete-vm.sh[71-74]

## Suggested fix
- Add `RUNNER_TAG` to `delete-vm.sh` (defaulting to `fullsend-gitlab-runner`) and query with `tag_list=${RUNNER_TAG}`.
- Or better: use the token extracted from `config.toml` to identify the runner to delete (avoids tag/description ambiguity), and/or query runners without tag filter then match by description.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. RPM install errors ignored ✓ Resolved 🐞 Bug ☼ Reliability
Description
setup.sh suppresses failures from installing the OpenShell gateway RPM (|| true) and continues,
even though later steps assume the gateway systemd unit exists. This can lead to partial installs
and confusing downstream errors.
Code

hack/gitlab-runner-vm/setup.sh[R231-233]

+  sudo rpm -Uvh --nodeps "/tmp/${gateway_rpm}" 2>&1 || true
+  rm -f "/tmp/${gateway_rpm}" /tmp/openshell-checksums.txt
+
Relevance

●●● Strong

They generally prefer explicit error handling over || true in critical setup steps to avoid
confusing partial installs.

PR-#390
PR-#264

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The RPM install command explicitly discards failures, but later logic proceeds to start and validate
the gateway service, which depends on the RPM having been installed successfully.

hack/gitlab-runner-vm/setup.sh[218-236]
hack/gitlab-runner-vm/setup.sh[322-334]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`install_openshell()` ignores RPM installation errors, which can leave the system without the gateway service while the script continues.

## Issue Context
Later `start_gateway()` expects `openshell-gateway.service` to exist and start successfully.

## Fix Focus Areas
- hack/gitlab-runner-vm/setup.sh[218-236]
- hack/gitlab-runner-vm/setup.sh[322-334]

## Suggested fix
- Remove `|| true` so failures stop the setup immediately.
- If there are known non-fatal RPM exit codes you want to tolerate, handle them explicitly and still validate that:
 - the gateway binary/package is installed, and
 - the systemd unit file exists before proceeding.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. CA trust bootstrap unsafe ✓ Resolved 🐞 Bug ⛨ Security
Description
install_ca_certs() installs certificates retrieved from the first openssl s_client connection
directly into the system trust store, without independently validating that the retrieved CA is the
expected one. If that initial connection is intercepted/controlled, the VM can persistently trust
attacker-supplied certificates until the trust anchor is removed.
Code

hack/gitlab-runner-vm/setup.sh[R52-55]

+  echo | openssl s_client -connect "${host}:443" -showcerts 2>/dev/null \
+    | awk '/BEGIN CERTIFICATE/,/END CERTIFICATE/' \
+    | sudo tee /etc/pki/ca-trust/source/anchors/gitlab-chain.pem >/dev/null
+
Relevance

●● Moderate

Security/TOFU concern; could be accepted, but may be tolerated for internal bootstrap convenience.

PR-#2011

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The function extracts the remote-presented certificate chain and writes it into
/etc/pki/ca-trust/source/anchors followed by update-ca-trust, persisting it as a trust anchor.

hack/gitlab-runner-vm/setup.sh[40-62]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The script bootstraps trust by trusting whatever certificate chain is presented by the target GitLab endpoint on first contact.

## Issue Context
This runs specifically when the system doesn’t already trust the GitLab endpoint, which is exactly when you have no authenticated channel yet.

## Fix Focus Areas
- hack/gitlab-runner-vm/setup.sh[40-62]

## Suggested fix
- Require a CA bundle file to be provided out-of-band (e.g., `CA_BUNDLE_PATH`), and install that instead of scraping from the network.
- Or ship the CA via a trusted mechanism for the VM (cloud-init file injection, OpenShift Secret mounted/templated into the VM).
- If keeping TOFU behavior, add explicit warnings and store/remove logic; do not silently treat the scraped chain as a trusted anchor.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context
✅ Compliance rules (platform): 54 rules

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread hack/gitlab-runner-vm/create-vm.sh Outdated
Comment thread hack/gitlab-runner-vm/vm.yaml Outdated
Comment thread hack/gitlab-runner-vm/delete-vm.sh Outdated
Comment thread hack/gitlab-runner-vm/setup.sh Outdated
Comment thread hack/gitlab-runner-vm/setup.sh Outdated
Comment thread hack/gitlab-runner-vm/executor/prepare.sh Outdated
Comment thread hack/gitlab-runner-vm/setup.sh Outdated
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [yaml-injection] hack/gitlab-runner-vm/create-vm.sh:361 — SSH_PUBLIC_KEY is injected into the vm.yaml template via Python string replacement. The key is validated to start with ssh- or ecdsa- and to contain no newlines. Since the substitution occurs within a YAML | block scalar where line-internal metacharacters are inert, and newlines (the only block-scalar escape vector) are rejected, the practical risk is negligible.

  • [incomplete-architecture-coverage] docs/ADRs/0036-agent-execution-sandbox.md — ADR-0036's executor list does not mention the Podman custom executor variant implemented in hack/gitlab-runner-vm/. The ADR's mitigations section already mentions "Fedora/RHEL alternative for Podman-first environments" and the README cross-references ADR-0036, narrowing the gap. A minor annotation or addendum could complete the coverage.

  • [logic-error] hack/gitlab-runner-vm/setup.sh:688 — In patch_config(), the awk command uses -v block="${custom_block}" where custom_block is a multi-line string containing TOML config with file paths. POSIX awk -v processes C-style escape sequences. If paths ever contain backslash sequences, awk will interpret them as escapes, corrupting the config file. In practice, these are standard Linux home directory paths (/home/fedora/...) on a single-purpose VM that never contain backslashes.

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:340 — If the GitLab API returns valid JSON lacking the expected token or id keys, the python3 command raises a KeyError and exits non-zero. The behavior is correct (the ERR trap cleans up), but the error message is an unhandled Python traceback rather than a user-friendly message.

  • [error-handling] hack/gitlab-runner-vm/setup.sh:165 — In install_gitlab_runner(), the version comparison parses gitlab-runner --version output with awk '{print $2}'. If the output format changes across gitlab-runner versions, the comparison may fail to detect a version match, causing an unnecessary but harmless re-install on every run.

  • [edge-case] hack/gitlab-runner-vm/delete-vm.sh:117 — The pagination loop in the runner lookup is bounded at 50 pages (5,000 runners). If the runner is beyond page 50, it won't be found and the script will warn and skip deregistration.

  • [credential-sharing] hack/gitlab-runner-vm/executor/prepare.sh:96 — All concurrent CI jobs receive copies of the runner's long-lived gateway mTLS credentials (~/.config/openshell). The runner is scoped to a single project (runner_type=project_type, locked=true) and tag-matched (run_untagged=false), which limits exposure. The README explicitly documents this limitation and recommends replacing the mount with short-lived per-job tokens when the gateway supports them.

  • [trust-on-first-use] hack/gitlab-runner-vm/setup.sh:122 — The install_ca_certs function fetches the CA certificate chain directly from the target server via openssl s_client (TOFU model). Mitigations include TLS-specific exit code gating, cryptographic validation via crl2pkcs7, post-install verification with rollback on failure, and a separate filename (gitlab-chain-tofu.pem) to avoid clobbering operator-provided bundles. The README documents the out-of-band alternative for higher assurance.

  • [secrets-exposure] hack/gitlab-runner-vm/setup.sh:239register_runner passes REGISTRATION_TOKEN on the command line via --token, visible in /proc/<pid>/cmdline. Partially mitigated when called via create-vm.sh (env file via stdin + umask 077). gitlab-runner register does not support --token-file, so this is an upstream limitation. Short-lived registration token, single-tenant VM, one-time provisioning operation.

  • [container-isolation] hack/gitlab-runner-vm/executor/prepare.sh:106 — Job containers run with --network=host, giving them full access to the host's network stack. Hardened with --cap-drop=ALL, --security-opt=no-new-privileges, and --pids-limit 4096. The gateway endpoint is protected by mTLS. Documented architectural trade-off required for gateway connectivity.

  • [code-organization] hack/gitlab-runner-vm/create-vm.sh:197gl_curl() is copy-pasted identically in both create-vm.sh and delete-vm.sh. Both scripts could share a sourced file, similar to gitlab-runner-version.sh.

Previous run

Review

Findings

Medium

  • [logic-error] hack/gitlab-runner-vm/create-vm.sh:382executor/job_id.sh is copied to the VM (line 360) but omitted from both the sha256sum integrity verification (lines 382–383) and the preflight file-existence check (lines 166–167). The sha256sum check exists specifically because cat > file exits 0 on a short write (comment at line 377). A truncated or corrupted job_id.sh would pass verification, and then all three executor stages (prepare.sh, run.sh, cleanup.sh) would fail at runtime because they source job_id.sh.
    Remediation: Add executor/job_id.sh to the sha256sum command on line 382 (alongside executor/prepare.sh, executor/run.sh, executor/cleanup.sh) and to the preflight file loop on lines 166–167.

Low

  • [logic-error] hack/gitlab-runner-vm/setup.sh:688 — In patch_config(), the awk command uses -v block="${custom_block}" where custom_block is a multi-line string containing TOML config with file paths. POSIX awk -v processes C-style escape sequences. If paths ever contain backslash sequences, awk will interpret them as escapes, corrupting the config file. In practice, these are standard Linux home directory paths (/home/fedora/...) on a single-purpose VM that never contain backslashes.

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:340 — If the GitLab API returns valid JSON lacking the expected token or id keys, the python3 command raises a KeyError and exits non-zero. The behavior is correct (the ERR trap cleans up), but the error message is an unhandled Python traceback rather than a user-friendly message.

  • [error-handling] hack/gitlab-runner-vm/setup.sh:165 — In install_gitlab_runner(), the version comparison parses gitlab-runner --version output with awk '{print $2}'. If the output format changes across gitlab-runner versions, the comparison may fail to detect a version match, causing an unnecessary but harmless re-install on every run.

  • [edge-case] hack/gitlab-runner-vm/delete-vm.sh:116 — The pagination loop in the runner lookup is bounded at 50 pages (5,000 runners). If the runner is beyond page 50, it won't be found and the script will warn and skip deregistration.

  • [credential-sharing] hack/gitlab-runner-vm/executor/prepare.sh:96 — All concurrent CI jobs receive copies of the runner's long-lived gateway mTLS credentials (~/.config/openshell). The runner is scoped to a single project (runner_type=project_type, locked=true) and tag-matched (run_untagged=false), which limits exposure. The README explicitly documents this limitation and recommends replacing the mount with short-lived per-job tokens when the gateway supports them.

  • [trust-on-first-use] hack/gitlab-runner-vm/setup.sh:122 — The install_ca_certs function fetches the CA certificate chain directly from the target server via openssl s_client (TOFU model). Mitigations include TLS-specific exit code gating, cryptographic validation of the fetched bundle via crl2pkcs7, post-install verification with rollback on failure, and a separate filename (gitlab-chain-tofu.pem) to avoid clobbering operator-provided bundles. The README documents the out-of-band alternative for higher assurance.

  • [secrets-exposure] hack/gitlab-runner-vm/setup.sh:239register_runner passes REGISTRATION_TOKEN on the command line via --token, visible in /proc/<pid>/cmdline. Partially mitigated when called via create-vm.sh (env file via stdin + umask 077). gitlab-runner register does not support --token-file, so this is an upstream limitation. Short-lived registration token, single-tenant VM, one-time provisioning operation.

  • [container-isolation] hack/gitlab-runner-vm/executor/prepare.sh:106 — Job containers run with --network=host, giving them full access to the host's network stack. Hardened with --cap-drop=ALL, --security-opt=no-new-privileges, and --pids-limit 4096. The gateway endpoint is protected by mTLS. Documented architectural trade-off required for gateway connectivity.

  • [code-organization] hack/gitlab-runner-vm/create-vm.sh:197gl_curl() is copy-pasted identically in both create-vm.sh and delete-vm.sh. Both scripts could share a sourced file, similar to gitlab-runner-version.sh.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (2)

Review

Findings

Critical

  • [logic-error] hack/gitlab-runner-vm/create-vm.sh:361executor/job_id.sh is never copied to the VM or installed to ${EXECUTOR_DIR}. The copy loop (line 361) transfers only prepare.sh, run.sh, and cleanup.sh, but all three source job_id.sh at runtime via source "$(dirname "${BASH_SOURCE[0]}")/job_id.sh". The preflight file check (line 168), sha256sum verification (line 384), and setup.sh's install_executor() (line 628) also omit it. Every CI job will fail at the prepare stage with source: No such file or directory.
    Remediation: Add job_id.sh to the executor file copy loop in create-vm.sh (line 361), the sha256sum verification list (line 384), the preflight file-existence check (line 168), and the install_executor() copy loop in setup.sh (line 628).

Low

  • [credential-sharing] hack/gitlab-runner-vm/executor/prepare.sh:97 — All concurrent CI jobs receive identical copies of the runner's long-lived gateway mTLS credentials (~/.config/openshell). The README explicitly documents this limitation and recommends replacing the mount with short-lived per-job tokens when the gateway supports them.

  • [logic-error] hack/gitlab-runner-vm/setup.sh:689 — In patch_config(), the awk command uses -v block="${custom_block}" where custom_block is a multi-line string containing TOML config with file paths. POSIX awk -v processes C-style escape sequences. If paths ever contain backslash sequences, awk will interpret them as escapes, corrupting the config file. In practice, these are standard Linux home directory paths (/home/fedora/...) on a single-purpose VM that never contain backslashes.

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:341 — If the GitLab API returns valid JSON lacking the expected token or id keys, the python3 command raises a KeyError. The ERR trap fires and calls cleanup_runner() with an empty runner_id, which tells the user to check for orphaned runners.

  • [error-handling] hack/gitlab-runner-vm/setup.sh:166 — In install_gitlab_runner(), the version comparison parses gitlab-runner --version output with awk '{print $2}'. If the output format changes, awk would extract the wrong token, causing an unnecessary but harmless re-install on every run.

  • [edge-case] hack/gitlab-runner-vm/delete-vm.sh:117 — The pagination loop in the runner lookup is bounded at 50 pages (5,000 runners). If the runner is beyond page 50, it won't be found and the script will warn and skip deregistration.

  • [yaml-injection] hack/gitlab-runner-vm/create-vm.sh:244 — The vm.yaml template substitution uses Python str.replace() to inject SSH_PUBLIC_KEY into a YAML literal block scalar. The newline check and prefix validation limit the attack surface. Within a literal block scalar, YAML metacharacters are inert, so the practical injection risk is negligible.

  • [trust-on-first-use] hack/gitlab-runner-vm/setup.sh:123 — The install_ca_certs function fetches the CA certificate chain directly from the target server via openssl s_client (TOFU model). During initial VM provisioning, an active network attacker could present a rogue CA certificate. One-time provisioning operation on an internal network, with documented out-of-band alternative and post-install verification.

  • [secrets-exposure] hack/gitlab-runner-vm/setup.sh:240register_runner passes REGISTRATION_TOKEN on the command line via --token, visible in /proc/<pid>/cmdline. gitlab-runner register does not support file-based input for the token. Short-lived registration token, single-tenant VM, one-time provisioning operation.

  • [container-isolation] hack/gitlab-runner-vm/executor/prepare.sh:107 — Job containers run with --network=host, giving them full access to the host's network stack. While --cap-drop=ALL, --security-opt=no-new-privileges, and --pids-limit 4096 provide strong process-level hardening, a malicious job can access any service listening on the host. Documented as a deliberate design choice for gateway connectivity; gateway endpoint is protected by mTLS.

  • [code-organization] hack/gitlab-runner-vm/create-vm.sh:80gl_curl() is copy-pasted identically in both create-vm.sh and delete-vm.sh. Both scripts could share a sourced file, similar to gitlab-runner-version.sh.

  • [missing-cross-reference] docs/ADRs/0036-agent-execution-sandbox.md:174 — ADR-0036's 'Executors supported' section does not mention the Podman custom executor variant now implemented in hack/gitlab-runner-vm/. A minor annotation is permitted under ADR immutability policy.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (3)

Review

Findings

Medium

  • [credential-sharing] hack/gitlab-runner-vm/executor/prepare.sh:109 — All concurrent CI jobs receive identical copies of the runner's long-lived gateway mTLS credentials (~/.config/openshell). A malicious job from a trusted project could exfiltrate these credentials and use them to authenticate to the OpenShell gateway independently. The credentials are copied per-job (cp -a to a staging dir) but are the same long-lived material. The README acknowledges this limitation and recommends replacing the mount with short-lived per-job tokens when the gateway supports them.

Low

  • [logic-error] hack/gitlab-runner-vm/setup.sh:688 — In patch_config(), the awk command uses -v block="${custom_block}" where custom_block is a multi-line string containing TOML config with file paths. POSIX awk -v processes C-style escape sequences. If paths ever contain backslash sequences, awk will interpret them as escapes, corrupting the config file. In practice, these are standard Linux home directory paths (/home/fedora/...) on a single-purpose VM that never contain backslashes.

  • [yaml-injection] hack/gitlab-runner-vm/create-vm.sh:243 — The vm.yaml template substitution uses Python str.replace() to inject SSH_PUBLIC_KEY into a YAML literal block scalar. The newline check (line 234) and prefix validation (line 238) limit the attack surface. Within a literal block scalar, YAML metacharacters are inert, so the practical injection risk is negligible. The str.replace() approach remains fragile compared to proper YAML serialization.

  • [trust-on-first-use] hack/gitlab-runner-vm/setup.sh:122 — The install_ca_certs function fetches the CA certificate chain directly from the target server via openssl s_client (TOFU model). During initial VM provisioning, an active network attacker could present a rogue CA certificate. One-time provisioning operation on an internal network, with documented out-of-band alternative and post-install verification.

  • [secrets-exposure] hack/gitlab-runner-vm/setup.sh:235register_runner passes REGISTRATION_TOKEN on the command line via --token, visible in /proc/<pid>/cmdline. gitlab-runner register does not support file-based input for the token. Short-lived registration token, single-tenant VM, one-time provisioning operation.

  • [container-isolation] hack/gitlab-runner-vm/executor/prepare.sh:119 — Job containers run with --network=host, giving them full access to the host's network stack. While --cap-drop=ALL, --security-opt=no-new-privileges, and --pids-limit 4096 provide strong process-level hardening, a malicious job can access any service listening on the host. Documented as a deliberate design choice for gateway connectivity; gateway endpoint is protected by mTLS.

  • [permissions] hack/gitlab-runner-vm/setup.sh:649/etc/gitlab-runner set to mode 700 (improved from prior review's 755). The config.toml inside retains its own 0600 permissions from gitlab-runner register.

  • [privilege-scope] hack/gitlab-runner-vm/setup.sh:534 — The OCI CA-injection hook runs as root via createRuntime for ALL containers on the host ("always": true), not just CI job containers. The hook only writes CA certificates and includes robust safety checks (40-hop symlink limit, escape detection, atomic write).

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:340 — If the GitLab API returns valid JSON lacking the expected token or id keys, the python3 command raises a KeyError. The ERR trap fires and calls cleanup_runner() with an empty runner_id, which tells the user to check for orphaned runners.

  • [error-handling] hack/gitlab-runner-vm/setup.sh:165 — In install_gitlab_runner(), the version comparison parses gitlab-runner --version output with awk '{print $2}'. If the output format changes, awk would extract the wrong token, causing an unnecessary but harmless re-install on every run.

  • [edge-case] hack/gitlab-runner-vm/delete-vm.sh:116 — The pagination loop in the runner lookup is bounded at 50 pages (5,000 runners). If the runner is beyond page 50, it won't be found and the script will warn and skip deregistration.

  • [code-organization] hack/gitlab-runner-vm/create-vm.sh:67gl_curl() is copy-pasted identically in both create-vm.sh and delete-vm.sh. Both scripts could share a sourced file, similar to gitlab-runner-version.sh.

  • [code-organization] hack/gitlab-runner-vm/executor/prepare.sh:21resolve_job_id() is copy-pasted identically across all three executor scripts (prepare.sh:21, run.sh:16, cleanup.sh:12). A sourced helper would reduce the maintenance surface.

  • [error-handling-idiom] hack/gitlab-runner-vm/executor/run.sh:8 — ERROR messages in run.sh are written to stdout (lines 8, 27, 33, 39), while sibling prepare.sh consistently writes errors to stderr with >&2. Within the same executor suite the stderr convention should be uniform.

  • [missing-cross-reference] docs/ADRs/0036-agent-execution-sandbox.md:174 — ADR-0036's 'Executors supported' section does not mention the Podman custom executor variant now implemented in hack/gitlab-runner-vm/. A minor annotation is permitted under ADR immutability policy.

Previous run (4)

Review

Findings

Medium

  • [credential-sharing] hack/gitlab-runner-vm/executor/prepare.sh:51 — All concurrent CI jobs receive identical copies of the runner's long-lived gateway mTLS credentials (~/.config/openshell). A malicious job from a trusted project could exfiltrate these credentials and use them to authenticate to the OpenShell gateway independently. The credentials are copied per-job (cp -a to a staging dir) but are the same long-lived material. The README acknowledges this limitation and recommends replacing the mount with short-lived per-job tokens when the gateway supports them.

Low

  • [trust-on-first-use] hack/gitlab-runner-vm/setup.sh:100 — The install_ca_certs function fetches the CA certificate chain directly from the target server via openssl s_client (TOFU model). During initial VM provisioning, an active network attacker could present a rogue CA certificate that would then be permanently trusted by the host and injected into all future job containers via the OCI hook. This is a one-time provisioning operation on an internal network, and the README documents an out-of-band alternative for higher assurance.

  • [secrets-exposure] hack/gitlab-runner-vm/setup.sh:197register_runner passes REGISTRATION_TOKEN on the command line via --token, visible in /proc/<pid>/cmdline. gitlab-runner register does not support file-based input for the token. Short-lived registration token, single-tenant VM, one-time provisioning operation.

  • [container-isolation] hack/gitlab-runner-vm/executor/prepare.sh:58 — Job containers run with --network=host, giving them full access to the host's network stack. While --cap-drop=ALL, --security-opt=no-new-privileges, and --pids-limit 4096 provide strong process-level hardening, and the README documents this as a deliberate design choice for gateway connectivity, a malicious job can access any service listening on the host. The gateway endpoint is protected by mTLS.

  • [permissions] hack/gitlab-runner-vm/setup.sh:575/etc/gitlab-runner set to mode 755 (world-readable directory listing). The config.toml inside retains its own 0600 permissions from gitlab-runner register. Low risk on a single-tenant VM; 750 would be more restrictive.

  • [privilege-scope] hack/gitlab-runner-vm/setup.sh:476 — The OCI CA-injection hook runs as root via createRuntime for ALL containers on the host ("always": true), not just CI job containers. The hook only writes CA certificates and includes robust safety checks (40-hop symlink limit, escape detection, atomic write).

  • [logic-error] hack/gitlab-runner-vm/setup.sh:614 — In patch_config(), the awk command uses -v block="${custom_block}" where custom_block is a multi-line string containing TOML config with file paths. POSIX awk -v processes C-style escape sequences. If paths ever contain backslash sequences, awk will interpret them as escapes, corrupting the config file. In practice, these are standard Linux home directory paths (/home/fedora/...) on a single-purpose VM that never contain backslashes.

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:282 — If the GitLab API returns valid JSON lacking the expected token or id keys, the python3 command raises a KeyError. The ERR trap fires and calls cleanup_runner() with an empty runner_id, which tells the user to check for orphaned runners.

  • [edge-case] hack/gitlab-runner-vm/executor/run.sh:32 — The env-file produced for podman exec --env-file includes all CUSTOM_ENV_* variables with the prefix stripped. The env | grep pipeline uses line-delimited parsing, so multiline CI variable values would be split across lines, corrupting the env-file. The script documents this limitation in a comment; GitLab Runner's CUSTOM_ENV_* are single-line in practice.

  • [error-handling] hack/gitlab-runner-vm/setup.sh:127 — In install_gitlab_runner(), the version comparison parses gitlab-runner --version output with awk '{print $2}'. If the output format changes, awk would extract the wrong token, causing an unnecessary but harmless re-install on every run.

  • [edge-case] hack/gitlab-runner-vm/delete-vm.sh:116 — The pagination loop in the runner lookup is bounded at 50 pages (5,000 runners). If the runner is beyond page 50, it won't be found and the script will warn and skip deregistration.

  • [code-organization] hack/gitlab-runner-vm/create-vm.sh:67gl_curl() is copy-pasted identically in both create-vm.sh and delete-vm.sh. Both scripts could share a sourced file, similar to gitlab-runner-version.sh.

  • [missing-cross-reference] docs/ADRs/0036-agent-execution-sandbox.md:174 — ADR-0036 'Executors supported' section does not mention the Podman custom executor + OpenShift Virtualization deployment variant now implemented in hack/gitlab-runner-vm/. The new README.md states this is 'a deployment variant of the container isolation model described in ADR-0036', but ADR-0036 does not document this variant.

Previous run (5)

Review

Findings

Medium

  • [credential-sharing] hack/gitlab-runner-vm/executor/prepare.sh:51 — All concurrent CI jobs receive identical copies of the runner’s long-lived gateway mTLS credentials (~/.config/openshell). A malicious job from a trusted project could exfiltrate these credentials and use them to authenticate to the OpenShell gateway independently. The credentials are copied per-job (cp -a to a staging dir) but are the same long-lived material. The README acknowledges this limitation and recommends replacing the mount with short-lived per-job tokens when the gateway supports them.

Low

  • [trust-on-first-use] hack/gitlab-runner-vm/setup.sh:100 — The install_ca_certs function fetches the CA certificate chain directly from the target server via openssl s_client (TOFU model). During initial VM provisioning, an active network attacker could present a rogue CA certificate that would then be permanently trusted by the host and injected into all future job containers via the OCI hook. This is a one-time provisioning operation on an internal network, and the README documents an out-of-band alternative for higher assurance.

  • [logic-error] hack/gitlab-runner-vm/setup.sh:612 — In patch_config(), the awk command uses -v block="${custom_block}" where custom_block is a multi-line string containing TOML config with file paths. POSIX awk -v processes C-style escape sequences. If paths ever contain backslash sequences, awk will interpret them as escapes, corrupting the config file. In practice, these are standard Linux home directory paths (/home/fedora/...) on a single-purpose VM that never contain backslashes.

  • [error-handling] hack/gitlab-runner-vm/executor/prepare.sh:9prepare.sh uses exit 1 for errors rather than the GitLab Runner custom executor contract’s SYSTEM_FAILURE_EXIT_CODE or BUILD_FAILURE_EXIT_CODE. In practice, GitLab Runner treats non-zero prepare exits as system failures by default, so this works correctly. The sibling run.sh correctly uses the contract exit codes.

  • [race-condition] hack/gitlab-runner-vm/create-vm.sh:144 — Auto-numbering has a TOCTOU race: two concurrent create-vm.sh invocations compute the same next number from oc get vm output, causing a name collision. Mitigated by this being a manual operations tool unlikely to be run concurrently, and by the pre-existing check that errors if the VM already exists.

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:282 — If the GitLab API returns valid JSON lacking the expected token or id keys, the python3 command raises a KeyError. The ERR trap fires and calls cleanup_runner() with an empty runner_id, which tells the user to check for orphaned runners.

  • [edge-case] hack/gitlab-runner-vm/executor/run.sh:32 — The env-file produced for podman exec --env-file includes all CUSTOM_ENV_* variables with the prefix stripped. The env | grep pipeline uses line-delimited parsing, so multiline CI variable values would be split across lines, corrupting the env-file. The script documents this limitation in a comment; GitLab Runner’s CUSTOM_ENV_* are single-line in practice.

  • [error-handling] hack/gitlab-runner-vm/setup.sh:127 — In install_gitlab_runner(), the version comparison parses gitlab-runner --version output with awk '{print $2}'. If the output format changes, awk would extract the wrong token, causing an unnecessary but harmless re-install on every run.

  • [edge-case] hack/gitlab-runner-vm/delete-vm.sh:116 — The pagination loop in the runner lookup has no upper bound on page count. If the GitLab API consistently returns exactly 100 runners per page and none match the target description, the loop runs indefinitely. Bounded in practice by the finite number of runners.

  • [secrets-exposure] hack/gitlab-runner-vm/setup.sh:193register_runner passes REGISTRATION_TOKEN on the command line via --token, visible in /proc/<pid>/cmdline. gitlab-runner register does not support file-based input for the token. Short-lived registration token, single-tenant VM, one-time provisioning operation.

  • [container-isolation] hack/gitlab-runner-vm/executor/prepare.sh:58 — Job containers run with --network=host, giving them full access to the host’s network stack. While --cap-drop=ALL and --security-opt=no-new-privileges provide strong process-level hardening, and the README documents this as a deliberate design choice for gateway connectivity, a malicious job can access any service listening on the host.

  • [permissions] hack/gitlab-runner-vm/setup.sh:573/etc/gitlab-runner set to mode 755 (world-readable directory listing). The config.toml inside retains its own 0600 permissions from gitlab-runner register. Low risk on a single-tenant VM.

  • [privilege-scope] hack/gitlab-runner-vm/setup.sh:345 — The OCI CA-injection hook runs as root via createRuntime for ALL containers on the host ("always": true), not just CI job containers. The hook only writes CA certificates and includes robust safety checks (40-hop symlink limit, escape detection, atomic write).

  • [code-organization] hack/gitlab-runner-vm/create-vm.sh:67gl_curl() is copy-pasted identically in both create-vm.sh and delete-vm.sh. Both scripts could share a sourced file, similar to gitlab-runner-version.sh.

  • [naming-convention] hack/gitlab-runner-vm/executor/run.sh:37SCRIPT_DIR in run.sh holds the parent directory of the CI build script, while in every other script in this PR it refers to the directory containing the script itself. The semantic collision could cause maintenance confusion.

  • [missing-cross-reference] docs/ADRs/0036-agent-execution-sandbox.md:174 — ADR-0036 'Executors supported' section does not mention the Podman custom executor + OpenShift Virtualization deployment variant now implemented in hack/gitlab-runner-vm/. The new README.md states this is 'a deployment variant of the container isolation model described in ADR-0036', but ADR-0036 does not document this variant.

Previous run (6)

Review

Findings

Medium

  • [credential-sharing] hack/gitlab-runner-vm/executor/prepare.sh:51 — All concurrent CI jobs receive identical copies of the runner's long-lived gateway mTLS credentials (~/.config/openshell). A malicious job from a trusted project could exfiltrate these credentials and use them to authenticate to the OpenShell gateway independently. The credentials are copied per-job (cp -a to a staging dir) but are the same long-lived material. The README acknowledges this limitation and recommends replacing the mount with short-lived per-job tokens when the gateway supports them.

  • [trust-on-first-use] hack/gitlab-runner-vm/setup.sh:100 — The install_ca_certs function fetches the CA certificate chain directly from the target server via openssl s_client (TOFU model). During initial VM provisioning, an active network attacker could present a rogue CA certificate that would then be permanently trusted by the host and injected into all future job containers via the OCI hook. This is acknowledged in the README and code comments. Consider adding an optional CA_FINGERPRINT environment variable that, when set, verifies the fetched certificate's fingerprint before trusting it.

Low

  • [logic-error] hack/gitlab-runner-vm/setup.sh:612 — In patch_config(), the awk command uses -v block="${custom_block}" where custom_block is a multi-line string containing TOML config with file paths. POSIX awk -v processes C-style escape sequences. If paths ever contain backslash sequences, awk will interpret them as escapes, corrupting the config file. In practice, these are standard Linux home directory paths (/home/fedora/...) on a single-purpose VM that never contain backslashes.

  • [secrets-exposure] hack/gitlab-runner-vm/setup.sh:193register_runner passes REGISTRATION_TOKEN on the command line via --token, visible in /proc/<pid>/cmdline. gitlab-runner register does not support file-based input for the token. Short-lived registration token, single-tenant VM, one-time provisioning operation.

  • [error-handling] hack/gitlab-runner-vm/executor/prepare.sh:9prepare.sh uses exit 1 for errors rather than the GitLab Runner custom executor contract's SYSTEM_FAILURE_EXIT_CODE or BUILD_FAILURE_EXIT_CODE. In practice, GitLab Runner treats non-zero prepare exits as system failures by default, so this works correctly. The sibling run.sh correctly uses the contract exit codes.

  • [race-condition] hack/gitlab-runner-vm/create-vm.sh:144 — Auto-numbering has a TOCTOU race: two concurrent create-vm.sh invocations compute the same next number from oc get vm output, causing a name collision. Mitigated by this being a manual operations tool unlikely to be run concurrently, and by the pre-existing check that errors if the VM already exists.

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:282 — If the GitLab API returns valid JSON lacking the expected token or id keys, the python3 command raises a KeyError. The ERR trap fires and calls cleanup_runner() with an empty runner_id, which tells the user to check for orphaned runners.

  • [edge-case] hack/gitlab-runner-vm/executor/run.sh:32 — The env-file produced for podman exec --env-file includes all CUSTOM_ENV_* variables with the prefix stripped. The env | grep pipeline uses line-delimited parsing, so multiline CI variable values would be split across lines, corrupting the env-file. The script documents this limitation in a comment; GitLab Runner's CUSTOM_ENV_* are single-line in practice.

  • [error-handling] hack/gitlab-runner-vm/setup.sh:127 — In install_gitlab_runner(), the version comparison parses gitlab-runner --version output with awk '{print $2}'. If the output format changes, awk would extract the wrong token, causing an unnecessary but harmless re-install on every run.

  • [edge-case] hack/gitlab-runner-vm/delete-vm.sh:116 — The pagination loop in the runner lookup has no upper bound on page count. If the GitLab API consistently returns exactly 100 runners per page and none match the target description, the loop runs indefinitely. Bounded in practice by the finite number of runners.

  • [container-isolation] hack/gitlab-runner-vm/executor/prepare.sh:58 — Job containers run with --network=host, giving them full access to the host's network stack. While --cap-drop=ALL and --security-opt=no-new-privileges provide strong process-level hardening, and the README documents this as a deliberate design choice for gateway connectivity, a malicious job can access any service listening on the host.

  • [permissions] hack/gitlab-runner-vm/setup.sh:573/etc/gitlab-runner set to mode 755 (world-readable directory listing). The config.toml inside retains its own 0600 permissions from gitlab-runner register. Low risk on a single-tenant VM.

  • [privilege-scope] hack/gitlab-runner-vm/setup.sh:345 — The OCI CA-injection hook runs as root via createRuntime for ALL containers on the host ("always": true), not just CI job containers. The hook only writes CA certificates and includes robust safety checks (40-hop symlink limit, escape detection, atomic write).

  • [code-organization] hack/gitlab-runner-vm/create-vm.sh:67gl_curl() is copy-pasted identically in both create-vm.sh and delete-vm.sh. Both scripts could share a sourced file, similar to gitlab-runner-version.sh.

  • [naming-convention] hack/gitlab-runner-vm/executor/run.sh:37SCRIPT_DIR in run.sh holds the parent directory of the CI build script, while in every other script in this PR it refers to the directory containing the script itself. The semantic collision could cause maintenance confusion.

  • [missing-cross-reference] docs/ADRs/0036-agent-execution-sandbox.md:174 — ADR-0036 'Executors supported' section does not mention the Podman custom executor + OpenShift Virtualization deployment variant now implemented in hack/gitlab-runner-vm/. The new README.md states this is 'a deployment variant of the container isolation model described in ADR-0036', but ADR-0036 does not document this variant.

  • [stale-note] docs/ADRs/0036-agent-execution-sandbox.md:262 — ADR-0036 Open Questions section mentions 'Fedora-based base images may be considered when exploring rootless Podman support' but does not reference that a Fedora+Podman implementation now exists in hack/gitlab-runner-vm/setup.sh (which uses Fedora VMs and configures rootless Podman).

Previous run (7)

Review

Findings

Medium

  • [credential-sharing] hack/gitlab-runner-vm/executor/prepare.sh:38 — All concurrent CI jobs receive identical copies of the runner's long-lived gateway mTLS credentials (~/.config/openshell). A malicious job from a trusted project could exfiltrate these credentials and use them to authenticate to the OpenShell gateway independently. The credentials are copied per-job (cp -a to a staging dir) but are the same long-lived material. The README acknowledges this limitation and recommends replacing the mount with short-lived per-job tokens when the gateway supports them.

  • [trust-on-first-use] hack/gitlab-runner-vm/setup.sh:100 — The install_ca_certs function fetches the CA certificate chain directly from the target server via openssl s_client (TOFU model). During initial VM provisioning, an active network attacker could present a rogue CA certificate that would then be permanently trusted by the host and injected into all future job containers via the OCI hook. This is acknowledged in the README and code comments. Consider adding an optional CA_FINGERPRINT environment variable that, when set, verifies the fetched certificate's fingerprint before trusting it.

Low

  • [logic-error] hack/gitlab-runner-vm/setup.sh:612 — In patch_config(), the awk command uses -v block="${custom_block}" where custom_block is a multi-line string containing TOML config with file paths. POSIX awk -v processes C-style escape sequences. If paths ever contain backslash sequences, awk will interpret them as escapes, corrupting the config file. In practice, these are standard Linux home directory paths (/home/fedora/...) on a single-purpose VM that never contain backslashes.

  • [secrets-exposure] hack/gitlab-runner-vm/setup.sh:193register_runner passes REGISTRATION_TOKEN on the command line via --token, visible in /proc/<pid>/cmdline. gitlab-runner register does not support file-based input for the token. Short-lived registration token, single-tenant VM, one-time provisioning operation.

  • [error-handling] hack/gitlab-runner-vm/executor/prepare.sh:9prepare.sh uses exit 1 for errors rather than the GitLab Runner custom executor contract's SYSTEM_FAILURE_EXIT_CODE or BUILD_FAILURE_EXIT_CODE. In practice, GitLab Runner treats non-zero prepare exits as system failures by default, so this works correctly. The sibling run.sh correctly uses the contract exit codes.

  • [race-condition] hack/gitlab-runner-vm/create-vm.sh:144 — Auto-numbering has a TOCTOU race: two concurrent create-vm.sh invocations compute the same next number from oc get vm output, causing a name collision. Mitigated by this being a manual operations tool unlikely to be run concurrently, and by the pre-existing check that errors if the VM already exists.

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:278 — If the GitLab API returns valid JSON lacking the expected token or id keys, the python3 command raises a KeyError. The ERR trap fires and calls cleanup_runner() with an empty runner_id, which tells the user to check for orphaned runners.

  • [edge-case] hack/gitlab-runner-vm/executor/run.sh:32 — The env-file produced for podman exec --env-file includes all CUSTOM_ENV_* variables with the prefix stripped. The env | grep pipeline uses line-delimited parsing, so multiline CI variable values would be split across lines, corrupting the env-file. The script documents this limitation in a comment; GitLab Runner's CUSTOM_ENV_* are single-line in practice.

  • [error-handling] hack/gitlab-runner-vm/setup.sh:127 — In install_gitlab_runner(), the version comparison parses gitlab-runner --version output with awk '{print $2}'. If the output format changes, awk would extract the wrong token, causing an unnecessary but harmless re-install on every run.

  • [edge-case] hack/gitlab-runner-vm/delete-vm.sh:113 — The pagination loop in the runner lookup has no upper bound on page count. If the GitLab API consistently returns exactly 100 runners per page and none match the target description, the loop runs indefinitely. Bounded in practice by the finite number of runners.

  • [container-isolation] hack/gitlab-runner-vm/executor/prepare.sh:44 — Job containers run with --network=host, giving them full access to the host's network stack. While --cap-drop=ALL and --security-opt=no-new-privileges provide strong process-level hardening, and the README documents this as a deliberate design choice for gateway connectivity, a malicious job can access any service listening on the host.

  • [permissions] hack/gitlab-runner-vm/setup.sh:573/etc/gitlab-runner set to mode 755 (world-readable directory listing). The config.toml inside retains its own 0600 permissions from gitlab-runner register. Low risk on a single-tenant VM.

  • [privilege-scope] hack/gitlab-runner-vm/setup.sh:345 — The OCI CA-injection hook runs as root via createRuntime for ALL containers on the host ("always": true), not just CI job containers. The hook only writes CA certificates and includes robust safety checks (40-hop symlink limit, escape detection, atomic write).

  • [code-organization] hack/gitlab-runner-vm/create-vm.sh:67gl_curl() is copy-pasted identically in both create-vm.sh and delete-vm.sh. Both scripts could share a sourced file, similar to gitlab-runner-version.sh.

  • [naming-convention] hack/gitlab-runner-vm/executor/run.sh:37SCRIPT_DIR in run.sh holds the parent directory of the CI build script, while in every other script in this PR it refers to the directory containing the script itself. The semantic collision could cause maintenance confusion.

  • [error-handling-idiom] hack/gitlab-runner-vm/create-vm.sh:92create-vm.sh and delete-vm.sh use raw echo "ERROR:" patterns while setup.sh uses fail()/info()/ok() helpers. Inconsistent error-reporting idiom across scripts in the same directory.

Previous run (8)

Review

Findings

Medium

  • [injection] hack/gitlab-runner-vm/create-vm.sh:188SSH_PUBLIC_KEY is interpolated into the YAML template via Python str.replace(). The prefix validation (^(ssh-|ecdsa-)) does not reject newlines. A crafted SSH_PUBLIC_KEY with embedded newlines could inject additional cloud-init directives (e.g., runcmd commands). The key is typically read from ~/.ssh/*.pub (single-line by SSH format specification) or set directly by the operator, limiting the practical attack surface to self-provisioning scenarios.
    Remediation: Add newline validation: if [[ "${SSH_PUBLIC_KEY}" == *$'\n'* ]]; then echo 'ERROR: SSH_PUBLIC_KEY must not contain newlines' >&2; exit 1; fi

  • [credential-sharing] hack/gitlab-runner-vm/executor/prepare.sh:38 — All concurrent CI jobs receive identical copies of the runner's long-lived gateway mTLS credentials (~/.config/openshell). A malicious job from a trusted project could exfiltrate these credentials and use them to authenticate to the OpenShell gateway independently. The credentials are copied per-job (cp -a to a staging dir) but are the same long-lived material. The README acknowledges this limitation and recommends replacing the mount with short-lived per-job tokens when the gateway supports them.

  • [trust-on-first-use] hack/gitlab-runner-vm/setup.sh:100 — The install_ca_certs function fetches the CA certificate chain directly from the target server via openssl s_client (TOFU model). During initial VM provisioning, an active network attacker could present a rogue CA certificate that would then be permanently trusted by the host and injected into all future job containers via the OCI hook. This is acknowledged in the README and code comments. Consider adding an optional CA_FINGERPRINT environment variable that, when set, verifies the fetched certificate's fingerprint before trusting it.

  • [secrets-exposure] hack/gitlab-runner-vm/setup.sh:193register_runner passes REGISTRATION_TOKEN on the command line via --token, visible in /proc/<pid>/cmdline. gitlab-runner register does not support file-based input for the token. Short-lived token, single-tenant VM.

  • [logic-error] hack/gitlab-runner-vm/setup.sh:612 — In patch_config(), the awk command uses -v block="${custom_block}" where custom_block is a multi-line string containing TOML paths. POSIX awk -v processes C-style escape sequences (\n, \t, \\). If EXECUTOR_DIR, BUILDS_DIR, or CACHE_DIR contain backslash sequences, awk will interpret them as escapes, corrupting the config file. In practice, these are standard Linux home directory paths that never contain backslashes.

Low

  • [error-handling] hack/gitlab-runner-vm/executor/prepare.sh:9prepare.sh uses exit 1 for errors rather than the GitLab Runner custom executor contract's SYSTEM_FAILURE_EXIT_CODE or BUILD_FAILURE_EXIT_CODE. In practice, GitLab Runner treats non-zero prepare exits as system failures by default, so this works correctly. The sibling run.sh correctly uses the contract exit codes.

  • [race-condition] hack/gitlab-runner-vm/create-vm.sh:141 — Auto-numbering has a TOCTOU race: two concurrent create-vm.sh invocations compute the same next number from oc get vm output, causing a name collision. Mitigated by this being a manual operations tool unlikely to be run concurrently, and by the pre-existing check that errors if the VM already exists.

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:274 — If the GitLab API returns valid JSON lacking the expected token or id keys, the python3 command raises a KeyError. The ERR trap fires and calls cleanup_runner() with an empty runner_id, which tells the user to check for orphaned runners.

  • [edge-case] hack/gitlab-runner-vm/executor/run.sh:32 — The env-file produced for podman exec --env-file includes all CUSTOM_ENV_* variables with the prefix stripped. The env | grep pipeline uses line-delimited parsing, so multiline CI variable values would be split across lines, corrupting the env-file. The script documents this limitation in a comment; GitLab Runner's CUSTOM_ENV_* are single-line in practice.

  • [error-handling] hack/gitlab-runner-vm/setup.sh:127 — In install_gitlab_runner(), the version comparison parses gitlab-runner --version output with awk '{print $2}'. If the output format changes, awk would extract the wrong token, causing an unnecessary but harmless re-install on every run.

  • [edge-case] hack/gitlab-runner-vm/delete-vm.sh:113 — The pagination loop in the runner lookup has no upper bound on page count. If the GitLab API consistently returns exactly 100 runners per page and none match the target description, the loop runs indefinitely. Bounded in practice by the finite number of runners.

  • [container-isolation] hack/gitlab-runner-vm/executor/prepare.sh:44 — Job containers run with --network=host, giving them full access to the host's network stack. While --cap-drop=ALL and --security-opt=no-new-privileges provide strong process-level hardening, and the README documents this as a deliberate design choice for gateway connectivity, a malicious job can access any service listening on the host.

  • [permissions] hack/gitlab-runner-vm/setup.sh:573/etc/gitlab-runner set to mode 755 (world-readable directory listing). The config.toml inside retains its own 0600 permissions from gitlab-runner register. Low risk on a single-tenant VM.

  • [privilege-scope] hack/gitlab-runner-vm/setup.sh:345 — The OCI CA-injection hook runs as root via createRuntime for ALL containers on the host ("always": true), not just CI job containers. The hook only writes CA certificates and includes robust safety checks (40-hop symlink limit, escape detection, atomic write).

  • [code-organization] hack/gitlab-runner-vm/create-vm.shgl_curl() is copy-pasted identically in both create-vm.sh and delete-vm.sh. Both scripts could share a sourced file, similar to gitlab-runner-version.sh.

  • [naming-convention] hack/gitlab-runner-vm/executor/run.sh:37SCRIPT_DIR in run.sh holds the parent directory of the CI build script, while in every other script in this PR it refers to the directory containing the script itself. The semantic collision could cause maintenance confusion.

  • [error-handling-idiom] hack/gitlab-runner-vm/create-vm.shcreate-vm.sh and delete-vm.sh use raw echo "ERROR:" patterns while setup.sh uses fail()/info()/ok() helpers. Inconsistent error-reporting idiom across scripts in the same directory.

Previous run (9)

Review

Findings

Medium

  • [credential-sharing] hack/gitlab-runner-vm/executor/prepare.sh:38 — All concurrent CI jobs receive identical copies of the runner's long-lived gateway mTLS credentials (~/.config/openshell). A malicious job from a trusted project could exfiltrate these credentials and use them to authenticate to the OpenShell gateway independently. The credentials are copied per-job (cp -a to a staging dir) but are the same long-lived material. The README acknowledges this limitation and recommends replacing the mount with short-lived per-job tokens when the gateway supports them.

  • [trust-on-first-use] hack/gitlab-runner-vm/setup.sh:120 — The install_ca_certs function fetches the CA certificate chain directly from the target server via openssl s_client (TOFU model). During initial VM provisioning, an active network attacker (MITM) could present a rogue CA certificate that would then be permanently trusted by the host and injected into all future job containers via the OCI hook. This is acknowledged in the README and code comments. Consider adding an optional CA_FINGERPRINT environment variable that, when set, verifies the fetched certificate's fingerprint before trusting it.

Low

  • [error-handling] hack/gitlab-runner-vm/executor/prepare.sh:9prepare.sh uses exit 1 for errors rather than the GitLab Runner custom executor contract's SYSTEM_FAILURE_EXIT_CODE or BUILD_FAILURE_EXIT_CODE. In practice, GitLab Runner treats non-zero prepare exits as system failures by default, so this works correctly. The sibling run.sh correctly uses the contract exit codes.

  • [race-condition] hack/gitlab-runner-vm/create-vm.sh:143 — Auto-numbering has a TOCTOU race: two concurrent create-vm.sh invocations compute the same next number from oc get vm output, causing a name collision. Mitigated by this being a manual operations tool unlikely to be run concurrently, and by the pre-existing check that errors if the VM already exists.

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:275 — If the GitLab API returns valid JSON lacking the expected token or id keys, the python3 command raises a KeyError. The ERR trap fires and calls cleanup_runner() with an empty runner_id, which tells the user to check for orphaned runners. The error message accurately describes the state.

  • [edge-case] hack/gitlab-runner-vm/executor/run.sh:32 — The env-file produced for podman exec --env-file includes all CUSTOM_ENV_* variables with the prefix stripped. The env | grep pipeline uses line-delimited parsing, so multiline CI variable values would be split across lines, corrupting the env-file. The script documents this limitation in a comment; GitLab Runner's CUSTOM_ENV_* are single-line in practice.

  • [secrets-exposure] hack/gitlab-runner-vm/executor/run.sh:29 — The CI environment file containing CUSTOM_ENV_* variables is created via mktemp. On POSIX-compliant systems, mktemp creates files with mode 0600 regardless of umask (mandated by POSIX mkstemp(3)), so the practical risk is minimal. The file is properly cleaned up via trap on EXIT.

  • [container-isolation] hack/gitlab-runner-vm/executor/prepare.sh:44 — Job containers run with --network=host, giving them full access to the host's network stack. While --cap-drop=ALL and --security-opt=no-new-privileges provide strong process-level hardening, and the README documents this as a deliberate design choice for gateway connectivity, a malicious job can access any service listening on the host.

  • [architectural-decision-record-staleness] docs/ADRs/0036-agent-execution-sandbox.md — ADR-0036 describes the container-based sandbox architecture but does not mention the VM-based GitLab Runner deployment variant introduced in this PR. The ADR already covers the architectural decision and acknowledges Podman-based deployments; this PR implements a deployment variant that the README correctly cross-references back to ADR-0036.

Previous run (10)

Review

Findings

Medium

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:271 — The cleanup_runner() hint message tells the user to run NAMESPACE=${NAMESPACE} ./delete-vm.sh ${vm_name} but omits GL_TOKEN and GITLAB_URL, both of which are required by delete-vm.sh and will cause it to exit with an error. The earlier cleanup_vm() correctly includes both variables in its hint.

Low

  • [error-handling] hack/gitlab-runner-vm/executor/prepare.sh:9prepare.sh uses exit 1 for errors and relies on set -euo pipefail for other failures, producing the command's native exit code rather than the GitLab Runner custom executor contract's SYSTEM_FAILURE_EXIT_CODE or BUILD_FAILURE_EXIT_CODE. In practice, GitLab Runner treats non-zero prepare exits as system failures by default, so this works correctly. The sibling run.sh correctly uses the contract exit codes.

  • [race-condition] hack/gitlab-runner-vm/create-vm.sh:143 — Auto-numbering has a TOCTOU race: two concurrent create-vm.sh invocations compute the same next number from oc get vm output, causing a name collision. Mitigated by this being a manual operations tool unlikely to be run concurrently, and by the pre-existing check that errors if the VM already exists.

  • [error-handling] hack/gitlab-runner-vm/create-vm.sh:275 — If the GitLab API returns valid JSON lacking the expected token or id keys, the python3 command raises a KeyError. The ERR trap fires and calls cleanup_runner() with an empty runner_id, which tells the user to check for orphaned runners. The error message accurately describes the state.

  • [edge-case] hack/gitlab-runner-vm/executor/run.sh:32 — The env-file produced for podman exec --env-file includes all CUSTOM_ENV_* variables with the prefix stripped. The env | grep pipeline uses line-delimited parsing, so multiline CI variable values would be split across lines, corrupting the env-file. The script documents this limitation in a comment; GitLab Runner's CUSTOM_ENV_* are single-line in practice.

  • [logic-error] hack/gitlab-runner-vm/setup.sh:150install_gitlab_runner() uses grep with pattern /gitlab-runner-linux-${arch}$ to extract the checksum. The GitLab release.sha256 file uses paths like binaries/gitlab-runner-linux-amd64, where the / in the pattern correctly matches the directory separator. If the format changed to bare filenames, the leading / would prevent a match — but this is fail-safe (clear error message) and the current format works correctly.

  • [credential-scope] hack/gitlab-runner-vm/executor/prepare.sh:39 — All job containers receive read-only access to the runner's gateway mTLS credentials (~/.config/openshell) via per-job staging copies. The underlying credentials are shared across all jobs with the lifetime of the runner. The README documents this trade-off and the planned mitigation (per-job tokens when the gateway supports credential minting). The runner is project-scoped (run_untagged=false).

  • [env-var-forwarding] hack/gitlab-runner-vm/executor/run.sh:29run.sh forwards all CUSTOM_ENV_* environment variables into the job container by stripping the prefix. This follows the GitLab custom executor contract — the runner decides which variables to expose, and the executor forwards them. The env-file is written to /tmp with mktemp and cleaned up via an EXIT trap.

  • [trust-on-first-use] hack/gitlab-runner-vm/setup.sh:97install_ca_certs() uses TOFU to fetch the CA chain directly from the server being connected to. If an attacker can MITM the initial connection during VM provisioning on the internal cluster network, they could inject a rogue CA. The code and README explicitly document this trade-off and the out-of-band alternative.

  • [network-exposure] hack/gitlab-runner-vm/setup.sh:307 — The OpenShell gateway is bound to 0.0.0.0 (all interfaces) rather than 127.0.0.1. This is architecturally required for the Podman compute driver (sandbox containers register via host.containers.internal). mTLS protects the endpoint, and the VM runs inside an OpenShift namespace with masquerade networking.

  • [container-hardening] hack/gitlab-runner-vm/executor/prepare.sh:49 — Job containers are created with --network=host, giving full host network namespace access. Documented as required for reaching the OpenShell gateway. Hardened with --cap-drop=ALL, --security-opt=no-new-privileges, memory/CPU/PID limits, single-tenant VM, and gateway mTLS.

  • [config-permissions] hack/gitlab-runner-vm/setup.sh:574patch_config() sets /etc/gitlab-runner to mode 755. The config.toml file containing the runner token retains its original permissions, but the directory listing is world-readable. Low risk on a single-user VM.

  • [error-message consistency] hack/gitlab-runner-vm/setup.sh:59setup.sh uses fail()/info()/ok() helpers, while create-vm.sh and delete-vm.sh use raw echo "ERROR:". Inconsistent error-reporting idiom across scripts in the same directory.

  • [code-organization] hack/gitlab-runner-vm/delete-vm.sh:32gl_curl() is copy-pasted identically in both create-vm.sh and delete-vm.sh. Both scripts could share a sourced file, similar to gitlab-runner-version.sh.

  • [naming-convention] hack/gitlab-runner-vm/executor/run.sh:37SCRIPT_DIR in executor/run.sh holds the parent directory of the CI build script, while in every other script in this PR it refers to the directory containing the script itself. The semantic collision could cause maintenance confusion.

  • [incomplete-executor-list] docs/ADRs/0036-agent-execution-sandbox.md:174 — ADR-0036 lists supported executors without mentioning the Podman custom executor variant introduced by this PR. The ADR already contemplates Podman deployments in other sections, and the PR's README cross-references ADR-0036.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added component/runner Agent runner behavior and lifecycle component/sandbox OpenShell sandbox environment labels Aug 5, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional review findings (security-focused, deduplicated against existing bot review comments on this PR).

Comment thread hack/gitlab-runner-vm/executor/prepare.sh Outdated
Comment thread hack/gitlab-runner-vm/create-vm.sh Outdated
Comment thread hack/gitlab-runner-vm/executor/prepare.sh
Comment thread hack/gitlab-runner-vm/setup.sh Outdated
Comment thread hack/gitlab-runner-vm/create-vm.sh
Comment thread hack/gitlab-runner-vm/setup.sh
Comment thread hack/gitlab-runner-vm/setup.sh Outdated
@ggallen
ggallen force-pushed the gitlab-runner-vm-setup branch from e7dbc31 to ec18b60 Compare August 5, 2026 21:19
@fullsend-ai-review

ghost commented Aug 5, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:20 PM UTC · Completed 9:36 PM UTC
Commit: ec18b60 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen ggallen changed the title feat: add reproducible GitLab Runner VM setup with OCI CA hook chore: add reproducible GitLab Runner VM setup with OCI CA hook Aug 5, 2026
@ggallen
ggallen force-pushed the gitlab-runner-vm-setup branch from ec18b60 to 37c08ad Compare August 5, 2026 21:42
@fullsend-ai-review

ghost commented Aug 5, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:43 PM UTC · Completed 10:00 PM UTC
Commit: 37c08ad · View workflow run →

ghost left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional review findings (deduplicated against existing bot and human review comments on this PR).

Comment thread hack/gitlab-runner-vm/create-vm.sh
Comment thread hack/gitlab-runner-vm/setup.sh
Comment thread hack/gitlab-runner-vm/create-vm.sh Outdated
Comment thread hack/gitlab-runner-vm/setup.sh Outdated
Comment thread hack/gitlab-runner-vm/setup.sh Outdated
Comment thread hack/gitlab-runner-vm/setup.sh Outdated
Comment thread hack/gitlab-runner-vm/setup.sh
Comment thread hack/gitlab-runner-vm/executor/prepare.sh Outdated
Comment thread hack/gitlab-runner-vm/vm.yaml
Comment thread hack/gitlab-runner-vm/vm.yaml Outdated
fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen
ggallen force-pushed the gitlab-runner-vm-setup branch from 37c08ad to 665a8b1 Compare August 5, 2026 22:17
@fullsend-ai-review

ghost commented Aug 5, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:18 PM UTC · Completed 10:36 PM UTC
Commit: 665a8b1 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen
ggallen force-pushed the gitlab-runner-vm-setup branch from 665a8b1 to d723c2d Compare August 5, 2026 22:39
@fullsend-ai-review

ghost commented Aug 5, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 10:39 PM UTC · Completed 10:57 PM UTC
Commit: d723c2d · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen
ggallen force-pushed the gitlab-runner-vm-setup branch from d723c2d to ca5f6e6 Compare August 5, 2026 23:01

ghost left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review-only pass focused on two issues in the GitLab Runner VM setup: a KubeVirt networking gap that exposes the OpenShell gateway wider than the accepted mitigation assumed, and a CA-bundle path-selection bug that silently no-ops on RHEL/UBI-family images.

One additional finding (the install_ca_hook() comment still referencing rustls-native-certs) was already raised on this exact line in an existing thread, so it was skipped here as a duplicate.

Comment thread hack/gitlab-runner-vm/vm.yaml Outdated
Comment thread hack/gitlab-runner-vm/setup.sh
@fullsend-ai-review

ghost commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:56 PM UTC · Completed 10:17 PM UTC

Commit: b6b1dfc · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

ghost commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:21 PM UTC · Completed 10:36 PM UTC

Commit: 370e584 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

ghost left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review sweep: 5 findings verified against the current head commit (370e584) and deduplicated against the extensive existing review history on this PR (800+ prior review comments across bot and human reviewers). These are distinct, unaddressed issues not previously raised.

Comment thread hack/gitlab-runner-vm/create-vm.sh
Comment thread hack/gitlab-runner-vm/setup.sh Outdated
Comment thread hack/gitlab-runner-vm/create-vm.sh Outdated
Comment thread hack/gitlab-runner-vm/create-vm.sh Outdated
Comment thread hack/gitlab-runner-vm/executor/prepare.sh Outdated
@fullsend-ai-review

ghost commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 5:14 PM UTC · Completed 5:31 PM UTC

Commit: 1f71a39 · View workflow run →

@fullsend-ai-review

ghost commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:39 PM UTC · Completed 5:59 PM UTC

Commit: dcb42e1 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

ghost commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:39 PM UTC · Completed 6:56 PM UTC

Commit: 51f5350 · View workflow run →

@fullsend-ai-review

ghost commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:00 PM UTC · Completed 7:19 PM UTC

Commit: bf6c5aa · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

ghost commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 8:06 PM UTC · Ended 8:17 PM UTC

Commit: a18c3db · View workflow run →

@fullsend-ai-review

ghost commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:18 PM UTC · Completed 8:36 PM UTC

Commit: 07ce176 · View workflow run →

ghost left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at 07ce1766.

Verified the head is byte-identical to the tree I tested locally (shellcheck clean, full pre-commit + gitlint, prepare_validation_test.sh 21/21, merge-tree clean against main), and CI is fully green — test exercises the new fixture via script-test, plus e2e, behaviour, functional-tests, test-sandbox-darwin, commit-lint, DCO.

Composite state reads coherently: your PRE_COMMIT_HOME/GOCACHE envs and the chmod 700 on /etc/gitlab-runner sit cleanly alongside the round-11–13 fixes (job identity from JOB_RESPONSE_FILE, canonicalised bind-mount validation, --env NAME forwarding, single-session .env + terminating signal traps, not_protected default with RUNNER_ACCESS_LEVEL/VM_USER overrides, seeded gateway.toml, non-fail-open gateway registration).

Two follow-ups worth a note, neither blocking:

  • resolve_job_id is duplicated in all three executor stages; a shared executor/job_id.sh would keep future tightening from drifting (needs the copy list / manifest / preflight updated).
  • VM sizing (4c/14Gi) still has no measured baseline — a podman stats / free -m sample during one real agent job would settle it and whether per-container caps are needed.

Squash the two commits into one whenever you like — nothing here depends on the split.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

ghost commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:50 PM UTC · Completed 9:09 PM UTC

Commit: d03596f · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

ghost commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 9:15 PM UTC · Ended 9:16 PM UTC

Commit: 5a6c694 · View workflow run →

@fullsend-ai-review

ghost commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:17 PM UTC · Completed 9:34 PM UTC

Commit: f839383 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Adds hack/gitlab-runner-vm/ with scripts to create, configure, and
delete GitLab Runner VMs on OpenShift Virtualization with Podman custom
executor and OpenShell gateway integration.

Includes hardened executor path handling, SHA256 verification of
transferred files, preflight checks, and a validation test suite.

Signed-off-by: Greg Allen <gallen@redhat.com>
Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
@fullsend-ai-review

ghost commented Aug 17, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:40 PM UTC · Completed 9:57 PM UTC

Commit: e9dd531 · View workflow run →

Comment thread hack/gitlab-runner-vm/create-vm.sh
Comment thread hack/gitlab-runner-vm/setup.sh
Comment thread hack/gitlab-runner-vm/create-vm.sh
Comment thread hack/gitlab-runner-vm/setup.sh
Comment thread hack/gitlab-runner-vm/delete-vm.sh
Comment thread hack/gitlab-runner-vm/executor/prepare.sh
Comment thread hack/gitlab-runner-vm/setup.sh
Comment thread hack/gitlab-runner-vm/setup.sh
Comment thread hack/gitlab-runner-vm/executor/prepare.sh
Comment thread hack/gitlab-runner-vm/create-vm.sh
@fullsend-ai-retro

ghost commented Aug 18, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 12:06 AM UTC · Completed 12:22 AM UTC

Commit: e9dd531 · View workflow run →

@fullsend-ai-retro

ghost commented Aug 18, 2026

Copy link
Copy Markdown

Retro: PR #5951 — GitLab Runner VM setup with OCI CA hook

PR: #5951chore: add reproducible GitLab Runner VM setup with OCI CA hook
Author: ggallen (human-authored) | Merged: 2026-08-18 | +2,077 lines across 12 files (shell scripts, YAML, executor scripts)

Workflow summary

This PR added infrastructure provisioning scripts under hack/gitlab-runner-vm/. The review agent ran 34 review dispatches (24 success, 2 failure, 8 cancelled) producing 424 inline comments across 41 review cycles. Human reviewer waynesun09 provided 3 rounds of detailed security-focused review with 39 inline comments.

Key finding: significant review quality gap

The review agent and human reviewer had starkly different outcomes:

Metric Review agent Human reviewer
CRITICAL findings 0 7
HIGH findings 1 ~14
Unique MEDIUM findings ~8 ~50+
LOW findings (repeated) ~12 (repeated 30-47x each) ~15

The human reviewer's most impactful findings came from three capabilities the review agent lacks:

  1. Compositional security reasoning — The bot noted --network=host (low) and gateway credential mount (low) separately. The human recognized their combination as CRITICAL: host-networked containers with mTLS credentials expose those credentials to attacker-influenced CI jobs.

  2. Cross-file data flow tracing — The human traced gateway credentials from setup.shprepare.sh → job containers, identifying the end-to-end exposure vector. The bot reviewed each file independently.

  3. End-to-end execution verification — The human found 4+ cases where code always fails (grep pattern doesn't match real release.sha256 format, --tag-list is fatal with glrt-* tokens, setup.sh depends on files never copied to VM). The bot assumed all external integrations work as claimed.

Failed runs

Two review.yml failures (runs 32049382924, 32050298888) were caused by a GitHub API 503 outage during post-review.sh. The review agent itself completed successfully in both cases.

Evidence for existing issues (not filed as new proposals)

  • Comment deduplication (agents#721, fullsend#5007, fullsend#5139, fullsend#5760): This PR is one of the most extreme cases — 424 inline comments with the top 7 categories each appearing 30-47 times verbatim. The same [low] secrets-handling, [low] edge-case, [low] error-handling findings were re-posted on every review cycle.

  • Per-PR review budget (fullsend#2599, fullsend#2587): 34 review dispatches at ~18-19 minutes each = ~11 hours of compute on a single PR, producing overwhelmingly repetitive low-severity findings.

  • ADR cross-referencing (agents#734, agents#638): The human's most impactful findings came from verifying implementation claims against ADRs 0017, 0036, and 0067. The bot never searched for or referenced any ADR.

  • Incorporating human reviews (agents#447): waynesun09's detailed security review (Aug 5-12) was never incorporated into the bot's subsequent re-reviews (Aug 17). The bot kept posting the same low-severity findings while ignoring the human's critical findings.

Proposals filed

Three new proposals target the agents repo (fullsend-ai/agents) to address the structural review quality gaps demonstrated by this PR.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/ci CI pipelines and checks component/runner Agent runner behavior and lifecycle component/sandbox OpenShell sandbox environment ready-for-merge All reviewers approved — ready to merge type/chore Maintenance and housekeeping tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants