🛡️ Sentinel: [HIGH] Fix insecure postMessage target origin and origin verification#18
Conversation
… verification - Restrict postMessage target origin in backend `/redirect` endpoint to the platform's public origin. - Use strict equality for origin verification in frontend OAuth2 utility. - Fix typo in redirect message. Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThis PR addresses OAuth security vulnerabilities by implementing proper origin verification and dynamic target origin resolution in cross-origin postMessage calls, moving away from insecure wildcard targets and inadequate startsWith validation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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.
Reviewed commit: 97e544be16
ℹ️ 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 "@codex address that feedback".
| const targetOrigin = new URL(publicUrl).origin | ||
| return reply | ||
| .type('text/html') | ||
| .send( | ||
| `<script>if(window.opener){window.opener.postMessage({ 'code': '${encodeURIComponent( | ||
| params.code, | ||
| )}' },'*')}</script> <html>Redirect succuesfully, this window should close now</html>`, | ||
| )}' }, '${targetOrigin}')}</script> <html>Redirect successfully, this window should close now</html>`, |
There was a problem hiding this comment.
The /redirect handler now hard-codes postMessage to the platform public origin, which can differ from the actual opener origin for valid deployments (for example, a platform with a custom domain where users are still on the default cloud host, or any alternate frontend host behind the same backend). In those cases the browser drops the message because targetOrigin does not match window.opener’s real origin, so OAuth popups never resolve and connection/login flows hang.
Useful? React with 👍 / 👎.
… verification - Restrict postMessage target origin in backend `/redirect` endpoint to the platform's public origin. - Use strict equality for origin verification in frontend OAuth2 utility. - Fix typo in redirect message. - Ensure correct import order for linting. Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-ui/src/lib/oauth2-utils.ts`:
- Around line 77-82: The message handler currently only checks origin and
redirectUrl which is vulnerable to same-origin forged posts; update the handler
in oauth2-utils.ts to also verify event.source matches the popup window
reference you opened (e.g. the auth popup variable used when launching OAuth)
before accepting event.data['code'], and ensure you remove the listener after
success; reference the existing handler function, event, expectedOrigin, and
redirectUrl when implementing the event.source === popupWindowRef check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6884ca8-125d-4b10-afcd-ec5383512c3f
📒 Files selected for processing (3)
.jules/sentinel.mdpackages/react-ui/src/lib/oauth2-utils.tspackages/server/api/src/app/app.ts
| window.addEventListener('message', function handler(event) { | ||
| if ( | ||
| redirectUrl && | ||
| redirectUrl.startsWith(event.origin) && | ||
| event.origin === expectedOrigin && | ||
| event.data['code'] | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n --type=ts -C3 "addEventListener\\('message'" packages/react-ui/src/lib/oauth2-utils.ts
rg -n --type=ts -C3 "event\\.source\\s*===\\s*currentPopup|currentPopup\\s*===\\s*event\\.source" packages/react-ui/src/lib/oauth2-utils.tsRepository: AGI-Corporation/Route.X
Length of output: 384
🏁 Script executed:
#!/bin/bash
# Read the full context around the handler
sed -n '70,95p' packages/react-ui/src/lib/oauth2-utils.ts
# Search for currentPopup declaration and usage in the file
rg -n "currentPopup" packages/react-ui/src/lib/oauth2-utils.tsRepository: AGI-Corporation/Route.X
Length of output: 889
🏁 Script executed:
#!/bin/bash
# Check if there are other message listeners in the file
rg -n "addEventListener.*message" packages/react-ui/src/lib/oauth2-utils.ts
# Look at the openWindow function to understand the popup context
rg -n -B2 -A8 "function openWindow" packages/react-ui/src/lib/oauth2-utils.ts
# Check the full file to see the complete flow
wc -l packages/react-ui/src/lib/oauth2-utils.tsRepository: AGI-Corporation/Route.X
Length of output: 415
Bind OAuth message acceptance to the popup window (event.source) as well as origin.
event.origin === expectedOrigin is necessary, but not sufficient for popup integrity. A different same-origin window can still post a forged { code } message and satisfy this check.
🔐 Proposed hardening
function getCode(redirectUrl: string): Promise<string> {
const expectedOrigin = new URL(redirectUrl).origin;
return new Promise<string>((resolve) => {
window.addEventListener('message', function handler(event) {
if (
- redirectUrl &&
+ event.source === currentPopup &&
event.origin === expectedOrigin &&
- event.data['code']
+ typeof event.data === 'object' &&
+ event.data !== null &&
+ 'code' in event.data &&
+ typeof (event.data as { code?: unknown }).code === 'string'
) {
- resolve(decodeURIComponent(event.data.code));
+ resolve(decodeURIComponent((event.data as { code: string }).code));
currentPopup?.close();
window.removeEventListener('message', handler);
}
});
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react-ui/src/lib/oauth2-utils.ts` around lines 77 - 82, The message
handler currently only checks origin and redirectUrl which is vulnerable to
same-origin forged posts; update the handler in oauth2-utils.ts to also verify
event.source matches the popup window reference you opened (e.g. the auth popup
variable used when launching OAuth) before accepting event.data['code'], and
ensure you remove the listener after success; reference the existing handler
function, event, expectedOrigin, and redirectUrl when implementing the
event.source === popupWindowRef check.
- Restrict postMessage target origin in backend `/redirect` endpoint to the platform's public origin. - Use strict equality for origin verification in frontend OAuth2 utility. - Fix typo in redirect message. - Correct import order in `app.ts`. Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
- Restrict postMessage target origin in backend `/redirect` endpoint. - Use strict equality for origin verification in frontend OAuth utility. - Fix typo in redirect message and correct import order. Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
1 participant
🚨 Severity: HIGH
💡 Vulnerability:
/redirectendpoint used a wildcard*as the target origin forpostMessage, which could allow malicious sites to intercept OAuth authorization codes.startsWithfor origin verification, which can be bypassed.🎯 Impact: An attacker could potentially steal OAuth authorization codes if they can get a reference to the redirect window.
🔧 Fix:
/redirectinpackages/server/api/src/app/app.tsto resolve the platform-specific origin usingdomainHelperand use it as the target origin.getCodeinpackages/react-ui/src/lib/oauth2-utils.tsto use strict equality (===) for origin verification.✅ Verification:
PR created automatically by Jules for task 3844969624917860543 started by @AGI-Corporation
Summary by CodeRabbit
Bug Fixes
Documentation