fix(security): remove wildcard CORS from netlify.toml (TAY-190)#1166
fix(security): remove wildcard CORS from netlify.toml (TAY-190)#1166Computer8004 wants to merge 258 commits intolearningeconomy:mainfrom
Conversation
… into LC-1558-themes
… into LC-1558-themes
… origins whitelist - Remove Access-Control-Allow-Origin: * from netlify.toml static headers - Add TRUSTED_ORIGINS constant and validation helpers for runtime origin checks Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
|
To enable Rovo Dev code reviews, link your GitHub account to your Atlassian account. This is a one-time task that takes less than a minute. Once your account is linked, resubmit the pull request to trigger a code review. |
👷 Deploy request for staging-learncardapp pending review.Visit the deploys page to approve it
|
✅ Deploy Preview for learncarddocs canceled.
|
✅ Deploy Preview for app-store-1-basic-launchpad-app canceled.
|
![gitstream-cm[bot]](https://avatars.githubusercontent.com/in/230441?s=60&v=4){kind=small}
There was a problem hiding this comment.
✨ PR Review
The PR adds tenant configuration infrastructure but fails to achieve its stated security goal: wildcard CORS headers remain in edge functions, contradicting the PR description.
2 issues detected:
🔒 Security - Wildcard CORS persists despite PR claiming to remove it
Details: The PR title claims to "remove wildcard CORS from netlify.toml" but the wildcard CORS header
Access-Control-Allow-Origin: "*"remains in this edge function at line 6 and is used in responses (lines 62, 80). This allows any origin to access the interaction endpoints, which is a security vulnerability that the PR was supposed to fix.
File:apps/learn-card-app/netlify/edge-functions/interact.ts (5-9)🔒 Security - New code adds wildcard CORS when PR aims to remove it 🛠️
Details: This new edge function introduces wildcard CORS
Access-Control-Allow-Origin: '*'on line 33, allowing any origin to fetch tenant configuration data. This contradicts the PR's security goal of removing wildcard CORS headers.File:
apps/learn-card-app/netlify/edge-functions/tenant-config.ts (33-33)
🛠️ A suggested code correction is included in the review comments.
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'Cache-Control': 'public, max-age=300, stale-while-revalidate=3600', | ||
| 'Access-Control-Allow-Origin': '*', |
There was a problem hiding this comment.
🔒 Security - New Wildcard CORS Added: Restrict CORS to trusted origins or remove the wildcard. Since this serves tenant configuration, consider validating the requesting origin against the tenant's configured domains before setting the CORS header.
| 'Access-Control-Allow-Origin': '*', | |
| 'Access-Control-Allow-Origin': url.origin, |
Is this review accurate? Use 👍 or 👎 to rate it
If you want to tell us more, use /gs feedback e.g. /gs feedback this review doesn't make sense, I disagree, and it keeps repeating over and over
…tlify.toml - Add shared cors.ts edge-function helper with explicit origin whitelist - Update interact.ts to use dynamic origin validation instead of '*' - Update tenant-config.ts to use dynamic origin validation instead of '*' - Remove Access-Control-Allow-Origin wildcard headers from scouts netlify.toml Co-Authored-By: Paperclip <noreply@paperclip.ing>
There was a problem hiding this comment.
✨ PR Review
LGTM
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how
4 participants
Summary
Removes wildcard
Access-Control-Allow-Origin: *headers fromnetlify.tomland introduces a runtime trusted-origins whitelist.Changes
apps/learn-card-app/netlify.tomlAccess-Control-Allow-Origin: "*"from/manifest.json,/\*,/assets/icon/icon.png,/assets/icon/favicon.png, and/assets/\*apps/learn-card-app/src/constants/trustedOrigins.ts(new)TRUSTED_ORIGINSarray andisTrustedOrigin()/isTrustedOriginPattern()helpersNetlify static headers cannot express dynamic multi-origin CORS, so the fix relies on removing the dangerous wildcard and letting browser same-origin policy block unsolicited cross-origin requests. Application-level postMessage validation already handles partner-origin trust at runtime.
Closes TAY-190
✨ PR Description
Purpose: Remove wildcard CORS (
Access-Control-Allow-Origin: *) from Netlify edge functions and implement tenant-based whitelist validation to prevent unauthorized cross-origin access to API endpoints.Main changes:
buildCorsHeaders()helper inshared/cors.tsthat validates origin against ALLOWED_ORIGINS whitelist and dynamic patterns (e.g.,*.learncard.app, deploy previews) instead of wildcardshared/tenant-resolver.tsimporting generated config bundle andgenerate-edge-tenant-configs.tsauto-discovery script to eliminate manual tenant importsinteract.tsedge function to use tenant-aware CORS headers, resolve LCN API URL from tenant config with fallback chain (env → tenant config → deploy context), and redirect to tenant-specific originGenerated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how