Fix(security): remove NEXT_PUBLIC_ usage from server-only route handlers#75
Open
Fix(security): remove NEXT_PUBLIC_ usage from server-only route handlers#75
Conversation
📝 WalkthroughWalkthroughSystematically replaces the backend URL environment variable source from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels
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)
📝 Coding Plan
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 Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
Ayush8923
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
app/api/evaluations/stt/samples/[sample_id]/route.ts (1)
8-8: Fail fast whenBACKEND_URLis missing.After this env rename, silently falling back to
http://localhost:8000can hide misconfiguration in non-local environments. Prefer explicit failure (or fallback only in development).Proposed change
- const backendUrl = process.env.BACKEND_URL || 'http://localhost:8000'; + const backendUrl = + process.env.BACKEND_URL ?? + (process.env.NODE_ENV === 'development' ? 'http://localhost:8000' : undefined); + + if (!backendUrl) { + return NextResponse.json( + { success: false, error: 'Server misconfiguration: BACKEND_URL is not set' }, + { status: 500 } + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/stt/samples/`[sample_id]/route.ts at line 8, The code currently sets backendUrl = process.env.BACKEND_URL || 'http://localhost:8000', which hides missing configuration; change this so that when process.env.BACKEND_URL is not set it throws an explicit error (or only falls back to 'http://localhost:8000' when process.env.NODE_ENV === 'development'). Update the initialization of backendUrl in app/api/evaluations/stt/samples/[sample_id]/route.ts (the backendUrl constant) to validate process.env.BACKEND_URL and fail fast with a clear error message if absent in non-development environments.app/api/evaluations/tts/runs/route.ts (1)
16-20: Pre-existing issue: Inconsistent error handling compared to POST handler.The GET handler returns the raw error object while the POST handler (line 49) correctly uses
error instanceof Error ? error.message : String(error). Consider aligning the error handling for consistency.♻️ Suggested fix
} catch (error) { return NextResponse.json( - { success: false, error: error, data: null }, + { success: false, error: error instanceof Error ? error.message : String(error), data: null }, { status: 500 } ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/tts/runs/route.ts` around lines 16 - 20, The GET handler's catch currently returns the raw error object; update the catch block in the GET route to format the error the same way the POST handler does (use error instanceof Error ? error.message : String(error)) and pass that string into NextResponse.json instead of the raw error so both handlers return consistent error messages via NextResponse.json.app/api/languages/route.ts (1)
16-20: Pre-existing issue: Raw error object may not serialize properly.The error object is returned directly which may not serialize correctly to JSON and could potentially leak stack traces in production. Consider using
error instanceof Error ? error.message : String(error)like other handlers in this PR.♻️ Suggested fix
} catch (error) { return NextResponse.json( - { success: false, error: error, data: null }, + { success: false, error: error instanceof Error ? error.message : String(error), data: null }, { status: 500 } ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/languages/route.ts` around lines 16 - 20, The catch block currently returns the raw error object via NextResponse.json which may not serialize and can leak stack traces; replace the returned error value with a sanitized string using a check like error instanceof Error ? error.message : String(error) (same pattern used elsewhere) so the response becomes { success: false, error: <sanitized-message>, data: null } while keeping the NextResponse.json call and status 500 intact.app/api/configs/route.ts (1)
7-12: Consider adding API key validation to the GET handler for consistency.The POST handler validates the API key and returns a 401 if missing, but the GET handler passes an empty string when the key is absent. This inconsistency may allow unauthenticated requests to reach the backend.
♻️ Suggested improvement
export async function GET(request: Request) { const backendUrl = process.env.BACKEND_URL || 'http://localhost:8000'; const apiKey = request.headers.get('X-API-KEY'); + + if (!apiKey) { + return NextResponse.json( + { error: 'Missing X-API-KEY header' }, + { status: 401 } + ); + } try { const response = await fetch(`${backendUrl}/api/v1/configs/`, { headers: { - 'X-API-KEY': apiKey || '', + 'X-API-KEY': apiKey, }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/configs/route.ts` around lines 7 - 12, The GET handler in app/api/configs/route.ts currently sends 'X-API-KEY': apiKey || '' to the backend without validating it; change the GET handler to validate apiKey the same way the POST handler does and return a 401 Unauthorized response when the key is missing/empty before performing the fetch. Locate the GET handler (the function that performs the fetch to `${backendUrl}/api/v1/configs/`) and add the same API key existence check and early return used by the POST handler so unauthenticated requests are blocked.app/api/collections/jobs/[job_id]/route.ts (1)
3-3: Consider centralizing backend URL resolution in one server-only helper.This replacement is correct, but the same env-resolution snippet is duplicated across many handlers. Extracting it to a shared utility will reduce config drift and make future security/config changes one-touch.
♻️ Example pattern to apply consistently
-const backendUrl = process.env.BACKEND_URL || 'http://localhost:8000'; +const backendUrl = getBackendUrl();// e.g. server-only helper export function getBackendUrl(): string { return process.env.BACKEND_URL || 'http://localhost:8000'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/collections/jobs/`[job_id]/route.ts at line 3, The duplicated inline backend URL resolution (const backendUrl = process.env.BACKEND_URL || 'http://localhost:8000') should be extracted into a single server-only helper (e.g., export function getBackendUrl(): string) and all handlers should import and call getBackendUrl() instead of repeating the expression; create the helper in a shared server utilities module, replace references to the backendUrl variable in route.ts and other handlers with getBackendUrl(), and ensure the helper is only used on the server side (no client bundling).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/api/collections/jobs/`[job_id]/route.ts:
- Line 3: The duplicated inline backend URL resolution (const backendUrl =
process.env.BACKEND_URL || 'http://localhost:8000') should be extracted into a
single server-only helper (e.g., export function getBackendUrl(): string) and
all handlers should import and call getBackendUrl() instead of repeating the
expression; create the helper in a shared server utilities module, replace
references to the backendUrl variable in route.ts and other handlers with
getBackendUrl(), and ensure the helper is only used on the server side (no
client bundling).
In `@app/api/configs/route.ts`:
- Around line 7-12: The GET handler in app/api/configs/route.ts currently sends
'X-API-KEY': apiKey || '' to the backend without validating it; change the GET
handler to validate apiKey the same way the POST handler does and return a 401
Unauthorized response when the key is missing/empty before performing the fetch.
Locate the GET handler (the function that performs the fetch to
`${backendUrl}/api/v1/configs/`) and add the same API key existence check and
early return used by the POST handler so unauthenticated requests are blocked.
In `@app/api/evaluations/stt/samples/`[sample_id]/route.ts:
- Line 8: The code currently sets backendUrl = process.env.BACKEND_URL ||
'http://localhost:8000', which hides missing configuration; change this so that
when process.env.BACKEND_URL is not set it throws an explicit error (or only
falls back to 'http://localhost:8000' when process.env.NODE_ENV ===
'development'). Update the initialization of backendUrl in
app/api/evaluations/stt/samples/[sample_id]/route.ts (the backendUrl constant)
to validate process.env.BACKEND_URL and fail fast with a clear error message if
absent in non-development environments.
In `@app/api/evaluations/tts/runs/route.ts`:
- Around line 16-20: The GET handler's catch currently returns the raw error
object; update the catch block in the GET route to format the error the same way
the POST handler does (use error instanceof Error ? error.message :
String(error)) and pass that string into NextResponse.json instead of the raw
error so both handlers return consistent error messages via NextResponse.json.
In `@app/api/languages/route.ts`:
- Around line 16-20: The catch block currently returns the raw error object via
NextResponse.json which may not serialize and can leak stack traces; replace the
returned error value with a sanitized string using a check like error instanceof
Error ? error.message : String(error) (same pattern used elsewhere) so the
response becomes { success: false, error: <sanitized-message>, data: null }
while keeping the NextResponse.json call and status 500 intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5895d65e-a271-443c-8a56-b360805cb358
📒 Files selected for processing (27)
app/api/assistant/[assistant_id]/route.tsapp/api/collections/[collection_id]/route.tsapp/api/collections/jobs/[job_id]/route.tsapp/api/collections/route.tsapp/api/configs/[config_id]/route.tsapp/api/configs/[config_id]/versions/[version_number]/route.tsapp/api/configs/[config_id]/versions/route.tsapp/api/configs/route.tsapp/api/document/[document_id]/route.tsapp/api/document/route.tsapp/api/evaluations/[id]/route.tsapp/api/evaluations/datasets/[dataset_id]/route.tsapp/api/evaluations/datasets/route.tsapp/api/evaluations/route.tsapp/api/evaluations/stt/datasets/[dataset_id]/route.tsapp/api/evaluations/stt/datasets/route.tsapp/api/evaluations/stt/files/route.tsapp/api/evaluations/stt/results/[result_id]/route.tsapp/api/evaluations/stt/runs/[run_id]/route.tsapp/api/evaluations/stt/runs/route.tsapp/api/evaluations/stt/samples/[sample_id]/route.tsapp/api/evaluations/tts/datasets/[dataset_id]/route.tsapp/api/evaluations/tts/datasets/route.tsapp/api/evaluations/tts/results/[result_id]/route.tsapp/api/evaluations/tts/runs/[run_id]/route.tsapp/api/evaluations/tts/runs/route.tsapp/api/languages/route.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
instructions/CLAUDE.md (1)
8-298: Documentation expansions beyond the PR scope.While the core objective of this PR is to remove
NEXT_PUBLIC_usage from server-only routes, this file includes extensive documentation additions covering components, type systems, features, and architecture details. These additions improve the documentation quality but are outside the stated PR scope.Consider separating documentation improvements into a dedicated PR to keep changes focused and make reviews easier to follow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@instructions/CLAUDE.md` around lines 8 - 298, This PR accidentally bundles large documentation expansions in instructions/CLAUDE.md unrelated to the stated scope (removing NEXT_PUBLIC_ usage); revert or remove the unrelated additions from this PR so the diff only contains server-only env changes (search for the NEXT_PUBLIC_ token to validate scope), and move the architecture/component/type system content into a separate docs PR or a new file (e.g., ARCHITECTURE.md) so reviews remain focused; ensure the CLAUDE.md in this PR only contains the minimal changes required for the env var cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@instructions/CLAUDE.md`:
- Around line 8-298: This PR accidentally bundles large documentation expansions
in instructions/CLAUDE.md unrelated to the stated scope (removing NEXT_PUBLIC_
usage); revert or remove the unrelated additions from this PR so the diff only
contains server-only env changes (search for the NEXT_PUBLIC_ token to validate
scope), and move the architecture/component/type system content into a separate
docs PR or a new file (e.g., ARCHITECTURE.md) so reviews remain focused; ensure
the CLAUDE.md in this PR only contains the minimal changes required for the env
var cleanup.
vprashrex
approved these changes
Mar 19, 2026
Collaborator
vprashrex
left a comment
vprashrex
left a comment
There was a problem hiding this comment.
@Ayush8923 @Prajna1999 After the merge .env need to be updated
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.
Issue: #45
Summary
NEXT_PUBLIC_environment variables from server-only route handlers and replaces them with server-side environment variables.process.env.NEXT_PUBLIC_BACKEND_URLwithprocess.env.BACKEND_URLin all server-only route handlersNotes
BACKEND_URLis properly defined in environment variables for all environments (local, staging, production)Summary by CodeRabbit
Chores
Documentation