fix: harden release workflows and widget snippets#7616
Open
fairlighteth wants to merge 6 commits into
Open
Conversation
- validate release tags against main before secret-bearing jobs run - sanitize widget embed params to block script and tradeType injection
Deploying explorer-dev with
|
| Latest commit: |
196741e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1e8e3715.explorer-dev-dxz.pages.dev |
| Branch Preview URL: | https://fix-deepsec-high-1-8.explorer-dev-dxz.pages.dev |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughTightens release commit validation across deployment workflows and parameterizes Vercel builds with a ChangesRelease Commit Validation and Safe Deployment Pipeline
Widget Snippet Security Hardening
Sequence Diagram(s)sequenceDiagram
participant Push as push/tag event
participant Validate as validate-release-tag
participant PreProd as vercel-pre-prod
participant VercelWF as .github/workflows/vercel.yml
Push->>Validate: run release validation script
Validate->>PreProd: output release-commit
PreProd->>VercelWF: workflow_call with checkout_ref
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 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 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: |
196741e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ad906104.swap-dev-5u6.pages.dev |
| Branch Preview URL: | https://fix-deepsec-high-1-8.swap-dev-5u6.pages.dev |
- load the staging sync path from a trusted ref before using app creds - call the tag-triggered Vercel reusable workflow from main instead of the tag workspace
Contributor
Author
✅ AI Review (Codex GPT-5, worked 2m): no new non-duplicate findingsReview completed. I found no new non-duplicate comments worth posting on the current PR head. Review scope and related contextRechecked on the latest PR head:
Verification gap:
|
Contributor
Author
✅ PR - Automated QA (Codex GPT-5, worked 12m): widget-configurator snippet hardening verified in browserOutcome
Source under test
Environment:
Wallet state reached:
If re-running locally:
Expected results:
Browser coverage:
Why these adjacent flows were included:
Note: unrelated Cypress failureImpact:
Observed behavior:
Interpretation:
Commands run:
Still manual if desired:
Artifacts:
Residual gaps:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/widget-configurator/src/app/embedDialog/utils/formatParameters.test.ts (1)
31-67: ⚡ Quick winAdd a regression case for malformed shape inputs (
enabledTradeTypesnon-array / falsytradeType).Current tests catch injected strings, but they miss the malformed-shape path that can break TS snippet generation. A single regression test here would lock this down.
🧪 Suggested test addition
+ it('handles malformed trade type shapes safely', () => { + const malformedParams = { + appCode: 'Widget App', + tradeType: '', + enabledTradeTypes: 'swap', + } as unknown as CowSwapWidgetParams + + expect(() => tsExample(malformedParams, defaultPalette)).not.toThrow() + const snippet = tsExample(malformedParams, defaultPalette) + expect(snippet).not.toContain('tradeType') + expect(snippet).not.toContain('enabledTradeTypes') + })🤖 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/widget-configurator/src/app/embedDialog/utils/formatParameters.test.ts` around lines 31 - 67, Add a regression test in formatParameters.test.ts to cover malformed-shape inputs for tsExample (and optionally jsExample): create params where enabledTradeTypes is not an array (e.g., null or a string) and tradeType is falsy/undefined, cast to CowSwapWidgetParams, invoke tsExample (and jsExample if desired), and assert the generated snippet does not include injected payloads like 'alert(1)' or malformed enum text and that valid enums (e.g., TradeType.SWAP) still appear when present; use the existing test style and functions tsExample, jsExample, CowSwapWidgetParams, enabledTradeTypes, and tradeType to locate where to add this case.
🤖 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 @.github/workflows/deployment.yml:
- Line 71: The workflow currently references the reusable workflow with a
mutable ref ("uses: cowprotocol/cowswap/.github/workflows/vercel.yml@main");
replace the "`@main`" ref with a pinned immutable ref (a specific commit SHA or a
release tag) so the reusable workflow cannot change between validation and
execution—update the "uses:
cowprotocol/cowswap/.github/workflows/vercel.yml@main" entry to use "@" followed
by the commit hash or stable tag.
In `@apps/widget-configurator/src/app/embedDialog/utils/sanitizeParameters.ts`:
- Around line 20-26: Ensure malformed trade-type fields are normalized: for
sanitized.tradeType, validate with isTradeType and set to undefined (or delete)
whenever it is not a valid trade type (including falsy empty string), not only
when truthy; for sanitized.enabledTradeTypes, coerce non-array values to an
empty array and then filter using isTradeType (i.e., if typeof
sanitized.enabledTradeTypes !== 'object' || !Array.isArray(...), set to []
before applying .filter). Update the logic around the sanitized object
(references: sanitized.tradeType and sanitized.enabledTradeTypes) so downstream
code that maps over enabledTradeTypes (used by formatParameters) always receives
an array of valid trade types.
---
Nitpick comments:
In `@apps/widget-configurator/src/app/embedDialog/utils/formatParameters.test.ts`:
- Around line 31-67: Add a regression test in formatParameters.test.ts to cover
malformed-shape inputs for tsExample (and optionally jsExample): create params
where enabledTradeTypes is not an array (e.g., null or a string) and tradeType
is falsy/undefined, cast to CowSwapWidgetParams, invoke tsExample (and jsExample
if desired), and assert the generated snippet does not include injected payloads
like 'alert(1)' or malformed enum text and that valid enums (e.g.,
TradeType.SWAP) still appear when present; use the existing test style and
functions tsExample, jsExample, CowSwapWidgetParams, enabledTradeTypes, and
tradeType to locate where to add this case.
🪄 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: 2e81d223-147f-47c4-9b6a-57e17e9c9dfa
📒 Files selected for processing (6)
.github/workflows/deployment-v2.yml.github/workflows/deployment.yml.github/workflows/vercel.ymlapps/widget-configurator/src/app/embedDialog/utils/formatParameters.test.tsapps/widget-configurator/src/app/embedDialog/utils/formatParameters.tsapps/widget-configurator/src/app/embedDialog/utils/sanitizeParameters.ts
shoom3301
reviewed
Jun 9, 2026
| if: startsWith(github.ref, 'refs/tags') | ||
| uses: ./.github/workflows/vercel.yml | ||
| secrets: inherit | ||
| needs: validate-release-tag |
Collaborator
There was a problem hiding this comment.
As far as I understand, we should remove deployment.yml soon due to the new deployment flow. @alfetopito Am I right? If yes, probably it's better to not tuch this file at all in order to reduce the PR changes scope
Collaborator
There was a problem hiding this comment.
Yes!
It's in my todo list after we move prod over to cf-pages.
Collaborator
|
For the first test case, the bad code is still there. Shouldn't the script be removed completely instead? 
On the 3rd case, the trade types match current behaviour on prod: 
|
alfetopito
reviewed
Jun 10, 2026
Collaborator
There was a problem hiding this comment.
As Sasha mentioned, this one can remain untouched as it'll be removed soon.
shoom3301
reviewed
Jun 15, 2026
| @@ -1,10 +1,12 @@ | |||
| import { CowSwapWidgetParams } from '@cowprotocol/widget-lib' | |||
Collaborator
There was a problem hiding this comment.
I would not touch widget-configurator as well to avoid conflicts with #7604
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Why
This PR addresses the following DeepSec high findings from the May 5, 2026 scan report:
Release-tag jobs expose GitHub App credentials to code loaded from the tag refRelease-tag deployment inherits secrets into a reusable workflow from the tag refToken input can break out of the generated HTML embed snippetCustom token list data can poison the generated HTML embed snippetGenerated Pure HTML snippet can break out of its script tagGenerated HTML embed snippet can break out of its inline scriptGenerated JavaScript snippet can contain injected code via tradeTypeGenerated TypeScript snippet trusts raw tradeType when emitting enum syntaxThose findings fall into two groups:
Release-tag workflow trust issues
Widget configurator code-generation injection issues
</script>content.What changed
Release workflow hardening
Files:
.github/workflows/deployment-v2.yml.github/workflows/deployment.yml.github/workflows/vercel.ymlChanges:
mainbefore any secret-bearing path runsmainsecrets: inheritin the release deployment path with an explicit secret allowlistSecurity effect:
Widget snippet hardening
Files:
apps/widget-configurator/src/app/embedDialog/utils/formatParameters.tsapps/widget-configurator/src/app/embedDialog/utils/sanitizeParameters.tsapps/widget-configurator/src/app/embedDialog/utils/formatParameters.test.tsChanges:
tradeTypeinterpolation in generated JS/TS snippetsTradeType.*enum syntax for known valid trade typestradeTypevalues instead of serializing attacker-controlled content into snippetsSecurity effect:
</script>breakout or rawtradeTypecode injection through configurator paramsScope
This PR is intentionally limited to the eight findings listed above.
It does not attempt to solve unrelated DeepSec findings.
Reviewer guide
Recommended review order:
.github/workflows/deployment.yml.github/workflows/deployment-v2.yml.github/workflows/vercel.ymlformatParameters.tssanitizeParameters.tsformatParameters.test.tsQA
1. Widget configurator regression check
Goal: confirm generated snippets no longer contain injectable payloads.
Setup:
pnpm start:widgetTest A: HTML snippet escaping
</script><script>alert(1)</script>\u003crather than raw</script>.<script>tag.Test B: invalid trade type in generated code
tradeTypevalue.TradeType.SWAP,TradeType.LIMIT, etc. in the TypeScript snippet.Expected result:
tradeTypeinput is removed/neutralizedTradeTypevalues still generate correct snippets2. Automated widget checks
Run:
pnpm exec jest --config apps/widget-configurator/jest.config.ts --runInBand apps/widget-configurator/src/app/embedDialog/utils/formatParameters.test.tspnpm exec eslint apps/widget-configurator/src/app/embedDialog/utils/formatParameters.ts apps/widget-configurator/src/app/embedDialog/utils/sanitizeParameters.ts apps/widget-configurator/src/app/embedDialog/utils/formatParameters.test.tsExpected result:
formatParameters.test.ts3. Release workflow review check
Goal: verify secret-bearing release jobs no longer trust tag workspace code.
Review points:
deployment.yml, confirm release tags are validated againstmaininvalidate-release-tag.vercel-pre-proddepends on that validation and passescheckout_refinto the reusable workflow.secrets: inheritis gone from the release deployment path.vercel.yml, confirm checkout usesinputs.checkout_refwhen provided.deployment-v2.yml, confirm staging/prod sync jobs check outmain, not the tag ref, before running the release sync action.Expected result:
Verification run for this PR
git diff --checkpnpm install --frozen-lockfilepnpm exec jest --config apps/widget-configurator/jest.config.ts --runInBand apps/widget-configurator/src/app/embedDialog/utils/formatParameters.test.tspnpm exec eslint apps/widget-configurator/src/app/embedDialog/utils/formatParameters.ts apps/widget-configurator/src/app/embedDialog/utils/sanitizeParameters.ts apps/widget-configurator/src/app/embedDialog/utils/formatParameters.test.tsNotes
pnpx nx run widget-configurator:test --runInBand --testPathPattern=formatParameters.test.tsandpnpx nx run widget-configurator:lintwere blocked in this worktree by repo-wide Nx project-graph/environment issues unrelated to the patch.Summary by CodeRabbit
New Features
Bug Fixes
tradeTypevalues and filteringenabledTradeTypesto valid entries only.Tests
Chores