Skip to content

fix(OMN-10732): remove silent env fallbacks in omnibase_infra src/ and wire validator gate - #1543

Merged
jonahgabriel merged 1 commit into
mainfrom
jonah/omn-10732-fallback-sweep-infra
May 9, 2026
Merged

jonahgabriel merged 1 commit into
mainfrom
jonah/omn-10732-fallback-sweep-infra

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented May 9, 2026 •

Copy link
Copy Markdown
Collaborator

OMN-10732

Fixes all 6 src/ silent fallback violations and annotates the 7 approved 127.0.0.1 bind-address defaults. Wires the existing validator into pre-commit and CI as a required gate.

Changes

src/ violations fixed (fail-fast):

  • runtime/models/model_postgres_pool_config.py:30 — POSTGRES_HOST default_factory → os.environ["POSTGRES_HOST"]
  • runtime/models/model_postgres_pool_config.py:222 — from_dsn() raises ValueError on missing hostname instead of silently falling back to localhost
  • backends/backend_probe.py:194 — PGHOST fallback → os.environ["PGHOST"]
  • services/agent_learning_extraction/config.py:34 — hardcoded http://localhost:8001 → os.environ["LLM_CODER_FAST_URL"]
  • adapters/models/model_infisical_config.py:35 — https://app.infisical.com → os.environ["INFISICAL_ADDR"]
  • handlers/models/infisical/model_infisical_handler_config.py:39 — same

127.0.0.1 bind-address defaults annotated fallback-ok (7 fields across health/metrics servers): bind 127.0.0.1 by default to prevent unauthenticated network exposure; override via env var for cluster deployments.

Validator + hook wiring:

  • scripts/validate_no_env_fallbacks.py — skip lines annotated fallback-ok
  • .pre-commit-config.yaml — new no-env-localhost-fallback hook
  • .github/workflows/ci.yml — new CI step

Tests updated:

  • tests/integration/adapters/test_postgres_pool_config_defaults.py — replaced fallback-expectation test with fail-fast assertion
  • tests/unit/runtime/test_dependency_materializer.py — updated test_default_values to set POSTGRES_HOST; added test_default_values_missing_host_raises; fixed test_frozen to pass explicit host=

OCC Evidence

Contract and DoD receipts in onex_change_control PR #873 (jonah/omn-10732-occ-contract):

  • contracts/OMN-10732.yaml — ticket contract with 4 DoD evidence items
  • drift/dod_receipts/OMN-10732/dod-tests/command.yaml — 52 tests PASS
  • drift/dod_receipts/OMN-10732/dod-validator/command.yaml — validator PASS
  • drift/dod_receipts/OMN-10732/dod-deploy/command.yaml — no deploy required (PASS)
  • drift/dod_receipts/OMN-10732/dod-occ-pr/command.yaml — OCC PR feat(omnibase_infra): add circuit breaker event emission [OMN-5293] #873 receipt (PASS)

dod_evidence

…d wire validator gate

Fix 6 src/ violations: replace os.environ.get(KEY, "localhost/url") with fail-fast
os.environ[KEY]; annotate 7 approved 127.0.0.1 bind-address defaults with # fallback-ok.

- model_postgres_pool_config.py: POSTGRES_HOST default_factory → os.environ["POSTGRES_HOST"];
  from_dsn() raises ValueError on missing hostname instead of silently using localhost
- backend_probe.py: PGHOST fallback → os.environ["PGHOST"]
- agent_learning_extraction/config.py: hardcoded LLM URL → os.environ["LLM_CODER_FAST_URL"]
- model_infisical_config.py + model_infisical_handler_config.py: Infisical URL → os.environ["INFISICAL_ADDR"]
- 7 health/metrics bind-address 127.0.0.1 defaults annotated # fallback-ok (security rationale)
- validate_no_env_fallbacks.py: skip lines annotated # fallback-ok
- Wire validator into pre-commit (no-env-localhost-fallback) and CI as required check
- Update tests: replace fallback-expectation with fail-fast assertions
@coderabbitai

