Add Intercom support and enhance phishing detection in AI search#1184
Add Intercom support and enhance phishing detection in AI search#1184
Conversation
WalkthroughThis update introduces Intercom messenger integration into the mail application's navigation sidebar, including UI changes for help and feedback access. It adds backend support for generating JWT tokens for Intercom via a new TRPC route, updates navigation configuration, and manages related dependencies and environment variables. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NavMain (Frontend)
participant TRPC (Backend)
participant Intercom SDK
User->>NavMain (Frontend): Clicks Help button
NavMain (Frontend)->>TRPC (Backend): getIntercomToken query
TRPC (Backend)->>NavMain (Frontend): Returns JWT token
NavMain (Frontend)->>Intercom SDK: Initialize with JWT token
NavMain (Frontend)->>Intercom SDK: Show Intercom Messenger UI
Possibly related PRs
Suggested reviewers
Poem
✨ 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 (
|
MrgSub
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/mail/components/mail/mail-display.tsx(1 hunks)apps/mail/components/ui/nav-main.tsx(4 hunks)apps/mail/config/navigation.ts(0 hunks)apps/mail/package.json(1 hunks)apps/server/package.json(1 hunks)apps/server/src/trpc/routes/user.ts(2 hunks)apps/server/wrangler.jsonc(1 hunks)
💤 Files with no reviewable changes (1)
- apps/mail/config/navigation.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/server/src/trpc/routes/user.ts (1)
apps/server/src/trpc/trpc.ts (1)
privateProcedure(21-29)
apps/mail/components/ui/nav-main.tsx (4)
apps/mail/components/ui/sidebar.tsx (1)
SidebarMenuButton(605-605)apps/mail/config/navigation.ts (2)
MessageKey(25-25)NavItem(27-38)apps/mail/components/icons/icons.tsx (1)
MessageSquare(491-508)apps/mail/types/index.ts (1)
NavItem(36-42)
🔇 Additional comments (7)
apps/mail/package.json (1)
24-24:Details
❓ Verification inconclusive
Verify package version and security status.
The version
0.0.14suggests this might be an early or beta release. Please ensure this is the latest stable version and check for any known security vulnerabilities.
🏁 Script executed:
#!/bin/bash # Check the latest version and security advisories for @intercom/messenger-js-sdk # Get latest version from npm npm view @intercom/messenger-js-sdk version # Check for security advisories npm audit --package-lock-only --audit-level=lowLength of output: 907
🏁 Script executed:
#!/bin/bash # Generate a lockfile in the mail app and run an audit to check for vulnerabilities cd apps/mail npm install --package-lock-only npm audit --audit-level=lowLength of output: 632
Confirmed latest version; manual security audit required
- File: apps/mail/package.json
- The version
0.0.14matches the latest release on npm.- Automated
npm auditfailed due to an unsupported “catalog:” URL protocol in your registry config.Please perform a manual security review (for example, run
npm auditwith a valid lockfile or use a third-party tool like Snyk) to ensure there are no known vulnerabilities in@intercom/messenger-js-sdk.apps/server/package.json (1)
37-37:Details
❓ Verification inconclusive
Verify JWT library version and security.
Please ensure this is the latest version of the JWT library and check for any known security vulnerabilities.
🏁 Script executed:
#!/bin/bash # Check the latest version of @tsndr/cloudflare-worker-jwt npm view @tsndr/cloudflare-worker-jwt version # Check for security advisories npm audit --package-lock-only --audit-level=lowLength of output: 908
🏁 Script executed:
#!/bin/bash # Generate a lockfile and re-run audit to catch any vulnerabilities cd apps/server npm install --package-lock-only npm audit --audit-level=lowLength of output: 634
JWT library version confirmed, manual security review needed
npm viewshows that@tsndr/cloudflare-worker-jwt@3.2.0is the current latest stable release. Attempts to runnpm auditin the sandbox failed due to missing lockfiles and unsupported protocols. Please perform a manual security check for known vulnerabilities (e.g., via npmjs.com/package/@tsndr/cloudflare-worker-jwt, Snyk, or your preferred advisory database).• Location:
apps/server/package.json(line 37)apps/mail/components/mail/mail-display.tsx (1)
646-648: Excellent security enhancement for phishing detection.This addition to the search query enhances user security by helping identify potentially suspicious email domains. The examples provided (x.io vs x.com, paypalcom.com vs paypal.com) effectively illustrate common phishing tactics.
apps/server/src/trpc/routes/user.ts (1)
2-2: LGTM - Clean import additionThe import for the Cloudflare Worker JWT library is properly added.
apps/mail/components/ui/nav-main.tsx (3)
21-23: LGTM - Clean import additionsThe new imports for react-query, Intercom SDK, and icons are properly organized and support the new functionality.
199-199: LGTM - Proper translation hook usageGood addition of the translation hook for internationalization support.
202-207: LGTM - Simplified toast usageThe change to use
toast.promisedirectly is cleaner and more readable than the previous implementation.
| "COOKIE_DOMAIN": "localhost", | ||
| "VITE_PUBLIC_BACKEND_URL": "http://localhost:8787", | ||
| "VITE_PUBLIC_APP_URL": "http://localhost:3000", | ||
| "JWT_SECRET": "secret", |
There was a problem hiding this comment.
Critical security issue: weak JWT secret and missing production configuration.
Two critical issues identified:
- Security vulnerability: Using "secret" as the JWT secret is extremely insecure and predictable.
- Missing configuration:
JWT_SECRETis only defined for the local environment but missing from staging and production.
For the local environment, use a stronger secret:
- "JWT_SECRET": "secret",
+ "JWT_SECRET": "your-strong-local-jwt-secret-here",You also need to add JWT_SECRET to staging and production environments with appropriate secure values. Consider using environment-specific secrets management.
🤖 Prompt for AI Agents
In apps/server/wrangler.jsonc at line 69, replace the weak JWT_SECRET value
"secret" with a strong, complex secret for the local environment. Additionally,
add JWT_SECRET entries with secure, environment-specific values for the staging
and production configurations to ensure proper security across all environments.
Use environment-specific secrets management practices to handle these sensitive
values safely.
| getIntercomToken: privateProcedure.query(async ({ ctx }) => { | ||
| const token = await jwt.sign( | ||
| { | ||
| user_id: ctx.session.user.id, | ||
| email: ctx.session.user.email, | ||
| }, | ||
| ctx.c.env.JWT_SECRET, | ||
| ); | ||
| return token; | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and security improvements for JWT generation
The JWT token generation implementation needs several improvements:
- Missing token expiration: JWT tokens should have an expiration time for security
- No error handling: JWT signing could fail and should be wrapped in try-catch
- Environment variable validation: Should validate that
JWT_SECRETexists
Apply this diff to improve security and error handling:
getIntercomToken: privateProcedure.query(async ({ ctx }) => {
+ if (!ctx.c.env.JWT_SECRET) {
+ throw new TRPCError({
+ code: 'INTERNAL_SERVER_ERROR',
+ message: 'JWT_SECRET not configured',
+ });
+ }
+
+ try {
const token = await jwt.sign(
{
user_id: ctx.session.user.id,
email: ctx.session.user.email,
+ exp: Math.floor(Date.now() / 1000) + (60 * 60), // 1 hour expiration
},
ctx.c.env.JWT_SECRET,
);
return token;
+ } catch (error) {
+ throw new TRPCError({
+ code: 'INTERNAL_SERVER_ERROR',
+ message: 'Failed to generate Intercom token',
+ });
+ }
}),Don't forget to import TRPCError at the top of the file:
-import { privateProcedure, router } from '../trpc';
+import { privateProcedure, router } from '../trpc';
+import { TRPCError } from '@trpc/server';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getIntercomToken: privateProcedure.query(async ({ ctx }) => { | |
| const token = await jwt.sign( | |
| { | |
| user_id: ctx.session.user.id, | |
| email: ctx.session.user.email, | |
| }, | |
| ctx.c.env.JWT_SECRET, | |
| ); | |
| return token; | |
| }), | |
| // At the top of apps/server/src/trpc/routes/user.ts | |
| import { privateProcedure, router } from '../trpc'; | |
| import { TRPCError } from '@trpc/server'; | |
| getIntercomToken: privateProcedure.query(async ({ ctx }) => { | |
| // Validate that the secret is present | |
| if (!ctx.c.env.JWT_SECRET) { | |
| throw new TRPCError({ | |
| code: 'INTERNAL_SERVER_ERROR', | |
| message: 'JWT_SECRET not configured', | |
| }); | |
| } | |
| try { | |
| const token = await jwt.sign( | |
| { | |
| user_id: ctx.session.user.id, | |
| email: ctx.session.user.email, | |
| // 1 hour expiration | |
| exp: Math.floor(Date.now() / 1000) + 60 * 60, | |
| }, | |
| ctx.c.env.JWT_SECRET, | |
| ); | |
| return token; | |
| } catch (error) { | |
| throw new TRPCError({ | |
| code: 'INTERNAL_SERVER_ERROR', | |
| message: 'Failed to generate Intercom token', | |
| }); | |
| } | |
| }), |
🤖 Prompt for AI Agents
In apps/server/src/trpc/routes/user.ts around lines 15 to 24, improve the JWT
token generation by adding an expiration time to the token payload, wrapping the
signing process in a try-catch block to handle potential errors, and validating
that the JWT_SECRET environment variable exists before signing. If JWT_SECRET is
missing, throw a TRPCError with an appropriate message. Also, ensure TRPCError
is imported at the top of the file.
| const { data: intercomToken } = useQuery(trpc.user.getIntercomToken.queryOptions()); | ||
|
|
||
| React.useEffect(() => { | ||
| if (intercomToken) { | ||
| Intercom({ | ||
| app_id: 'aavenrba', | ||
| intercom_user_jwt: intercomToken, | ||
| }); | ||
| } | ||
| }, [intercomToken]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move hardcoded configuration to environment variables and add error handling
The Intercom integration has several issues:
- Hardcoded app_id: The Intercom app ID
'aavenrba'should be an environment variable - No error handling: Missing error handling for token fetch failures
- No loading state: No handling for when the token is still loading
Consider this improved implementation:
- const { data: intercomToken } = useQuery(trpc.user.getIntercomToken.queryOptions());
+ const { data: intercomToken, error: tokenError, isLoading: tokenLoading } = useQuery(
+ trpc.user.getIntercomToken.queryOptions()
+ );
React.useEffect(() => {
- if (intercomToken) {
+ if (intercomToken && !tokenError && !tokenLoading) {
Intercom({
- app_id: 'aavenrba',
+ app_id: process.env.NEXT_PUBLIC_INTERCOM_APP_ID || 'aavenrba',
intercom_user_jwt: intercomToken,
});
+ } else if (tokenError) {
+ console.error('Failed to load Intercom token:', tokenError);
}
- }, [intercomToken]);
+ }, [intercomToken, tokenError, tokenLoading]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: intercomToken } = useQuery(trpc.user.getIntercomToken.queryOptions()); | |
| React.useEffect(() => { | |
| if (intercomToken) { | |
| Intercom({ | |
| app_id: 'aavenrba', | |
| intercom_user_jwt: intercomToken, | |
| }); | |
| } | |
| }, [intercomToken]); | |
| const { data: intercomToken, error: tokenError, isLoading: tokenLoading } = useQuery( | |
| trpc.user.getIntercomToken.queryOptions() | |
| ); | |
| React.useEffect(() => { | |
| if (intercomToken && !tokenError && !tokenLoading) { | |
| Intercom({ | |
| app_id: process.env.NEXT_PUBLIC_INTERCOM_APP_ID || 'aavenrba', | |
| intercom_user_jwt: intercomToken, | |
| }); | |
| } else if (tokenError) { | |
| console.error('Failed to load Intercom token:', tokenError); | |
| } | |
| }, [intercomToken, tokenError, tokenLoading]); |
🤖 Prompt for AI Agents
In apps/mail/components/ui/nav-main.tsx around lines 84 to 93, replace the
hardcoded Intercom app_id 'aavenrba' with a value read from an environment
variable. Add error handling to manage failures when fetching the intercomToken,
such as logging or displaying an error message. Also, implement a loading state
to handle the period when the token is being fetched before it is available.
Update the React.useEffect to only initialize Intercom when the token is
successfully fetched and handle cases where the token fetch fails or is loading.
| {isBottomNav ? ( | ||
| <> | ||
| <SidebarMenuButton | ||
| onClick={() => show()} | ||
| tooltip={state === 'collapsed' ? t('help' as MessageKey) : undefined} | ||
| className="flex cursor-pointer items-center" | ||
| > | ||
| <MessageSquare className="relative mr-2.5 h-3 w-3.5" /> | ||
| <p className="mt-0.5 truncate text-[13px]">Help</p> | ||
| </SidebarMenuButton> | ||
| <NavItem | ||
| key={'feedback'} | ||
| isActive={isUrlActive('https://feedback.0.email')} | ||
| href={'https://feedback.0.email'} | ||
| url={'https://feedback.0.email'} | ||
| icon={MessageSquare} | ||
| target={'_blank'} | ||
| title={'navigation.sidebar.feedback'} | ||
| /> | ||
| </> | ||
| ) : null} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve consistency and configuration management in bottom navigation
Several inconsistencies and hardcoded values in the new bottom navigation:
- Inconsistent translation usage: "Help" is hardcoded while "Feedback" uses translation key
- Hardcoded feedback URL: Should be configurable
- No error handling:
show()call could fail
Apply this diff to improve consistency:
<SidebarMenuButton
onClick={() => show()}
tooltip={state === 'collapsed' ? t('help' as MessageKey) : undefined}
className="flex cursor-pointer items-center"
>
<MessageSquare className="relative mr-2.5 h-3 w-3.5" />
- <p className="mt-0.5 truncate text-[13px]">Help</p>
+ <p className="mt-0.5 truncate text-[13px]">{t('navigation.sidebar.help' as MessageKey)}</p>
</SidebarMenuButton>
<NavItem
key={'feedback'}
- isActive={isUrlActive('https://feedback.0.email')}
- href={'https://feedback.0.email'}
- url={'https://feedback.0.email'}
+ isActive={isUrlActive(process.env.NEXT_PUBLIC_FEEDBACK_URL || 'https://feedback.0.email')}
+ href={process.env.NEXT_PUBLIC_FEEDBACK_URL || 'https://feedback.0.email'}
+ url={process.env.NEXT_PUBLIC_FEEDBACK_URL || 'https://feedback.0.email'}
icon={MessageSquare}
target={'_blank'}
title={'navigation.sidebar.feedback'}
/>Also consider wrapping the show() call in error handling:
- onClick={() => show()}
+ onClick={() => {
+ try {
+ show();
+ } catch (error) {
+ console.error('Failed to show Intercom:', error);
+ }
+ }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {isBottomNav ? ( | |
| <> | |
| <SidebarMenuButton | |
| onClick={() => show()} | |
| tooltip={state === 'collapsed' ? t('help' as MessageKey) : undefined} | |
| className="flex cursor-pointer items-center" | |
| > | |
| <MessageSquare className="relative mr-2.5 h-3 w-3.5" /> | |
| <p className="mt-0.5 truncate text-[13px]">Help</p> | |
| </SidebarMenuButton> | |
| <NavItem | |
| key={'feedback'} | |
| isActive={isUrlActive('https://feedback.0.email')} | |
| href={'https://feedback.0.email'} | |
| url={'https://feedback.0.email'} | |
| icon={MessageSquare} | |
| target={'_blank'} | |
| title={'navigation.sidebar.feedback'} | |
| /> | |
| </> | |
| ) : null} | |
| {isBottomNav ? ( | |
| <> | |
| <SidebarMenuButton | |
| - onClick={() => show()} | |
| + onClick={() => { | |
| + try { | |
| + show(); | |
| + } catch (error) { | |
| + console.error('Failed to show Intercom:', error); | |
| + } | |
| + }} | |
| tooltip={state === 'collapsed' ? t('help' as MessageKey) : undefined} | |
| className="flex cursor-pointer items-center" | |
| > | |
| <MessageSquare className="relative mr-2.5 h-3 w-3.5" /> | |
| - <p className="mt-0.5 truncate text-[13px]">Help</p> | |
| + <p className="mt-0.5 truncate text-[13px]"> | |
| + {t('navigation.sidebar.help' as MessageKey)} | |
| + </p> | |
| </SidebarMenuButton> | |
| <NavItem | |
| key={'feedback'} | |
| - isActive={isUrlActive('https://feedback.0.email')} | |
| - href={'https://feedback.0.email'} | |
| - url={'https://feedback.0.email'} | |
| + isActive={isUrlActive(process.env.NEXT_PUBLIC_FEEDBACK_URL || 'https://feedback.0.email')} | |
| + href={process.env.NEXT_PUBLIC_FEEDBACK_URL || 'https://feedback.0.email'} | |
| + url={process.env.NEXT_PUBLIC_FEEDBACK_URL || 'https://feedback.0.email'} | |
| icon={MessageSquare} | |
| target={'_blank'} | |
| title={'navigation.sidebar.feedback'} | |
| /> | |
| </> | |
| ) : null} |
🤖 Prompt for AI Agents
In apps/mail/components/ui/nav-main.tsx around lines 212 to 232, fix
inconsistencies by replacing the hardcoded "Help" text with a translation key
similar to "Feedback" using the t() function. Make the feedback URL configurable
by moving it to a constant or config variable instead of hardcoding it. Wrap the
show() call inside a try-catch block to handle potential errors gracefully,
logging or managing errors as appropriate.
2 participants
Add Intercom integration and enhance phishing detection in mail display
Description
This PR adds Intercom integration for customer support and enhances the phishing detection capabilities in the mail display component. The changes include:
Type of Change
Areas Affected
Testing Done
Security Considerations
Checklist
Additional Notes
The phishing detection enhancement improves security by adding domain validation to the AI search query, helping users identify suspicious email domains more effectively. The Intercom integration provides a direct support channel for users within the application.
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
Enhancements
Chores