(feat): secure Gemini API architecture and harden AI prompt pipeline#51
Open
vraj826 wants to merge 4 commits into
Open
(feat): secure Gemini API architecture and harden AI prompt pipeline#51vraj826 wants to merge 4 commits into
vraj826 wants to merge 4 commits into
Conversation
|
@vraj826 is attempting to deploy a commit to the nirvik34's projects Team on Vercel. A member of the Team first needs to authorize it. |
Author
|
@nirvik34 The PR is ready to be reviewed. |
Contributor
|
Warning Review limit reached
More reviews will be available in 38 minutes and 58 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Warning
|
| Layer / File(s) | Summary |
|---|---|
Validation schemas and tests frontend/src/lib/validation.ts, frontend/src/lib/validation.test.ts |
Zod schemas define generate request shape, commit-result constraints, success/error responses, and inferred TS types; tests exercise valid/invalid cases and safe-default normalization. |
Prompt sanitization and tests frontend/src/lib/sanitizePrompt.ts, frontend/src/lib/sanitizePrompt.test.ts |
Regex-based sanitization removes control characters, normalizes whitespace/blank lines, replaces instruction-override phrases with a placeholder, truncates to MAX_PROMPT_LENGTH, and formats untrusted prompt blocks; tests verify normalization, injection filtering, and truncation. |
Rate limiter and tests frontend/src/lib/rateLimit.ts, frontend/src/lib/rateLimit.test.ts |
Module-level Map-backed per-identifier rate limiter with env-configurable windowMs, maxRequests, and cooldownMs, proxy-aware client identifier resolution, bucket cleanup, checkRateLimit/resetRateLimit, and tests for allow/block/retry-after/cooldown behaviors. |
Server-side API route and tests frontend/src/app/api/generate/route.ts, frontend/src/app/api/generate/route.test.ts |
POST validates payload via Zod, enforces IP-based rate limiting and server-only GEMINI_API_KEY, sanitizes prompt, calls Gemini with timeout/abort, parses/validates model JSON, and returns structured responses; GET returns 405. Tests cover success, prompt-injection sanitization, validation errors, method handling, missing config, and proxy-aware rate-limiting. |
Client adapter and fallback logic frontend/src/lib/gemini.ts, frontend/src/lib/generateCommit.ts, frontend/src/lib/generateCommit.test.ts |
callGemini now POSTs to /api/generate, validates the API response, and throws GenerateApiError for invalid/unavailable responses; generateCommit conditionally falls back to rule-based generation for network/config/invalid-response errors; tests cover fallback and error propagation. |
Project config and docs frontend/package.json, frontend/vitest.config.ts, frontend/README.md |
Adds vitest script and dependency, adds zod dependency, configures Vitest (@ path alias, node env), and updates README to require server-side GEMINI_API_KEY and mention new lib/ modules. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested labels
level:intermediate, quality:clean, gssoc:approved, mentor:nirvik34
Poem
🐰 I hop to clean the prompt and gate the key,
Server-side whispers keep secrets free.
Rate limits guard the rolling tide,
Tests and schemas walk beside.
A tidy spring — secure commits for thee.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly summarizes the main security-focused changes: moving Gemini to server-side and hardening the prompt pipeline against injection attacks. |
| Linked Issues check | ✅ Passed | All key objectives from issue #49 are implemented: server-side /api/generate route, server-only GEMINI_API_KEY, prompt sanitization, rate limiting, Zod validation, and improved fallback handling. |
| Out of Scope Changes check | ✅ Passed | All changes are directly aligned with securing the Gemini architecture and hardening the prompt pipeline. No unrelated modifications detected in the changeset. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
frontend/src/app/api/generate/route.test.ts (1)
106-127: ⚡ Quick winAdd a multi-hop/forged forwarding-header regression test.
Given IP-based throttling, add a case with crafted
x-forwarded-forchains to lock expected identifier behavior and prevent future bypass regressions.Also applies to: 130-136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/api/generate/route.test.ts` around lines 106 - 127, Add a new test that verifies IP-based throttling cannot be bypassed by forged X-Forwarded-For chains: call the exported POST handler (imported from "./route") twice with the same effective client identifier but different X-Forwarded-For header values (e.g., a multi-hop comma-separated list like "203.0.113.10, 198.51.100.5") and assert the second call is rate-limited (status 429 and Retry-After "60") after setting AI_RATE_LIMIT_MAX and AI_RATE_LIMIT_COOLDOWN_MS; reuse the existing request(...) helper and generateContentMock setup so this regression test ensures the route's rate-limit logic uses the intended IP from the X-Forwarded-For chain.frontend/src/lib/rateLimit.test.ts (1)
26-31: ⚡ Quick winAdd a regression test for
cooldownMs > windowMs.Current tests won’t catch the cooldown-bypass edge case. Add one case that exceeds the limit, advances past
windowMsbut notcooldownMs, and still expectsallowed: false.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/lib/rateLimit.test.ts` around lines 26 - 31, Add a regression test in rateLimit.test.ts that exercises the cooldown>window edge-case: call checkRateLimit("127.0.0.1", 0, config) to consume the allowed request, then call checkRateLimit again at a timestamp greater than windowMs but less than cooldownMs and assert allowed is false; use the existing checkRateLimit function name and a config such as { windowMs: 500, maxRequests: 1, cooldownMs: 1_000 } and name the spec something like "respects cooldown when cooldownMs > windowMs" so it fails if cooldown is incorrectly bypassed after window expiry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/app/api/generate/route.ts`:
- Around line 108-110: The getClientIdentifier function currently trusts
x-forwarded-for directly which is spoofable; change it to prefer a
server-derived address (e.g., connection remote address /
request.socket.remoteAddress or framework-provided trusted client IP) and only
accept x-forwarded-for when you explicitly trust a proxy (controlled by a config
flag like PROXY_TRUSTED); validate the chosen value as a proper IP (IPv4/IPv6)
and fall back to x-real-ip or "anonymous" only after these checks. Ensure you
update getClientIdentifier to check an env/config flag before reading
x-forwarded-for, parse and validate the first entry if used, and otherwise use
the server-side remote address so per-IP rate limiting isn’t bypassable.
- Around line 67-71: generateWithGemini currently calls
model.generateContent(...) with no timeout or cancellation; update
generateWithGemini to create an AbortController, pass a RequestOptions object to
model.generateContent(prompt, { timeout: <reasonable ms>, signal:
controller.signal }) and ensure you clear/abort the controller on timeout or
when the request finishes (pick e.g. 15000 ms). Also change getClientIdentifier
to stop trusting the first token of the x-forwarded-for header for
rate-limiting; use the transport-level address (e.g., req.socket.remoteAddress
or connection.remoteAddress) as the primary client identifier and only consider
x-forwarded-for when you explicitly trust the proxy chain, to prevent client IP
spoofing.
In `@frontend/src/lib/rateLimit.ts`:
- Around line 38-40: The bucket reset logic currently resets based on resetAt
before checking cooldown, allowing a client with blockedUntil > now to bypass
the cooldown when config.cooldownMs > config.windowMs; update the logic around
the const bucket assignment (using variables existing, now, config.windowMs,
cooldownMs, and the bucket.blockedUntil field) to first check if
existing?.blockedUntil > now and keep the existing bucket (do not reset) while
still honoring resetAt otherwise; apply the same change to the analogous logic
in the block handling code referenced (the section around lines 42-49) so
blockedUntil is always respected before any reset.
---
Nitpick comments:
In `@frontend/src/app/api/generate/route.test.ts`:
- Around line 106-127: Add a new test that verifies IP-based throttling cannot
be bypassed by forged X-Forwarded-For chains: call the exported POST handler
(imported from "./route") twice with the same effective client identifier but
different X-Forwarded-For header values (e.g., a multi-hop comma-separated list
like "203.0.113.10, 198.51.100.5") and assert the second call is rate-limited
(status 429 and Retry-After "60") after setting AI_RATE_LIMIT_MAX and
AI_RATE_LIMIT_COOLDOWN_MS; reuse the existing request(...) helper and
generateContentMock setup so this regression test ensures the route's rate-limit
logic uses the intended IP from the X-Forwarded-For chain.
In `@frontend/src/lib/rateLimit.test.ts`:
- Around line 26-31: Add a regression test in rateLimit.test.ts that exercises
the cooldown>window edge-case: call checkRateLimit("127.0.0.1", 0, config) to
consume the allowed request, then call checkRateLimit again at a timestamp
greater than windowMs but less than cooldownMs and assert allowed is false; use
the existing checkRateLimit function name and a config such as { windowMs: 500,
maxRequests: 1, cooldownMs: 1_000 } and name the spec something like "respects
cooldown when cooldownMs > windowMs" so it fails if cooldown is incorrectly
bypassed after window expiry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6090d198-8b54-484c-996f-edffbaeb78fe
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
frontend/README.mdfrontend/package.jsonfrontend/src/app/api/generate/route.test.tsfrontend/src/app/api/generate/route.tsfrontend/src/lib/gemini.tsfrontend/src/lib/generateCommit.test.tsfrontend/src/lib/generateCommit.tsfrontend/src/lib/rateLimit.test.tsfrontend/src/lib/rateLimit.tsfrontend/src/lib/sanitizePrompt.test.tsfrontend/src/lib/sanitizePrompt.tsfrontend/src/lib/validation.test.tsfrontend/src/lib/validation.tsfrontend/vitest.config.ts
Author
|
@nirvik34 any updates on the PR? |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/app/api/generate/route.ts`:
- Around line 136-137: The code currently trusts the x-real-ip header
unconditionally; update the IP extraction in generate/route.ts so that you only
read and validate request.headers.get("x-real-ip") (variable realIp) when the
proxy-trust flag (e.g., PROXY_TRUSTED or equivalent env/config) is enabled; if
proxy trust is disabled, do not use header-based IPs and return "anonymous"
instead, ensuring isValidIp(realIp) is only called inside the proxy-trust branch
and all header-based logic is gated by the proxy flag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 436ad367-be7d-401c-bd02-249f81351b79
📒 Files selected for processing (4)
frontend/src/app/api/generate/route.test.tsfrontend/src/app/api/generate/route.tsfrontend/src/lib/rateLimit.test.tsfrontend/src/lib/rateLimit.ts
nirvik34
requested changes
May 26, 2026
Owner
nirvik34
left a comment
nirvik34
left a comment
There was a problem hiding this comment.
- Current code trusts the x-forwarded-for header directly without validation:
if (isValidIp(forwardedFor)) {
return forwardedFor;
}
- Only use x-forwarded-for when explicitly trusted (e.g., behind a known proxy)
- Check a PROXY_TRUSTED environment variable first
- Prefer server-derived client IP (e.g., request.socket.remoteAddress)
Verify that multi-hop X-Forwarded-For chains (spoofed IPs) are properly rejected when proxy trust is disabled.
Author
|
@nirvik34 I have made the requested changes, can you review this! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/lib/rateLimit.ts (1)
117-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the block duration cover the remaining window.
When
windowMs > cooldownMs, this returnsRetry-After = cooldownMs, but the bucket still cannot reset untilresetAt. The/api/generateroute forwardsretryAfterSecondsdirectly as the HTTPRetry-Afterheader, so clients that wait exactly what we advertise can still get another 429 immediately. ClampblockedUntilto at leastbucket.resetAtwhen the limit is exceeded.🔧 Proposed fix
if (bucket.count > config.maxRequests) { - bucket.blockedUntil = now + config.cooldownMs; + bucket.blockedUntil = Math.max(bucket.resetAt, now + config.cooldownMs); buckets.set(key, bucket); return { allowed: false, retryAfterSeconds: secondsUntil(bucket.blockedUntil, now), remaining: 0, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/lib/rateLimit.ts` around lines 117 - 123, When the bucket count exceeds config.maxRequests, clamp bucket.blockedUntil to cover the remaining window by setting it to the later of now + config.cooldownMs and bucket.resetAt (instead of always now + config.cooldownMs), update buckets.set(key, bucket) with that clamped value, and compute retryAfterSeconds via secondsUntil(bucket.blockedUntil, now) so the returned retryAfterSeconds cannot understate how long the bucket remains blocked; apply this change in the branch that returns { allowed: false, retryAfterSeconds, remaining: 0 }.
🧹 Nitpick comments (1)
frontend/src/lib/rateLimit.ts (1)
21-27: ⚡ Quick winTighten IP validation in
resolveIdentifier(regex-based).
frontend/src/app/api/generate/route.tsalready derives the rate-limit key usingnode:netisIP, so malformedx-forwarded-forvalues won’t pass in the live request path.- The handwritten
IP_REinfrontend/src/lib/rateLimit.tsonly impactsresolveIdentifier(no production call sites found—onlyfrontend/src/lib/rateLimit.test.tsuses it). IfresolveIdentifieris meant to be used, hardenisValidIpand add regression tests for invalid IPv4/IPv6 inputs (e.g.,999.999.999.999,:::).🔧 Proposed fix
+import { isIP } from "node:net"; + -// Regex for a bare IPv4 or IPv6 address (no port, no CIDR). -const IP_RE = - /^(?:(?:\d{1,3}\.){3}\d{1,3}|(?:[0-9a-fA-F]{0,4}:){2,7}[0-9a-fA-F]{0,4}|::(?:[fF]{4}:)?\d{1,3}(?:\.\d{1,3}){3})$/; - /** Returns true for a syntactically valid IP address (v4 or v6). */ function isValidIp(value: string): boolean { - return IP_RE.test(value.trim()); + return isIP(value.trim()) !== 0; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/lib/rateLimit.ts` around lines 21 - 27, The current IP_RE used by isValidIp is too permissive; update isValidIp to use stricter validation by replacing the single loose regex with separate, robust IPv4 and IPv6 checks (e.g., an IPv4 regex that enforces 0-255 per octet and a stricter IPv6 pattern) or delegate to a well-tested utility if available, and ensure resolveIdentifier (which relies on isValidIp) uses the tightened function; add regression tests covering invalid inputs such as "999.999.999.999", ":::","abcd::12345", empty string, and malformed mixed IPv4/IPv6 to frontend/src/lib/rateLimit.test.ts to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontend/src/lib/rateLimit.ts`:
- Around line 117-123: When the bucket count exceeds config.maxRequests, clamp
bucket.blockedUntil to cover the remaining window by setting it to the later of
now + config.cooldownMs and bucket.resetAt (instead of always now +
config.cooldownMs), update buckets.set(key, bucket) with that clamped value, and
compute retryAfterSeconds via secondsUntil(bucket.blockedUntil, now) so the
returned retryAfterSeconds cannot understate how long the bucket remains
blocked; apply this change in the branch that returns { allowed: false,
retryAfterSeconds, remaining: 0 }.
---
Nitpick comments:
In `@frontend/src/lib/rateLimit.ts`:
- Around line 21-27: The current IP_RE used by isValidIp is too permissive;
update isValidIp to use stricter validation by replacing the single loose regex
with separate, robust IPv4 and IPv6 checks (e.g., an IPv4 regex that enforces
0-255 per octet and a stricter IPv6 pattern) or delegate to a well-tested
utility if available, and ensure resolveIdentifier (which relies on isValidIp)
uses the tightened function; add regression tests covering invalid inputs such
as "999.999.999.999", ":::","abcd::12345", empty string, and malformed mixed
IPv4/IPv6 to frontend/src/lib/rateLimit.test.ts to prevent regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b8c54157-c24a-4dad-a002-2bb4494789bc
📒 Files selected for processing (2)
frontend/src/lib/rateLimit.test.tsfrontend/src/lib/rateLimit.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #49
Implements a secure server-side AI architecture for Gitbun by migrating Gemini inference behind a protected Next.js API route and hardening the AI prompt pipeline against injection and abuse vectors.
Changes Made
/api/generateserver routeNEXT_PUBLIC_GEMINI_API_KEYwith server-onlyGEMINI_API_KEYScreenshots
Secure server-side Gemini routing
Frontend requests are routed through
/api/generate, ensuring Gemini API credentials remain server-side and are never exposed to the browser bundle.Request validation using Zod schemas
Malformed and empty payloads are rejected before reaching the AI pipeline.
Prompt injection safeguards
Unsafe instruction override attempts are intercepted and safely handled through the secured AI pipeline.
Oversized payload protection
Large payloads are blocked through validation and sanitization safeguards to reduce abuse risks.
Type of change
GSSoC '26 Contribution Details
Please select only one difficulty level that was assigned to you in the issue:
level:beginner
level:intermediate
level:advanced
level:critical
I have been assigned to this issue by a maintainer. (PRs without prior assignment will not count toward GSSoC).
How Has This Been Tested?
npm testpasses locallynpm run lintpasses without errorsnpm run devand verified the outputVerification
Result:
Checklist:
Summary by CodeRabbit
New Features
Tests
Chores