Fix processing status proxy route#489
Conversation
|
@atul-upadhyay-7 is attempting to deploy a commit to the firefistisdead's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a new Express gateway route ChangesProcessing status proxy endpoint
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new API endpoint to fetch processing progress from the RAG service and expands/adjusts tests to cover the new behavior and updated auth requirements.
Changes:
- Added
GET /processing-status/:sessionIdroute that proxies to the RAG service with internal auth headers and handles upstream 404s. - Added tests for the new route’s success and 404 flows.
- Updated existing tests to use a signed Supabase JWT instead of a placeholder token.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| server.js | Introduces the /processing-status/:sessionId proxy route and error handling. |
| server.test.js | Adds coverage for the new route and updates auth headers to use a signed JWT. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server.js (1)
1106-1106: ⚡ Quick winValidate sessionId format to fail fast on malformed input.
The
sessionIdURL parameter is forwarded without validation. Malformed input (non-UUID) will propagate to the RAG service. While the RAG service should handle this, gateway-level validation provides faster feedback and reduces unnecessary upstream calls.✅ Proposed validation
app.get("/processing-status/:sessionId", async (req, res) => { + const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + if (!uuidPattern.test(req.params.sessionId)) { + return res.status(400).json({ error: "Invalid session ID format." }); + } + try {🤖 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 `@server.js` at line 1106, The GET route handler registered at app.get("/processing-status/:sessionId") forwards the sessionId without validating its format; add an early validation step in that handler to check the sessionId param is a well-formed UUID (e.g., UUID v4) using a regex or a UUID validation helper (validateUuid/uuid.validate) and if invalid immediately return a 400 response with a clear JSON error body instead of calling the downstream RAG service; ensure the validation runs before any RAG/service calls or DB lookups in the route handler.
🤖 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 `@server.js`:
- Around line 1106-1119: The GET /processing-status/:sessionId route lacks rate
limiting and must use the existing inferenceLimiter middleware to prevent
unrestricted polling; update the route definition for
app.get("/processing-status/:sessionId", ...) to apply the inferenceLimiter (the
same limiter used elsewhere like for inference endpoints) so requests pass
through inferenceLimiter before the async handler, ensuring the existing 60
req/min policy is enforced and preventing abuse/session enumeration.
- Around line 1106-1119: The GET handler for /processing-status/:sessionId
currently allows unauthenticated polling; require a session_secret query param
and validate ownership before forwarding to the RAG service: in the
app.get("/processing-status/:sessionId") handler, reject requests missing
req.query.session_secret with a 400, then call the RAG service with both
sessionId and session_secret (or perform a local ownership check) instead of
blindly proxying; keep using ragAuthHeaders() for service auth and still use
propagateRagError for downstream errors, and update the RAG service (or local
validation) so it only returns status when the session_secret matches the
sessionId.
---
Nitpick comments:
In `@server.js`:
- Line 1106: The GET route handler registered at
app.get("/processing-status/:sessionId") forwards the sessionId without
validating its format; add an early validation step in that handler to check the
sessionId param is a well-formed UUID (e.g., UUID v4) using a regex or a UUID
validation helper (validateUuid/uuid.validate) and if invalid immediately return
a 400 response with a clear JSON error body instead of calling the downstream
RAG service; ensure the validation runs before any RAG/service calls or DB
lookups in the route handler.
🪄 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 Plus
Run ID: 89a91e71-96d1-416b-8ee8-d51ec9fea3b8
📒 Files selected for processing (2)
server.jsserver.test.js
Fixes #450. Added Express proxy route for /processing-status/ and updated server tests to verify internal token logic.
Summary by CodeRabbit