feat: allow link cloaking for Organization accounts in workflow emails#26528
feat: allow link cloaking for Organization accounts in workflow emails#26528
Conversation
Organization accounts are paid accounts with lower spam/scam risk, so they are allowed to use cloaked links (URL behind text) in workflow emails. Changes: - Frontend: Allow 'link' toolbar item in Editor for Organization accounts - Backend: Skip replaceCloakedLinksInHtml sanitization for Organization workflows - Repository: Include team.isOrganization in workflow reminder query 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:
|
E2E results are ready! |
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
Co-Authored-By: peer@cal.com <peer@cal.com>
…king-1767773716' into devin/org-link-cloaking-1767773716
…ked links for orgs
Ryukemeister
left a comment
There was a problem hiding this comment.
email reminders and workflow reminders were missing logic for enabling cloaked emails for orgs, updated the code to fix that. all good now!
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
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/api/scheduleEmailReminders.ts">
<violation number="1" location="packages/features/ee/workflows/api/scheduleEmailReminders.ts:360">
P2: This change contradicts the stated security design in the PR description. The description explicitly says this deprecated file should "default to sanitizing all links (safer behavior)" as the intentional security posture for edge cases. Adding the organization check here allows cloaked links for organizations, which is inconsistent with that design. If the stricter default is intended for deprecated paths, remove these organization checks and let all emails have links sanitized in this file.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const isOrganization = reminder.workflowStep?.workflow?.team?.isOrganization ?? false; | ||
| const processedEmailBody = isOrganization | ||
| ? emailContent.emailBody | ||
| : replaceCloakedLinksInHtml(emailContent.emailBody); |
There was a problem hiding this comment.
P2: This change contradicts the stated security design in the PR description. The description explicitly says this deprecated file should "default to sanitizing all links (safer behavior)" as the intentional security posture for edge cases. Adding the organization check here allows cloaked links for organizations, which is inconsistent with that design. If the stricter default is intended for deprecated paths, remove these organization checks and let all emails have links sanitized in this file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/ee/workflows/api/scheduleEmailReminders.ts, line 360:
<comment>This change contradicts the stated security design in the PR description. The description explicitly says this deprecated file should "default to sanitizing all links (safer behavior)" as the intentional security posture for edge cases. Adding the organization check here allows cloaked links for organizations, which is inconsistent with that design. If the stricter default is intended for deprecated paths, remove these organization checks and let all emails have links sanitized in this file.</comment>
<file context>
@@ -355,10 +355,17 @@ export async function handler(req: NextRequest) {
+ // Organization accounts are allowed to use cloaked links (URL behind text)
+ // since they are paid accounts with lower spam/scam risk
+ const isOrganization = reminder.workflowStep?.workflow?.team?.isOrganization ?? false;
+ const processedEmailBody = isOrganization
+ ? emailContent.emailBody
</file context>
Fix confidence (alpha): 8/10
| const isOrganization = reminder.workflowStep?.workflow?.team?.isOrganization ?? false; | |
| const processedEmailBody = isOrganization | |
| ? emailContent.emailBody | |
| : replaceCloakedLinksInHtml(emailContent.emailBody); | |
| const processedEmailBody = replaceCloakedLinksInHtml(emailContent.emailBody); |
Devin AI is addressing Cubic AI's review feedbackA Devin session has been created to address the issues identified by Cubic AI. |
sean-brydon
left a comment
There was a problem hiding this comment.
Fixed one of the queries to follow best practise.
LGTM
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 allows Organization accounts to use cloaked links (URL behind text) in workflow emails, while keeping the restriction for non-Organization accounts.
Context: Previously, PR #26292 prevented all link cloaking in workflow emails to help recipients identify potentially malicious URLs. This change relaxes that restriction for Organization accounts since they are paid accounts with lower spam/scam risk.
Changes:
replaceCloakedLinksInHtmlsanitization for Organization workflowsteam.isOrganizationin the workflow reminder queryImplementation Scope:
EmailWorkflowService.handleSendEmailWorkflowTask(the main tasker-based scheduled email path)emailReminderManager, deprecatedscheduleEmailReminders.ts) default to sanitizing all links (safer behavior)Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Organization account test:
<a href="https://example.com">Click here</a>)Non-Organization account test:
Human Review Checklist
isOrganizationprop inWorkflowStepContainer.tsxis derived from the same source asworkflow.team?.isOrganizationin the backendWorkflowReminderRepository.findByIdIncludeStepAndWorkflowis the query used for tasker-based email sendsscheduleEmailReminders.tsstill sanitizes all links (intentional - safer default)Link to Devin run: https://app.devin.ai/sessions/c245edc8038c4ac99ae88d65cbe65b71
Requested by: peer@cal.com (@PeerRich)