Feat/translate about#228
Conversation
…ages - Add about page translations (EN, UK, PL) - Add auth.fields.validation translations for form errors
- Use dynamic import for groq-sdk (Netlify compatibility) - Bypass rate limiting for unknown IPs (serverless safety) - Safe JSON parsing with request.text() + empty body check - Fix ReferenceError: remove undefined errorMessage variable - Remove sensitive debug info from client responses - Add i18n keys: pricing.heading, sponsors.ctaAriaLabel
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe pull request introduces comprehensive internationalization support to the About page and its components using next-intl, adds metadata generation with translations, significantly enhances the AI explanation API route with rate limiting and diagnostics, implements client-side validation for authentication form fields, extends translation files across multiple locales, and updates Netlify and Next.js configurations for groq-sdk support. Changes
Sequence DiagramsequenceDiagram
actor Client
participant RateLimit as Rate Limiter
participant Validation as Request Validator
participant GroqSDK as Groq SDK
participant GroqAPI as Groq API
participant ErrorHandler as Error Handler
Client->>RateLimit: POST /api/ai/explain + IP
alt Rate limit exceeded
RateLimit->>ErrorHandler: Rate limit exceeded
ErrorHandler->>Client: 429 error
else Within limit
RateLimit->>Validation: Proceed with request
Validation->>Validation: Parse body, validate format
alt Invalid/empty body
Validation->>ErrorHandler: Invalid request
ErrorHandler->>Client: 400 error
else Valid body
Validation->>GroqSDK: Initialize Groq client
alt SDK init fails
GroqSDK->>ErrorHandler: SDK initialization error
ErrorHandler->>Client: 503 SDK_INIT_ERROR
else SDK ready
GroqSDK->>GroqAPI: Send explain request
alt API call succeeds
GroqAPI->>Client: Return explanation
else API call fails
GroqAPI->>ErrorHandler: Auth or model error
ErrorHandler->>Client: 401/503 error
end
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/components/about/CommunitySection.tsx (1)
140-142:⚠️ Potential issue | 🟡 MinorMissing translation for "via" prefix.
The word "via" is hardcoded but should be translated for consistency with the rest of the i18n work.
Suggested fix
The
TestimonialCardwould need access to the translation function. Consider either:
- Pass the translation function as a prop, or
- Call
useTranslationswithinTestimonialCard:function TestimonialCard({ name, role, avatar, content, platform, icon: Icon, color }: Testimonial) { + const t = useTranslations("about.community") return ( // ... <div className="mt-3 text-[10px] text-gray-400 dark:text-gray-600 font-mono flex items-center gap-1"> - via {platform} <ExternalLink size={8} /> + {t("via")} {platform} <ExternalLink size={8} /> </div>
🤖 Fix all issues with AI agents
In `@frontend/app/`[locale]/about/page.tsx:
- Around line 11-17: The generateMetadata function currently calls
getTranslations() with no locale; update its signature to accept params and pass
the locale to getTranslations (e.g., change to async function generateMetadata({
params }) and call getTranslations(params.locale, "about") or equivalent),
keeping the returned object with title: t("metaTitle") and description:
t("metaDescription"); this mirrors other pages (q&a/page.tsx, blog/page.tsx,
dashboard/page.tsx) and ensures metadata is generated in the correct locale.
In `@frontend/app/api/ai/explain/route.ts`:
- Around line 342-359: The GET health handler exposes sensitive environment
details; update the GET function to stop returning groqKeyLength, nodeEnv,
isNetlify, and context (or move them behind auth) — keep only non-sensitive
status/service/timestamp and a boolean hasGroqKey, or protect the route with
authentication/ACL so only internal users can fetch full diagnostics; locate the
GET export in route.ts and remove the sensitive fields from the
NextResponse.json payload (or wrap the detailed payload behind an auth check)
and ensure the response status logic using apiKey remains unchanged.
- Around line 93-97: The checkRateLimit function currently bypasses limits for
ip === 'unknown', which is exploitable; update checkRateLimit to not silently
allow unlimited requests: when ip === 'unknown' either apply a conservative
fallback rate (e.g., much lower MAX_REQUESTS_PER_WINDOW) or still enforce the
normal limits but keyed to a special "unknown" bucket, and emit a monitored
log/metric indicating the fallback was used (use the existing
logger/processLogger or telemetry). Also update callers (e.g., getClientIp) to
surface when IP extraction failed so the log contains context, and ensure the
returned object sets skipped to true only when a safe monitoring path is
enabled.
In `@frontend/lib/security/client-ip.ts`:
- Around line 25-27: The Netlify client IP header is currently trusted
unconditionally: update the logic in frontend/lib/security/client-ip.ts around
the netlifyIp variable to gate trusting 'x-nf-client-connection-ip' behind a new
environment flag (e.g., TRUST_NETLIFY_CLIENT_IP or similar) consistent with how
TRUST_CF_CONNECTING_IP and TRUST_FORWARDED_HEADERS are used; only read and
return netlifyIp when the env flag is truthy and isIP(netlifyIp) passes,
otherwise skip it so header spoofing is prevented and downstream IP resolution
falls back to existing checks.
In `@netlify.toml`:
- Around line 18-23: The CORS headers on the route defined by the [[headers]]
block for = "/api/ai/*" are overly permissive (Access-Control-Allow-Origin =
"*"); change the static header to a specific allowed origin or comma-separated
list of trusted domains instead of "*" or remove the static header and implement
dynamic CORS in the API code (validate Origin against an allowlist and set
Access-Control-Allow-Origin per-request) in your route handler (the API that
serves /api/ai/*), and ensure Access-Control-Allow-Methods and
Access-Control-Allow-Headers remain correct when you switch to the more
restrictive approach.
🧹 Nitpick comments (7)
netlify.toml (1)
13-13: Empty function configuration section.The
[functions."api-ai-explain-*"]section is empty and has no effect. Either add configuration options (e.g.,included_files,schedule, timeouts) or remove this line to avoid confusion.frontend/components/auth/fields/PasswordField.tsx (1)
18-27: Missing else clause may leave stale custom validity message.If the input becomes invalid for a reason not explicitly handled (e.g., if a
patternattribute is added later), the custom validity message from a previous state could persist. Add an else clause to reset custom validity for unhandled cases:const handleInvalid = (e: React.InvalidEvent<HTMLInputElement>) => { const input = e.target; if (input.validity.valueMissing) { input.setCustomValidity(t("validation.required")); } else if (input.validity.tooShort && minLength) { input.setCustomValidity( t("validation.passwordTooShort", { minLength }) ); + } else { + input.setCustomValidity(""); } };frontend/components/auth/fields/EmailField.tsx (1)
14-21: Clear custom validity before setting a new message.If the validity state changes between invocations (e.g., from
valueMissingtotypeMismatch), the previous custom validity message may persist sincesetCustomValidityis only called conditionally.Proposed fix
const handleInvalid = (e: React.InvalidEvent<HTMLInputElement>) => { const input = e.target; + input.setCustomValidity(""); // Clear first if (input.validity.valueMissing) { input.setCustomValidity(t("validation.required")); } else if (input.validity.typeMismatch) { input.setCustomValidity(t("validation.invalidEmail")); } };frontend/app/api/ai/explain/route.ts (1)
15-22: Consider removing API key length from logs in production.Logging the API key length (
apiKey.length) could provide minor information to attackers about the key format. While not critical, consider conditionally logging this only in development.Optional: Conditional logging
function logEnvironmentDiagnostics() { const apiKey = process.env.GROQ_API_KEY; console.log('[ENV] GROQ_API_KEY configured:', !!apiKey); - console.log('[ENV] GROQ_API_KEY length:', apiKey ? apiKey.length : 0); + if (process.env.NODE_ENV === 'development') { + console.log('[ENV] GROQ_API_KEY length:', apiKey ? apiKey.length : 0); + } console.log('[ENV] NODE_ENV:', process.env.NODE_ENV);frontend/components/about/HeroSection.tsx (1)
94-123: Consider adding proper TypeScript types for widget props.Both
GlassWidgetandMobileStatItemuseanyfor props, which loses type safety. Consider defining a shared interface.Suggested type definition
+interface StatWidgetProps { + icon: React.ComponentType<{ size: number }>; + color: string; + bg: string; + label: string; + value: string; +} + -function GlassWidget({ icon: Icon, color, bg, label, value }: any) { +function GlassWidget({ icon: Icon, color, bg, label, value }: StatWidgetProps) {Apply the same type to
MobileStatItem.frontend/components/about/PricingSection.tsx (1)
79-87: Avoid using translated strings as React keys.Using
key={item}whereitemis a translated string can cause issues:
- If two features have identical translations, React will produce duplicate key warnings
- Keys should be stable identifiers, not content that varies by locale
♻️ Proposed fix: Use index-based or stable keys
<ul className="space-y-4 mb-8 flex-1"> - {juniorFeatures.map((item) => ( - <li key={item} className="flex items-center gap-3 text-sm text-gray-700 dark:text-neutral-300"> + {juniorFeatures.map((item, index) => ( + <li key={`junior-feature-${index}`} className="flex items-center gap-3 text-sm text-gray-700 dark:text-neutral-300"><ul className="space-y-4 mb-8 flex-1"> - {heroFeatures.map((item) => ( - <li key={item} className="flex items-center gap-3 text-sm text-gray-900 dark:text-white font-medium"> + {heroFeatures.map((item, index) => ( + <li key={`hero-feature-${index}`} className="flex items-center gap-3 text-sm text-gray-900 dark:text-white font-medium">Alternatively, consider using translation keys as identifiers (similar to the approach in
FeaturesSection.tsx):const juniorFeatureKeys = ["unlimited", "fullAccess", "noCard", "noGuilt"] // Then map with: key={key} and t(`junior.features.${key}`)Also applies to: 129-137
frontend/components/about/FeaturesSection.tsx (1)
305-310: Consider SSR hydration for initialisMobilestate.The initial state uses
window.innerWidthwhich is only available on the client. While the conditional checktypeof window !== "undefined"prevents errors, during SSR this will always befalse, potentially causing a hydration mismatch if the server renders the desktop version but the client is on mobile.♻️ Optional: Use consistent initial state
For consistent SSR/CSR behavior, consider defaulting to
falseunconditionally and letting theuseEffecthandle the actual value:- const [isMobile, setIsMobile] = useState(() => - typeof window !== "undefined" ? window.innerWidth < 768 : false - ) + const [isMobile, setIsMobile] = useState(false)This ensures the server and client render the same initial state, with the client updating after hydration. The brief flash is typically acceptable, or you could use CSS media queries for the initial render.
| export async function generateMetadata() { | ||
| const t = await getTranslations("about") | ||
| return { | ||
| title: t("metaTitle"), | ||
| description: t("metaDescription"), | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing locale parameter in generateMetadata.
Other pages in this codebase (e.g., q&a/page.tsx, blog/page.tsx, dashboard/page.tsx) explicitly extract the locale from params and pass it to getTranslations. Without this, the metadata may not be generated in the correct locale.
Proposed fix to match existing patterns
-export async function generateMetadata() {
- const t = await getTranslations("about")
+export async function generateMetadata({
+ params,
+}: {
+ params: Promise<{ locale: string }>;
+}) {
+ const { locale } = await params;
+ const t = await getTranslations({ locale, namespace: "about" });
return {
title: t("metaTitle"),
description: t("metaDescription"),
}
}🤖 Prompt for AI Agents
In `@frontend/app/`[locale]/about/page.tsx around lines 11 - 17, The
generateMetadata function currently calls getTranslations() with no locale;
update its signature to accept params and pass the locale to getTranslations
(e.g., change to async function generateMetadata({ params }) and call
getTranslations(params.locale, "about") or equivalent), keeping the returned
object with title: t("metaTitle") and description: t("metaDescription"); this
mirrors other pages (q&a/page.tsx, blog/page.tsx, dashboard/page.tsx) and
ensures metadata is generated in the correct locale.
| function checkRateLimit(ip: string): { allowed: boolean; remaining: number; resetIn: number; skipped: boolean } { | ||
| // Bypass rate limiting for unknown IPs (serverless safety) | ||
| if (ip === 'unknown') { | ||
| return { allowed: true, remaining: MAX_REQUESTS_PER_WINDOW, resetIn: RATE_LIMIT_WINDOW_MS, skipped: true }; | ||
| } |
There was a problem hiding this comment.
Rate limit bypass for unknown IPs may be exploitable.
Bypassing rate limiting when ip === 'unknown' could allow abuse if getClientIp fails to extract IPs in certain environments (e.g., misconfigured proxies). Consider logging when rate limiting is skipped to monitor for abuse, or applying a stricter fallback.
Suggested improvement
function checkRateLimit(ip: string): { allowed: boolean; remaining: number; resetIn: number; skipped: boolean } {
// Bypass rate limiting for unknown IPs (serverless safety)
if (ip === 'unknown') {
+ console.warn('[RATE_LIMIT] Skipped - unknown IP');
return { allowed: true, remaining: MAX_REQUESTS_PER_WINDOW, resetIn: RATE_LIMIT_WINDOW_MS, skipped: true };
}📝 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.
| function checkRateLimit(ip: string): { allowed: boolean; remaining: number; resetIn: number; skipped: boolean } { | |
| // Bypass rate limiting for unknown IPs (serverless safety) | |
| if (ip === 'unknown') { | |
| return { allowed: true, remaining: MAX_REQUESTS_PER_WINDOW, resetIn: RATE_LIMIT_WINDOW_MS, skipped: true }; | |
| } | |
| function checkRateLimit(ip: string): { allowed: boolean; remaining: number; resetIn: number; skipped: boolean } { | |
| // Bypass rate limiting for unknown IPs (serverless safety) | |
| if (ip === 'unknown') { | |
| console.warn('[RATE_LIMIT] Skipped - unknown IP'); | |
| return { allowed: true, remaining: MAX_REQUESTS_PER_WINDOW, resetIn: RATE_LIMIT_WINDOW_MS, skipped: true }; | |
| } |
🤖 Prompt for AI Agents
In `@frontend/app/api/ai/explain/route.ts` around lines 93 - 97, The
checkRateLimit function currently bypasses limits for ip === 'unknown', which is
exploitable; update checkRateLimit to not silently allow unlimited requests:
when ip === 'unknown' either apply a conservative fallback rate (e.g., much
lower MAX_REQUESTS_PER_WINDOW) or still enforce the normal limits but keyed to a
special "unknown" bucket, and emit a monitored log/metric indicating the
fallback was used (use the existing logger/processLogger or telemetry). Also
update callers (e.g., getClientIp) to surface when IP extraction failed so the
log contains context, and ensure the returned object sets skipped to true only
when a safe monitoring path is enabled.
| export async function GET() { | ||
| const apiKey = process.env.GROQ_API_KEY; | ||
| return NextResponse.json( | ||
| { | ||
| status: apiKey ? 'ok' : 'misconfigured', | ||
| service: 'ai-explain', | ||
| timestamp: new Date().toISOString(), | ||
| env: { | ||
| hasGroqKey: !!apiKey, | ||
| groqKeyLength: apiKey ? apiKey.length : 0, | ||
| nodeEnv: process.env.NODE_ENV, | ||
| isNetlify: !!process.env.NETLIFY, | ||
| context: process.env.CONTEXT ?? 'unknown', | ||
| }, | ||
| }, | ||
| { status: apiKey ? 200 : 503 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Health endpoint exposes potentially sensitive environment details.
The GET endpoint publicly exposes groqKeyLength, nodeEnv, isNetlify, and deployment context. This information could aid attackers in fingerprinting the environment. Consider restricting access or reducing the information exposed.
Suggested: Reduce public exposure
export async function GET() {
const apiKey = process.env.GROQ_API_KEY;
return NextResponse.json(
{
status: apiKey ? 'ok' : 'misconfigured',
service: 'ai-explain',
timestamp: new Date().toISOString(),
- env: {
- hasGroqKey: !!apiKey,
- groqKeyLength: apiKey ? apiKey.length : 0,
- nodeEnv: process.env.NODE_ENV,
- isNetlify: !!process.env.NETLIFY,
- context: process.env.CONTEXT ?? 'unknown',
- },
},
{ status: apiKey ? 200 : 503 }
);
}If detailed diagnostics are needed, consider protecting this endpoint with authentication or moving it to a separate internal route.
📝 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.
| export async function GET() { | |
| const apiKey = process.env.GROQ_API_KEY; | |
| return NextResponse.json( | |
| { | |
| status: apiKey ? 'ok' : 'misconfigured', | |
| service: 'ai-explain', | |
| timestamp: new Date().toISOString(), | |
| env: { | |
| hasGroqKey: !!apiKey, | |
| groqKeyLength: apiKey ? apiKey.length : 0, | |
| nodeEnv: process.env.NODE_ENV, | |
| isNetlify: !!process.env.NETLIFY, | |
| context: process.env.CONTEXT ?? 'unknown', | |
| }, | |
| }, | |
| { status: apiKey ? 200 : 503 } | |
| ); | |
| } | |
| export async function GET() { | |
| const apiKey = process.env.GROQ_API_KEY; | |
| return NextResponse.json( | |
| { | |
| status: apiKey ? 'ok' : 'misconfigured', | |
| service: 'ai-explain', | |
| timestamp: new Date().toISOString(), | |
| }, | |
| { status: apiKey ? 200 : 503 } | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@frontend/app/api/ai/explain/route.ts` around lines 342 - 359, The GET health
handler exposes sensitive environment details; update the GET function to stop
returning groqKeyLength, nodeEnv, isNetlify, and context (or move them behind
auth) — keep only non-sensitive status/service/timestamp and a boolean
hasGroqKey, or protect the route with authentication/ACL so only internal users
can fetch full diagnostics; locate the GET export in route.ts and remove the
sensitive fields from the NextResponse.json payload (or wrap the detailed
payload behind an auth check) and ensure the response status logic using apiKey
remains unchanged.
| const netlifyIp = (headers.get('x-nf-client-connection-ip') ?? '').trim(); | ||
| if (netlifyIp && isIP(netlifyIp)) return netlifyIp; | ||
|
|
There was a problem hiding this comment.
Unconditional trust of x-nf-client-connection-ip creates a header spoofing risk.
Unlike cf-connecting-ip (gated by TRUST_CF_CONNECTING_IP) and forwarded headers (gated by TRUST_FORWARDED_HEADERS), the Netlify header is trusted unconditionally. If the app is deployed outside Netlify or behind a different proxy, attackers can spoof this header to bypass IP-based rate limiting or security controls.
Consider adding an environment variable gate for consistency:
🔒 Proposed fix to add trust gate for Netlify header
const trustCf = envBool('TRUST_CF_CONNECTING_IP', false);
+ const trustNetlify = envBool('TRUST_NETLIFY_IP', false);
- const netlifyIp = (headers.get('x-nf-client-connection-ip') ?? '').trim();
- if (netlifyIp && isIP(netlifyIp)) return netlifyIp;
+ if (trustNetlify) {
+ const netlifyIp = (headers.get('x-nf-client-connection-ip') ?? '').trim();
+ if (netlifyIp && isIP(netlifyIp)) return netlifyIp;
+ }🤖 Prompt for AI Agents
In `@frontend/lib/security/client-ip.ts` around lines 25 - 27, The Netlify client
IP header is currently trusted unconditionally: update the logic in
frontend/lib/security/client-ip.ts around the netlifyIp variable to gate
trusting 'x-nf-client-connection-ip' behind a new environment flag (e.g.,
TRUST_NETLIFY_CLIENT_IP or similar) consistent with how TRUST_CF_CONNECTING_IP
and TRUST_FORWARDED_HEADERS are used; only read and return netlifyIp when the
env flag is truthy and isIP(netlifyIp) passes, otherwise skip it so header
spoofing is prevented and downstream IP resolution falls back to existing
checks.
| [[headers]] | ||
| for = "/api/ai/*" | ||
| [headers.values] | ||
| Access-Control-Allow-Origin = "*" | ||
| Access-Control-Allow-Methods = "POST, OPTIONS" | ||
| Access-Control-Allow-Headers = "Content-Type" |
There was a problem hiding this comment.
Overly permissive CORS configuration.
Access-Control-Allow-Origin = "*" allows any origin to make requests to your AI API endpoints. Since this API likely processes authenticated user requests and may have rate limiting tied to user identity, consider restricting this to your specific domains:
[[headers]]
for = "/api/ai/*"
[headers.values]
- Access-Control-Allow-Origin = "*"
+ Access-Control-Allow-Origin = "https://devlovers.net"
Access-Control-Allow-Methods = "POST, OPTIONS"
Access-Control-Allow-Headers = "Content-Type"Alternatively, if you need multiple origins, handle CORS dynamically in the API route itself to validate against an allowlist.
📝 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.
| [[headers]] | |
| for = "/api/ai/*" | |
| [headers.values] | |
| Access-Control-Allow-Origin = "*" | |
| Access-Control-Allow-Methods = "POST, OPTIONS" | |
| Access-Control-Allow-Headers = "Content-Type" | |
| [[headers]] | |
| for = "/api/ai/*" | |
| [headers.values] | |
| Access-Control-Allow-Origin = "https://devlovers.net" | |
| Access-Control-Allow-Methods = "POST, OPTIONS" | |
| Access-Control-Allow-Headers = "Content-Type" |
🤖 Prompt for AI Agents
In `@netlify.toml` around lines 18 - 23, The CORS headers on the route defined by
the [[headers]] block for = "/api/ai/*" are overly permissive
(Access-Control-Allow-Origin = "*"); change the static header to a specific
allowed origin or comma-separated list of trusted domains instead of "*" or
remove the static header and implement dynamic CORS in the API code (validate
Origin against an allowlist and set Access-Control-Allow-Origin per-request) in
your route handler (the API that serves /api/ai/*), and ensure
Access-Control-Allow-Methods and Access-Control-Allow-Headers remain correct
when you switch to the more restrictive approach.
2 participants
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.