Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

@icecrasher321 icecrasher321 commented Jul 23, 2025

Description

  • Change to aliased imports for provider in real files (not all test files)
  • Move provider logic one level up so KB files were not importing from same level of tree structure
  • Actual logic to do write / admin checks for creating docs, uploading docs / chunks
  • Only need read to see them

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Screen.Recording.2025-07-23.at.8.08.26.PM.mov

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally and in CI (bun run test)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have updated version numbers as needed (if needed)
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Security Considerations:

  • My changes do not introduce any new security vulnerabilities
  • I have considered the security implications of my changes

@vercel
Copy link

vercel bot commented Jul 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sim ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2025 3:15am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
docs ⬜️ Skipped (Inspect) Jul 24, 2025 3:15am

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR implements a comprehensive workspace permissions system for knowledge bases by reusing the existing workspace permissions infrastructure. The changes introduce granular access controls where users need different permission levels for different operations:

  • Read access: Required to view knowledge bases and documents
  • Write access: Required to create, upload, modify, or delete documents and knowledge bases
  • Admin access: Required for advanced operations like workspace-level knowledge base management

The implementation involves several key architectural changes:

  1. Provider restructuring: The WorkspacePermissionsProvider was moved from w/components/providers/ to components/providers/ (one level up) to make it accessible to both workflow and knowledge base components without same-level imports

  2. Import standardization: All real application files (excluding test files) were updated to use aliased imports (@/app/workspace/[workspaceId]/components/providers/) instead of relative imports, following established path alias patterns

  3. Permission enforcement: Knowledge base API routes now use distinct access control functions:

    • checkKnowledgeBaseAccess for read operations (GET requests)
    • checkKnowledgeBaseWriteAccess for write operations (POST, PUT, DELETE requests)
    • Similar patterns applied to document and chunk operations
  4. UI integration: Knowledge base components now disable buttons and show appropriate tooltips when users lack required permissions, maintaining consistent UX patterns with the rest of the application

The system integrates with the existing workspace permissions model where knowledge bases can belong to workspaces and inherit permission structures. Users can access knowledge bases through direct ownership OR through workspace membership with appropriate permission levels. This creates a unified authorization model across workflows, knowledge bases, and other workspace features.

Confidence score: 4/5

  • This PR appears safe to merge with proper permission controls implemented throughout the knowledge base system
  • The confidence score reflects the comprehensive nature of the changes and proper separation of read/write access controls
  • The files that need most attention are the API route handlers (apps/sim/app/api/knowledge/*/route.ts) and the permissions utility functions (apps/sim/app/api/knowledge/utils.ts) as they contain the core security logic

29 files reviewed, 5 comments

Edit Code Review Bot Settings | Greptile

@vercel vercel bot temporarily deployed to Preview – docs July 24, 2025 03:07 Inactive
@icecrasher321 icecrasher321 merged commit 14e1c17 into staging Jul 24, 2025
4 of 5 checks passed
@waleedlatif1 waleedlatif1 deleted the improvment/kb-perms branch July 27, 2025 01:34
arenadeveloper02 pushed a commit to arenadeveloper02/p2-sim that referenced this pull request Sep 19, 2025
…i#761)

* improvement(kb-perms): use workspace perms system for kbs

* readd test file

* fixed test

* address greptile comments

* fix button disabling logic

* update filter condition for legacy kbs

* fix kb selector to respect the workspace scoping

* remove testing code

* make workspace selection and prevent cascade deletion

* make workspace selector pass lint

* lint fixed

* fix type error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants