diff --git a/contracts/OMN-14051.yaml b/contracts/OMN-14051.yaml index 5a67841695f..e5e2acf49ff 100644 --- a/contracts/OMN-14051.yaml +++ b/contracts/OMN-14051.yaml @@ -65,3 +65,15 @@ dod_evidence: checks: - check_type: command check_value: gh pr view ${PR_NUMBER} --repo ${REPO} --json number,state,headRefOid + - id: dod-pr4426-head-contains-5120e3fa + description: 'Add-only supersession for PR #4426 after OCC#4427 evidence was merged into the branch. + The PR head moved from 5120e3fa4f056d6ee51fe16589ebf0fc641d7c45, but it still contains the validated + contract-compliance validator fix commit 5120e3fa4f056d6ee51fe16589ebf0fc641d7c45; later evidence-only + commits need not predict their own SHA.' + source: generated + evidence_artifact: supersedes_dod_evidence:dod-onex_change_control-pr-4426 + checks: + - check_type: command + check_value: 'PRODUCT_PR_NUMBER=4426; PRODUCT_REPO=OmniNode-ai/onex_change_control; PRODUCT_FIX_COMMIT=5120e3fa4f056d6ee51fe16589ebf0fc641d7c45; + : ${PR_NUMBER}; git merge-base --is-ancestor "$PRODUCT_FIX_COMMIT" HEAD && test "$(gh pr view + $PRODUCT_PR_NUMBER --repo $PRODUCT_REPO --json state --jq .state)" = "OPEN"' diff --git a/drift/dod_receipts/OMN-14051/dod-pr4426-head-contains-5120e3fa/command.yaml b/drift/dod_receipts/OMN-14051/dod-pr4426-head-contains-5120e3fa/command.yaml new file mode 100644 index 00000000000..8793fc6fb89 --- /dev/null +++ b/drift/dod_receipts/OMN-14051/dod-pr4426-head-contains-5120e3fa/command.yaml @@ -0,0 +1,22 @@ +--- +schema_version: 1.0.0 +ticket_id: OMN-14051 +evidence_item_id: dod-pr4426-head-contains-5120e3fa +check_type: command +check_value: 'PRODUCT_PR_NUMBER=4426; PRODUCT_REPO=OmniNode-ai/onex_change_control; PRODUCT_FIX_COMMIT=5120e3fa4f056d6ee51fe16589ebf0fc641d7c45; + : ${PR_NUMBER}; git merge-base --is-ancestor "$PRODUCT_FIX_COMMIT" HEAD && test "$(gh pr view $PRODUCT_PR_NUMBER + --repo $PRODUCT_REPO --json state --jq .state)" = "OPEN"' +contract_entry_sha256: sha256:9f4bb73cc5a3ca0b78ea3e61cb528f25b6866ffa71a6a81097c440fd4192e475 +status: PASS +run_timestamp: '2026-07-19T05:45:55Z' +commit_sha: 35ef8cb3219e020db8323e0812ab2888f14885a4 +runner: codex-letter +verifier: github-pr-head-supersession-readback +probe_command: git merge-base --is-ancestor 5120e3fa4f056d6ee51fe16589ebf0fc641d7c45 HEAD && gh pr view + 4426 --repo OmniNode-ai/onex_change_control --json state --jq .state +probe_stdout: OPEN +actual_output: 'PASS: repair branch head 35ef8cb3219e020db8323e0812ab2888f14885a4 contains validated product + commit 5120e3fa4f056d6ee51fe16589ebf0fc641d7c45 and PR #4426 remains open before push.' +exit_code: 0 +pr_number: 4426 +branch: jonah/omn-14051-reject-non-hermetic-check-value diff --git a/scripts/ci/run_contract_compliance_check.py b/scripts/ci/run_contract_compliance_check.py index 74dd46b8226..96a85ae2d0c 100755 --- a/scripts/ci/run_contract_compliance_check.py +++ b/scripts/ci/run_contract_compliance_check.py @@ -47,6 +47,7 @@ _load_legacy_allowlist, _load_yaml, _maybe_demote_precommit, + _non_hermetic_reason, _run, _run_dod_checks, _run_single_check, @@ -83,6 +84,7 @@ "_load_legacy_allowlist", "_load_yaml", "_maybe_demote_precommit", + "_non_hermetic_reason", "_run", "_run_dod_checks", "_run_single_check", diff --git a/src/onex_change_control/scripts/contract_compliance_check.py b/src/onex_change_control/scripts/contract_compliance_check.py index cda074d49b8..831ac4aa434 100755 --- a/src/onex_change_control/scripts/contract_compliance_check.py +++ b/src/onex_change_control/scripts/contract_compliance_check.py @@ -46,6 +46,7 @@ import json import os import re +import shlex import subprocess import sys from dataclasses import dataclass @@ -95,6 +96,197 @@ def _is_inert_check(check_value: Any) -> bool: return any(p.search(text) for p in _INERT_CHECK_PATTERNS) +# OMN-14051 -- non-hermetic check_value guard (reject at validation time). +# +# A dod_evidence check of check_type "command" has its check_value executed +# verbatim by this runner inside the CI environment. CI runners have no ssh, no +# route to the .201 Tailscale/LAN hosts, and no live docker/k8s daemon, so a +# command that shells out to ssh/scp, a live container daemon, or network egress +# to a non-loopback host can NEVER pass in CI -- it 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): a `dod-deploy-scope` item +# inlined `ssh jonah@100.109.203.94 "docker ps ..."` and produced +# "3/4 PASS, 1 BLOCK". +# +# The canonical, hermetic pattern is to run the live probe OUT OF BAND, record +# it in a committed receipt, and have the contract grep that receipt: +# +# check_value: >- +# grep -q '^status: PASS$' +# "$CONTRACT_REPO_DIR/drift/dod_receipts///command.yaml" +# +# This guard rejects the non-hermetic form up front with that actionable +# message, so the failure surfaces at authoring/validation time instead of as a +# late, cryptic CI error. It is folded into the *existing* validator (no new +# gate/workflow -- net-negative-surface) and is subject to the same OMN-14436 +# demotion rules as any other BLOCK: a grandfathered (content-pinned legacy) +# contract is demoted to WARN -- so the pre-existing corpus of `docker exec` +# runtime-proof checks is reported but not wedged -- while a NEW or touched +# contract is enforced from its first PR. +# +# Detection is deliberately conservative (per the ticket: "start conservative, +# expand as needed"). Known gap: a binary hidden inside a command substitution +# (`$(ssh ...)`) is not yet detected; only command-position invocations are. +_MAX_SNIPPET_LEN = 160 + +# Shell tokens that begin a new command word (so the next token is in command +# position). shlex emits these as standalone tokens when whitespace-separated. +_SHELL_OPERATORS: frozenset[str] = frozenset( + {"|", "||", "&&", ";", ";;", "&", "|&", "(", ")", "{", "}"} +) +# Command wrappers whose *argument* is the real command; stay in command +# position past them (and past `env FOO=bar` assignment tokens). +_WRAPPER_BINS: frozenset[str] = frozenset( + {"sudo", "env", "time", "nice", "nohup", "command", "exec", "xargs", "then", "do"} +) +# Remote shell / remote copy: always non-hermetic. +_REMOTE_SHELL_BINS: frozenset[str] = frozenset( + {"ssh", "scp", "sftp", "rsync", "telnet"} +) +# Container / orchestration: need a live daemon absent from CI runners. +_DAEMON_BINS: frozenset[str] = frozenset( + {"docker", "docker-compose", "podman", "nerdctl", "kubectl"} +) +# Network fetch: non-hermetic only when the target host is not loopback. +_NET_FETCH_BINS: frozenset[str] = frozenset({"curl", "wget", "nc", "ncat"}) + +_ASSIGNMENT_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*=") +# Full-octet IPv4 in the private / Tailscale-CGNAT ranges. Anchored to a +# complete dotted quad so version strings ("10.2") never match. +_LAN_IP_RE = re.compile( + r"\b(" + r"192\.168\.\d{1,3}\.\d{1,3}" + r"|10\.\d{1,3}\.\d{1,3}\.\d{1,3}" + r"|172\.(?:1[6-9]|2\d|3[01])\.\d{1,3}\.\d{1,3}" + r"|100\.(?:6[4-9]|[7-9]\d|1[01]\d|12[0-7])\.\d{1,3}\.\d{1,3}" + r")\b" +) +_URL_HOST_RE = re.compile(r"https?://([^/\s]+)") +_LOOPBACK_HOSTS: frozenset[str] = frozenset( + {"localhost", "127.0.0.1", "::1", "ip6-localhost"} +) + + +def _command_binaries(command: str) -> list[str]: + """Basenames of the binaries invoked in command position in ``command``. + + Quote-aware via ``shlex`` so a binary name embedded in a quoted argument -- + e.g. the word "docker" in ``grep -q 'no docker exec' file`` -- is NOT + reported; only tokens that actually start a command word are. Shell + operators (``;`` ``&&`` ``||`` ``|``) and wrappers (``sudo``, ``env VAR=x``) + keep the scan in command position. + """ + try: + tokens = shlex.split(command, posix=True) + except ValueError: + # Unbalanced quotes etc. -- best-effort whitespace split rather than + # silently passing the guard on a malformed command. + tokens = command.split() + binaries: list[str] = [] + expect_command = True + for token in tokens: + if token in _SHELL_OPERATORS: + expect_command = True + continue + if not expect_command: + continue + if _ASSIGNMENT_RE.match(token): + # `env FOO=bar cmd` -- leading assignments are not the command. + continue + binary = token.rsplit("/", 1)[-1] + if binary in _WRAPPER_BINS: + continue # stay in command position; the next token is the command + binaries.append(binary) + expect_command = False + return binaries + + +def _is_loopback_host(host: str) -> bool: + """True if ``host`` (optionally ``user@host:port``) is a loopback target.""" + text = host.strip().strip("\"'[]").lower() + if "@" in text: + text = text.rsplit("@", 1)[1] + if text.startswith("127.") or text in _LOOPBACK_HOSTS: + return True + return text.split(":", 1)[0] in _LOOPBACK_HOSTS + + +def _has_nonloopback_url(command: str) -> bool: + """True if ``command`` contains an http(s) URL whose host is not loopback.""" + return any(not _is_loopback_host(h) for h in _URL_HOST_RE.findall(command)) + + +def _non_hermetic_message(reason: str, command: str) -> str: + """Actionable rejection message steering the author to the receipt pattern.""" + snippet = ( + command + if len(command) <= _MAX_SNIPPET_LEN + else command[:_MAX_SNIPPET_LEN] + "..." + ) + return ( + f"NON-HERMETIC check_value -- {reason}. A CI runner has no ssh, no route " + "to the .201/LAN/Tailscale hosts, and no live docker/k8s daemon, so this " + "command can never pass in CI (OMN-14051); it would BLOCK the PR with a " + "cryptic runtime error even when the work is real. Run the live probe out " + "of band, record it in a committed receipt " + "(drift/dod_receipts///command.yaml with fields " + "probe_command, probe_stdout, exit_code, status: PASS), then set the " + "contract check to grep that receipt:\n" + " check_value: >-\n" + " grep -q '^status: PASS$'\n" + ' "$CONTRACT_REPO_DIR/drift/dod_receipts/' + '//command.yaml"\n' + f" offending check_value: {snippet}" + ) + + +def _non_hermetic_reason(check: Any) -> str | None: + """Rejection message if ``check``'s command check_value is non-hermetic, else None. + + "Non-hermetic" == the command depends on ssh/remote-copy, a live + docker/k8s daemon, or network egress to a non-loopback host -- none of which + exist on a CI runner. Returns ``None`` for a hermetic command (local file + assertions, receipt greps, pure compute, loopback probes) so those still run. + + Only ``check_type: command`` is inspected: its check_value is executed as a + shell command, so it is the only surface where these binaries actually run. + grep / file_exists / test_exists check_values are file/glob assertions that + may legitimately *contain* an IP or the word "docker" (e.g. grepping a + committed config), so scanning them would be a false-positive machine. + """ + if not isinstance(check, dict) or check.get("check_type") != "command": + return None + command = str(check.get("check_value", "")) + if not command.strip(): + return None + + binaries = _command_binaries(command) + for binary in binaries: + if binary in _REMOTE_SHELL_BINS: + return _non_hermetic_message( + f"invokes the remote-access binary {binary!r}", command + ) + if binary in _DAEMON_BINS: + return _non_hermetic_message( + f"invokes {binary!r}, which needs a live container/orchestration " + "daemon that CI runners do not have", + command, + ) + lan_ip = _LAN_IP_RE.search(command) + if lan_ip is not None: + return _non_hermetic_message( + f"references the non-routable LAN/Tailscale host {lan_ip.group(1)!r}", + command, + ) + for binary in binaries: + if binary in _NET_FETCH_BINS and _has_nonloopback_url(command): + return _non_hermetic_message( + f"performs network egress via {binary!r} to a non-loopback host", + command, + ) + return None + + def _contract_digest(contract_path: Path) -> str: """sha256 of the contract file's exact bytes. @@ -542,6 +734,15 @@ def _run_single_check( context: _CheckContext, ) -> tuple[str, str, str]: """Run a single ModelDodCheck and return (check_type, result, detail).""" + reason = _non_hermetic_reason(check) + if reason is not None: + # OMN-14051: reject an ssh/live-docker/network-egress check_value up + # front with an actionable message instead of executing it and surfacing + # a cryptic "command not found" BLOCK. _demote still applies the + # OMN-14436 grandfather downgrade, so the legacy corpus is reported, not + # wedged. + return str(check.get("check_type", "")), _RESULT_BLOCK, reason + check_type = check.get("check_type", "") check_value = check.get("check_value", "") diff --git a/tests/test_contract_compliance_check.py b/tests/test_contract_compliance_check.py index 92b7772a390..02671f991b1 100644 --- a/tests/test_contract_compliance_check.py +++ b/tests/test_contract_compliance_check.py @@ -29,8 +29,11 @@ _check_file_exists, _check_grep, _check_test_exists, + _command_binaries, + _contract_digest, _extract_ticket_id, _find_contracts_dir, + _non_hermetic_reason, run_compliance_check, ) @@ -675,3 +678,189 @@ def test_superseded_dod_item_is_warn_not_block(tmp_path: Path) -> None: ): rc = run_compliance_check(1, "OmniNode-ai/omnimarket", contracts, tmp_path) assert rc == 0 + + +# --------------------------------------------------------------------------- +# OMN-14051 -- non-hermetic check_value guard +# --------------------------------------------------------------------------- + + +def _cmd(check_value: str) -> dict[str, str]: + return {"check_type": "command", "check_value": check_value} + + +def test_non_hermetic_rejects_ssh() -> None: + """An ssh probe (the OMN-14001 / PR #3642 case) is rejected with a message.""" + reason = _non_hermetic_reason( + _cmd('ssh jonah@100.109.203.94 "docker ps" && echo no-out-of-band-deploy') + ) + assert reason is not None + assert "NON-HERMETIC" in reason + assert "ssh" in reason + # Steers the author to the canonical receipt-preservation pattern. + assert "drift/dod_receipts/" in reason + assert "status: PASS" in reason + + +def test_non_hermetic_rejects_live_docker() -> None: + """`docker exec` against a live daemon (absent in CI) is rejected.""" + reason = _non_hermetic_reason( + _cmd("docker exec omninode-runtime python -c 'import omnimarket'") + ) + assert reason is not None + assert "docker" in reason + + +def test_non_hermetic_rejects_lan_ip_curl() -> None: + """Network egress to a LAN/Tailscale host is rejected.""" + reason = _non_hermetic_reason(_cmd("curl -fsS http://192.168.86.201:8085/health")) + assert reason is not None + assert "192.168.86.201" in reason + + +def test_non_hermetic_rejects_scp_and_rsync() -> None: + assert _non_hermetic_reason(_cmd("scp host:/tmp/x .")) is not None + assert _non_hermetic_reason(_cmd("rsync -a host:/src/ ./dst/")) is not None + + +def test_non_hermetic_rejects_sudo_prefixed_ssh() -> None: + """A wrapper (sudo/env) does not hide the non-hermetic binary.""" + assert _non_hermetic_reason(_cmd("sudo ssh box uptime")) is not None + assert _non_hermetic_reason(_cmd("env FOO=bar docker ps")) is not None + + +def test_hermetic_receipt_grep_accepted() -> None: + """The canonical receipt-grep pattern is NOT flagged as non-hermetic.""" + receipt = ( + "grep -q '^status: PASS$' " + '"$CONTRACT_REPO_DIR/drift/dod_receipts/OMN-14051/dod-x/command.yaml"' + ) + assert _non_hermetic_reason(_cmd(receipt)) is None + + +def test_hermetic_local_file_checks_accepted() -> None: + assert ( + _non_hermetic_reason(_cmd("test -f src/onex_change_control/__init__.py")) + is None + ) + assert _non_hermetic_reason(_cmd("python -c 'import onex_change_control'")) is None + + +def test_hermetic_loopback_curl_accepted() -> None: + """curl to a loopback host is hermetic (the ticket carves out non-loopback).""" + assert _non_hermetic_reason(_cmd("curl -fsS http://localhost:8085/health")) is None + assert _non_hermetic_reason(_cmd("curl -fsS http://127.0.0.1:9000/ready")) is None + + +def test_hermetic_quoted_docker_word_accepted() -> None: + """The word "docker" inside a quoted grep pattern must NOT trip the guard.""" + check = _cmd("grep -q 'No runtime deploy, docker exec, rpk produce' report.txt") + assert _non_hermetic_reason(check) is None + + +def test_non_command_check_types_are_ignored() -> None: + """grep/file_exists/test_exists may legitimately contain IPs or "docker".""" + assert ( + _non_hermetic_reason( + {"check_type": "grep", "check_value": {"pattern": "192.168.1.1"}} + ) + is None + ) + assert ( + _non_hermetic_reason( + {"check_type": "file_exists", "check_value": "deploy/ssh/config"} + ) + is None + ) + + +def test_command_binaries_is_quote_aware() -> None: + assert _command_binaries("grep -q 'no docker here' f && ssh host") == [ + "grep", + "ssh", + ] + assert _command_binaries("sudo docker ps") == ["docker"] + assert _command_binaries("env FOO=bar docker exec c cmd") == ["docker"] + assert _command_binaries("/usr/bin/ssh host") == ["ssh"] + + +def test_non_hermetic_contract_blocks_new_ticket( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A NEW (non-grandfathered) contract with an ssh check_value BLOCKs (rc=1).""" + contracts = tmp_path / "contracts" + contracts.mkdir() + contract_yaml = textwrap.dedent(""" + schema_version: "1.0.0" + ticket_id: "OMN-2051" + summary: "Non-hermetic ssh probe in dod" + is_seam_ticket: false + interface_change: false + interfaces_touched: [] + evidence_requirements: [] + emergency_bypass: + enabled: false + justification: "" + follow_up_ticket_id: "" + dod_evidence: + - id: dod-deploy-scope + description: "Prove no out-of-band deploy" + checks: + - check_type: command + check_value: "ssh jonah@100.109.203.94 hostname" + status: pending + """) + (contracts / "OMN-2051.yaml").write_text(contract_yaml) + with patch( + "onex_change_control.scripts.contract_compliance_check._extract_ticket_id", + return_value="OMN-2051", + ): + rc = run_compliance_check(1, "OmniNode-ai/omnimarket", contracts, tmp_path) + out = capsys.readouterr().out + assert rc == 1 + assert "NON-HERMETIC" in out + + +def test_non_hermetic_contract_grandfathered_is_warn_not_block( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A grandfathered (content-pinned) contract demotes the non-hermetic BLOCK to WARN. + + Proves the guard respects the OMN-14436 ratchet: the pre-existing corpus of + ssh/docker-exec runtime-proof checks is reported, not wedged. + """ + contracts = tmp_path / "contracts" + contracts.mkdir() + contract_yaml = textwrap.dedent(""" + schema_version: "1.0.0" + ticket_id: "OMN-2052" + summary: "Legacy non-hermetic probe" + is_seam_ticket: false + interface_change: false + interfaces_touched: [] + evidence_requirements: [] + emergency_bypass: + enabled: false + justification: "" + follow_up_ticket_id: "" + dod_evidence: + - id: dod-legacy-probe + description: "Legacy live probe" + checks: + - check_type: command + check_value: "docker exec omninode-runtime true" + status: verified + """) + contract_path = contracts / "OMN-2052.yaml" + contract_path.write_text(contract_yaml) + legacy = {"OMN-2052": _contract_digest(contract_path)} + with patch( + "onex_change_control.scripts.contract_compliance_check._extract_ticket_id", + return_value="OMN-2052", + ): + rc = run_compliance_check( + 1, "OmniNode-ai/omnimarket", contracts, tmp_path, legacy_tickets=legacy + ) + out = capsys.readouterr().out + assert rc == 0 + assert "GRANDFATHERED" in out