Repository navigation
ops(OMN-18415): the fleet also needs the LLM trust boundary, not just the key - #3614
Conversation
… the key Provisioning the signing key alone moved the reviewer's failure three seconds later in the same call path and changed nothing a reader could see. MixinLlmHttpTransport rejects a local LLM call before any HTTP is attempted unless the destination falls inside LLM_ENDPOINT_CIDR_ALLOWLIST, and it refuses to default that variable: a trust boundary nobody declared is not a boundary. That variable was absent from every fleet runner too. Measured, not inferred: reproducing the runner's exact environment (signing key present, the rest of the LLM family absent) fails with "LLM_ENDPOINT_CIDR_ALLOWLIST is required but not set" and exit 1, which is what the gate's suppressed stderr was hiding. With both present, the same command reports the model succeeding. Declared the same way as the key, on the shared anchor, fail-closed, reaching only the 60 fleet services. It is not a secret, but it is still :? rather than a literal default, because mirroring the library's own refusal is the point and a host whose boundary differs must say so rather than inherit this lab's.
|
OCC autobind did not mint a companion for this PR: no changed-file candidate could be proven RED against the merge base, and emitting a PR-existence probe instead would be non-falsifiable evidence (OMN-15247). Hand-authored evidence is required. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: glm-review
Models failed: codex
New finding threads: 3
Deduped (already posted on this PR): 0
Nit-level findings suppressed: 2
The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.
Findings demoted from threads (anchor rejected)
-
[MINOR] hostile-reviewer (glm-review)
Malformed CIDR value passes all compose-level tests | The new test coverage verifies presence, fail-fast interpolation, and single-declaration for LLM_ENDPOINT_CIDR_ALLOWLIST, but no test validates that the interpolated value is syntactically valid CIDR. A runner host provisioned with a typo (e.g. '10.0.0.0/8 ' trailing whitespace or '10.0.0.0/33') passes every test in this file and docker compose config, then fails at runtime inside _parse_cidr_allowlist with the same empty-log failure mode this change exi
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
CIDR allowlist treated as secret-equivalent in leak scope test | test_out_of_scope_runners_do_not_receive_the_key now asserts LLM_ENDPOINT_CIDR_ALLOWLIST is absent from out-of-scope services, but the diff's own comments state the CIDR variable is 'Not a secret: a network boundary declaration.' Out-of-scope services that make local LLM calls would legitimately need the boundary declaration without the signing key. Bundling them into one REQUIRED_REVIEWER_ENV_VARS tuple couples two contracts with different co
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
No negative coverage for the new CIDR assertions | test_the_presence_assertion_can_actually_fail demonstrates the secret assertion can fail, but the diff adds no analogous mutation test for the CIDR assertions (removal from the anchor, a :- soft default, or an inline literal). The test_the_presence_assertion_can_actually_fail test is not updated to iterate the new tuple, so the claim of fail-closed test coverage holds for one variable by demonstration and for the other by assumption. | Evidence: test_the_pr
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
|
| Surface | Meaning | Blocks merge? |
|---|---|---|
| Review threads | Per-finding, posted by the reviewer | No (informational) |
Hostile Review Thread Gate |
Deterministic: unresolved hostile-reviewer threads exist | Fails until resolved (not yet a required context) |
degraded verdict |
Fewer than 2 models succeeded (infra) | No |
Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review: qwen3-review, qwen3-review-b, glm-review (OMN-8468/OMN-8524/OMN-17492)
#9771) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#3614 * evidence: OCC companion self-bind for #9771 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
… the key (#3614) Provisioning the signing key alone moved the reviewer's failure three seconds later in the same call path and changed nothing a reader could see. MixinLlmHttpTransport rejects a local LLM call before any HTTP is attempted unless the destination falls inside LLM_ENDPOINT_CIDR_ALLOWLIST, and it refuses to default that variable: a trust boundary nobody declared is not a boundary. That variable was absent from every fleet runner too. Measured, not inferred: reproducing the runner's exact environment (signing key present, the rest of the LLM family absent) fails with "LLM_ENDPOINT_CIDR_ALLOWLIST is required but not set" and exit 1, which is what the gate's suppressed stderr was hiding. With both present, the same command reports the model succeeding. Declared the same way as the key, on the shared anchor, fail-closed, reaching only the 60 fleet services. It is not a secret, but it is still :? rather than a literal default, because mirroring the library's own refusal is the point and a host whose boundary differs must say so rather than inherit this lab's.
OMN-18415 — the other half of the same fail-closed contract
The merged change gave the 60-runner fleet the local-LLM signing key. Rolling it to
every runner and rerunning the gate showed the reviewer still failing, in the same
call path, a few seconds later:
MixinLlmHttpTransportalso rejects a local LLM callbefore any HTTP is attempted unless the destination falls inside
LLM_ENDPOINT_CIDR_ALLOWLIST, and it refuses to default that variable — a trustboundary nobody declared is not a boundary. That variable was absent from every fleet
runner too.
Measured, not inferred
Reproducing the runner's exact environment (signing key present, the rest of the LLM
family absent):
With both present, the same command against the same pull request:
Exit 1 is what the gate reported on both attempts, and the gate discards the reviewer's
stderr, which is why the cause was invisible in CI and had to be reproduced by hand.
That suppression is the ticket's third acceptance criterion and is not addressed here.
What changed
One declaration on the
&runner-envanchor, fail-closed (:?), reaching only the 60omninode-runner-Nservices. The deploy runner and both customer-plane runners definetheir own environment blocks and stay untouched.
Not a secret — a network boundary declaration — but still
:?rather than a literaldefault: mirroring the library's own refusal is the point, and a host whose boundary
differs must say so rather than inherit this lab's.
The fleet script's required-variable block now documents both variables as one pair,
with the reason provisioning either alone accomplishes nothing.
The compose tests now assert both variables over the whole fleet, each with its
positive control, and the
docker compose configtest supplies the new one.uv run pytest tests/ci/test_runner_local_llm_secret_env_omn18415.py tests/ci/test_runner_kubeconfig_mount.py tests/unit/scripts/test_deploy_runners_rolling_mode_omn18415.py tests/ci/test_customer_plane_runner_credential_free_omn18392.py→ 48 passed.The value is on the runner host beside the signing key, in the compose project
directory's env file, and is absent from the fleet script's sync list.
Application to the live fleet is the same serial, busy-checked roll, one runner at a
time, with a per-runner readback.
Evidence-Ticket: OMN-18415
Evidence-Source: OCC#9771