fix(internal): restrict isAbsoluteURL scheme regex to http(s)#1888
Open
ibondarenko1 wants to merge 1 commit into
Open
fix(internal): restrict isAbsoluteURL scheme regex to http(s)#1888ibondarenko1 wants to merge 1 commit into
ibondarenko1 wants to merge 1 commit into
Conversation
The SDK typed resource methods use hardcoded relative paths, so the only schemes isAbsoluteURL needs to recognize as absolute are http and https. The previous regex matched any RFC 3986 scheme (file:, gopher:, ftp:, data:, javascript:), which could bypass baseURL when an absolute-URL path is passed to client.get / client.post.
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
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.
Problem
isAbsoluteURLatsrc/internal/utils/values.ts:6-10uses/^[a-z][a-z0-9+.-]*:/i. The regex follows the WHATWG URL spec definition of "URI scheme string" and matches any RFC 3986 scheme. When an absolute-URL path reachesbuildURL, the path is used as the complete URL vianew URL(path)andbaseURLis bypassed.The SDK's typed resource methods (
chat,completions,embeddings, etc.) all use hardcoded relative paths. The only legitimate consumer use of absolute-URL paths is via the rawclient.get/client.postmethods, and those should only construct HTTP or HTTPS requests.Schemes accepted by the current regex that have no legitimate use here include
file:,gopher:,ftp:,data:,javascript:.Change
One regex narrow in
src/internal/utils/values.ts:6-10. The new pattern accepts^https?:only. A comment explains the SDK-side reasoning so a future contributor does not loosen the regex back to the spec form.Impact
client.get("file:///etc/passwd"),client.get("gopher://internal/..."),client.get("data:text/html,..."),client.get("javascript:...")are now treated as relative paths rather than absolute, sobuildURLrejects them via the standardnew URL(baseURL + path)parse failure.http://andhttps://absolute paths continue to work.Backwards compatibility
http://orhttps://absolute paths to rawclient.get/client.post: no behavior change.client.get: previously bypassedbaseURL; now treated as a relative path. This was not a documented use case and there are no resource methods that generate such schemes, so no internal SDK call site is affected.Tests
yarn prettier --check src/internal/utils/values.ts: clean.yarn eslint src/internal/utils/values.ts: clean.yarn jest tests/internal tests/path.test.ts tests/index.test.ts: 212 pass.Full
yarn testrequires the Stainless mock server (steady on port 4010) which is not available on the contributor side. The change is type-neutral and narrows the matching set; tests that previously passed with the broader regex still pass with the narrower one as long as they do not depend on a non-HTTP scheme.Why this is a hardening contribution
I was reading the SDK's request pipeline and noticed the regex breadth. The SDK has no documented call site that constructs non-HTTP schemes, so the regex can be narrowed without changing intended behavior.