Skip to content

fix(OMN-14162): drop hardcoded LAN Kafka default from master env template - #2239

Merged
jonahgabriel merged 1 commit into
devfrom
jonah/omn-14162-kafka-tailscale-bootstrap
Jul 9, 2026
Merged

jonahgabriel merged 1 commit into
devfrom
jonah/omn-14162-kafka-tailscale-bootstrap

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Jul 8, 2026 •

Copy link
Copy Markdown
Collaborator

Evidence-Source: OCC#3738
Evidence-Ticket: OMN-14162

Summary

OMN-14162: local skill-dispatch Kafka bootstrap was pointed at a LAN-only default via omnibase_infra/docs/env-master-template.env, which every consuming repo (omniclaude, omniintelligence, omnidash, omnimemory, omnimarket) is documented to copy for local .env setup.

  • docs/env-master-template.env:28 hardcoded KAFKA_BOOTSTRAP_SERVERS=192.168.86.201:19092. 192.168.86.201 is a LAN IP, unroutable off-network — copying this template while traveling silently breaks local dispatch with no error until the Kafka client times out.
  • Blanked the default and added a comment documenting both valid addresses so the operator picks the one that matches their network:
    • On-LAN: 192.168.86.201:19092
    • Off-network / Tailscale: 100.109.203.94:39092 (omninode-pc.tail75df5e.ts.net:39092)

This matches the repo's existing fail-fast convention — every real (non-doc) call site already reads KAFKA_BOOTSTRAP_SERVERS from the environment with no default (kafka_publisher_base.get_kafka_bootstrap_servers() explicitly removed a localhost default under OMN-7227 "to prevent silent local connections"; omniclaude/.env.example already ships with KAFKA_BOOTSTRAP_SERVERS= blank). The only actual defect was this doc template presenting a LAN IP as the value to copy in.

Scope: this repo's docker-compose files and the .201-server advertise-listener defaults (docker-compose.generated.yml, catalog/services/redpanda.yaml) legitimately hardcode 192.168.86.201 for container/advertise addressing and are untouched — they're not part of the local-macOS skill-dispatch bootstrap path this ticket targets.

Test plan

  • bash scripts/validation/check_kafka_no_hardcoded_fallback.sh — passes (this hook only scans .py/.sh/.yaml/.yml; confirms no functional Kafka-fallback gate regresses)
  • pre-commit run --files docs/env-master-template.env — all applicable hooks pass
  • Confirmed scripts/validate_env.py already treats KAFKA_BOOTSTRAP_SERVERS as conditionally-required (not defaulted), so blanking the template value doesn't change validator behavior
  • Confirmed no script sources docs/env-master-template.env programmatically (grep -rl env-master-template) — it's a manual copy-and-edit doc, so CI/docker/.201-server paths are unaffected

Summary by CodeRabbit

  • Documentation
    • Clarified the Kafka setup template by marking the broker setting as required.
    • Removed the default LAN broker value and replaced it with an empty placeholder.
    • Added guidance for both off-network and LAN-accessible configurations, plus example broker endpoints.

@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The Kafka bootstrap servers template setting in docs/env-master-template.env was changed from a hardcoded LAN IP default to an empty required value, with added comments explaining LAN vs. off-network broker address usage.

Changes

Kafka env template documentation

Layer / File(s) Summary
Kafka bootstrap servers template value and guidance comments
docs/env-master-template.env
Replaced the hardcoded LAN IP default for KAFKA_BOOTSTRAP_SERVERS with an empty required value and added comments explaining LAN vs. off-network/Tailscale broker addressing with example endpoints.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: removing the hardcoded LAN Kafka default from the master env template.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jonah/omn-14162-kafka-tailscale-bootstrap

Comment @coderabbitai help to get the list of available commands.

…S in master env template

The master env template hardcoded KAFKA_BOOTSTRAP_SERVERS=192.168.86.201:19092
as the value for omniclaude/omniintelligence/omnidash/omnimemory/omnimarket to
copy into a working .env. 192.168.86.201 is a LAN address, unroutable
off-network — copying this template while traveling silently points local
skill-dispatch at an unreachable broker.

