fix: prevent link cloaking and add malicious URL detection in workflows#26292
Conversation
- Exclude 'link' toolbar item from Editor for all workflow actions (not just SMS) - Add replaceCloakedLinksInHtml utility to sanitize HTML and replace cloaked links with visible URLs - Apply sanitization to email content before sending in both event and form workflows This ensures recipients can see the actual destination of URLs, helping them identify potentially malicious links. Co-Authored-By: peer@cal.com <peer@cal.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
- Changed regex from [^<]* to [\s\S]*? to capture nested HTML content - Added HTML tag stripping before comparing link text with href - Added comprehensive unit tests covering: - Basic cloaked links - Nested HTML tags (<b>, <strong>, <span>) - Already-visible URLs (should remain unchanged) - Empty link text - Multiple links in content - Edge cases (multiline, single quotes, etc.) Co-Authored-By: peer@cal.com <peer@cal.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Co-Authored-By: peer@cal.com <peer@cal.com>
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/ee/workflows/lib/reminders/utils.ts">
<violation number="1" location="packages/features/ee/workflows/lib/reminders/utils.ts:146">
P1: XSS vulnerability: The `href` value is inserted directly into HTML text content without escaping. If a malicious href contains `</a><script>...</script>`, it would close the anchor tag and execute arbitrary JavaScript. The `href` should be HTML-escaped before being inserted into text content.</violation>
</file>
Reply to cubic to teach it or ask questions. Tag @cubic-dev-ai to re-run a review.
Co-Authored-By: peer@cal.com <peer@cal.com>
…tps://git-manager.devin.ai/proxy/github.com/calcom/cal.com into devin/prevent-link-cloaking-workflows-1767043625
E2E results are ready! |
|
Unfortunately, this PR is a modest idea. Users should be free to activate or deactivate this option. Especially with a self-hosted instance, this is a form of paternalism that I would very much like to reverse. |
- Exclude 'link' toolbar item from Editor for all workflow actions (not just SMS) - Add replaceCloakedLinksInHtml utility to sanitize HTML and replace cloaked links with visible URLs - Apply sanitization to email content before sending in all workflow email paths This ensures recipients can see the actual destination of URLs, helping them identify potentially malicious links. Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
i agree. we are reverting this change |
This reverts the changes from PR #26292 and PR #26528 to restore link cloaking functionality for all users. The plan is to fight malicious links with Cloudflare URL Scanner instead of restricting link cloaking to Organization accounts only. Changes: - Remove replaceCloakedLinksInHtml function and its usage - Restore link toolbar item for all users in workflow editor - Remove isOrganization checks from email workflow processing - Delete the replaceCloakedLinksInHtml test file Co-Authored-By: peer@cal.com <peer@cal.com>
What does this PR do?
This PR adds two security features for workflow emails and SMS:
1. Link Cloaking Prevention - Prevents users from hiding URLs behind custom text (e.g., "Click here" linking to a phishing URL). The full URL is always displayed to recipients.
2. Malicious URL Detection - Integrates Cloudflare Radar URL Scanner to detect malicious URLs in workflow content and event type redirect URLs. When malicious URLs are detected, the user's account is locked.
Link Cloaking Prevention Changes:
replaceCloakedLinksInHtmlutility that transforms<a href="url">text</a>→<a href="url">url</a>Malicious URL Detection Changes:
urlScanner.tsutility for Cloudflare Radar URL Scanner API integrationscanWorkflowUrlsasync task with submit → poll pattern (15s intervals, max 10 attempts)scanWorkflowBodytask (runs after Iffy spam scan)successRedirectUrl)MALICIOUS_URL_IN_WORKFLOWlock reasonwhitelistWorkflowsflag - whitelisted users won't be auto-lockedEnvironment Variables Required
For malicious URL detection:
CLOUDFLARE_URL_SCANNER_API_TOKEN- API token for Cloudflare RadarCLOUDFLARE_ACCOUNT_ID- Cloudflare account IDURL scanning is automatically disabled if these are not set.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Link Cloaking Prevention:
Malicious URL Detection:
CLOUDFLARE_URL_SCANNER_API_TOKENandCLOUDFLARE_ACCOUNT_IDenvironment variablesscanWorkflowUrlstaskChecklist
Human Review Checklist
/client/v4/accounts/{account_id}/urlscanner/v2/scan)whitelistWorkflowsflag correctly prevents auto-lockingImportant Review Notes
Fail-open behavior: If Cloudflare API submissions fail or max poll attempts reached, the workflow step is marked as verified (not blocked). This prevents legitimate workflows from being blocked due to API issues.
Async scanning: URL scanning is async via the tasker system. Workflow steps have
verifiedAtset to null until scanning completes. Steps with nullverifiedAtshould not be sent.Pre-existing fixes: This PR also fixes two pre-existing lint issues:
parseInt(..., 0)→parseInt(..., 10)in constants.tssecondaryEmail && secondaryEmail.emailVerified→secondaryEmail?.emailVerifiedin update.handler.tsLink to Devin run: https://app.devin.ai/sessions/c245edc8038c4ac99ae88d65cbe65b71
Requested by: peer@cal.com (@PeerRich)