Skip to content

fix: route egress-blocked requests to local fallback - #97370

Open
mrkillbob wants to merge 37 commits into
NousResearch:mainfrom
mrkillbob:codex/local-egress-fallback-clean
Open

mrkillbob wants to merge 37 commits into
NousResearch:mainfrom
mrkillbob:codex/local-egress-fallback-clean

Conversation

@mrkillbob

Copy link
Copy Markdown

Summary

Routes egress-blocked requests to a local fallback instead of failing outright.

Cherry-picked cleanly from the original working branch (which also carried an
unrelated large upstream-replay commit); this PR contains only the actual fix.

Commit: 7745fb1

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 28, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Overall: Adds egress-aware fallback routing so a request rejected by egress policy does not walk another remote provider.

What it does

  • agent/chat_completion_helpers.py adds _fallback_destination_class(fb: dict) resolving fallback base_url (falls back to load_config().providers[provider].api|base_url if fb.base_url empty, since fallback entries may omit base_url and rely on config), then classify_destination(provider, base_url, api_mode) from agent.llm_egress_firewall. Returns unknown if base_url still empty (cannot inherit local trust). Used by fallback selection to route egress-blocked requests to local fallback only.

Non-blocking notes

  • load_config() inside helper may be called per fallback entry; fallback count is small so overhead negligible, but consider caching if hot path.
  • Provider name not treated as security boundary — correctly resolves URL instead — good.

No blocking issues.

Non-blocking — please use your judgment.

@mrkillbob

Copy link
Copy Markdown
Author

Transplanted the local-only fallback routing from this PR into PR #95850.

Source PR commit: a3dd3f8
Target PR #95850 integrated commit / verified remote head: 15e43f1
Focused tests passed on #95850 after integration.

Mike DeMott and others added 9 commits September 2, 2026 16:34
…ithub-pr-feedback/2498d0fbd766c8a12c8b683425453e0ccfbe9bf4de13432a34c7101dd848b2fc

# Conflicts:
#	agent/auxiliary_client.py
#	agent/chat_completion_helpers.py
#	agent/codex_runtime.py
#	agent/coding_context.py
#	agent/context_references.py
#	agent/error_classifier.py
#	agent/kanban_stop.py
#	agent/prompt_builder.py
#	agent/system_prompt.py
#	agent/tool_executor.py
#	cli.py
#	hermes_cli/cli_agent_setup_mixin.py
#	hermes_cli/config_defaults.py
#	hermes_cli/dump.py
#	hermes_cli/kanban_db.py
#	run_agent.py
#	tools/file_tools.py
#	tools/kanban_tools.py
#	tui_gateway/server.py
…ithub-pr-feedback/5feed73d935692037817322f3f5564ea4a70c9ad4fa054e3bbe5cf29da68320a
@mrkillbobbot

Copy link
Copy Markdown

Hermes automated repair (pr-repair-steward)

Published the approved downstream integration at 8caab97. It includes repaired prerequisite dfced73 and preserves the local-only fallback rule in the refactored candidate filter. Remote and unknown destinations are skipped after egress denial without constructing another remote client. Relative to the repaired prerequisite, this follow-up changes two files (fallback logic and regression tests).

scripts/run_tests.sh passed all 536 focused fallback, classifier, firewall, runtime, provider-boundary and auxiliary tests. No full local-CI or merge-readiness claim.

@mrkillbobbot

Copy link
Copy Markdown

@codex review

…ithub-pr-feedback/11199c9eae143a4e50ec8580afb80d184124c5f1fbee19afcb557358bf80eab4

# Conflicts:
#	agent/prompt_builder.py
@mrkillbobbot

Copy link
Copy Markdown

Refreshed onto main at 08f170a and resolved the prompt backend probe merge conflict while preserving probe-only SSH setup and cleanup. Focused verification: python -m pytest -q tests/agent/test_prompt_builder.py -k "probe_remote_backend_ssh_is_probe_only_and_torn_down or probe_remote_backend_tears_down_its_sandbox or probe_remote_backend_tears_down_sandbox_on_failure" (3 passed, 74 deselected). Resolved head: f004b9e.

@mrkillbobbot

Copy link
Copy Markdown
Refreshed onto main at 08f170a and resolved the prompt backend probe conflict. Focused verification passed: 3 tests, 74 deselected.

@mrkillbobbot

Copy link
Copy Markdown

@codex review

@mrkillbobbot

Copy link
Copy Markdown