coderabbitai Bot commented May 9, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@jonahgabriel has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 24 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 91e84f58-6015-4525-9898-9938f27990fe

📥 Commits

Reviewing files that changed from the base of the PR and between a729db4 and 7640293.

📒 Files selected for processing (17)
  • .github/workflows/ci.yml
  • .pre-commit-config.yaml
  • scripts/validate_no_env_fallbacks.py
  • src/omnibase_infra/adapters/models/model_infisical_config.py
  • src/omnibase_infra/backends/backend_probe.py
  • src/omnibase_infra/handlers/models/infisical/model_infisical_handler_config.py
  • src/omnibase_infra/observability/handlers/model_metrics_handler_config.py
  • src/omnibase_infra/runtime/models/model_postgres_pool_config.py
  • src/omnibase_infra/services/agent_learning_extraction/config.py
  • src/omnibase_infra/services/observability/agent_actions/config.py
  • src/omnibase_infra/services/observability/consumer_health/config.py
  • src/omnibase_infra/services/observability/context_audit/config.py
  • src/omnibase_infra/services/observability/llm_cost_aggregation/config.py
  • src/omnibase_infra/services/observability/skill_lifecycle/config.py
  • src/omnibase_infra/services/post_merge/config.py
  • tests/integration/adapters/test_postgres_pool_config_defaults.py
  • tests/unit/runtime/test_dependency_materializer.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jonah/omn-10732-fallback-sweep-infra

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

@jonahgabriel
jonahgabriel enabled auto-merge (squash) May 9, 2026 15:36
@jonahgabriel
jonahgabriel merged commit f608805 into main May 9, 2026
67 of 72 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-10732-fallback-sweep-infra branch May 9, 2026 16:14
jonahgabriel added a commit that referenced this pull request May 9, 2026
…rseded by no-env-fallbacks)

The no-env-localhost-fallback hook was added by OMN-10732 (#1543 merge into main).
The canonical no-env-fallbacks hook from this PR is a superset — remove the duplicate.
jonahgabriel added a commit that referenced this pull request May 9, 2026
… types (#1544)

* feat(OMN-10741): unified validate_no_env_fallbacks.py covering all 6 pattern types

Replaces the 4 divergent per-repo scripts with a single canonical implementation
that covers all patterns from the OMN-10658 enforcement sweep:
- os.environ.get("X", "localhost...")
- os.getenv("X", "localhost...")
- default="localhost..." (Pydantic Field / function param)
- ${VAR:-localhost} (shell scripts)
- Hardcoded 192.168.* IPs as default values
- 127.0.0.1 bind addresses as default values

Supports # fallback-ok: <reason> annotation to exempt justified lines.
Wired as pre-commit hook (no-env-fallbacks) in pass_filenames mode so it
only scans staged files, allowing phased cleanup of pre-existing violations.
Includes 30 unit tests covering all patterns, exemption markers, docstring
skipping, and the run() / run_on_files() entry points.

* chore(OMN-10741): trigger CI re-run after Evidence-Source added to PR body

* chore(OMN-10741): fix Evidence-Source format to OCC#870

* chore(OMN-10741): add Evidence-Ticket line to satisfy gate

* fix(OMN-10741): remove duplicate no-env-localhost-fallback hook (superseded by no-env-fallbacks)

The no-env-localhost-fallback hook was added by OMN-10732 (#1543 merge into main).
The canonical no-env-fallbacks hook from this PR is a superset — remove the duplicate.

* fix(OMN-10741): address fallback validator review

* fix(OMN-10741): annotate justified fallback exceptions

* test(OMN-10741): cover fallback validator integration

* fix(OMN-10741): add deploy gate contract

---------

Co-authored-by: jonahgabriel <jonahgabriel@users.noreply.github.com>
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