fix: Encode userId path parameter in user API key methods#1664
fix: Encode userId path parameter in user API key methods#1664devin-ai-integration[bot] wants to merge 3 commits into
Conversation
createUserApiKey and listUserApiKeys interpolated the userId argument directly into the request path without encodeURIComponent. Because the HTTP client builds the URL with new URL(path, baseURL), a userId containing ../ segments and a ? was normalized and query-split, letting the SDK's environment-key-authenticated request be redirected to an arbitrary API path (e.g. /organizations/org_.../api_keys). Wrap userId in encodeURIComponent, matching the SDK's established pattern (e.g. getUserConnectedAccount), so the argument stays confined to the user resource route. Add regression tests.
Original prompt from Linear User
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Greptile SummaryThis PR fixes a path-traversal vulnerability where
Confidence Score: 3/5The fix is correct for user-management.ts but the same path-traversal pattern remains live in multi-factor-auth.ts and feature-flags.ts, leaving the attack surface partially open. The fix is correct and complete within user-management.ts, but the identical unencoded userId interpolation pattern remains in multi-factor-auth.ts (createUserAuthFactor, listUserAuthFactors) and feature-flags.ts (listUserFeatureFlags), all callable with the SDK environment key. The partial fix reduces but does not eliminate the attack surface. src/multi-factor-auth/multi-factor-auth.ts (lines 171, 200, 207) and src/feature-flags/feature-flags.ts (lines 189, 196) need the same encodeURIComponent fix applied. Important Files Changed
Reviews (3): Last reviewed commit: "Encode userId in updateUser path" | Re-trigger Greptile |
Extend the encodeURIComponent fix to the other user-scoped endpoints that share the same raw-interpolation path-traversal primitive: getUser, sendVerificationEmail, verifyEmail, listSessions, deleteUser, and getUserIdentities. Add regression tests for getUser and deleteUser.
|
Good catch on the incomplete fix. The same raw Addressed in 088d805: all |
updateUser interpolated payload.userId raw into the PUT path, leaving the same path-traversal primitive open. Encode it and add a regression test.
|
Fixed the |
|
Confirmed the same unencoded pattern exists in Scoping note: SEC-2175 is specifically the user-management API-key methods, and this PR already extends the fix to cover all |
Description
Automated first-pass security fix for SEC-2175. Requires human security review before merge.
createUserApiKeyandlistUserApiKeysinterpolated theuserIdargument directly into the request path withoutencodeURIComponent:`/user_management/users/${userId}/api_keys`Because
HttpClient.getResourceURLbuilds the URL withnew URL(path, baseURL)(src/common/net/http-client.ts:83), auserIdcontaining../segments and a?is normalized and query-split by the WHATWG URL parser. This lets auserIdthat reaches these methods redirect the SDK's environment-key-authenticated request to an arbitrary API path.Reproduced locally against a mock HTTP server (no production traffic), observing the actual outbound request:
The fix wraps
userIdinencodeURIComponent, matching the SDK's established pattern (e.g.getUserConnectedAccountatsrc/pipes/pipes.ts). After the fix the same input stays confined to the route template:Added regression tests for both methods asserting the encoded pathname.
Note: this is a client-side SDK defect. The full confused-deputy impact (whether the WorkOS API accepts the redirected request and mints an org-scoped key) depends on server-side behavior that was not tested against production; encoding the parameter closes the SDK-side primitive regardless.
Documentation
Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.
Link to Devin session: https://app.devin.ai/sessions/19376786c7a44cceb39716ee31a45cfb