fix(api): tighten collection lookup with workspaceId and preserve HTTPException status codes#1193
fix(api): tighten collection lookup with workspaceId and preserve HTTPException status codes#1193shivamashtikar wants to merge 1 commit intomainfrom
Conversation
…PException status codes - Add `workspaceId` parameter to `getRecordBypath` and filter collections by that workspace - Pass `workspaceId` from `AgentMessageApi` to the lookup - Update query logic to enforce multi‑workspace isolation - Refine API error handler to re‑throw `HTTPException` instances before falling back to custom `APIError`, preserving original status codes
shivamashtikar
requested review from
devesh-juspay,
junaid-shirur,
kalpadhwaryu and
zereraz
as code owners
WalkthroughThe changes add workspace-scoped validation to path-based record access by updating the Changes
Sequence Diagram(s)sequenceDiagram
participant AgentAPI as AgentMessageApi<br/>(agents.ts)
participant KnowledgeBase as getRecordBypath<br/>(knowledgeBase.ts)
participant DB as Database
Note over AgentAPI,DB: New Workspace-Scoped Flow
AgentAPI->>KnowledgeBase: getRecordBypath(path, workspace.id, trx)
activate KnowledgeBase
KnowledgeBase->>DB: Query collection WHERE name = ? AND workspace_id = ? AND deleted = false
DB-->>KnowledgeBase: collection result
alt Collection found
KnowledgeBase-->>AgentAPI: Return record
else Collection not found
KnowledgeBase-->>AgentAPI: Return null
end
deactivate KnowledgeBase
Note over AgentAPI: Error Handling
alt HTTPException thrown
AgentAPI-->>AgentAPI: Rethrow HTTPException (preserves status)
else Other error
AgentAPI-->>AgentAPI: Generic APIError handling
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-07-29T09:29:29.401ZApplied to files:
📚 Learning: 2025-09-16T08:57:58.762ZApplied to files:
📚 Learning: 2025-09-16T08:57:58.762ZApplied to files:
🧬 Code graph analysis (2)server/api/chat/agents.ts (1)
server/db/knowledgeBase.ts (2)
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 |
Summary of ChangesHello @shivamashtikar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on improving the application's data security and API robustness. It enforces stricter multi-workspace data isolation by integrating Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two important fixes. First, it tightens the security of collection lookups by adding a workspaceId filter, ensuring proper data isolation between workspaces. This is a great improvement. Second, it refines the API error handling to preserve original HTTPException status codes, which will make API error responses more accurate for clients. The implementation of these changes is sound. I've added one comment regarding a potential issue in the error handling logic that could be improved for more robustness.
| if (error instanceof HTTPException) { | ||
| // Re-throw HTTPException to preserve the original status code (400, 403, etc.) | ||
| throw error | ||
| } else if (error instanceof APIError) { |
There was a problem hiding this comment.
This change to re-throw HTTPException is a good improvement for preserving specific error status codes. However, the surrounding catch block has a potential issue. It attempts to write to an SSE stream in a context where an error might have occurred before the stream was initialized (e.g., during chat creation). This could lead to a TypeError if stream is undefined, masking the original error.
Consider refactoring this catch block to only handle pre-stream errors by throwing an HTTPException. Any logic that writes errors to the stream should be located within the streamSSE onError callback to ensure the stream is active.
1 participant
Description
workspaceIdparameter togetRecordBypathand filter collections by that workspaceworkspaceIdfromAgentMessageApito the lookupHTTPExceptioninstances before falling back to customAPIError, preserving original status codesTesting
Additional Notes
Summary by CodeRabbit