fix: vercel -> cfpages changes#7401
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughThis PR establishes Sentry production sourcemap uploads in the Vite build pipeline, adds HTTP security headers for Cloudflare Pages deployment, documents the complete Cloudflare Pages migration, and updates build infrastructure dependencies and configuration. ChangesDeployment Infrastructure & Sentry Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Deploying swap-dev with
|
| Latest commit: |
e6e8a2d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c8459c19.swap-dev-5u6.pages.dev |
| Branch Preview URL: | https://fix-vercel-cfpages.swap-dev-5u6.pages.dev |
alfetopito
force-pushed
the
fix/build-envs
branch
2 times, most recently
from
ceac06b to
86967f0
Compare
alfetopito
force-pushed
the
fix/vercel-cfpages
branch
from
0c24c61 to
71c5466
Compare
![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 (4)
package.json (1)
26-26: 💤 Low valueSignificant memory increase for production builds.
The Node.js memory limit has been doubled from ~4GB (default) to 8GB. While this addresses Cloudflare Pages build failures (as noted in the commit messages), consider monitoring actual memory usage to determine if the full 8GB is necessary long-term, or if a smaller increase would suffice.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 26, The npm script "build:cowswap" hardcodes NODE_OPTIONS=--max-old-space-size=8192 which doubles Node's memory cap; change this to a configurable or smaller value and add monitoring: replace the hardcoded 8192 with an env-driven value (e.g., use an env var like MAX_OLD_SPACE_SIZE or a script that falls back to a sensible default such as 4096), update the "build:cowswap" script to reference that env var, and add a short note in the repo README or CI docs instructing how to override and to collect build memory metrics to determine the minimal needed size.apps/cowswap-frontend/public/_headers (2)
2-2: 🏗️ Heavy liftConsider tightening overly permissive CSP directives.
Several CSP directives use wildcards that significantly reduce the security benefits of CSP:
connect-src *- allows connections to any originstyle-src *- allows styles from any sourcefont-src *- allows fonts from any sourceframe-src *- allows framing any external contentWhile these may be necessary for DeFi integrations (wallet connectors, blockchain RPCs, third-party widgets), consider explicitly listing known domains to reduce the attack surface. At minimum, document why each wildcard is required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cowswap-frontend/public/_headers` at line 2, The CSP in public/_headers is overly permissive because several directives use wildcards (notably connect-src, style-src, font-src, frame-src); replace these wildcards with explicit, minimal whitelists of required origins (e.g., known RPCs, analytics, widget domains and CDN hosts) and remove any unnecessary '*' entries, or if wildcard is absolutely required add an inline comment documenting why; update the directives named connect-src, style-src, font-src, and frame-src in the Content-Security-Policy header to list only the trusted domains and test the app to ensure wallet connectors, RPC calls and third-party widgets still function.
2-2: ⚖️ Poor tradeoff
'unsafe-eval'and'unsafe-inline'weaken script CSP protection.The script-src directive includes both
'unsafe-eval'and'unsafe-inline', which significantly reduce the effectiveness of CSP against XSS attacks. These are often required for certain frameworks and third-party scripts, but verify they are truly necessary.If these are required for specific libraries (e.g., analytics, ads), consider using CSP nonces or hashes for inline scripts instead of the blanket
'unsafe-inline'permission.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cowswap-frontend/public/_headers` at line 2, The Content-Security-Policy header's script-src currently includes 'unsafe-eval' and 'unsafe-inline' which weakens CSP; audit the third-party hosts listed in the script-src directive and remove 'unsafe-eval' and 'unsafe-inline' where possible, then replace blanket inline permissions by adopting CSP nonces or script hashes: update the Content-Security-Policy header string (the line beginning with Content-Security-Policy and the script-src token) to remove those unsafe tokens and instead add either 'nonce-<value>' (and implement server-side injection of that nonce into inline <script> tags) or add sha256/... hashes for any required inline scripts, and narrow allowed origins in script-src/connect-src to only the third-party domains actually needed.docs/CLOUDFLARE_PAGES_MIGRATION.md (1)
86-91: ⚡ Quick winEnhance validation checklist with security and behavior testing.
The current checklist verifies that headers are present but doesn't validate their correctness or security implications. Consider adding:
- Verify
frame-ancestorsCSP directive aligns with security requirements (test iframe embedding)- Test deep-link routing behavior matches Vercel (compare specific routes between Vercel and CF Pages)
- Validate that CSP headers don't block legitimate app functionality (wallet connections, external resources)
- Confirm CORS and COOP/CORP headers work correctly with wallet integrations
📋 Proposed enhancement to validation checklist
## Validation checklist - `apps/cowswap-frontend/public/_headers` is copied into `build/cowswap/_headers`. - `build/cowswap` and `build/explorer` do not contain a top-level `404.html`. - Deep links for both apps resolve correctly on Pages preview deployments. - `cowswap` document responses include the migrated security headers. +- Security headers function as intended: + - Test iframe embedding behavior matches requirements. + - Verify CSP doesn't block wallet connections or legitimate external resources. + - Confirm COOP/CORP headers work with wallet integrations. +- SPA routing behavior matches Vercel production (test multiple route paths).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/CLOUDFLARE_PAGES_MIGRATION.md` around lines 86 - 91, Extend the Validation checklist to include security and behavioral tests: add items to verify the `frame-ancestors` CSP directive (attempt iframe embedding of `cowswap` to ensure it matches security requirements), add explicit deep-link routing comparisons between Vercel and Cloudflare Pages for representative routes (e.g., root, /swap, /pool, and a known deep-link), add tests that exercise wallet connections and external resource loads to confirm CSP headers in `build/cowswap` don’t block legitimate app functionality, and add checks that CORS and COOP/CORP headers on `cowswap` document responses work correctly with wallet integrations (e.g., cross-origin requests and credentialed fetches).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cowswap-frontend/public/_headers`:
- Line 2: The Content-Security-Policy header currently sets "frame-ancestors *",
which permits clickjacking; update the CSP line in public/_headers to replace
frame-ancestors * with a restrictive value (e.g., frame-ancestors 'self' or
frame-ancestors 'none', or an explicit allowlist like frame-ancestors 'self'
https://trusted-partner.com) and ensure the rest of the policy string remains
unchanged; locate the CSP header string (the long line beginning with
"Content-Security-Policy:") and edit only the frame-ancestors token to the
chosen restrictive option.
---
Nitpick comments:
In `@apps/cowswap-frontend/public/_headers`:
- Line 2: The CSP in public/_headers is overly permissive because several
directives use wildcards (notably connect-src, style-src, font-src, frame-src);
replace these wildcards with explicit, minimal whitelists of required origins
(e.g., known RPCs, analytics, widget domains and CDN hosts) and remove any
unnecessary '*' entries, or if wildcard is absolutely required add an inline
comment documenting why; update the directives named connect-src, style-src,
font-src, and frame-src in the Content-Security-Policy header to list only the
trusted domains and test the app to ensure wallet connectors, RPC calls and
third-party widgets still function.
- Line 2: The Content-Security-Policy header's script-src currently includes
'unsafe-eval' and 'unsafe-inline' which weakens CSP; audit the third-party hosts
listed in the script-src directive and remove 'unsafe-eval' and 'unsafe-inline'
where possible, then replace blanket inline permissions by adopting CSP nonces
or script hashes: update the Content-Security-Policy header string (the line
beginning with Content-Security-Policy and the script-src token) to remove those
unsafe tokens and instead add either 'nonce-<value>' (and implement server-side
injection of that nonce into inline <script> tags) or add sha256/... hashes for
any required inline scripts, and narrow allowed origins in
script-src/connect-src to only the third-party domains actually needed.
In `@docs/CLOUDFLARE_PAGES_MIGRATION.md`:
- Around line 86-91: Extend the Validation checklist to include security and
behavioral tests: add items to verify the `frame-ancestors` CSP directive
(attempt iframe embedding of `cowswap` to ensure it matches security
requirements), add explicit deep-link routing comparisons between Vercel and
Cloudflare Pages for representative routes (e.g., root, /swap, /pool, and a
known deep-link), add tests that exercise wallet connections and external
resource loads to confirm CSP headers in `build/cowswap` don’t block legitimate
app functionality, and add checks that CORS and COOP/CORP headers on `cowswap`
document responses work correctly with wallet integrations (e.g., cross-origin
requests and credentialed fetches).
In `@package.json`:
- Line 26: The npm script "build:cowswap" hardcodes
NODE_OPTIONS=--max-old-space-size=8192 which doubles Node's memory cap; change
this to a configurable or smaller value and add monitoring: replace the
hardcoded 8192 with an env-driven value (e.g., use an env var like
MAX_OLD_SPACE_SIZE or a script that falls back to a sensible default such as
4096), update the "build:cowswap" script to reference that env var, and add a
short note in the repo README or CI docs instructing how to override and to
collect build memory metrics to determine the minimal needed size.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a9bccaaf-5f66-40b0-bfb0-3c92c4d62424
📒 Files selected for processing (7)
.nvmrcREADME.mdapps/cowswap-frontend/.envapps/cowswap-frontend/public/_headersapps/cowswap-frontend/vite.config.mtsdocs/CLOUDFLARE_PAGES_MIGRATION.mdpackage.json
| @@ -0,0 +1,6 @@ | |||
| /* | |||
| Content-Security-Policy: default-src 'self'; script-src 'self' https://*.cow.fi https://telegram.org https://*.appzi.io https://www.googletagmanager.com https://s3.amazonaws.com https://www.redditstatic.com https://www.google-analytics.com https://www.clarity.ms https://tag.safary.club https://static.ads-twitter.com https://scripts.clarity.ms https://api.hypelab.com https://googleads.g.doubleclick.net https://tag.adrsbl.io https://cdn.spindl.xyz https://r2.ixncdn.com https://cdn.id5-sync.com 'unsafe-eval' 'unsafe-inline'; style-src * 'unsafe-inline'; img-src 'self' data: blob: https:; font-src * data:; connect-src *; media-src * data: blob:; frame-src *; frame-ancestors *; worker-src 'self'; manifest-src 'self'; base-uri 'self'; form-action 'self'; object-src 'none'; upgrade-insecure-requests; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the security risks of frame-ancestors wildcard in Content Security Policy for DeFi applications?
💡 Result:
Using CSP frame-ancestors with a wildcard like '*' is generally high risk for DeFi apps because it allows any website to embed your UI, enabling clickjacking (UI redressing) and other framing-based user deception. 1) Direct clickjacking / UI redressing risk - The frame-ancestors directive controls which parent origins may embed your page in an iframe (and related embedding elements) [1]. If you effectively allow all origins (e.g., via a wildcard), attackers can frame your DeFi app inside their own site and trick users into clicking UI they believe they control. - This “unauthorized framing” is described as enabling clickjacking attacks in CSP wildcard findings [2]. 2) Bypass of the security boundary you intended for “sensitive” screens - DeFi frontends are particularly sensitive because UI actions often lead to signing or confirming transfers/approvals. Guidance on clickjacking defenses emphasizes that protections must be applied to every page with interactive elements (not just a login page), since attackers target the highest-value actions like transfer confirmations and other authenticated operations [3]. A wildcard frame-ancestors undermines that boundary by letting any origin embed those sensitive pages. 3) Increased blast radius for frontend compromise scenarios - In DeFi, trust failure in the frontend delivery path can translate into harmful user interactions (e.g., deceptive transaction prompts, routing/approval abuse) [4][5]. While frame-ancestors wildcard is not the same as script-src being overly permissive, letting any origin embed your app expands the opportunities for attackers to present deceptive UI flows around those compromised or manipulated states. 4) Policy “failure mode” is easy to misunderstand - If you accidentally deploy a too-permissive frame-ancestors source expression, it effectively changes who can embed your site; frame-ancestors does not fall back from default-src [1]. So relying on other CSP directives or default-src for safety while using an over-broad frame-ancestors expression can leave your app embeddable by unintended parties. Practical DeFi guidance (safer patterns) - Prefer denying all embedding unless you have a business need: frame-ancestors 'none' [1]. - If you must allow embedding, use an explicit allowlist of exact trusted origins (and their required subdomains) rather than broad wildcards; the directive supports allowing specific sources, and the allowed set determines embedding authorization [1]. In short: frame-ancestors wildcard (allowing arbitrary origins) removes the main technical protection against hostile framing and increases clickjacking risk for DeFi user interactions, making it a bad fit for hardened DeFi security posture.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Content-Security-Policy/frame-ancestors
- 2: https://docs.stackhawk.com/vulnerabilities/10055/
- 3: https://encryptcodec.com/blog/clickjacking-attacks-x-frame-options-and-frame-ancestors-csp
- 4: https://www.zealynx.io/glossary/content-security-policy
- 5: https://cypro.li/defi-frontend-supply-chain-security.html
🏁 Script executed:
cat -n apps/cowswap-frontend/public/_headers | head -20Repository: cowprotocol/cowswap
Length of output: 1100
Critical: frame-ancestors * allows clickjacking attacks.
The CSP directive frame-ancestors * permits this application to be embedded in iframes on any untrusted website. For a DeFi application handling financial transactions, this creates a serious clickjacking risk where attackers can overlay transparent iframes to trick users into authorizing unintended transactions.
Restrict to trusted domains or block iframe embedding entirely:
frame-ancestors 'self'(only allow same-origin embedding)frame-ancestors 'none'(block all iframe embedding)- Or explicitly list trusted domains:
frame-ancestors 'self' https://trusted-partner.com
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/cowswap-frontend/public/_headers` at line 2, The Content-Security-Policy
header currently sets "frame-ancestors *", which permits clickjacking; update
the CSP line in public/_headers to replace frame-ancestors * with a restrictive
value (e.g., frame-ancestors 'self' or frame-ancestors 'none', or an explicit
allowlist like frame-ancestors 'self' https://trusted-partner.com) and ensure
the rest of the policy string remains unchanged; locate the CSP header string
(the long line beginning with "Content-Security-Policy:") and edit only the
frame-ancestors token to the chosen restrictive option.
There was a problem hiding this comment.
We do embed the page in an iframe when running the widget (see widget.cow.fi for an example).
Wouldn't changing this break the widget behaviour?
There was a problem hiding this comment.
🧩 Analysis chain
Good point — let me check the widget architecture to better understand the embedding pattern.
🏁 Script executed:
#!/bin/bash
# Check if there's a widget app or widget-specific routes/config
fd -t f -e html -e ts -e tsx -e json --full-path | xargs grep -l "widget" 2>/dev/null | grep -i widget | head -20
# Check if there are any existing frame-ancestors or X-Frame-Options settings
rg -rn "frame-ancestors|X-Frame-Options" --type-add 'config:*.{json,yaml,yml,toml,conf,ini,headers,txt}' --type config -g '!node_modules' 2>/dev/null | head -20
# Check for any widget app
fd -t d "widget" --max-depth 3 | head -10
# List apps directory to understand widget vs cowswap-frontend
ls apps/ 2>/dev/null
Length of output: 909
Let me also check how the widget constructs iframe URLs to understand the embedding pattern better.
🏁 Script executed:
#!/bin/bash
cat libs/widget-lib/src/cowSwapWidget.ts
echo "---"
cat libs/widget-lib/src/urlUtils.ts
Length of output: 18895
@alfetopito, you're absolutely right — changing frame-ancestors globally to 'none' or 'self' would break the widget, since libs/widget-lib is designed to embed cowswap-frontend (e.g. swap.cow.fi) as an iframe on any arbitrary third-party website. That's the core product design.
However, there is a meaningful middle ground. Looking at libs/widget-lib/src/urlUtils.ts, widget routes follow the path pattern /:chainId/widget/:tradeType/:sell/:buy. Cloudflare Pages _headers supports path-scoped rules, so you could scope frame-ancestors * only to widget routes and restrict all other routes:
# Non-widget routes - no iframe embedding needed
/*
Content-Security-Policy: default-src 'self'; ... frame-ancestors 'self'; ...
# Widget routes - must be embeddable anywhere
/:chainId/widget/*
Content-Security-Policy: default-src 'self'; ... frame-ancestors *; ...
This way:
- The main app at
/swap,/limit,/about, etc. is protected against clickjacking. - Widget routes remain embeddable by any third-party host, preserving the widget use case.
It doesn't eliminate the risk entirely (attackers could still frame a widget URL), but it meaningfully reduces the attack surface by limiting the embeddable surface to only the routes that actually need it.
Would you like me to draft the updated _headers file with this split?
✏️ Learnings added
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 7401
File: apps/cowswap-frontend/public/_headers:2-2
Timestamp: 2026-05-12T17:23:47.226Z
Learning: In the cowprotocol/cowswap repository, `cowswap-frontend` is intentionally embedded as an iframe on arbitrary third-party websites via the CowSwap widget (libs/widget-lib). Widget routes follow the URL pattern `/:chainId/widget/:tradeType/:sell/:buy`. The default widget host is `https://swap.cow.fi`. `frame-ancestors *` in the CSP is required for the widget use case; however, it can be scoped to widget routes only using Cloudflare Pages path-based `_headers` rules.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
If the access is limited to app paths, what would happen to static files? Do we need to manually whitelist any path that's supposed to be served by the app?
There was a problem hiding this comment.
@alfetopito, great question — there are actually two good news here:
1. Static files don't need manual whitelisting for frame-ancestors
frame-ancestors controls which documents are allowed to embed this document in a nested browsing context like an <iframe>. Browsers only enforce frame-ancestors when rendering a response as a document inside a frame. Static assets like JS bundles, CSS, images, and fonts are fetched as subresources — they're never framed directly — so frame-ancestors on their response headers has no meaningful security effect. You don't need to list /assets/*, /static/*, etc. separately.
2. However, the path-scoping approach I suggested has a catch with Cloudflare Pages
An incoming request which matches multiple rules' URL patterns will inherit all rules' headers, and if a header is applied twice, the values are joined with a comma separator.
This means if you had:
/*
Content-Security-Policy: ... frame-ancestors 'self'; ...
/:chainId/widget/*
Content-Security-Policy: ... frame-ancestors *; ...
Widget routes would match both rules, and the browser would receive two Content-Security-Policy headers joined with a comma. Per the CSP spec, when multiple CSP headers are present, the browser enforces all of them simultaneously (intersection), so frame-ancestors 'self' + frame-ancestors * would effectively leave only 'self' — the permissive widget rule would be silently negated.
To work around this on Cloudflare Pages, you'd need to:
- Use
!Content-Security-Policyin the widget rule to remove the base rule, then re-add the full widget-specific CSP. But this means maintaining the full CSP string in two places. - Or use a Cloudflare Worker to set headers conditionally based on path (more robust but more complex).
Given that complexity, the pragmatic trade-off is to leave frame-ancestors * globally as-is (current state), since:
- Widget embedding requires it
frame-ancestorson static assets is a no-op for browsers- The main framing risk is on interactive HTML pages, which realistically need the same policy as widget routes for this app anyway
Would you like me to explore the Worker-based approach, or are you comfortable keeping the current global frame-ancestors *?
elena-zh
left a comment
elena-zh
left a comment
There was a problem hiding this comment.
works as described, thank you
alfetopito
force-pushed
the
fix/vercel-cfpages
branch
from
5efbb17 to
e6e8a2d
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
2 participants
Summary
Adapt app/cf pages setup to vercel specific changes
To Test
curl -I https://fix-vercel-cfpages.swap-dev-5u6.pages.dev/