Only show external images banner if there was a CSP violation#648
Only show external images banner if there was a CSP violation#648ahmetskilinc merged 3 commits intoMail-0:stagingfrom
Conversation
|
@danteissaias is attempting to deploy a commit to the Zero Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes update the CSP handling in two parts of the mail application. In the mail iframe component, a new state variable tracks CSP violations by listening to window messages and conditionally renders a warning. In the email utilities, a nonce-based CSP is implemented by generating a nonce for the meta tag and adding a listener for security policy violation events. Both modifications bolster security by controlling script execution and handling CSP violations. Changes
Sequence Diagram(s)sequenceDiagram
participant Window as Parent Window
participant MailIframe as MailIframe Component
Window->>MailIframe: "csp-violation" message event
MailIframe->>MailIframe: setCspViolation(true)
MailIframe-->>User: Render warning message
sequenceDiagram
participant Template as Email Template Generator
participant Script as CSP Event Listener
participant Parent as Parent Window
Template->>Template: Generate nonce and embed in CSP meta tag
Template->>Script: Insert script to listen for securitypolicyviolation events
Script->>Parent: Send violation message event
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/mail/lib/email-utils.ts (1)
23-24: Review CSP configuration for potential security risks
Allowing'unsafe-inline'for styles (and'img-src *'if imagesEnabled) can pose a security risk. Verify whether these directives are required for emails, or consider tightening them if feasible.apps/mail/components/mail/mail-iframe.tsx (1)
92-105: Validate event origin for enhanced security
When listening formessageevents, consider checkingevent.originor otherwise verifying the sender to mitigate spoofed messages from other windows.window.addEventListener( 'message', (event) => { - if (event.data.type === 'csp-violation') { + if (event.origin === window.location.origin && event.data?.type === 'csp-violation') { setCspViolation(true); } }, { signal: ctrl.signal }, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/mail/components/mail/mail-iframe.tsx(3 hunks)apps/mail/lib/email-utils.ts(1 hunks)
🔇 Additional comments (4)
apps/mail/lib/email-utils.ts (1)
27-38: Script approach for detecting CSP violations looks solid
This addition effectively logs CSP violations and lets the parent know about them. Looks good as is.apps/mail/components/mail/mail-iframe.tsx (3)
16-16: Well-structured state variable for tracking CSP violations
ThecspViolationstate variable is straightforward and supports the new banner logic.
109-125: Conditionally rendering the banner meets PR goals
Displaying the banner only when there's a CSP violation and images are disabled is precisely in line with the objective to avoid unnecessary warnings.
149-149: Updated sandbox attribute aligns with nonce-based CSP
Addingallow-scriptsis consistent with the new CSP handling where only scripts with the proper nonce can run.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
Description
This PR introduces a way to detect if an email's iframe violates our content security policy, for example trying to load external images when the user hasn't enable them.
We now only show the "allow external images" yellow banner if we detect there has actually been a violation.
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Describe the tests you've done:
Security Considerations
For changes involving data or authentication:
Checklist
Additional Notes
N/A
Screenshots/Recordings
N/A
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit