Repository navigation
fix(OMN-15134): self-heal workspace-reset hook against root-owned debris - #2448
Conversation
Root-owned .venv artifacts under the omninode-deploy-runner Actions job workspace jammed the built-in job-started cleanup hook (run 30171892373), blocking the OMN-14900 stability deploy hop before any workflow step ran. Root cause (verified live, not inferred): this image's ENTRYPOINT legitimately starts as root (docker-socket GID fix) before gosu-dropping to `runner` for every job step -- so a bare `docker exec omninode-deploy-runner ...` without `-u runner` (a same-session `uv run` verification probe cited in PR #2446's body, mistakenly described as read-only) silently ran as root and left a root-owned `.venv` (confirmed: `/root/.cache/uv` populated with a matching timestamp) that the unprivileged workspace-reset hook could not `rm -rf`. Fix: harden runner-job-started.sh to fail loud (name the offending paths, never silently succeed or hang) then self-heal via a narrowly scoped NOPASSWD sudo rule (Dockerfile, one command + one argument pattern, confined to this runner's own `_work` tree -- never a general root shell). Rejected alternatives: (a) flip the image's default USER to `runner` -- rejected, the ENTRYPOINT's root-phase init (docker-socket GID fix, OMNI_HOME chown, operator-env copy) genuinely needs root before it gosu-drops, so this would either break that init or not change docker exec's default identity (a compose-level `user:` override still shows up as the container's effective Config.User); (c) forbid `.venv` in the bind-mounted job workspace -- N/A here, the workspace is NOT bind-mounted from host (verified via a write-visibility probe), so this was never a DooD path-mismatch bug. Verified live on omninode-deploy-runner (ssh omni-201-ts): cleaned the 18 confirmed root-owned paths + /root/.cache/uv (count_after=0); reproduced the exact RED condition (simulated root-owned debris) against the real Ubuntu 22.04 GNU-realpath container and confirmed the hardened hook fails loud with the offending path + a clear "sudo not installed" diagnostic (pre-rebuild, no sudoers rule yet) and exits non-zero with a manual-remediation instruction -- never silently wedging or succeeding. Added test_runner_job_started_root_owned_debris.py (happy path + undeletable-debris fail-loud + pre-existing path-confinement regression guard); skips on non-GNU-realpath hosts (this repo's macOS gate host), runs for real on Linux CI. Gates run on .200 (patch-transfer + sha256 verify): ruff format/check, mypy, full pre-commit (shellcheck included) -- all clean. Refs: OMN-15134, OMN-14900 (parent stability deploy hop), OMN-15131/PR #2446 (the same-session probe that produced the debris). Evidence-Ticket: OMN-15134
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
| Verdict | Meaning | Blocks merge? |
|---|---|---|
passed |
No critical findings | No |
blocked |
CRITICAL findings found | Yes |
degraded |
All models unavailable (infra) | No (pilot) |
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8468/OMN-8524)
…ibase_infra#2448 (#4865) * evidence(OMN-15134): author OCC companion for OmniNode-ai/omnibase_infra#2448 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 3d64b29e1ee9915739429467d2e4de5ffb812b95. * evidence(OMN-15134): self-bind OCC#4865 + rebind contract_sha256 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
The docker-compose.runners.yml GIT_CONFIG_* change from the previous commit is reverted: it is a "runtime path" per deploy-gate, which correctly blocked the PR (DEPLOY GATE FAILED: no cited ticket has a falsifiable deploy probe) since this container-level env only takes effect on a deploy-runner container recreate -- out of scope for a code-only PR, same split as OMN-15134/#2448 (land now, deploy separately). Kept: the test file correction (_ENSURE_REPOS now has 6 entries including omnibase_spi, matching ensure_runner_clones.sh's real behavior -- this part is NOT deploy-gated, it is test-only and was a genuine CI regression introduced by the sibling_clone_manifest.sh fix). Reverted the compose-specific GIT_CONFIG_COUNT/KEY/VALUE assertions in that same test file back to 5, with a note that the container-level omnibase_spi safe.directory gap is a tracked, non-blocking residual (the load-bearing per-op `-c safe.directory=` scoping in the scripts already covers it). Verified (.200): full regression set (unit/scripts + scripts/) 87/87 passed; pre-commit (scoped) all hooks passed. Evidence-Ticket: OMN-15137
…ifest (#2450) * fix(OMN-15137): provision omnibase_spi via a shared sibling-clone manifest ensure_runner_clones.sh never provisioned an omnibase_spi clone under the deploy runner private OMNI_HOME, even though stage_workspace.sh sibling-pin preflight (PREFLIGHT_REPO_ARGS -> check_sibling_lock_pins.py's DEFAULT_PACKAGE_REPO_DIRS) has required a live OMNI_HOME/omnibase_spi clone since OMN-12977. The two lists were independently hardcoded 5-repo and 6-repo bash arrays that silently drifted apart. This blocked OMN-14900 stability deploy-hop re-fire iteration N+3 3 steps downstream of clone provisioning: ERROR: cannot resolve clone pin for omnibase-spi: missing pyproject.toml for omnibase-spi: /data/omninode/runner_omni_home/omnibase_spi/pyproject.toml Fix targets the ROOT pattern, not just the missing repo: adds scripts/runtime_build/sibling_clone_manifest.sh as the single source of truth for the full 6-repo sibling-clone set (omnibase_infra, omnibase_core, omnibase_spi, omnibase_compat, onex_change_control, omnimarket). Both ensure_runner_clones.sh (RUNNER_CLONE_REPOS) and stage_workspace.sh (PREFLIGHT_REPO_ARGS) now source this one file instead of hardcoding their own copy, so the two can never diverge again. ensure_runner_clones.sh's existing existence/operability loop is the fail-fast preflight the ticket asks for -- once fed the complete manifest it already names any missing clone before the deploy proceeds, with no new logic required. Audited full sibling set consumed by the workspace build (Dockerfile.runtime + stage_workspace.sh): omnibase_infra is staged as the Docker build context itself (never a vendored "sibling"); omnibase_spi is installed from the published PyPI wheel via `uv sync` (never staged from local source) but still needs a live OMNI_HOME clone so the pin-preflight can read its pyproject.toml version + git SHA; omnibase_core/omnibase_compat/ onex_change_control/omnimarket are vendored as source via stage_workspace.sh's SIBLING_REPOS. No 7th sibling exists in any lock file (check_sibling_lock_pins.py's DEFAULT_PACKAGE_REPO_DIRS has exactly these 6 entries). Tests (new): - tests/scripts/test_ensure_runner_clones.py: end-to-end against real file:// git remotes -- proves all 6 repos including omnibase_spi are cloned, idempotent re-run, fail-closed + named-repo error on missing upstream, fail-closed on an unwritable clone .git dir, fail-closed on missing OMNI_HOME. Confirmed RED against the pre-fix 5-repo manifest (7/8 assertions fail without omnibase_spi in the list). - tests/scripts/test_sibling_clone_manifest_parity.py: cross-language recurrence ratchet asserting sibling_clone_manifest.sh's bash arrays exactly match check_sibling_lock_pins.py's DEFAULT_PACKAGE_REPO_DIRS, so a future 7th sibling added on one side and forgotten on the other fails CI immediately instead of failing 3 deploy hops deep on a real runner. Verification (.200, env -u PYTHONPATH uv run pytest, homebrew bash 5.3 on PATH -- macOS system /bin/bash 3.2 has a pre-existing, unrelated nounset empty-array bug in stage_workspace.sh line ~204 that reproduces identically on unmodified dev tip; out of scope for this ticket): - New files: 8/8 passed. - Full regression set (new + existing stage_workspace/check_sibling_lock_pins coverage): 67/67 passed. - ruff format --check + ruff check: clean. - shellcheck --severity=warning (all 3 shell files): clean. - bash -n: syntax OK. - pre-commit (scoped to changed files): all hooks passed, including SPDX. dod_evidence: - Ticket: OMN-15137 (parent OMN-14900). - Root cause reproduced live, per ticket run https://github.com/OmniNode-ai/omnibase_infra/actions/runs/30175447119. - Acceptance per ticket: a fresh release-train-lab.yml stability-lane execute=true run reaching past check_sibling_lock_pins.py without an omnibase-spi clone-pin resolution error. This PR does not itself trigger that run (requires a fresh cut/deploy hop, out of scope for this PR); a RED-confirmed test now proves the fix and the next stability deploy hop is the live acceptance readback. Not merging this PR myself -- Codex owns merge queue / merge per repo policy. Evidence-Ticket: OMN-15137 * chore(OMN-15137): retrigger CI to pick up Evidence-Source autobind * fix(OMN-15137): close the same drift in compose GIT_CONFIG_* + its test docker-compose.runners.yml GIT_CONFIG_COUNT/GIT_CONFIG_KEY_*/VALUE_* (the container-level defense-in-depth safe.directory list for the deploy runner) was ALSO hardcoded to 5 entries, missing omnibase_spi -- the same independently-hardcoded-list drift this ticket fixes elsewhere. Bumped to 6 and added the omnibase_spi entry. tests/unit/scripts/test_runner_private_omni_home_safe_directory.py (pre-existing OMN-14900 coverage) asserted the old 5-repo shape in three places and its own file:// bare-fixture fixture only seeded 5 upstream repos -- this is what CI Tests (Split 1/1) caught on PR #2450 after the sibling_clone_manifest.sh fix made ensure_runner_clones.sh try to clone a 6th repo the test fixture never provided. Updated _ENSURE_REPOS to 6 (added omnibase_spi), the GIT_CONFIG_COUNT/KEY/VALUE assertions to 6, and renamed the now-inaccurate test_..._all_five_... to test_..._all_and_is_idempotent. Verified (.200): tests/unit/scripts/test_runner_private_omni_home_safe_directory.py 20/20 passed; full regression set (with the two new OMN-15137 test files) 87/87 passed; pre-commit (scoped) all hooks passed. Evidence-Ticket: OMN-15137 * fix(OMN-15137): revert compose GIT_CONFIG_* change, keep it out of scope The docker-compose.runners.yml GIT_CONFIG_* change from the previous commit is reverted: it is a "runtime path" per deploy-gate, which correctly blocked the PR (DEPLOY GATE FAILED: no cited ticket has a falsifiable deploy probe) since this container-level env only takes effect on a deploy-runner container recreate -- out of scope for a code-only PR, same split as OMN-15134/#2448 (land now, deploy separately). Kept: the test file correction (_ENSURE_REPOS now has 6 entries including omnibase_spi, matching ensure_runner_clones.sh's real behavior -- this part is NOT deploy-gated, it is test-only and was a genuine CI regression introduced by the sibling_clone_manifest.sh fix). Reverted the compose-specific GIT_CONFIG_COUNT/KEY/VALUE assertions in that same test file back to 5, with a note that the container-level omnibase_spi safe.directory gap is a tracked, non-blocking residual (the load-bearing per-op `-c safe.directory=` scoping in the scripts already covers it). Verified (.200): full regression set (unit/scripts + scripts/) 87/87 passed; pre-commit (scoped) all hooks passed. Evidence-Ticket: OMN-15137 --------- Co-authored-by: jonahgabriel <jonahgabriel@users.noreply.github.com>
…t catalog, T0/T1 readback, dev gotchas, prod pointer (#2454) Extends docs/runbooks/release-train-lab.md (existing mechanism doc, not a new file) with the operator-facing procedure proven live on run 30180376657 (2026-07-26, tag lab/stability/20260725T235956Z-87ec5b3165ce, overall: PASS) — the terminal success of the six-iteration OMN-14900 hardening chain. - Copy-paste tag-cut + watch procedure with expected output and a tag-content WARNING (never reuse a parked tag name). - Preflight catalog: what each of the 6 landed fixes (#2450/#2446/#2444/ #2452/#2434/#2448) catches, with pre-fix failure signatures. - T0/T1 readback discipline: health 18085/18086, contract-count floor vs. regression, discovery_errors baseline, consumer groups, vcs_ref ancestry; FAILED_ROLLED_BACK equality-proof discipline. - Dev-lane gotchas: stale workspace-build config YAML, OMN-14968 false FAILED on runtime-worker; pointer (not duplicate) to cold-lane-full-bringup.md for cold bring-up. - Prod: pointer-only section citing CLAUDE.md rules 2a/12 and the onex_change_control#4892 prep-only grant-PR pattern; explicit raw-docker-mutation prohibition citing the no-raw-prod-bypass gate. - Verification checklist + rollback section. Co-authored-by: jonahgabriel <jonahgabriel@users.noreply.github.com>
Summary
Fixes OMN-15134: root-owned
.venvartifacts under theomninode-deploy-runnerActions job workspace jammed the built-in "Set up runner" job-started cleanup hook (run 30171892373), blocking the OMN-14900 stability deploy hop before any workflow-defined step ran:Root cause (verified live, not inferred)
This image's
ENTRYPOINTlegitimately starts as root (docker-socket GID fix,OMNI_HOMEchown, operator-env copy) beforegosu-dropping torunnerfor every job step — so a baredocker exec omninode-deploy-runner ...without-u runnersilently inherits root. PR #2446's own body cites the exact command that did this, describing it as a "read-only" verification probe:uv runis not read-only — it transparently runsuv syncwhen no.venvexists, creating one. Run via a baredocker exec(no-u runner), this created the.venvas root. Confirmed live:.venv, all timestamped18:56/root/.cache/uv/interpreter-v4/...msgpackpopulated (root's own uv cache — direct proofuvexecuted withHOME=/root)The workspace-reset hook itself (
runner-job-started.sh) correctly runs as the unprivilegedrunneruser (same identity every job step executes as) and correctly could not remove root-owned files — it just gave no diagnostic and no recovery path, wedging every subsequent job on this single-runner label until an operator hand-ran a rootdocker exec rm -rf.Fix
Harden
runner-job-started.shto fail loud, then self-heal:rm -rf(unchanged happy path).rmerror + afind -not -user $(id -u)listing of the offending paths.sudo -n rm -rffallback — one command, one argument pattern (/bin/rm -rf -- <path under this runner's own _work tree>), added as a NOPASSWD sudoers rule in the Dockerfile. Never a general root shell.Rejected alternatives
USERtorunner— rejected. TheENTRYPOINT's root-phase init (docker-socket GID fix,OMNI_HOMEchown, operator-env copy) genuinely needs root before it drops privilege viagosu. Making the image default non-root either breaks that init, or (if compose'suser: rootoverrides it back for the compose-managed lifecycle) doesn't change whatdocker execdefaults to anyway —docker inspect --format '{{.Config.User}}'reflects the container's effective configured user regardless of whether it came from the image default or a runtime override, so a split default wouldn't have closed the ad hocdocker execvector that actually produced this incident..venvin the bind-mounted job workspace — N/A. Verified live: this job workspace is not bind-mounted from host at all (see write-visibility probe above), so there's no host-path mismatch to fix here.Verification (live,
.201, read-only-then-scoped-mutation per standing rule)/root/.cache/uvon the liveomninode-deploy-runnercontainer (count_after=0, re-verified viafind -user root).docker exec -u root) against the real Ubuntu 22.04 / GNU-realpath environment — confirmed the hook fails loud with the offending path and a clear "sudo not installed" diagnostic (pre-image-rebuild, no sudoers rule live yet), and exits non-zero with the manual-remediation instruction. Confirmed the happy path (workspace owned byrunner) still resets clean.tests/scripts/test_runner_job_started_root_owned_debris.py: happy path, undeletable-debris fail-loud + fallback-unavailable path, and the pre-existing path-confinement regression guard. Skips on non-GNU-realpathhosts (this repo's macOS.200gate host); runs for real on Linux CI.Gates run on
.200(patch-transfer + sha256 verify, never edited in place):ruff format --check,ruff check,mypy, fullpre-commit run(shellcheck included) — all clean.Follow-up (tracked separately, not in this PR)
The image itself still needs rebuilding + the running container recreating to pick up the sudoers rule — tracked as the deploy step for this same ticket (OMN-15134), performed after this PR merges.
Refs: OMN-15134, OMN-14900 (parent stability deploy hop), OMN-15131 / PR #2446 (the same-session probe that produced the debris)
Evidence-Ticket: OMN-15134
Evidence-Source: OCC#4865
Evidence-Commit: aa73c9a6f5129ec1d768289d82877ed33e392974