Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe updates adjust how email content and privacy settings are managed and displayed. The Changes
Sequence Diagram(s)sequenceDiagram
participant MailDisplay
participant MailContent
participant EmailProcessor
MailDisplay->>MailContent: Render with { id, html, senderEmail }
MailContent->>MailContent: useQuery([id, ...])
MailContent->>EmailProcessor: Sanitize html (allowing img tags)
EmailProcessor-->>MailContent: Sanitized HTML
MailContent-->>MailDisplay: Rendered content
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Minor but important fixes across email handling and security components, focusing on content identification and sanitization.
- Added required 'id' prop to
apps/mail/components/mail/mail-content.tsxfor proper email content caching and identification - Enhanced security in
apps/server/src/lib/email-processor.tsby implementing stricter HTML tag whitelisting while preserving image support - Fixed form handling in
apps/mail/app/(routes)/settings/privacy/page.tsxto properly initialize and manage external image settings - Improved mail navigation in
apps/mail/hooks/use-mail-navigation.tsby removing redundant URL checks and updating state management
5 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile
| const form = useForm<z.infer<typeof userSettingsSchema>>({ | ||
| resolver: zodResolver(userSettingsSchema), | ||
| defaultValues: { | ||
| externalImages: true, | ||
| trustedSenders: [], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
logic: Form validation will fail until form is initialized. Add default values here to match schema.
| const form = useForm<z.infer<typeof userSettingsSchema>>({ | |
| resolver: zodResolver(userSettingsSchema), | |
| defaultValues: { | |
| externalImages: true, | |
| trustedSenders: [], | |
| }, | |
| }); | |
| const form = useForm<z.infer<typeof userSettingsSchema>>({ | |
| resolver: zodResolver(userSettingsSchema), | |
| defaultValues: { | |
| externalImages: false, | |
| trustedSenders: [] | |
| } | |
| }); |
| const [threadId] = useQueryState('threadId'); | ||
| const [isCommandPaletteOpen] = useQueryState('isCommandPaletteOpen'); |
There was a problem hiding this comment.
style: Both these query state hooks should specify their types. Add generic type parameters to useQueryState.
| const [threadId] = useQueryState('threadId'); | |
| const [isCommandPaletteOpen] = useQueryState('isCommandPaletteOpen'); | |
| const [threadId] = useQueryState<string | null>('threadId'); | |
| const [isCommandPaletteOpen] = useQueryState<boolean>('isCommandPaletteOpen'); |
| const sanitizeConfig: sanitizeHtml.IOptions = { | ||
| allowedTags: false, | ||
| allowedTags: sanitizeHtml.defaults.allowedTags.concat(['img']), | ||
| allowedAttributes: false, |
There was a problem hiding this comment.
logic: Setting allowedAttributes: false permits all attributes. Consider whitelisting specific attributes needed for email display (src, href, style, etc).
| allowedAttributes: false, | |
| allowedAttributes: { | |
| '*': ['style', 'class'], | |
| 'a': ['href', 'target', 'rel'], | |
| 'img': ['src', 'alt', 'width', 'height'] | |
| }, |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: UI Reactivity Lost with State Change
The change from form.watch('externalImages') to data?.settings.externalImages breaks UI reactivity. The trusted senders section's visibility no longer updates immediately when the user toggles the 'external images' switch, as it now depends on the saved server state rather than the current form state. This results in inconsistent UI behavior until the form is saved.
apps/mail/app/(routes)/settings/privacy/page.tsx#L36-L37
Zero/apps/mail/app/(routes)/settings/privacy/page.tsx
Lines 36 to 37 in 5790d77
Bug: Navigation Logic Error Due to Variable Shadowing
Variable shadowing causes incorrect navigation logic. A local threadId (derived from message.id) shadows the threadId from useQueryState within the navigateToThread function. This makes the if (threadId) condition evaluate the local message.id (always truthy for valid messages) instead of the threadId from the URL query parameter. Consequently, navigation now always occurs if a message ID exists, deviating from the original behavior which required a threadId URL parameter.
apps/mail/hooks/use-mail-navigation.ts#L26-L92
Zero/apps/mail/hooks/use-mail-navigation.ts
Lines 26 to 92 in 5790d77
Bug: Hotkey Conflicts Due to Command Palette State Handling
The isCommandPaletteOpen variable, which controls hotkey enablement, was incorrectly changed from useCommandPalette().open to useQueryState('isCommandPaletteOpen'). As useQueryState returns an array (which is always truthy), !isCommandPaletteOpen will always evaluate to false. This causes keyboard navigation hotkeys to remain enabled even when the command palette is open, leading to conflicting inputs.
apps/mail/hooks/use-mail-navigation.ts#L27-L28
Zero/apps/mail/hooks/use-mail-navigation.ts
Lines 27 to 28 in 5790d77
BugBot free trial expires on July 22, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
1 participant
READ CAREFULLY THEN REMOVE
Remove bullet points that are not relevant.
PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.
Description
Please provide a clear description of your changes.
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
Add any other context about the pull request here.
Screenshots/Recordings
Add screenshots or recordings here if applicable.
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes