feat: enhance CowSwapWidget with Safe SDK bridge option and improve e…#7460
feat: enhance CowSwapWidget with Safe SDK bridge option and improve e…#7460fairlighteth wants to merge 8 commits intodevelopfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
WalkthroughAdds a remark-based HTML image extractor and integrates it into article rendering (removes rehype_raw), introduces an optional enableSafeSdkBridge flag with widget lifecycle and test refactors, extracts DUNE API key handling to a helper and removes hardcoded env value, and standardizes route cache/CSP headers. ChangesSafe SDK Bridge Configuration
Markdown HTML Image Processing Rewrite
Configuration & Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: |
755edff
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
Hey @fairlighteth , thank you, but could you please provide a bit more instructions on how to test these steps:
-
Open or mock an article body containing a raw HTML image, e.g.
<img src="https://..." alt="...">;the image should render through the lazy image path. -
Raw non-image HTML such as <script>, <iframe>, or inline event handlers should not render.
-
Unsafe image sources such as javascript: should be ignored.
-
Confirm apps/cow-fi/.env no longer contains a real Dune API key.
-
Provide DUNE_API_KEY through local/deployment secrets and verify Dune-backed data still loads.
-
Without DUNE_API_KEY, the app should fail closed for Dune calls instead of using a committed fallback secret.
-
Response headers for /widget should include Content-Security-Policy: frame-ancestors 'self' and X-Frame-Options: SAMEORIGIN.
-
Safe SDK bridge forwarding should be disabled for the cow-fi widget page.
As for the rest steps, I:
- opened articles page: the only thing that I noticed that a new article blinks (shakes) 2-3 times while it is being loaded. I don't observe the same behavior on Prod.
- widget: is loaded, trades can be placed from it https://explorer.cow.fi/base/orders/0x31921244657582675e104dc156415a9e89d2ecb6cfaa0589c575653506c2c5e59fa3c00a92ec5f96b1ad2527ab41b3932efeda5869fb8897
|
Hey @elena-zh, thanks for checking. Some of these are easier to verify from the implementation side, so no need to spend too much QA time on the lower-level/security ones. I can cover those and report back with the exact evidence. For QA, the most useful checks would be:
I’ll validate the rest myself (and front-end):
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
libs/widget-lib/src/types.ts (1)
65-65: ⚡ Quick winAdd JSDoc for the new public
enableSafeSdkBridgeprop.This is a public widget API addition. Surrounding props are documented with JSDoc that explains intent, defaults, and security/behavior tradeoffs. Without docs, consumers won't know:
- the default is
true,- what it actually disables (Safe SDK forwarding while preserving widget params/app data/deeplinks/parent message handling),
- when they should set it to
false.📝 Proposed documentation
onReady?(): void - enableSafeSdkBridge?: boolean + /** + * Controls whether the widget creates an `IframeSafeSdkBridge` to forward Safe SDK + * messages between the host page and the iframe. + * + * Set to `false` to disable Safe SDK forwarding while keeping widget params, + * app-data updates, deeplinks, and parent message handling intact. + * + * Defaults to `true`. + */ + enableSafeSdkBridge?: boolean }🤖 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 `@libs/widget-lib/src/types.ts` at line 65, Add JSDoc for the new public prop enableSafeSdkBridge in libs/widget-lib/src/types.ts: document that its default is true, describe that setting it to false disables Safe SDK forwarding while still preserving widget params, app data, deeplinks, and parent message handling, and explain recommended use cases and security/behavior tradeoffs (when to set false). Place the JSDoc immediately above the enableSafeSdkBridge? boolean declaration and follow the same style/annotations as adjacent props so consumers see defaults and implications.apps/cow-fi/util/markdownHtmlImages.test.ts (1)
12-93: 💤 Low valueSolid coverage of the safe-list contract.
The mix of positive-extraction, container-wrapping, multi-image, scheme-injection (including the

obfuscation), disallowed-parent rejection, and AST round-trip with mixed allowed/disallowed siblings exercises the most important invariants ofmarkdownHtmlImages.ts.If you want to lock down the contract a bit more, consider adding cases for:
data-srcpreference oversrc, HTML-entity-encodedaltround-tripping (e.g.,&→&), comment stripping (<!-- ... --><img ...>), and nested HTML inside a list item or blockquote so the recursivetransformAllowedHtmlImagesis exercised at depth.🤖 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/cow-fi/util/markdownHtmlImages.test.ts` around lines 12 - 93, Add tests covering data-src fallback/preference, HTML-entity-encoded alt decoding, comment-stripping, and nested-container recursion: specifically add cases asserting getAllowedHtmlImage prefers data-src over src (e.g., <img data-src="/uploads/x.png" src="/bad.png">), that alt values with entities (e.g., "&") decode to "&", that leading HTML comments before an image (<!--...--><img ...>) still yield a valid image, and that remarkAllowedHtmlImages/transformAllowedHtmlImages correctly transform images when they appear nested inside list items or blockquotes to exercise depth recursion; reference getAllowedHtmlImage, getAllowedHtmlImages, and remarkAllowedHtmlImages in the new tests.apps/cow-fi/util/markdownHtmlImages.ts (1)
175-191: 💤 Low valueConsider broader HTML entity coverage for
alttext.
decodeHtmlAttributeonly decodesamp/apos/gt/lt/quotplus numeric references. Common CMS-authored alt-text entities like©, ,™,—,…will be rendered/announced literally.Two pragmatic options:
- Extend the local
HTML_ENTITIESmap with the handful of entities your CMS actually emits (lowest effort, recommended for this utility's scope).- Use
helibrary (most robust, but adds 121.2 KB; overkill if CMS output is limited).Either way, keep the order: decode → safety-check, exactly as today.
This is purely a cosmetic/accessibility quality issue for
alttext —srcis unaffected because URLs are percent-encoded by authors anyway.🤖 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/cow-fi/util/markdownHtmlImages.ts` around lines 175 - 191, The decodeHtmlAttribute function only covers a few named entities; extend its decoding to handle common CMS entities by adding entries to the HTML_ENTITIES map (e.g., copy, nbsp, trade, mdash, hellip, etc.) or alternatively replace the local decoder with a call to a robust library like he in decodeHtmlAttribute while preserving the current decode → safety-check flow; update references in decodeHtmlAttribute and ensure the normalizedEntity lookup still uses HTML_ENTITIES[normalizedEntity] so existing fallback numeric decoding and String.fromCodePoint behavior remain unchanged.
🤖 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.
Nitpick comments:
In `@apps/cow-fi/util/markdownHtmlImages.test.ts`:
- Around line 12-93: Add tests covering data-src fallback/preference,
HTML-entity-encoded alt decoding, comment-stripping, and nested-container
recursion: specifically add cases asserting getAllowedHtmlImage prefers data-src
over src (e.g., <img data-src="/uploads/x.png" src="/bad.png">), that alt values
with entities (e.g., "&") decode to "&", that leading HTML comments before
an image (<!--...--><img ...>) still yield a valid image, and that
remarkAllowedHtmlImages/transformAllowedHtmlImages correctly transform images
when they appear nested inside list items or blockquotes to exercise depth
recursion; reference getAllowedHtmlImage, getAllowedHtmlImages, and
remarkAllowedHtmlImages in the new tests.
In `@apps/cow-fi/util/markdownHtmlImages.ts`:
- Around line 175-191: The decodeHtmlAttribute function only covers a few named
entities; extend its decoding to handle common CMS entities by adding entries to
the HTML_ENTITIES map (e.g., copy, nbsp, trade, mdash, hellip, etc.) or
alternatively replace the local decoder with a call to a robust library like he
in decodeHtmlAttribute while preserving the current decode → safety-check flow;
update references in decodeHtmlAttribute and ensure the normalizedEntity lookup
still uses HTML_ENTITIES[normalizedEntity] so existing fallback numeric decoding
and String.fromCodePoint behavior remain unchanged.
In `@libs/widget-lib/src/types.ts`:
- Line 65: Add JSDoc for the new public prop enableSafeSdkBridge in
libs/widget-lib/src/types.ts: document that its default is true, describe that
setting it to false disables Safe SDK forwarding while still preserving widget
params, app data, deeplinks, and parent message handling, and explain
recommended use cases and security/behavior tradeoffs (when to set false). Place
the JSDoc immediately above the enableSafeSdkBridge? boolean declaration and
follow the same style/annotations as adjacent props so consumers see defaults
and implications.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 50eb3a86-b5da-4307-aab8-52a604404473
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
apps/cow-fi/.envapps/cow-fi/app/(main)/widget/page.tsxapps/cow-fi/components/ArticlePageComponent.tsxapps/cow-fi/next.config.tsapps/cow-fi/package.jsonapps/cow-fi/services/dune/index.tsxapps/cow-fi/util/lazyLoadImages.tsapps/cow-fi/util/markdownHtmlImages.test.tsapps/cow-fi/util/markdownHtmlImages.tslibs/ui/src/index.tslibs/widget-lib/src/cowSwapWidget.spec.tslibs/widget-lib/src/cowSwapWidget.tslibs/widget-lib/src/types.tslibs/widget-react/package.jsonlibs/widget-react/src/lib/CowSwapWidget.spec.tsxlibs/widget-react/src/lib/CowSwapWidget.tsx
💤 Files with no reviewable changes (2)
- apps/cow-fi/util/lazyLoadImages.ts
- apps/cow-fi/package.json
|
Hey @fairlighteth , thank you for the clarification. Everything works great, however, I still can see the page blinking when an article is being loaded. Also, this article cannot be loaded for me https://cowfi-git-fix-deepsec-cowswap.vercel.app/learn/what-you-need-to-know-about-crypto-linked-orders-in-2026, however, it workd fine on Prod. Could you please check why? |
3 participants
Summary
Addresses the HIGH findings from the vulnerability report for apps/cow-fi.
This PR hardens the cow-fi app while preserving expected user-facing behavior:
Screenshots not applicable; this is security hardening plus behavior preservation.
To Test
<img src="https://..." alt="...">;the image should render through the lazy image path.Background
The vulnerability report flagged cow-fi issues around exposed secrets, unsafe raw markdown/HTML handling, and widget embedding behavior.
The initial safe-looking markdown fix removed rehypeRaw, but that also dropped CMS-authored raw![]()
tags that existing article content relies on. This PR keeps the hardening by leaving skipHtml enabled, then explicitly allowlists only raw HTML image
tags and turns them into markdown image nodes before rendering.
For /widget, replacing the widget with a raw iframe would remove the parent-side widget bridge. Instead, this PR keeps the existing widget wrapper and adds a narrow enableSafeSdkBridge flag so only the risky Safe SDK forwarding is disabled while normal widget messaging remains intact.
Summary by CodeRabbit
New Features
Improvements
Tests
Chores