Blank the default (fail-fast, matching the existing pattern in
omniclaude/.env.example and kafka_publisher_base.get_kafka_bootstrap_servers()
per OMN-7227) and document both the on-LAN and Tailscale broker addresses in
a comment so operators pick the one that matches their network.
@jonahgabriel
jonahgabriel force-pushed the jonah/omn-14162-kafka-tailscale-bootstrap branch from 8318e1d to c468280 Compare July 9, 2026 02:28
@jonahgabriel
jonahgabriel merged commit 010dec3 into dev Jul 9, 2026
121 of 131 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-14162-kafka-tailscale-bootstrap branch July 9, 2026 02:43
jonahgabriel added a commit that referenced this pull request Aug 31, 2026
…lic repo (#3074)

OMN-17288 scrubbed a live tenant slug out of five files in this repo (#3062,
3f10ee5) and established a synthetic-identifier convention in its place. Three
hours later an unrelated lane reintroduced the same slug in omnimarket (#2239),
and the rebase carried it onto omnimarket#2241 -- the PR whose own acceptance
criterion was "zero grep hits" -- with every enforced gate green. Nothing in
either repo was looking. The convention was documentation, and documentation
lost a race in three hours. Operating Rule #5: detection that is not a gate
gets ignored.

Why digests and not a plaintext pattern list: this repo is PUBLIC. Writing a
forbidden customer identifier into a pattern file here would create exactly the
fresh, greppable, current-tree occurrence the class exists to prevent, and would
force that file to be exempt from its own rule -- a special file holding the
forbidden value, that nobody scans and that people copy from. That is the shape
of the incident, not a fix for it. Entries are salted SHA-256 plus a class label
and owning ticket; the loader refuses to load an entry carrying a value/literal/
plaintext field. The salt is committed, so this is obfuscation and not secrecy,
and that is stated in the file rather than implied: the OMN-17288 values are
already public in git history and the operator ruled document-and-accept on that
history. What the format buys is FORWARD safety -- the next entry may be a live
identifier that has NOT leaked, where a plaintext denylist would be an active
disclosure.

Matching windows inside each identifier token, so a literal is caught bare,
embedded (tenant_<slug>_v2), and inside a path or URL segment. Findings print
path:line:col plus match length and entry id, never the value. Encoded forms are
deliberately NOT decoded -- OMN-17180 owns that class, and claiming coverage
here would be a false claim.

One escape hatch, per line, ticket + reason required:
  # onex-allow-exposed-identifier OMN-XXXXX reason="<concrete reason>"
A bare annotation is rejected, matching every other onex-allow class. There is
deliberately NO file-level waiver and NO self-exempt file: a whole-file waiver is
how a forbidden value survives in a corner nobody reads. The gate is subject to
its own rule.

Wired in this same PR on both surfaces (Rule #5):
- pre-commit hook `exposed-identifier-gate`
- CI job `Exposed Identifier Gate (OMN-17320)` in ci.yml, registered in
  scripts/ci/ci_summary_gate.py::STRICT_GATE_JOBS. That registration is half the
  mechanism: dev requires exactly one context (CI Summary), and while the
  default-deny sweep already fails on a FAILING job, an unregistered job that is
  skipped or deleted yields SUCCESS -- so without it, removing this job would
  silently restore the unenforced state that produced the recurrence.

Evidence:
- Incident replay (OMN-15547 convention) over the real pre-scrub artifact,
  captured from git object 6527db3 (= 3f10ee5^). ONE same-length redaction of
  the slug, recorded in registry.yaml with the pre-redaction sha256 so the git
  object can be re-fetched and diffed; every other byte of the 8455 is verbatim
  and offsets are preserved, so the finding is asserted at the slug's real
  position (line 7, col 379). An accept-control in the same module requires the
  SHIPPED denylist to PASS those same bytes, so a reject-everything guard cannot
  satisfy the case.
- Non-vacuity of all five real entries proven OUT OF TREE (committed tests cannot
  carry this without defeating the gate's own rule): pre-scrub content extracted
  from git objects identifies the slug, slug-body, uuid and uuid-prefix entries
  by digest, and the uuid-hex entry is confirmed by transforming the recovered
  UUID. Scanner exit 1 with 15 findings across bare/embedded/path-segment forms.
- 29 tests pass; full-tree scan clean.

Ticket: OMN-17320
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant