test(fallback): verify the credential-name trust boundary - #100
Closed
seonghobae wants to merge 48 commits into
Closed
test(fallback): verify the credential-name trust boundary#100seonghobae wants to merge 48 commits into
seonghobae wants to merge 48 commits into
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
Require the policy-only CLI to accept an explicit trusted set of available credential names, reject the legacy environment selector, and prove that planning never reads environment values.
Add a permanent read-only exact-head workflow that runs the fallback policy behavior tests, 100% statement/branch coverage, 100% docstrings, compilation, and diff checks. The current test-first head is expected to fail before the CLI boundary is repaired.
Run the permanent read-only fallback quality gate for path-matching pushes as well as pull requests so a stacked non-default-base branch produces exact-head evidence instead of waiting for its ancestor to merge.
|
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 |
Preserve ordinary locale/process environment access used by argparse while failing specifically on attempts to inspect configured provider credential values. The feature remains red until the CLI accepts declarative names and removes the environment selector.
Contributor
Author
|
Temporary verification is complete on exact head The preserved red run demonstrated that the former CLI read credential values through Closing this temporary PR without merge. Product integration and the remaining repository-wide exact-head gates continue in #94 after prerequisite #96. |
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 verification completed — do not merge
This Draft PR existed only to obtain pull-request-triggered exact-head evidence for stacked product PR #94.
Exact head
0b04029363438aa490c885ac978f51788e83e0dcpassed permanent read-onlyModel fallback policy qualityrun30989893429:The temporary PR is now closed without merge. Product integration and all full-stack gates remain on Draft PR #94 after #96.