fix(#7218): ephemeral per-job OpenShell gateway on GitLab runners - #7244
Conversation
GitLab runner VMs kept one long-lived systemd --user gateway across
every job. The registry went stale (profile import is a no-op on
"already exists"), the OpenShell version drifted from the repo pin,
and leaked sandboxes pinned old policy. GitHub Actions does not have
this: each job installs a fresh, version-matched gateway.
prepare.sh now reaps leftover gateway/sandbox containers, installs
the OpenShell version the job image pins, and starts a gateway
against a wiped store (~/.local/state/openshell/{gateway,tls}).
cleanup.sh tears it down. setup.sh no longer enables the unit.
ImportProfileVerified applies the OpenAI ForgetProfileCache ->
import -> ProfileExists -> retry pattern to the generic/Vertex
import path so a stale os.TempDir() cache cannot skip the send
against a fresh gateway.
Closes #7218
|
🤖 Finished Review · ✅ Success · Started 7:32 PM UTC · Completed 7:58 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $9.23 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Looks good to me Previous runReviewFindingsMedium
Next steps:
Previous run (2)ReviewFindingsHigh
Medium
Low
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 8:00 PM UTC · Completed 8:15 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $2.60 |
Address the security-review findings on the ephemeral OpenShell gateway: - gateway.sh: only install the OpenShell version a job image reports when it matches the Renovate-tracked pin (.github/scripts/openshell-version.sh), and fetch install.sh from that pin's commit SHA instead of a job-supplied release tag. A job could otherwise steer the host installer to an arbitrary/retagged/older NVIDIA/OpenShell release, bypassing the commit-SHA allowlist install-openshell.sh already enforces (High). - gateway.sh: harden the podman run that probes a job image's OpenShell version with the same flags prepare.sh applies to the real job container (--cap-drop=ALL, --security-opt=no-new-privileges, --pids-limit), and distinguish a hard podman error from a clean "image has no CLI" run instead of silently keeping the host version on either (Medium x2). - gateway.sh: stop before disabling the gateway unit after install, and fail if disable does not succeed, instead of swallowing the failure (Low). - setup.sh: wait for the seed gateway start to become active (reusing wait_for_openshell_gateway) and fail setup() if it does not, instead of a verify() check that is trivially true either way (Low). - sandbox.go: ImportProfileVerified's retry path now mirrors the first ProfileExists check - a listing error is wrapped and returned directly instead of being folded into a generic "not on the gateway" message (Low). Addresses #7244
🔧 Fix agent — iteration 1 (bot-triggered)Addressed all 6 review findings on PR #7244 (1 High, 2 Medium, 3 Low), all in hack/gitlab-runner-vm/executor/gateway.sh, hack/gitlab-runner-vm/setup.sh, and internal/sandbox/sandbox.go. The core fix ties job-image OpenShell installs to the Renovate-tracked commit-SHA pin instead of trusting a job-supplied version tag, hardens the version-probe podman run, distinguishes hard probe errors from a legitimate no-CLI image, makes the gateway-unit disable after install fail-closed, makes setup.sh's seed gateway start an explicit smoke test, and fixes an error-wrapping bug in ImportProfileVerified's retry path. Updated/added shell tests in gateway_test.sh and a Go test in import_verified_test.go to cover the new behavior; all existing gateway_test.sh, lib_test.sh, prepare_validation_test.sh, and the relevant Go test suites (internal/sandbox, internal/cli) pass. Fixed (6):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 8:17 PM UTC · Completed 8:35 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $4.59 |
|
Risk Assessment: moderate (2/5) DetailsSizeable but non-protected, non-security-sensitive, dependency-free change to a high-churn CLI/sandbox area with modest test coverage, an unflagged behavioral change to existing gateway/import logic, and an issue whose acceptance criteria are claimed but not independently confirmed by the issue checkboxes -- standard careful review is warranted but nothing crosses into elevated-risk territory. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Fix · ✅ Success · Started 8:37 PM UTC · Completed 8:47 PM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $1.69 |
Addresses review feedback on PR #7244 (logic-error, gateway.sh:24). gateway.sh's two relative guesses for .github/scripts/openshell-version.sh (VM source-tree layout and repo checkout layout) never match the layout prepare.sh/cleanup.sh actually source it from at per-job runtime: setup.sh's install_executor flattens job_id.sh/prepare.sh/run.sh/cleanup.sh/gateway.sh into EXECUTOR_DIR with no .github/scripts sibling. OPENSHELL_VERSION/ OPENSHELL_SHA end up unset there, so ensure_job_openshell_gateway's version- mismatch branch fails every job whose image reports a different OpenShell version than the host, exactly the version-drift scenario #7218 was filed to fix. - install_executor now also copies openshell-version.sh into EXECUTOR_DIR/.github/scripts/, alongside the flattened scripts. - gateway.sh tries that flattened-layout path first, before falling back to the VM and repo-checkout guesses. - Added a regression test that sources gateway.sh from an isolated temp dir laid out like the flattened EXECUTOR_DIR (no .github/scripts reachable via the other two guesses) and asserts the pin resolves from that layout specifically, using sentinel values so the assertion can't pass via environment leakage from the test's own earlier sourcing of gateway.sh. Addresses #7244
🔧 Fix agent — iteration 2 (bot-triggered)Fixed the one active Medium finding: gateway.sh now finds openshell-version.sh from the flattened EXECUTOR_DIR layout that prepare.sh/cleanup.sh actually run from in production, via install_executor shipping the pin file alongside the flattened scripts and a new first-priority relative guess in gateway.sh. Added a regression test reproducing the flattened layout in isolation. All findings from the prior review iteration (inside the Details'Previous run' block) were already addressed in an earlier commit and were not re-litigated per the fix-review procedure.Fixed (1):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 8:49 PM UTC · Completed 9:01 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $2.59 |
Superseded by updated review
fullsend-ai-review
Bot
added
the
ready-for-merge
|
🤖 Finished Retro · ✅ Success · Started 2:31 AM UTC · Completed 2:44 AM UTC Commit: Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $2.31 |
|
PR #7244 (ephemeral per-job OpenShell gateway on GitLab runners, closing #7218) is a healthy example of the pipeline working as designed. The code agent correctly diagnosed a subtle root cause (long-lived gateway + no-op profile import + version drift) and the automated review agent caught a real High-severity security issue in round 1 (installer piping a job-controlled version tag into Proposals filed
|
2 participants
Summary
GitLab runner VMs no longer keep a long-lived OpenShell gateway. Each job now gets a fresh, version-matched gateway with an empty profile registry (GitHub Actions parity), and URL-resolved profile import no longer trusts a stale
os.TempDir()cache against that empty registry.Related Issue
Addresses the silent
policy_deniedfailure from a stale_provider_vertex_aiprofile (job 60629238):openshell provider profile importis a no-op on "already exists", so a gateway that outlived its pin never picked up**/claude.exe.Changes
prepare.shreaps leftoveropenshell.managed/openshell-*containers, installs the OpenShell version from the job image when it differs from the host, and starts the systemd user unit against a wiped~/.local/state/openshell/{gateway,tls}store.cleanup.shalways tears the gateway down (even if the job id is missing).setup.shno longer enables the unit, so a reboot cannot resurrect a long-lived daemon.sandbox.ImportProfileVerified(ForgetProfileCache → import →ProfileExists→ retry) is used by the generic/Vertex import path inrun.goand byensureOpenAIProfile.executor/gateway.sh(copied by bothcreate-*-vm.shpaths). Existing VMs pick this up by re-runningsetup.sh.Testing
go testforImportProfileVerified(stale-cache-vs-empty-gateway, retry success, import/list errors) andTestEnsureOpenAIProfile*/TestRunAgent_*ininternal/clihack/gitlab-runner-vm/executor/gateway_test.sh(wipe, orphan reaping, nosystemctl enable, version-mismatch install from the release tag)prepare_validation_test.shandlib_test.shChecklist
fix(#7218): …)Closes #7218
Post-script verification
agent/7218-ephemeral-openshell-gateway)773149d0e4f04f7ee5d831acbcf3b083baf9aa2b..HEAD)