Rechecked the exact PR head f004b9e. The automated note is non-blocking: the fallback helper already resolves an omitted base_url through the configured provider URL and only permits local/loopback destinations after an egress denial. I evaluated caching the config lookup, but kept the current bounded implementation so it preserves live config resolution; the focused egress fallback tests pass (2 passed, 29 deselected). No additional code change is warranted for this note.

@mrkillbobbot

Copy link
Copy Markdown

Validated at exact PR head f004b9e: the reported local-only fallback routing is already present via ancestor commit a3dd3f8, so no additional patch or push was needed. Focused verification: ./.venv/bin/python -m pytest tests/run_agent/test_provider_fallback.py -q — 31 passed in 8.06s.

@mrkillbobbot

Copy link
Copy Markdown

Confirmed at commit f004b9e. Focused verification: python -m pytest -q tests/agent/test_prompt_builder.py -k "probe_remote_backend_ssh_is_probe_only_and_torn_down or probe_remote_backend_tears_down_its_sandbox or probe_remote_backend_tears_down_sandbox_on_failure" — 3 passed, 74 deselected.

…ithub-pr-feedback/88c8f954300c4cc462c771875c3cbcf70cc7a5785930d49e241e440a1f0e4655

# Conflicts:
#	tools/file_tools.py
@mrkillbobbot

Copy link
Copy Markdown

Merged main at 693641a and pushed merge commit e6999af. Focused pytest: 30 passed, 1 failed because FOREGROUND_MAX_TIMEOUT evaluated to 3570 while the test expects 600; no test or gate was weakened.

@mrkillbobbot

Copy link
Copy Markdown

Fixed the test fixture to clear TERMINAL_MAX_FOREGROUND_TIMEOUT before each foreground-timeout test, isolating the documented 600-second default from worker runtime overrides. Focused pytest: 16 passed.

@mrkillbobbot

Copy link
Copy Markdown

@codex review

…ithub-pr-feedback/e08ea998e93c2b2071e930c0461eb132e2cda6a1868377e2f7ac8c9deb67f9c2

# Conflicts:
#	hermes_cli/config_defaults.py
@mrkillbobbot

Copy link
Copy Markdown

Repaired the merge conflict against target base 08b140d and pushed commit 6c27c3f. Focused checks: scripts/run_tests.sh tests/hermes_cli/test_gpt56_registration.py tests/agent/test_context_estimator_multimodal.py -q (10 passed).

@mrkillbobbot

Copy link
Copy Markdown

@codex review

@mrkillbob

Copy link
Copy Markdown
Author

@Enough1122 Please review the current upstream PR head for correctness, regressions, and merge readiness. This request is specifically for your AI review; do not route it to Codex.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Reviewed current head 6c27c3ff0e72 — 1 blocking issue, 1 non-blocking note.

Blocking

Auxiliary egress binding omits the nous-portal / nousresearch aliases, so aux calls on those labels bypass the firewall entirelyagent/auxiliary_client.py:2443-2456, cf. agent/chat_completion_helpers.py:685

The main-agent gate _EGRESS_PROTECTED_PROVIDERS (chat_completion_helpers.py:685) covers five labels: anthropic, openai-codex, nous, nous-portal, nousresearch. But _AUX_EGRESS_PROVIDERS (auxiliary_client.py:2443) covers only three — nous-portal and nousresearch are missing — and _normalize_aux_provider (auxiliary_client.py:539) has no alias mapping those names to nous, so they pass through unchanged, fail the membership test at line 2455, and _auxiliary_egress_binding returns None, which makes _authorize_auxiliary_request invoke the provider callback with the raw kwargs and no authorization. The same labels are reachable on the aux path: _normalize_aux_provider("main") resolves through _read_main_provider(), so a session whose main provider is nous-portal/nousresearch sends auxiliary traffic (compression, triage/router calls — same source-bearing content the main path scans) with zero firewall coverage and no egress-denial fallback. Fix: add the two aliases to _AUX_EGRESS_PROVIDERS (safer than touching _normalize_aux_provider, which affects routing).

Non-blocking

Local fallback is unreachable for hostname-based local endpoints (http://localhost:11434)agent/auxiliary_egress_recovery.py:19-23 combined with classify_destination hostname handling. local_fallback_steps only accepts LOCAL_PROCESS/LOOPBACK, and classify_destination deliberately classifies localhost (non-numeric hostname, no DNS trust) as REMOTE, so a conventional Ollama/vLLM-at-localhost fallback candidate is silently skipped and the original EgressBlocked is re-raised with no local retry. Fail-closed, but it narrows the PR's core promise (egress-blocked → local fallback) to numeric-literal loopback URLs and in-process modes. Consider resolving localhost explicitly or documenting the numeric-literal requirement for fallback endpoints.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants