[feature request] #447 partial support external urls in paths #1434
Conversation
🦋 Changeset detectedLatest commit: 0a81ae6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| createRequestsMap = ( | ||
| resolvedSwaggerSchema: ResolvedSwaggerSchema, | ||
| routeInfoByMethodsMap: AnyObject, | ||
| ) => { | ||
| const parameters = lodash.get(routeInfoByMethodsMap, "parameters"); | ||
|
|
||
| return lodash.reduce( | ||
| routeInfoByMethodsMap, | ||
| (acc, requestInfo, method) => { | ||
| if ( | ||
| method.startsWith("x-") || | ||
| ["parameters", "$ref"].includes(method) | ||
| ) { | ||
| (acc, anything, property) => { | ||
| if (property.startsWith("x-") || ["parameters"].includes(property)) { | ||
| return acc; | ||
| } | ||
|
|
||
| if (property === "$ref") { | ||
| const refData = resolvedSwaggerSchema.getRef(anything); | ||
| if (typeGuard.isObject(refData)) { | ||
| Object.assign( | ||
| acc, | ||
| this.createRequestsMap(resolvedSwaggerSchema, refData), | ||
| ); |
There was a problem hiding this comment.
[P1] Preserve path-level params when resolving $ref
When a path item contains both a $ref and its own parameters, the new createRequestsMap recurses into the referenced object but never passes the local parameters along. Any parameters declared next to the $ref are dropped, so externalized routes lose required path/query args (e.g. /pets/{id} with an external path file). Now that external paths are supported this merge needs to happen, otherwise generated client types omit those parameters.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
src/swagger-schema-resolver.ts
Outdated
| resolve: { | ||
| external: true, | ||
| http: { | ||
| ...this.config.requestOptions, | ||
| headers: Object.assign( | ||
| {}, | ||
| this.config.authorizationToken | ||
| ? { | ||
| Authorization: this.config.authorizationToken, | ||
| } | ||
| : {}, | ||
| this.config.requestOptions?.headers ?? {}, | ||
| ), |
There was a problem hiding this comment.
[P0] Avoid sending auth headers to arbitrary $ref URLs
The resolver now calls SwaggerParser.resolve with resolve.http.headers built from authorizationToken and requestOptions and applies those headers to every HTTP request when dereferencing external paths. A spec can embed $ref: https://attacker.example.com/foo.yml and the generator will automatically send the caller’s Authorization token (and any other configured headers) to that host. This leaks credentials and enables SSRF against arbitrary URLs referenced in the spec. Consider scoping headers to the original spec’s origin, allow users to whitelist hosts, or omit sensitive headers for external refs.
Useful? React with 👍 / 👎.
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| }); | ||
|
|
||
| return this.tryToResolveRef(fixedRef); | ||
| } |
There was a problem hiding this comment.
Bug: JSON Pointer Normalization Fails for Valid Paths
The getRef method's logic for normalizing local references is flawed. The regex /#[a-z]/ is too restrictive, missing valid JSON pointer paths that don't start with a lowercase letter. When it does match, the replacement logic incorrectly truncates the path (e.g., #<char> becomes #/char), leading to invalid references and preventing proper schema resolution.
2 participants
No description provided.