Skip to content

Conversation

LogicalGuy77
Copy link
Contributor

@LogicalGuy77 LogicalGuy77 commented Sep 18, 2025

User description

The email sending logic has been moved to a secure server-side API route at send-email.js. The client-side sendEmail function now calls this API route, so your API key is no longer exposed to the client.

In Next.js, any environment variable prefixed with NEXT_PUBLIC_ is embedded into the client-side JavaScript bundle and is accessible in the browser. This means anyone inspecting frontend code could see the API key


PR Type

Enhancement, Fixes #22


Description

  • Move email sending logic to secure server-side API endpoint

  • Remove client-side API key exposure vulnerability

  • Improve error handling and validation

  • Update environment variable naming convention


Diagram Walkthrough

flowchart LR
  A["Client emailService.js"] -- "POST request" --> B["Server API /send-email"]
  B -- "Brevo API call" --> C["External Brevo Service"]
  D["Environment Variable"] --> B
Loading

File Walkthrough

Relevant files
Enhancement
send-email.js
Create secure server-side email API endpoint                         

pages/api/send-email.js

  • Create new server-side API endpoint for email sending
  • Add email validation and error handling
  • Implement secure Brevo API integration with server-side key access
  • Return appropriate HTTP status codes and error messages
+53/-0   
emailService.js
Refactor client-side email service to use API                       

utils/services/emailService.js

  • Replace direct Brevo API calls with internal API endpoint calls
  • Simplify client-side logic by removing API key handling
  • Improve error handling with server response parsing
  • Maintain existing notification functionality
+11/-33 
Configuration changes
.env.example
Update environment variable naming convention                       

.env.example

  • Update environment variable name from NEXT_PUBLIC_BREVO_API_KEY to
    NEXT_PUBLIC_BREVO_API
+1/-1     

Copy link

vercel bot commented Sep 18, 2025

@LogicalGuy77 is attempting to deploy a commit to the lighthouse-storage Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 Security concerns

Environment variable exposure:
The PR correctly moves the API key handling to the server side, but the environment variable is still named with the NEXT_PUBLIC_ prefix (line 9 in send-email.js). Variables with this prefix are exposed to the client in Next.js applications. The environment variable should be renamed to remove this prefix (e.g., BREVO_API instead of NEXT_PUBLIC_BREVO_API) to ensure the API key remains secure.

⚡ Recommended focus areas for review

Environment Variable Naming

The API key is still using NEXT_PUBLIC_ prefix on the server side. This prefix should be removed since the variable is now only used server-side and should not be exposed to the client.

const brevo_key = process.env.NEXT_PUBLIC_BREVO_API;
Unused Variable

The brevo_key variable is still defined but no longer used in the refactored code. It should be removed to avoid confusion.

const brevo_key = process.env.NEXT_PUBLIC_BREVO_API;
Email Validation Redundancy

Email validation is performed both client-side and server-side. While this is good for security, the client-side validation could use the same validation function to ensure consistency.

if (!email || !validateEmail(email)) {
  return res.status(400).json({ error: "Invalid email address" });
}

Copy link

codiumai-pr-agent-free bot commented Sep 18, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Prevent API key client-side exposure

Remove the NEXT_PUBLIC_ prefix from the NEXT_PUBLIC_BREVO_API environment
variable to prevent exposing the API key on the client-side.

.env.example [1]

-NEXT_PUBLIC_BREVO_API=your_brevo_api_key_here
+BREVO_API=your_brevo_api_key_here
  • Apply / Chat
Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies a critical security vulnerability where an API key is exposed to the client-side due to the NEXT_PUBLIC_ prefix, which undermines the PR's goal of securing the key.

High
Use server-only environment variable

Replace process.env.NEXT_PUBLIC_BREVO_API with a server-only environment
variable (e.g., process.env.BREVO_API) to prevent exposing the API key on the
client-side.

pages/api/send-email.js [9]

-const brevo_key = process.env.NEXT_PUBLIC_BREVO_API;
+const brevo_key = process.env.BREVO_API;
  • Apply / Chat
Suggestion importance[1-10]: 10

__

Why: The suggestion correctly points out the use of a client-exposed environment variable on the server, which is a critical security flaw that defeats the purpose of the PR, and proposes the correct fix.

High
  • Update

@LogicalGuy77
Copy link
Contributor Author

@arpitB-dev hi, could you please review it and merge when you get time?
Also I would like to contribute more on this project, is there any major feature or bug that you suggest I could work on?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improvement: Move Email sending logic to server side
1 participant