fix(sdk): guard the generated TypeScript http.ts chokepoint against dot-segment path params - #936
Open
AmirF194 wants to merge 1 commit into
Open
Conversation
…ot-segment path params RequestContext's constructor hands its templated url straight to new URL(), which silently collapses a "." or ".." path segment before Middleware.pre() or RetryHttpLibrary.send() ever see the request, so a caller-controlled value like a domain of ".." retargets a call at a different, larger-scoped resource. tokencanopy#929 closed this for the Python SDK's single param_serialize chokepoint; the TypeScript generated layer had no equivalent, and tokencanopy#915 explains why the two proposed client-side hooks can't see the collapse in time. Since http.ts is regenerated by make generate and isn't in .openapi-generator-ignore, the guard is injected by a new codegen post-processing step (scripts/guard-dot-segment-path.py, wired into generate-oag.sh) rather than hand-edited, mirroring the existing guard-optional-header-params.py pattern. Fixes tokencanopy#915
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.
Summary
#915 (filed during review of #909) shows that the per-call-site dot-segment
guard in the ergonomic wrapper classes never runs if a caller uses a
generated
*Apiclass directly, or MCP/CLI code that goes straight throughthe generated layer:
encodeURIComponentdoes not escape., so a valueof
..survives into the templated URL string, andnew URL()thencollapses it before either
Middleware.pre()orRetryHttpLibrary.send()ever see the request.
RequestContext's constructor (and its unusedsetUrltwin) is the only place that still holds the raw, un-collapsedstring, so that is where the guard has to live.
http.tsis regenerated bymake generateand is not in.openapi-generator-ignore, so a hand edit would not survive the nextcodegen run. This PR adds a post-processing step
(
scripts/guard-dot-segment-path.py, wired intogenerate-oag.sh) thatinjects the guard into the generated file, mirroring the existing
guard-optional-header-params.pypattern for the same reason: a staticaudit (the new
dot-segment-path-guard.test.ts) pins the emitted shape soa future generator upgrade fails loudly instead of silently re-blessing an
unguarded chokepoint.
Scope: this is the TypeScript half of #915's generic guard (the Python
half shipped in #929,
Refs #915). #915 also asks for a corrected,per-path-parameter-position router regression test (the "priority 2"
section) as a Go-side follow-up; that is not part of this PR, so this stays
Refs #915too, notFixes #915.Deleted the client-surface checklist below: no API surface, generated
types, or other client changed; this is a TypeScript-SDK-only hardening fix.
Operational risk
None. The guard only rejects a path segment of exactly
.or..; nolegitimate email, id, or domain value is a bare
./..segment, and thequery string is excluded from the check.
Test plan
dot-segment-path-guard.test.ts: constructsRequestContextand agenerated
DomainsApiRequestFactorydirectly, confirmingdomain=".."throws before a request is built while an ordinary domain value still
builds the expected URL. A static audit over the committed
http.tspins that both
new URL(ensureAbsoluteUrl(url))call sites stay guarded.scripts/test_guard_dot_segment_path.py: unit tests thepost-processing script directly (guards both call sites, idempotent
re-run, raises instead of silently no-opping if the generator's emitted
shape ever drifts from 2 call sites).
make generate-sdk-check(the repo's generated-code freshness gate,matching its own CI job: Go 1.26, Node 22, Python 3.12): regenerating
from the spec reproduces this PR's
http.tsdiff exactly,git diff --exit-codeclean.npm run test:coverage --workspace @e2a/sdk: typecheck, fulltest/v1unit suite (269 tests, including the 7 new ones), and type tests all
green.
ts-contractCI job (needs aPostgres-backed Go server). Its fixtures use realistic domain/email
values, none of which are a bare
./..path segment, so I don'texpect it to interact with this change, but I have not executed it.