Repository navigation
fix(OMN-18120): give the gateway forwarder the dev lane's SCRAM credential so the hook mirror can produce again - #3392
Conversation
…SCRAM credential OMN-18012 flipped the dev lane's broker to SASL and, in its own words, gave "all 15 clients the credential" (#3276). The gateway forwarder was not one of the fifteen. It lives in a different compose project (omninode-gateway) and configures its bus legs per-leg in a mounted YAML rather than through the shared x-dev-lane-broker-auth-env anchor, so the enumeration that credentialed the lane did not reach it. Both of its dev-lane legs -- local_bus and lane_mirror_buses.dev -- kept declaring PLAINTEXT against a listener that now closes a plaintext connection at handshake. Every republish failed at connect, and because the deployed build predates the delivery-confirmation fix it logged a delivery anyway. The stability->dev hook mirror has moved zero records since. Measured on .201 2026-09-10, read-only, each zero with a positive control: rpk --config <empty> -X brokers=<dev external> cluster metadata -> "SASL is required but not provided" rpk --config <empty> -X brokers=<stability external> cluster metadata -> CLUSTER / BROKERS ... all four dev hook topics' high watermarks static across 91s (tool-executed 599687, session-started 1251873, prompt-submitted 8660, session-ended 9759) while the forwarder logged 110 "Lane mirror delivered" lines in three minutes control: onex.dlq.omnibase-infra.quarantine.v1 11475729 -> 11475736 The credential is a REFERENCE resolved from an operator-supplied mounted map, never a literal and never an environment variable. beta-gateway-canary.yaml is tracked in this repo AND mounted into the container verbatim, so a value spelled there is a credential in git; an env var would be readable from `docker inspect` for the life of the container, and this repo's check-env-reads gate refuses new os.environ reads outside the overlay surfaces. A leg declares sasl_credential_ref and _materialize_lane_broker_credentials resolves it against the map passed as --lane-credential-map -- the same not-in-git, operator-mounted mechanism the cloud leg's endpoint already rides via --broker-ref-map. A single ambient KAFKA_SASL_* is deliberately not the mechanism: this process holds three legs on three brokers with three postures (cloud MSK SASL_SSL / AWS_MSK_IAM, the stability source leg still PLAINTEXT, the dev legs SASL_PLAINTEXT / SCRAM-SHA-256). One value cannot be right for all three and would break the leg that works. test_the_source_leg_stays_plaintext pins that asymmetry so a later "make them consistent" edit cannot undo it. Fails closed in every direction: no map, an absent file, a non-mapping file, a missing entry or an empty field all refuse to start naming the ref; a ref on a PLAINTEXT leg refuses rather than silently collecting a credential it would never send; a ref alongside a literal refuses as ambiguous. A bare literal is left alone on purpose so the loader can re-read its own model_dump() output -- the same trap _materialize_contract_https_ingest records for its null round trip; that the SHIPPED file carries no literal is asserted statically over the tracked YAML instead. NOT A ROTATION (Operating Rule 22). This binds the principal OMN-18012 phase A provisioned. Nothing is rotated, re-issued or revoked, and no value appears in the diff, the tests, the error messages or the logs. Red at parent bcbbf90: 15 failed / 1 passed, the one pass being the premise that the stability source leg stays plaintext. Green at head: 284 passed across the forwarder node, the runtime loader, the compose-credential and deploy-gateway suites.
…fra#3392 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head ac0cf833bcdc26325fcce913888fa592b407e0ca.
![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: 1
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 not anchored to a changed file
-
[MAJOR] hostile-reviewer (glm-review)
No detection mechanism for recurrence of the silent-failure mode | The incident root cause was that the forwarder logged 'Lane mirror delivered' per attempted republish while zero records crossed. The diff fixes the configuration but adds no counter, healthcheck field, or assertion that distinguishes an authenticated, producing leg from a leg failing at connect. The egress counters already mounted for the healthcheck are not extended to cover leg-level connect/auth outcomes, so the next misconfiguration (ro
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Loader mutates caller's input dict on the failure path | _materialize_lane_broker_credentials pops sasl_credential_ref from each leg before raising on ambiguity, protocol mismatch, or map failures. A caller that catches the ValueError and retries validation with the same dict silently loses the ref key, degrading the ambiguous-config error into a different (or absent) failure. _iter_bus_legs similarly replaces nested dicts in the caller's structure. | Evidence: ref_object = leg.pop(_SASL_CREDENTIAL_REF_KEY,
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Legs declaring sasl_mechanism without a ref or protocol are not validated | The boundary only validates legs carrying sasl_credential_ref. A leg declaring SASL_PLAINTEXT and sasl_mechanism but neither a ref nor literals fails model validation later, but a leg declaring PLAINTEXT plus sasl_mechanism SCRAM-SHA-256 (a contradiction in the other direction) passes materialization silently; whether it fails downstream depends on the pydantic model. The change's fail-closed claims are scoped to the ref key only. |
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
Findings demoted from threads (anchor rejected)
-
[MINOR] hostile-reviewer (glm-review)
Untested edge cases in credential materialization | The new test file covers missing map, absent file, missing entry, empty field, ref+literal, and protocol mismatch, but not: (a) a map entry that is a non-dict scalar (e.g. 'ref: not-a-mapping'), which exercises the isinstance(entry, dict) branch; (b) a non-string or whitespace-only sasl_credential_ref value; (c) multiple legs sharing one map confirming the map is read once and both resolve. Branches shipped without direct tests. | Evidence: if not isinstan
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Compose assertions are substring matches, not structural checks | test_compose_passes_the_dev_lane_credential_fail_closed asserts '${GATEWAY_LANE_CREDENTIAL_MAP_FILE:?' and '--lane-credential-map' appear anywhere in the compose file text. Either token could appear in a comment (as both in fact do, since the diff adds explanatory comments containing '--lane-credential-map' is not in a comment here, but '--lane-credential-map' assert could pass via a comment block) or in the wrong service. The mount could als
Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (glm-review)
Credential map file permissions and post-stop lifetime unaddressed | The diff correctly avoids git and env vars, but the mounted map is a world-readable-unless-operator-restricts plaintext password on the host and inside the container (mounted 0444-equivalent via :ro bind). Any process or user on the .201 host with read access to GATEWAY_LANE_CREDENTIAL_MAP_FILE obtains the SCRAM password; docker inspect exposure was the stated motivation, file readability is the same class of residual exposure. No mode/own
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)
#8905) * evidence(OMN-18120): author OCC companion for OmniNode-ai/omnibase_infra#3392 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head ac0cf833bcdc26325fcce913888fa592b407e0ca. * evidence(OMN-18120): self-bind OCC#8905 + rebind contract_sha256 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
…ts own loader demands (#3412) #3392 credentialed the gateway forwarder's dev-lane legs and did not credential the healthcheck that watches them. `onex-gateway-canary-probe` calls the SAME `load_gateway_forwarder_runtime_config` the forwarder does, and that loader fails closed -- correctly, by design -- as soon as any bus leg names a `sasl_credential_ref`. The probe inherited a hard refusal it was never given the argument to satisfy, so every 15s tick died in configuration load before dialing a broker: ValueError: gateway bus leg 'local_bus' names sasl_credential_ref=... but no lane-credential map was supplied; the gateway refuses to start rather than open a client that cannot authenticate Measured on .201 2026-09-10T18:17Z against the deployed fix, with the forwarder and its healthcheck read in the same window: the process was authenticated and delivering (`Authenticated as onex-dev-lane via SCRAM-SHA-256`, zero `Connection at redpanda:9092 closed`, dev `onex.evt.omniclaude.tool-executed.v1` 599687 -> 599711, and `omninode_internal.work_events` n_tup_ins 71208 -> 71237 with n_tup_upd unchanged at 94811, i.e. real INSERTs rather than the no-op upsert loop), while `docker inspect` reported the container `unhealthy` with FailingStreak 5 and the traceback above as its last probe output. That is the one state a healthcheck must never invent: it declared the traffic path dead while the path was carrying traffic. It is the exact inverse of the 2026-08-04 outage the probe was built for -- ready-file present, forwarding 0% for four days -- and it misleads in the same way, because both make the health signal independent of the thing it names. WHY THE EXISTING TEST DID NOT CATCH IT. `test_compose_passes_the_dev_lane_ credential_fail_closed` asserts the flag is `in compose` -- a substring match over the whole file. The flag WAS in the file, on the forwarder's `command:`. The healthcheck's `test:` array was never looked at, so a file-granularity assertion reported green on an invocation-granularity defect. The three tests added here are per-argv: the healthcheck carries the flag; the process and the probe name the SAME map path (two argv lists naming two maps would make the probe a check on a configuration nothing runs); and the probe's parser actually accepts it, since a flag argparse rejects would turn the healthcheck into a usage error on every tick. The first also asserts the healthcheck still invokes the probe at all, so it cannot pass vacuously if the healthcheck is later rewritten. WHY THE ARGUMENT IS OPTIONAL AND NOT REQUIRED, unlike `--broker-ref-map`: a deployment whose legs are all PLAINTEXT names no ref and needs no map. The loader already refuses by name when a ref is present and the map is absent, so making it required here would break that deployment in order to restate a refusal the loader owns. NOT A ROTATION (Operating Rule 22): no credential is minted, rotated, re-issued or revoked, and no value appears in the diff, the tests or any error message. Only the PATH of the operator-supplied map is named. Red at parent: 3 failed / 16 passed. Green at head: 35 passed across this file and the runtime loader suite; ruff, mypy --strict and pre-commit --all-files clean.
…ts own loader demands (#3412) #3392 credentialed the gateway forwarder's dev-lane legs and did not credential the healthcheck that watches them. `onex-gateway-canary-probe` calls the SAME `load_gateway_forwarder_runtime_config` the forwarder does, and that loader fails closed -- correctly, by design -- as soon as any bus leg names a `sasl_credential_ref`. The probe inherited a hard refusal it was never given the argument to satisfy, so every 15s tick died in configuration load before dialing a broker: ValueError: gateway bus leg 'local_bus' names sasl_credential_ref=... but no lane-credential map was supplied; the gateway refuses to start rather than open a client that cannot authenticate Measured on .201 2026-09-10T18:17Z against the deployed fix, with the forwarder and its healthcheck read in the same window: the process was authenticated and delivering (`Authenticated as onex-dev-lane via SCRAM-SHA-256`, zero `Connection at redpanda:9092 closed`, dev `onex.evt.omniclaude.tool-executed.v1` 599687 -> 599711, and `omninode_internal.work_events` n_tup_ins 71208 -> 71237 with n_tup_upd unchanged at 94811, i.e. real INSERTs rather than the no-op upsert loop), while `docker inspect` reported the container `unhealthy` with FailingStreak 5 and the traceback above as its last probe output. That is the one state a healthcheck must never invent: it declared the traffic path dead while the path was carrying traffic. It is the exact inverse of the 2026-08-04 outage the probe was built for -- ready-file present, forwarding 0% for four days -- and it misleads in the same way, because both make the health signal independent of the thing it names. WHY THE EXISTING TEST DID NOT CATCH IT. `test_compose_passes_the_dev_lane_ credential_fail_closed` asserts the flag is `in compose` -- a substring match over the whole file. The flag WAS in the file, on the forwarder's `command:`. The healthcheck's `test:` array was never looked at, so a file-granularity assertion reported green on an invocation-granularity defect. The three tests added here are per-argv: the healthcheck carries the flag; the process and the probe name the SAME map path (two argv lists naming two maps would make the probe a check on a configuration nothing runs); and the probe's parser actually accepts it, since a flag argparse rejects would turn the healthcheck into a usage error on every tick. The first also asserts the healthcheck still invokes the probe at all, so it cannot pass vacuously if the healthcheck is later rewritten. WHY THE ARGUMENT IS OPTIONAL AND NOT REQUIRED, unlike `--broker-ref-map`: a deployment whose legs are all PLAINTEXT names no ref and needs no map. The loader already refuses by name when a ref is present and the map is absent, so making it required here would break that deployment in order to restate a refusal the loader owns. NOT A ROTATION (Operating Rule 22): no credential is minted, rotated, re-issued or revoked, and no value appears in the diff, the tests or any error message. Only the PATH of the operator-supplied map is named. Red at parent: 3 failed / 16 passed. Green at head: 35 passed across this file and the runtime loader suite; ruff, mypy --strict and pre-commit --all-files clean.
OMN-18120: the gateway forwarder was never given the dev lane's SCRAM credential
OMN-18012 flipped the dev lane's broker to SASL and, in its own words, gave "all 15 clients the credential" (
OmniNode-ai/omnibase_infra#3276). The gateway forwarder was not one of the fifteen. It lives in a different compose project (omninode-gateway) and configures its bus legs per-leg in a mounted YAML rather than through the sharedx-dev-lane-broker-auth-envanchor, so the enumeration that credentialed the lane did not reach it.Both of its dev-lane legs kept declaring
PLAINTEXTagainst a listener that now closes a plaintext connection at handshake. Every republish failed at connect, and because the deployed build predates the delivery-confirmation fix it logged a delivery anyway. The stability -> dev hook mirror has moved zero records since 2026-09-06.Evidence, measured read-only on the
.201dev lane 2026-09-10, every zero with a positive controlThe listener asymmetry, same command both times, empty rpk profile so no ambient credential applies:
The consequence, two samples 91s apart:
Nothing moved, while the forwarder logged 110
Lane mirror deliveredlines in three minutes. Positive control on the same reads in the same window:onex.dlq.omnibase-infra.quarantine.v1advanced 11475729 -> 11475736, so the broker was taking writes and the zeros are real.enable_saslreadstrueon the dev broker. The projection downstream took zero writes across the window:n_tup_ins71208 andn_tup_upd94811, byte-identical at both readings.Why this is not the alias bug it resembles
The OMN-17919 alias defect was fixed.
getent hosts redpandainside the forwarder now answers the dev broker, and the dev leg is addressed at the dev lane's external listener anyway. Both are correct. The leg still cannot produce, because it declaresPLAINTEXT.The shape of the fix
The credential is a reference resolved from an operator-supplied mounted map, never a literal and never an environment variable.
beta-gateway-canary.yamlis tracked here and mounted into the container verbatim, so a value spelled there is a credential in git.docker inspectfor the life of the container, and this repo'scheck-env-readsgate refuses newos.environreads outside the overlay surfaces. It fired on the first draft of this change, correctly.A leg declares
sasl_credential_refand_materialize_lane_broker_credentialsresolves it against the map passed as--lane-credential-map. That is the same not-in-git, operator-mounted mechanism the cloud leg's endpoint already rides via--broker-ref-map.A single ambient
KAFKA_SASL_*is deliberately not the mechanism. This process holds three legs on three brokers with three postures: cloud MSK onSASL_SSL/AWS_MSK_IAM, the stability source leg stillPLAINTEXT, the dev legsSASL_PLAINTEXT/SCRAM-SHA-256. One value cannot be right for all three and would break the leg that works.test_the_source_leg_stays_plaintextpins that asymmetry so a later "make them consistent" edit cannot undo it.Fails closed in every direction. No map, an absent file, a non-mapping file, a missing entry or an empty field all refuse to start naming the ref. A ref on a
PLAINTEXTleg refuses rather than silently collecting a credential it would never send. A ref alongside a literal refuses as ambiguous. A bare literal is left alone on purpose so the loader can re-read its ownmodel_dump()output, the same trap_materialize_contract_https_ingestrecords for its null round trip; that the shipped file carries no literal is asserted statically over the tracked YAML instead.Not a rotation
Operating Rule 22. This binds the principal OMN-18012 phase A provisioned. Nothing is rotated, re-issued or revoked, and no value appears in the diff, the tests, the error messages or the logs.
Test evidence
Red at parent
bcbbf9060: 15 failed, 1 passed. The single pass is the premise test asserting the stability source leg stays plaintext, which is correct at parent and must stay correct.Green at head: 284 passed across the forwarder node suite, the runtime loader, the compose-credential suite and the deploy-gateway suite. Governed pre-push selector green; no
PREPUSH_*override, no--no-verify, no hooks-path override.Deploy note
This changes a mounted-file contract: the operator must supply
GATEWAY_LANE_CREDENTIAL_MAP_FILEbefore the gateway project is next brought up, or the forwarder refuses to start. That refusal is the intended behaviour and is strictly better than the silent inertness it replaces.Ticket
OMN-18120
Evidence-Ticket: OMN-18120
Evidence-Source: OCC#8905