(feat): secure Gemini API architecture and harden AI prompt pipeline#51
Open
vraj826 wants to merge 2 commits into
Open
(feat): secure Gemini API architecture and harden AI prompt pipeline#51vraj826 wants to merge 2 commits into
vraj826 wants to merge 2 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
|
| 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 and 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, 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: advanced, type:security, type:testing, type: feature, gssoc:approved, mentor:nirvik34, quality:clean
Suggested reviewers
- 2PieRadian
Poem
🐰 A rabbit's hop for safe AI days,
Keys tucked away from public gaze.
Prompts are scrubbed, rate limits keep,
The server guards while coders sleep.
A hop, a test, a tidy creed—secure commits proceed.
🚥 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 accurately describes the main architectural change: moving Gemini inference to a secure server-side route and hardening the prompt pipeline against injection attacks. |
| Linked Issues check | ✅ Passed | All five coding objectives from issue #49 are successfully implemented: server-side API route created, client-side exposure removed, server-only GEMINI_API_KEY configured, prompt sanitization added, rate limiting implemented, and Zod validation schemas created. |
| Out of Scope Changes check | ✅ Passed | All changes are directly aligned with the PR objectives and issue #49 requirements; no unrelated or out-of-scope code modifications detected. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Warning
Review ran into problems
🔥 Problems
Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.
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.
| } | ||
|
|
||
| if (process.env.PROXY_TRUSTED === "true") { | ||
| const forwardedFor = request.headers.get("x-forwarded-for")?.split(",")[0]?.trim(); |
Owner
There was a problem hiding this comment.
A malicious client can craft different x-forwarded-for values on each request to appear as different users, evading rate limiting. Rate limiting becomes ineffective.
- 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)
- Validate the IP before use
|
|
||
| const key = identifier || "anonymous"; | ||
| const existing = buckets.get(key); | ||
| const bucket = existing?.blockedUntil && existing.blockedUntil > now |
Owner
There was a problem hiding this comment.
The bucket reset logic checks resetAt before honoring blockedUntil, allowing a blocked client to immediately reset their bucket even if the cooldown hasn't elapsed.
- Check blockedUntil > now before resetting the bucket
- Only reset if both window and cooldown have expired
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