-
Notifications
You must be signed in to change notification settings - Fork 170
fix: vercel -> cfpages changes #7401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
alfetopito
wants to merge
6
commits into
develop
Choose a base branch
from
fix/vercel-cfpages
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
24be81e
fix: add headers to mimic vercel behaviour on cf pages
alfetopito e25893d
fix: pin node to actual version instead of version name to fix cfpages
alfetopito 0d9f06c
chore: add docs about cf migration
alfetopito 893f422
fix: sentry source maps upload to prevent cf error with large files
alfetopito 65d4852
fix: increase build size to 8g as 4g is breaking at cf!!!
alfetopito e6e8a2d
fix: get correct path to delete generated source map files
alfetopito File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| lts/jod | ||
| 22.15.1 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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; | ||
| Cross-Origin-Opener-Policy: same-origin-allow-popups | ||
| Cross-Origin-Resource-Policy: cross-origin | ||
| X-Content-Type-Options: nosniff | ||
| Referrer-Policy: strict-origin-when-cross-origin | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| # Cloudflare Pages Migration Notes | ||
|
|
||
| Scope: `apps/cowswap-frontend` and `apps/explorer` only. `cow-fi` is intentionally excluded. | ||
|
|
||
| ## Repo-backed equivalents | ||
|
|
||
| ### `apps/cowswap-frontend` | ||
|
|
||
| Vercel source of truth: | ||
| - `apps/cowswap-frontend/vercel.ts` | ||
|
|
||
| Cloudflare Pages equivalent: | ||
| - `apps/cowswap-frontend/public/_headers` | ||
|
|
||
| Captured in `_headers`: | ||
| - `Content-Security-Policy` | ||
| - `Cross-Origin-Opener-Policy` | ||
| - `Cross-Origin-Resource-Policy` | ||
| - `X-Content-Type-Options` | ||
| - `Referrer-Policy` | ||
|
|
||
| Notes: | ||
| - The Vercel redirect that sends non-asset routes to `/` is not copied into `_redirects`. | ||
| - Cloudflare Pages already provides SPA fallback for projects without a top-level `404.html`, which matches the current app output. | ||
|
|
||
| ### `apps/explorer` | ||
|
|
||
| Vercel source of truth: | ||
| - `apps/explorer/vercel.json` | ||
|
|
||
| Cloudflare Pages equivalent: | ||
| - No repo file is required for the current rewrite behavior. | ||
|
|
||
| Notes: | ||
| - The Vercel rewrite from `/(.*)` to `/index.html` is intentionally not translated to `_redirects`. | ||
| - Cloudflare Pages SPA fallback covers this app as long as the build output does not introduce a top-level `404.html`. | ||
|
|
||
| ## Cloudflare Pages UI or API settings | ||
|
|
||
| These settings do not have an equivalent static file in this repo for the current setup and must be configured per Pages project: | ||
| - Git repository connection | ||
| - Production branch | ||
| - Preview branch controls | ||
| - Root directory | ||
| - Build command | ||
| - Build output directory | ||
| - Environment variables | ||
| - Secrets | ||
| - Custom domains | ||
| - Branch-to-domain mappings, if used | ||
|
|
||
| Recommended project settings: | ||
|
|
||
| ### `cowswap` | ||
| - Root directory: repository root | ||
| - Build command: `pnpm run install:ci && pnpm run build:cowswap` | ||
| - Build output directory: `build/cowswap` | ||
| - Build variable: `SKIP_DEPENDENCY_INSTALL=1` | ||
| - Secret: `SENTRY_AUTH_TOKEN` for build-time sourcemap upload | ||
| - Optional overrides: | ||
| - `SENTRY_ORG=cowprotocol` | ||
| - `SENTRY_PROJECT=frontend` | ||
| - These are already the repo defaults and only need to be set if you want to override them. | ||
|
|
||
| ### `explorer` | ||
| - Root directory: repository root | ||
| - Build command: `pnpm run install:ci && pnpm run build:explorer` | ||
| - Build output directory: `build/explorer` | ||
| - Build variable: `SKIP_DEPENDENCY_INSTALL=1` | ||
|
|
||
| ## No exact Cloudflare Pages file alternative | ||
|
|
||
| ### `cowswap` Vercel regex redirect | ||
|
|
||
| Current Vercel behavior: | ||
| - `source: /((?!#|.*[\\w\\d\\.-]\\.\\w{2,15}$).+)` | ||
| - `destination: /` | ||
|
|
||
| Why it is not translated: | ||
| - Cloudflare Pages `_redirects` does not support the same regex semantics. | ||
| - Using a broad `_redirects` rule here would be a behavior change because Pages redirect rules run before header matching and static asset resolution logic differs from the Vercel config. | ||
|
|
||
| Fallback used instead: | ||
| - Native Cloudflare Pages SPA serving. | ||
|
|
||
| ## 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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:
🏁 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)frame-ancestors 'self' https://trusted-partner.com🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Good point — let me check the widget architecture to better understand the embedding pattern.
🏁 Script executed:
Length of output: 909
Let me also check how the widget constructs iframe URLs to understand the embedding pattern better.
🏁 Script executed:
Length of output: 18895
@alfetopito, you're absolutely right — changingframe-ancestorsglobally to'none'or'self'would break the widget, sincelibs/widget-libis designed to embedcowswap-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_headerssupports path-scoped rules, so you could scopeframe-ancestors *only to widget routes and restrict all other routes:This way:
/swap,/limit,/about, etc. is protected against clickjacking.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
_headersfile with this split?✏️ Learnings added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alfetopito, great question — there are actually two good news here:1. Static files don't need manual whitelisting for
frame-ancestorsframe-ancestorscontrols which documents are allowed to embed this document in a nested browsing context like an<iframe>. Browsers only enforceframe-ancestorswhen 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 — soframe-ancestorson 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:
Widget routes would match both rules, and the browser would receive two
Content-Security-Policyheaders joined with a comma. Per the CSP spec, when multiple CSP headers are present, the browser enforces all of them simultaneously (intersection), soframe-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:
!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.Given that complexity, the pragmatic trade-off is to leave
frame-ancestors *globally as-is (current state), since:frame-ancestorson static assets is a no-op for browsersWould you like me to explore the Worker-based approach, or are you comfortable keeping the current global
frame-ancestors *?