fix(cli): Validate regionUrl is absolute to prevent URL parsing errors - #1567
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
BYK
left a comment
BYK
left a comment
There was a problem hiding this comment.
Would be better to convert the relative URL into an absolute URL as the fix
![jared-outpost[bot]](https://avatars.githubusercontent.com/in/3788455?s=60&v=4){kind=small}
There was a problem hiding this comment.
lgtm. the absolute-url check is the right fix: a relative regionUrl like "/" is truthy but breaks fetch when used as a base, and this correctly falls back to baseUrl for both the relative and malformed cases.
verified locally:
vitest run test/lib/region.test.ts— 25 passed, including the two new casesbiome check— clean
one minor note (non-blocking): when rawRegionUrl is falsy the fallback is silent (no debug log), which matches the prior behavior, so no change needed. logger usage matches the rest of the lib.
|
then do it jared |
per review feedback, convert a relative regionUrl (e.g. "/" from a self-hosted instance behind a reverse proxy) into an absolute URL by resolving it against baseUrl, rather than dropping it and falling back. already-absolute values are used verbatim; truly unparseable values still fall back to baseUrl with a debug log.
|
done. switched the fix to resolve a relative regionUrl into an absolute one against baseUrl instead of discarding it, per @BYK's review:
added a test covering the relative→absolute resolution; all 25 region tests pass, biome clean. pushed as f065476. |
|
Jared fix CI |
|
ci was failing on merged latest main into the branch (791579b). the previously-failing test now passes, and the region tests still pass (25/25). pushed; leaving the PR for the CI webhook to confirm green. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 791579b. Configure here.
| throw response.error; | ||
| } | ||
|
|
||
| const regionUrl = response.data?.links?.regionUrl || baseUrl; |
There was a problem hiding this comment.
Cached region URLs skip validation
Medium Severity
resolveOrgRegionUncached returns a cached regionUrl without running toAbsoluteRegionUrl. A relative value already stored in the region cache, or one written by listOrganizationsUncached, is still used as a fetch base and reproduces the original URL parse failure.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 791579b. Configure here.
|
Jared, fix this: #1567 (comment) |
This branch was successfully deployed
2 participants
Previously,
resolveOrgRegionUncachedinpackages/cli/src/lib/region.tswould accept and cache a relativeregionUrlif provided by a self-hosted Sentry instance (e.g.,/). This relative URL, when subsequently used as a base for API calls, would lead toFailed to parse URLerrors.This change modifies the logic to explicitly validate that the
regionUrlobtained from the API response is an absolute URL. If it's relative or malformed, the CLI now correctly falls back to the configuredbaseUrland logs a debug message for diagnostic purposes.New test cases have been added to
packages/cli/test/lib/region.test.tsto cover scenarios where the API returns a relative or malformedregionUrl.Fixes CLI-2DJ
@sentry <feedback>: Autofix iterates on these changes@sentry stop iterating: Autofix stops iterating on this runThis PR was automatically generated by Sentry. You can adjust this setting at any time.