fix(security): route allowlisted provider validation through EgressWeave

[seonghobae]

Summary

• ModelClient._validate_provider's allowlisted-host branch replaces its hand-rolled resolve-then-check SSRF loop with EgressWeave's validate_egress_url_details (CWE-918/CWE-350).
• EgressWeave's already-validated, already-resolved addresses are reused directly to build the ProviderDestination — re-resolving would reopen the validate-then-connect DNS-rebinding gap EgressWeave closes.
• The no-allowlist-configured default (the common case today) keeps the prior hand-rolled check unchanged, since EgressWeave's allowlist model can't express "no host restriction" — an empty allowlist there rejects every host, not none.
• The local-provider branch is untouched; it's self-contained and never used EgressWeave's allowlist mechanism.

Resolves #1041.

Implementation notes (scoped in the issue first)

The PyPI-published egressweave is 0.1.0 — materially simpler than the unreleased main branch I initially read (no port-authority dimension, just a host allowlist, matching this codebase's existing host-only allowed_provider_hosts semantics exactly). See the issue comments for the full scoping trail, including the correction after downloading and inspecting the actual installable wheel.

• Added egressweave>=0.1.0,<0.2.0 to pyproject.toml.
• Regenerated requirements.lock via uv pip compile --universal --python-version 3.10 --extra=api --extra=db --extra=queue --generate-hashes --output-file=requirements.lock pyproject.toml (universal mode was necessary to preserve the existing python_full_version >= '3.12' markers on fast-mlsirm/numpy — a plain single-interpreter compile silently dropped them).
• Regenerated uv.lock via uv lock.
• Rewrote test_provider_allowlist_ignores_request_time_environment_changes: it previously mocked ModelClient._resolve_addresses, a seam EgressWeave's allowlisted path no longer calls (EgressWeave resolves DNS itself via socket.getaddrinfo). Now patches at that shared boundary instead.
• Added test_provider_allowlist_rejects_allowlisted_host_resolving_to_private_address covering the case an allowlisted host still needs address-safety checking.

Verification

• PYTHONPATH=. python -m pytest tests/test_security_hardening.py -q — 26 passed.
• PYTHONPATH=. python -m pytest tests/ -q (full suite, pre-lock-regen venv) — 3391 passed, 2 skipped.
• uv run --locked --extra api --extra db --extra queue --group dev python -m pytest -q (exact CI command, post-lock-regen) — 3391 passed, 2 skipped in 657s.
• python -m pip install --disable-pip-version-check --no-input --require-hashes -r requirements.lock (exact security.yml command) — installs cleanly.
• python -m pip_audit -r requirements.lock — no known vulnerabilities found.

Developer experience

No API surface change for existing no-allowlist deployments. Allowlist-configured deployments gain a stricter, more thoroughly-audited SSRF/DNS-rebinding guard at the cost of two new transitive dependencies (httpx, httpcore) that this codebase doesn't otherwise use — accepted per the issue's scoping discussion since they're EgressWeave's own hard package dependency regardless of which of its symbols are imported.

User experience

No behavior change for the default (no-allowlist) configuration. Allowlist-configured deployments get equivalent-or-stricter address-safety rejection (EgressWeave's is_global check catches more reserved-address classes than the prior hand-rolled is_private/is_loopback/is_link_local/is_multicast/is_reserved check).

🤖 Generated with Claude Code

Summary by CodeRabbit

• 보안 강화
  • 허용된 프로바이더 호스트의 DNS 재바인딩 및 사설 주소 접근 검증을 강화했습니다.
  • 검증된 주소와 실제 연결 대상이 일치하도록 보호 기능을 개선했습니다.
• 버그 수정
  • 허용 목록에 등록된 호스트가 사설 IP로 해석될 때 연결이 거부됩니다.
• 문서
  • 보안 변경 사항을 변경 로그에 추가했습니다.

Current restack authority

• exact head: 7ebe9128245c3ad212f8f2d6456bd6d968cc5139
• base: trusted NIM successor fix(nim): preserve current hosted-access evidence on trusted branch #1068 at fa5446294ae7ae69f1c2958aa1ab6c071fa760bc
• effective diff: the original EgressWeave boundary files only; successor ancestry removes duplicated NIM evidence from the three-dot diff
• focused security/workflow tests: 47 passed
• NIM expiry and security regressions: 59 passed
• exact stacked full suite: 3396 passed, 2 skipped
• git diff --check: pass

The prior Noema and Strix failures were provider-infrastructure failures, not transferable source approvals. Fresh hosted checks and current-head review remain required. #1063 stays open until #1068 reaches protected main and exact-tree equivalence is verified.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 3e67ebda-679d-476e-9e92-9aae56a07317

📥 Commits

Reviewing files that changed from the base of the PR and between c594b68 and dca1084.

⛔ Files ignored due to path filters (2)

• requirements.lock is excluded by !**/*.lock
• uv.lock is excluded by !**/*.lock

📒 Files selected for processing (4)

• CHANGELOG.md
• contextual_orchestrator/orchestrator.py
• pyproject.toml
• tests/test_security_hardening.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

허용 목록이 설정된 프로바이더 호스트 검증을 EgressWeave에 위임합니다. EgressWeave가 검증한 IPv4 또는 IPv6 주소를 연결 대상에 사용합니다. DNS 해석 경계를 반영하도록 테스트를 수정하고 사설 주소 거부 테스트를 추가합니다.

Changes

프로바이더 Egress 검증

Layer / File(s)	Summary
EgressWeave 검증 경로 통합 pyproject.toml, contextual_orchestrator/orchestrator.py	egressweave 의존성을 추가합니다. 허용 목록 경로에서 EgressPolicy와 validate_egress_url_details를 사용합니다. 검증된 주소를 ProviderDestination으로 변환합니다.
보안 회귀 검증 및 변경 기록 tests/test_security_hardening.py, CHANGELOG.md	테스트가 socket.getaddrinfo 경계를 패치하도록 변경됩니다. 허용된 호스트가 사설 주소로 해석될 때 거부되는지 검증합니다. 보안 변경 사항을 기록합니다.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to dca10

Allowlisted provider URLs now receive SSRF and DNS-rebinding validation while preserving existing behavior when no allowlist is configured. The validated address is used for the provider connection, and the available regression coverage includes private-address rejection.

Sequence Diagram(s)

sequenceDiagram
  participant ModelClient
  participant EgressWeave
  participant DNSResolver
  ModelClient->>EgressWeave: EgressPolicy와 프로바이더 URL 전달
  EgressWeave->>DNSResolver: 호스트 주소 해석
  DNSResolver-->>EgressWeave: 주소 목록 반환
  EgressWeave-->>ModelClient: 검증된 주소 또는 거부 오류 반환

Loading

Suggested reviewers: claude

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (3 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	PR은 이슈 #1041의 관련 범위인 allowlisted provider 검증의 EgressWeave 위임을 구현합니다. 기존 no-allowlist 경로와 local-provider 처리를 유지하고, 의존성과 회귀 테스트도 추가했습니다. 잠금 파일은 경로 필터로 제외되어 검증 대상에서 제외됩니다.
Out of Scope Changes check	✅ Passed	변경 사항은 EgressWeave 기반 provider egress 검증, 의존성 갱신, 보안 변경 기록, 관련 테스트로 구성됩니다. 제공된 범위에서 이슈와 무관한 변경은 확인되지 않습니다.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	제목은 allowlisted provider 검증을 EgressWeave로 라우팅하는 핵심 변경을 정확하고 간결하게 설명합니다.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (3 skipped: 2 unsupported, 1 too large.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch egressweave-provider-ssrf-check-1041

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[seonghobae]

최신 main 병합 및 provider-validation slice 검증

PR head 78cbf732c0b28899470e3804f1468c8eafc9e094는 protected main@60c562defc81fb1897fa97ebdb5bf8f69eae0c55를 non-force merge했습니다. 이 PR의 완료 범위는 목표 #7 전체가 아니라 provider validation slice입니다.

• 의존성: pyproject.toml은 egressweave>=0.1.0,<0.2.0, requirements.lock과 uv.lock은 0.1.0 및 httpx/httpcore 전이를 포함합니다. uv lock --check와 Python 3.10 pip --require-hashes --dry-run이 통과했습니다.
• fail-closed: allowlist 불일치, DNS 실패, 빈 주소, 비공개 주소는 EgressWeave의 EgressNotAllowedError 경계에서 provider validation 실패로 닫힙니다. 검증 결과가 없거나 주소 변환이 실패해도 연결로 진행하지 않습니다.
• DNS pinning/TLS/SNI: EgressWeave가 검증한 첫 주소를 ProviderDestination으로 직접 넘겨 재해석하지 않습니다. _open_provider는 그 sockaddr로 연결하면서 HTTPSConnection에는 원래 URL hostname을 유지하므로 인증서 검증과 SNI가 보존됩니다. no-allowlist 및 local-provider 경로는 기존 구현을 유지합니다.

검증 결과:

• Python 3.10 focused security/provider/lock contracts: 42 passed
• Python 3.14 expanded security/provider/local transport/lock contracts: 91 passed
• uv lock --check: pass
• git diff --check: pass
• 미해결 review thread: 0

현재 Ready 상태는 유지합니다. Auto-merge가 안전하려면 이 exact head의 required tests/security scans가 모두 명시적 성공 상태가 되고, 최신 head에 유효한 필수 robot/human review가 충족되며, protected-branch ruleset이 merge를 허용해야 합니다. 대기 중인 체크를 이전 head 결과로 대체하지 않습니다.

[seonghobae]

Status note, since a cross-session triage described this PR as a lock-file change conflicting with #1030 on requirements.lock/uv.lock/pyproject.toml. That is not what is actually blocking it.

This is a stacked PR, not a main-based one. gh pr view 1046 --json baseRefName → base is codex/nim-evidence-successor@fa544629, not main. That is why GitHub reports 6 changed files while git diff $(git merge-base origin/main <head>) <head> reports 13: the extra seven come from the branches underneath it.

The real conflict against main is the NIM benchmark surface, not the lock files. Test-merging the head onto origin/main (a080297) leaves exactly four conflicted paths:

contextual_orchestrator/nim_benchmark.py
docs/doctoring/nim-benchmark-evidence-grade.md
docs/nim_benchmark.md
tests/test_nim_benchmark_release_acceptance.py

pyproject.toml, requirements.lock and uv.lock merge cleanly — git does not list them as conflicted. The cause is main's a080297 ("fix(benchmark): refresh NVIDIA cost evidence", #1073) landing the same evidence refresh this stack carries in its own commits (1db3c625, 1d5221f3, and the earlier 9928e4ab/efd8032a).

So resolving this is NIM-benchmark work, not dependency-lock work. Whoever owns that surface should take the resolution; the EgressWeave delta itself (egressweave>=0.1.0,<0.2.0 plus the orchestrator.py change) is untouched by the conflict.

One more thing worth flagging: commit 6e7a08fc in this stack carries the same change as #1060 ("ci(security): run checks on stacked pull requests") — same message and same security.yml edit, though a different commit object than #1060's head 489fedc6, so it is a content match rather than the same commit. If #1060 is closed in favour of #1066 (as I have recommended there on the evidence), that decision needs to account for the copy riding inside this stack, or the two will diverge.

All checks run on a clone verified 0 commits behind origin/main.