Repository navigation
feat(OMN-14051): reject non-hermetic check_value in contract-compliance validator - #4426
Conversation
…ce validator
Fold a non-hermetic check_value guard into the existing contract-compliance
engine (onex_change_control.scripts.contract_compliance_check) so an ssh /
live-docker / network-egress / LAN-Tailscale-IP command check_value is rejected
up front with an actionable message steering the author to the receipt-grep
pattern, instead of surfacing as a late, cryptic CI BLOCK ('sh: 1: ssh: not
found', exit 127). No new gate/workflow (net-negative-surface); the guard reuses
the OMN-14436 demotion path so grandfathered legacy contracts are reported (WARN)
not wedged, while new/touched contracts are enforced.
Observed on OCC PR #3642 (OMN-14001).
- _non_hermetic_reason() + quote-aware _command_binaries() command-position scan
- guard called at top of _run_single_check(); _demote() still applies ratchet
- re-exported via the thin scripts/ci wrapper
- 20 new focused tests (reject ssh/docker/LAN-IP/scp/rsync/sudo-wrapped; accept
receipt-grep/local-file/loopback-curl/quoted-docker-word; new-ticket BLOCK vs
grandfathered WARN integration)
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 31 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 (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
OMN-14051 — OCC contract compliance: reject non-hermetic check_value at authoring time
Evidence-Ticket: OMN-14051
Evidence-Source: OCC#4427
Evidence-Commit: 489887a
Closes OMN-14051.
Problem
An OCC contract's
dod_evidence[].checks[].check_valueis executed verbatim by the contract-compliance runner in CI. When an author inlines a non-hermetic command (ssh / live docker / network egress / LAN-Tailscale IP), it can never pass in CI and BLOCKs the PR with a cryptic runtime error (sh: 1: ssh: not found, exit 127) even when the underlying work is real. Observed on OCC PR #3642 (OMN-14001): adod-deploy-scopeitem inlinedssh jonah@100.109.203.94 "docker ps ..."→ "3/4 PASS, 1 BLOCK". The failure surfaced late (in CI) instead of at validation time.Change (net-negative-surface — NO new gate/workflow)
Folded a non-hermetic
check_valueguard into the existing validator (onex_change_control.scripts.contract_compliance_check):_non_hermetic_reason(check)— inspects onlycheck_type: command(the executed-shell surface) and returns an actionable rejection message when the command, in command position, invokes a remote-shell binary (ssh/scp/sftp/rsync/telnet), a live container/orchestration daemon (docker/podman/kubectl/…), network egress to a non-loopback host (curl/wget/ncto a non-localhost/127.*URL), or references a private/Tailscale-CGNAT IPv4 literal (192.168.*,10.*,172.16–31.*,100.64–127.*).shlex(_command_binaries): the word "docker" inside a quoted grep pattern is not flagged — only real command-position invocations are;sudo/env FOO=barwrappers are seen through._run_single_checkand returns aBLOCK; the existing OMN-14436_demotepath still applies, so a grandfathered (content-pinned legacy) contract is demotedBLOCK → WARN(reported, not wedged) while a new/touched contract is enforced from its first PR. This keeps the pre-existing corpus ofdocker execruntime-proof checks (25 command-position instances) from wedging.check_valueand steers the author to the canonical receipt-preservation pattern (drift/dod_receipts/<TICKET>/<evidence-id>/command.yamlwithprobe_command/probe_stdout/exit_code/status: PASS, thencheck_value: grep -q '^status: PASS$' "$CONTRACT_REPO_DIR/…/command.yaml").scripts/ci/run_contract_compliance_check.pywrapper.Conservative by design (per ticket): known gap — a binary hidden inside a command substitution (
$(ssh …)) is not yet detected; documented inline for a later expansion.Acceptance criteria mapping
_non_hermetic_reasonreturnsNone).BLOCKvs grandfatheredWARNintegration.dod_evidence
src/onex_change_control/scripts/contract_compliance_check.py—_non_hermetic_reason+_command_binaries+ guard wired into_run_single_check.tests/test_contract_compliance_check.py— 20 new tests. Local:uv run pytest tests/test_contract_compliance_check.py tests/test_dod_runner_ratchet.py tests/test_ci_script_exists.py -q→ 66 passed.ruff formatclean,ruff checkclean,mypy --strictclean (module + wrapper + tests),pre-commiton changed files all Pass/Skip, pre-push mypy Pass.OCC companion
OCC evidence companion is owed to Codex (net-new-file-only; machine-authored per OMN-14055 — implementer does not self-author their own companion). Not authored in this PR.