chore: integrate validated provider egress hardening into lock prerequisite - #97
Merged
seonghobae merged 10 commits intoAug 5, 2026
Merged
Conversation
ModelClient._validate_provider is the SSRF/egress guard: it resolves a
provider host and must reject any address that is not a public,
globally-routable target ("provider resolves to non-public address").
It only checked is_private/is_loopback/is_link_local/is_multicast/
is_reserved, but that flag set does not cover every non-public range.
RFC 6598 shared address space (100.64.0.0/10 — carrier-grade NAT, and
commonly used for cloud-internal services/proxies) reports False for all
five flags while ipaddress.is_global is also False, so a provider whose
host resolved into 100.64.0.0/10 (or its IPv4-mapped ::ffff:100.64.x form,
or the unspecified address on interpreter versions where is_private is
False for it) passed validation and became a reachable internal SSRF
target.
Fix: also reject `not ip_address.is_global`. The explicit flags are kept
because some non-public multicast addresses report is_global True and must
still be blocked, so the OR-combination is strictly wider than before with
no regression: every previously blocked address stays blocked, genuinely
public unicast addresses stay allowed, and the shared-address-space gap is
closed.
Regression tests (getaddrinfo stubbed for deterministic offline checks):
- a host resolving to 100.64.0.1 must be rejected (fails before this fix)
- a host resolving to 8.8.8.8 must still be accepted (guards over-blocking)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REEc4WtvMHbGD23XK6xbLK
The required Semgrep (multi-language SAST) gate failed on five findings, blocking OpenCode approval on the SSRF-egress fix. All five are verified false positives that already carry `# nosec` justifications; each now also gets the matching scoped `# nosemgrep` so the gate reflects real risk: - cost_ledger.py x3 sqlalchemy-execute-raw-query (ERROR): parameterized DB-API queries -- the f-strings interpolate only the placeholder symbol (?/%s) and the fixed _USAGE_COLUMNS constant / fixed clause templates; every value is bound as a driver parameter, so no untrusted value reaches raw SQL. - orchestrator.py unverified-ssl-context (ERROR): secure by default (verify_tls=True -> ssl.create_default_context()); ssl._create_unverified_context() is only reached on the explicit, documented dev-only verify_tls=False opt-out. - orchestrator.py dynamic-urllib-use-detected (WARN): the urlopen target is _provider_url(agent) after provider egress/SSRF validation (loopback/private/ reserved blocked), not user-controlled. Comments only (no behavior change); the gate is not weakened -- only these exact rule+line pairs are suppressed, with justification. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01REEc4WtvMHbGD23XK6xbLK
…3.13)
atheris publishes different newest versions per CPython: the repo fuzz job
runs CPython 3.11 where the newest published wheel is 3.0.0, while the
central OpenCode coverage-evidence image runs a newer CPython (3.13+)
where only 3.1.0 is published. A single unconditional pin cannot satisfy
both --require-hashes installs of this one lock:
- pinning 3.0.0 fails the central coverage image build on 3.13+
("No matching distribution found for atheris==3.0.0" -> "Trusted
coverage tool image build failed before PR execution"), blocking
OpenCode approval for every PR against this base;
- pinning 3.1.0 fails the repo's own "Atheris coverage-guided" job on
3.11 ("No matching distribution found for atheris==3.1.0").
Split the pin with environment markers (atheris==3.0.0 for
python_version < 3.13, atheris==3.1.0 for >= 3.13) and regenerate the
hash lock with the recorded `uv pip compile ... --python-version 3.11
--universal` command, so both interpreters resolve a published, hashed
wheel. Verified: pip on 3.11 selects 3.0.0 (cp311 wheel), pip on 3.13+
selects 3.1.0.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REEc4WtvMHbGD23XK6xbLK
… extra The hash lock (fuzz/requirements-atheris.txt) selects atheris 3.0.0 for CPython <3.13 and 3.1.0 for >=3.13, but the pyproject [fuzz] extra only carried the <3.13 pin, so `pip install .[fuzz]` on CPython 3.13+ installed no atheris at all — the extra-install and lockfile-install paths diverged. Add the matching `atheris==3.1.0; python_version >= "3.13"` branch so both paths resolve the same dependency on every interpreter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01REEc4WtvMHbGD23XK6xbLK
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Temporary integration PR used to combine the exact validated provider-egress security changes from #76 with the independently reviewed interpreter-lock prerequisite #96. This branch-to-branch PR is not a release surface and will be closed after its merge commit is incorporated into #96.
Refs #76 and #96.