fix(kb-workspace-selector): allow switching assigned workspace for KB #1567
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Summary
This PR implements workspace switching functionality for Knowledge Bases (KB) by enabling users to reassign knowledge bases between workspaces through the UI. The changes span the full stack from the database service layer to the frontend components.The implementation adds a workspaceId field to the knowledge base update operations, allowing the API to handle workspace reassignments. The frontend workspace selector component now immediately updates the local state when a workspace change occurs, eliminating the need for page reloads and providing a smoother user experience. The knowledge store gains a new updateKnowledgeBase method that handles partial updates to knowledge base metadata, keeping both cached records and lists synchronized.
The changes integrate well with the existing codebase architecture. The API route follows established patterns for validation and service delegation, the store method maintains consistency with other update operations, and the frontend components leverage existing state management patterns. The removal of the page reload callback in the base component aligns with modern React practices of optimistic updates and maintaining user context during state changes.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
| apps/sim/lib/knowledge/service.ts | 1/5 | Critical security vulnerability - allows workspace reassignment without permission validation |
| apps/sim/app/api/knowledge/[id]/route.ts | 4/5 | Adds workspaceId support to PUT endpoint with proper schema validation |
| apps/sim/stores/knowledge/store.ts | 5/5 | Adds updateKnowledgeBase method for partial updates with proper state management |
| apps/sim/app/workspace/[workspaceId]/knowledge/components/workspace-selector/workspace-selector.tsx | 5/5 | Implements immediate state synchronization for workspace changes |
| apps/sim/app/workspace/[workspaceId]/knowledge/[id]/base.tsx | 4/5 | Removes page reload callback to enable smoother workspace switching |
| apps/sim/app/api/knowledge/[id]/route.test.ts | 5/5 | Updates test assertions to match new API behavior with workspaceId parameter |
Confidence score: 1/5
- This PR has a critical security vulnerability that makes it unsafe to merge
- Score severely lowered due to missing permission validation in the service layer that could allow unauthorized workspace access
- Pay immediate attention to apps/sim/lib/knowledge/service.ts which lacks permission checks for target workspace access
Sequence Diagram
sequenceDiagram
participant User
participant WorkspaceSelector as "WorkspaceSelector Component"
participant API as "Knowledge Base API"
participant DB as "Database"
participant Store as "Knowledge Store"
User->>WorkspaceSelector: "Select different workspace"
WorkspaceSelector->>WorkspaceSelector: "Set isUpdating = true"
WorkspaceSelector->>API: "PUT /api/knowledge/[id] with workspaceId"
API->>API: "Validate session and parse body"
API->>API: "Check knowledge base write access"
API->>DB: "Update knowledge base workspaceId"
DB-->>API: "Return updated knowledge base"
API-->>WorkspaceSelector: "Success response with updated data"
WorkspaceSelector->>Store: "updateKnowledgeBase(id, { workspaceId })"
Store->>Store: "Update cached knowledge base and list"
WorkspaceSelector->>WorkspaceSelector: "Set isUpdating = false"
WorkspaceSelector->>WorkspaceSelector: "Call onWorkspaceChange callback"
6 files reviewed, 2 comments
| { | ||
| name: validatedData.name, | ||
| description: validatedData.description, | ||
| workspaceId: validatedData.workspaceId, |
There was a problem hiding this comment.
logic: Missing permission validation when changing workspaceId. Should verify user has access to target workspace before allowing the update.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/knowledge/[id]/route.ts
Line: 106:106
Comment:
**logic:** Missing permission validation when changing workspaceId. Should verify user has access to target workspace before allowing the update.
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| if (updates.name !== undefined) updateData.name = updates.name | ||
| if (updates.description !== undefined) updateData.description = updates.description | ||
| if (updates.workspaceId !== undefined) updateData.workspaceId = updates.workspaceId |
There was a problem hiding this comment.
logic: Missing permission validation when changing workspaceId. Users could move knowledge bases to unauthorized workspaces. Should verify user has permissions for target workspace before allowing the change.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/lib/knowledge/service.ts
Line: 166:166
Comment:
**logic:** Missing permission validation when changing workspaceId. Users could move knowledge bases to unauthorized workspaces. Should verify user has permissions for target workspace before allowing the change.
How can I resolve this? If you propose a fix, please make it concise.
Summary
Fix switching workspaces for KB.
Type of Change
Testing
Tested manually
Checklist