Skip to content

fix(providers issue #1939): accept http base URLs with any IP address - #2018

Open
jamesvren wants to merge 1 commit into
1jehuang:masterfrom
jamesvren:fix/http-any-ip-base-url
Open

jamesvren wants to merge 1 commit into
1jehuang:masterfrom
jamesvren:fix/http-any-ip-base-url

Conversation

@jamesvren

Copy link
Copy Markdown

Plaintext http was only accepted for localhost, *.local, and RFC 1918 / link-local / CGNAT addresses, so a self-hosted inference server on a public or otherwise unroutable-looking address (for example http://203.0.113.10:8080/v1) was rejected as an invalid base_url and the endpoint never reached the transport.

Any IPv4/IPv6 literal is now accepted for http, matching how these servers are actually reached. api_base_uses_localhost follows suit so a bare-IP endpoint still defaults to no API key, and the TUI plus env override messages name the accepted shapes.

… address

Plaintext http was only accepted for localhost, *.local, and RFC 1918 /
link-local / CGNAT addresses, so a self-hosted inference server on a
public or otherwise unroutable-looking address (for example
http://203.0.113.10:8080/v1) was rejected as an invalid base_url and the
endpoint never reached the transport.

Any IPv4/IPv6 literal is now accepted for http, matching how these
servers are actually reached. api_base_uses_localhost follows suit so a
bare-IP endpoint still defaults to no API key, and the TUI plus env
override messages name the accepted shapes.
@greptile-apps

greptile-apps Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes affect how users configure OpenAI-compatible provider endpoints.

Do not merge until the failing IPv6 assertion is corrected or the required URL and transport support is implemented.

Findings

  1. P1 New IPv6 test fails ▶
Fix with agent prompt
### Issue 1
crates/jcode-provider-metadata/src/lib.rs:404-407
`url::Url::parse` rejects `http://[fe80::1%25eth0]:8080/v1`, so `normalize_api_base` returns `None` before `ip_addr_from_host` can strip the zone ID. The new assertion expects `Some(...)`, causing `normalize_api_base_accepts_any_ip_http_host` and the metadata crate's test suite to fail. Running the normalization tests confirms this failure with exit code 101.

Remove this assertion and keep zone-qualified URLs unsupported, or add support through both URL parsing and the request transport before merging.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Allows HTTP provider URLs with any IPv4 or IPv6 address.

  • HTTP provider URLs now accept any IP address.
  • IP-based provider URLs now default to no API key.

Acknowledgment: jamesvren explicitly describes public-IP HTTP access and bare-IP no-key defaults as intentional. Plaintext exposure from that choice is not reported as a separate finding.

Reviews (1) · Last reviewed commit: "fix(providers issue #1939): accept http ..." · Reviewed by Greptile

Comment on lines +404 to +407
assert_eq!(
normalize_api_base("http://[fe80::1%25eth0]:8080/v1").as_deref(),
Some("http://[fe80::1%25eth0]:8080/v1")
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 New IPv6 test fails

url::Url::parse rejects http://[fe80::1%25eth0]:8080/v1, so normalize_api_base returns None before ip_addr_from_host can strip the zone ID. The new assertion expects Some(...), causing normalize_api_base_accepts_any_ip_http_host and the metadata crate's test suite to fail. Running the normalization tests confirms this failure with exit code 101.

Remove this assertion and keep zone-qualified URLs unsupported, or add support through both URL parsing and the request transport before merging.

Artifacts

Test command at PR HEAD

  • This executed shell command runs the exact supplied metadata test filter from the target checkout, making the failing invocation reproducible.

Test output at PR HEAD

  • The supplied test command ran at PR HEAD and failed at line 404 with None instead of the expected scoped IPv6 URL, confirming the assertion failure.

Test command at the parent commit

  • This executed shell command runs the same metadata test filter in the detached parent checkout using the warmed build cache, providing the baseline invocation.

Passing test output at the parent commit

  • The same test filter ran against the parent commit and passed both tests with exit zero, establishing the before-change baseline.

Source and checkout inspection command

  • This executed command records commit identities, tracked-file status, the relevant diff, numbered source excerpts, and the locked URL version, making the source evidence traceable.

Relevant source and commit evidence

  • The captured inspection output shows the exact assertion, early URL parsing, later zone stripping, and unchanged tracked files, locating the cause in the supplied finding.

Executed IPv6 URL parser probe source

  • The compiled probe asserts InvalidIpv6Address for the scoped URL and successful parsing without the zone ID, isolating the claimed parser behavior.

Parser probe compilation and execution command

  • This executed command compiles the attached Rust probe against the cached URL library and runs it, making the parser check reproducible.

Parser rejection and successful control output

  • The probe prints Err(InvalidIpv6Address) for the zone-ID URL and Ok for the unscoped control, confirming that parsing rejects the input before host processing.

View artifacts

TREX Ran code and verified through TREX

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-provider-metadata/src/lib.rs
Line: 404-407

Comment:
**New IPv6 test fails**

`url::Url::parse` rejects `http://[fe80::1%25eth0]:8080/v1`, so `normalize_api_base` returns `None` before `ip_addr_from_host` can strip the zone ID. The new assertion expects `Some(...)`, causing `normalize_api_base_accepts_any_ip_http_host` and the metadata crate's test suite to fail. Running the normalization tests confirms this failure with exit code 101.

Remove this assertion and keep zone-qualified URLs unsupported, or add support through both URL parsing and the request transport before merging.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant