🛡️ Sentinel: Fix insecure postMessage origin in OAuth redirects#15
🛡️ Sentinel: Fix insecure postMessage origin in OAuth redirects#15AGI-Corporation wants to merge 4 commits intomainfrom
Conversation
Restricted the target origin of `window.postMessage` in both server-side and client-side OAuth redirect handlers. Using a wildcard origin (`'*'`) is insecure as it allows sensitive authorization codes to be intercepted by malicious origins. - In `packages/server/api/src/app/app.ts`, the `/redirect` route now resolves the platform-specific frontend origin. - In `packages/react-ui/src/app/routes/redirect.tsx`, the `targetOrigin` is restricted to `window.location.origin`. - Updated `.jules/sentinel.md` with details of this vulnerability and prevention strategy. 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. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughClient and server OAuth redirect flows were hardened by replacing wildcard postMessage origins with explicitly resolved origins; origin validation on message receipt was tightened. A new security doc Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 46c64ebbe3
ℹ️ 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 platformId = await platformUtils.getPlatformIdForRequest(request) | ||
| const frontendUrl = await domainHelper.getPublicUrl({ platformId }) | ||
| const targetOrigin = new URL(frontendUrl).origin |
There was a problem hiding this comment.
Deriving targetOrigin from domainHelper.getPublicUrl({ platformId }) here can send the OAuth code to the wrong origin in cloud custom-domain setups that use AP_INTERNAL_URL: THIRD_PARTY_AUTH_PROVIDER_REDIRECT_URL is generated via getInternalUrl (shared/internal host), so this /redirect request may not map to a tenant host, getPlatformIdForRequest falls back to null, and the handler posts to the default frontend origin instead of the tenant opener origin. In that environment, window.opener.postMessage is blocked by origin mismatch and the OAuth/SSO popup never returns a code.
Useful? React with 👍 / 👎.
![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
🧹 Nitpick comments (1)
packages/react-ui/src/app/routes/redirect.tsx (1)
11-17: Security fix looks correct, but consider hardening the receiver validation.The change from
'*'towindow.location.origincorrectly restricts the postMessage target. However, note that the message receiver inpackages/react-ui/src/lib/oauth2-utils.ts(lines 78-79) usesredirectUrl.startsWith(event.origin)which is a weak validation—it could accept messages from origins sharing a common prefix (e.g.,https://app.example.com.attacker.comwould pass ifredirectUrlstarts withhttps://app.example.com).Consider tightening the receiver validation to use exact origin matching:
new URL(redirectUrl).origin === event.origin🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-ui/src/app/routes/redirect.tsx` around lines 11 - 17, The receiver currently validates origins using the startsWith check (see redirectUrl.startsWith(event.origin) in packages/react-ui/src/lib/oauth2-utils.ts); replace that weak prefix check with an exact origin comparison by deriving the origin from redirectUrl (using the URL API) and comparing it strictly to event.origin so only an exact match is accepted.
🤖 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/server/api/src/app/app.ts`:
- Around line 253-261: Add the missing import for platformUtils used in the
handler: import { platformUtils } from './platform/platform.utils'; this will
ensure platformUtils.getPlatformIdForRequest(request) resolves; update the top
of the file where other imports are declared so platformUtils is available
before its use in the code that calls platformUtils.getPlatformIdForRequest and
remove any linter/unused-import warnings if needed.
---
Nitpick comments:
In `@packages/react-ui/src/app/routes/redirect.tsx`:
- Around line 11-17: The receiver currently validates origins using the
startsWith check (see redirectUrl.startsWith(event.origin) in
packages/react-ui/src/lib/oauth2-utils.ts); replace that weak prefix check with
an exact origin comparison by deriving the origin from redirectUrl (using the
URL API) and comparing it strictly to event.origin so only an exact match is
accepted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83db884e-e75d-4d3c-aa6f-5af1f6a5136b
📒 Files selected for processing (3)
.jules/sentinel.mdpackages/react-ui/src/app/routes/redirect.tsxpackages/server/api/src/app/app.ts
This PR addresses a security vulnerability where `window.postMessage` was using a wildcard origin (`'*'`) or loose validation (`startsWith`), potentially leaking OAuth authorization codes to malicious origins. Key changes: - Restricted `targetOrigin` in server-side `/redirect` route to the platform's public frontend origin. - Restricted `targetOrigin` in client-side `RedirectPage` to `window.location.origin`. - Improved `postMessage` listener in `oauth2-utils.ts` to use strict origin equality and moved origin calculation for better performance/robustness. - Fixed a typo in the server response message. - Added missing import for `platformUtils` in `app.ts`. Closes #security-sentinel-01 (Sentinel security enhancement) Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react-ui/src/lib/oauth2-utils.ts (1)
74-88: Good security hardening with strict origin equality.The change from
startsWithto exact origin equality is the correct fix. The previous check was vulnerable to origin spoofing (e.g.,https://trusted.com.attacker.comwould pass astartsWith('https://trusted.com')check).One consideration:
new URL(redirectUrl)on line 76 will throw aTypeErrorifredirectUrlis malformed. This would result in an unhandled promise rejection since the error propagates up fromgetCode. The callers appear to always pass valid URLs from configuration, but you may want defensive handling:♻️ Optional: Add defensive URL parsing
function getCode(redirectUrl: string): Promise<string> { return new Promise<string>((resolve) => { - const expectedOrigin = redirectUrl ? new URL(redirectUrl).origin : null; + let expectedOrigin: string | null = null; + try { + expectedOrigin = redirectUrl ? new URL(redirectUrl).origin : null; + } catch { + console.error('Invalid redirectUrl for OAuth origin validation:', redirectUrl); + } window.addEventListener('message', function handler(event) {🤖 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 74 - 88, getCode currently calls new URL(redirectUrl) which will throw for malformed redirectUrl and cause an unhandled rejection; update getCode to accept a reject callback in the Promise, wrap the new URL(redirectUrl) parsing in a try/catch and call reject(new TypeError(...)) or reject(err) if parsing fails, and early-return so the message event handler is not registered; reference getCode, new URL(redirectUrl), expectedOrigin, and the window.addEventListener('message', handler) block when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/react-ui/src/lib/oauth2-utils.ts`:
- Around line 74-88: getCode currently calls new URL(redirectUrl) which will
throw for malformed redirectUrl and cause an unhandled rejection; update getCode
to accept a reject callback in the Promise, wrap the new URL(redirectUrl)
parsing in a try/catch and call reject(new TypeError(...)) or reject(err) if
parsing fails, and early-return so the message event handler is not registered;
reference getCode, new URL(redirectUrl), expectedOrigin, and the
window.addEventListener('message', handler) block when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad6920c9-099d-4fbf-a2ed-d670bb0a0f5e
📒 Files selected for processing (2)
packages/react-ui/src/lib/oauth2-utils.tspackages/server/api/src/app/app.ts
Restricted the target origin of `window.postMessage` in both server-side and client-side OAuth redirect handlers. Using a wildcard origin (`'*'`) or loose validation (`startsWith`) is insecure as it allows sensitive authorization codes to be intercepted by malicious origins. - In `packages/server/api/src/app/app.ts`, the `/redirect` route now resolves the platform-specific frontend origin. - In `packages/react-ui/src/app/routes/redirect.tsx`, the `targetOrigin` is restricted to `window.location.origin`. - Improved `postMessage` listener in `oauth2-utils.ts` to use strict origin equality and moved origin calculation for better performance/robustness. - Fixed a typo in the server response message. - Added missing import for `platformUtils` in `app.ts`. Closes #1 Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
Restricted the target origin of `window.postMessage` in both server-side and client-side OAuth redirect handlers. Using a wildcard origin (`'*'`) or loose validation (`startsWith`) is insecure as it allows sensitive authorization codes to be intercepted by malicious origins. - In `packages/server/api/src/app/app.ts`, the `/redirect` route now resolves the platform-specific frontend origin. - In `packages/react-ui/src/app/routes/redirect.tsx`, the `targetOrigin` is restricted to `window.location.origin`. - Improved `postMessage` listener in `oauth2-utils.ts` to use strict origin equality and added error handling for URL parsing. - Fixed a typo in the server response message. - Added missing import for `platformUtils` in `app.ts`. Closes #1 Co-authored-by: AGI-Corporation <186229839+AGI-Corporation@users.noreply.github.com>
1 participant
🛡️ Sentinel: [HIGH] Fix insecure postMessage origin in OAuth redirects
Restricted the target origin of
window.postMessagein both server-side and client-side OAuth redirect handlers. Using a wildcard origin ('*') is insecure as it allows sensitive authorization codes to be intercepted by malicious origins.packages/server/api/src/app/app.ts, the/redirectroute now resolves the platform-specific frontend origin.packages/react-ui/src/app/routes/redirect.tsx, thetargetOriginis restricted towindow.location.origin..jules/sentinel.mdwith details of this vulnerability and prevention strategy.PR created automatically by Jules for task 843941654461830414 started by @AGI-Corporation
Summary by CodeRabbit
Bug Fixes
Documentation