Repository navigation
feat: add TRELLO_ALLOWED_WORKSPACES env var for workspace restriction - #61
Conversation
Summary of ChangesHello @nathanschram, 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 implements a crucial security and organizational feature by allowing administrators to define a whitelist of Trello workspaces accessible through the MCP tools. By introducing the Highlights
Changelog
Activity
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
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b0683fac7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Determine the target workspace | ||
| const targetWorkspace = params.idOrganization ?? this.activeConfig.workspaceId; | ||
|
|
||
| // Validate workspace access if a workspace is specified and restrictions are enabled | ||
| if (targetWorkspace && this.hasWorkspaceRestriction) { | ||
| this.validateWorkspaceAccess(targetWorkspace); |
There was a problem hiding this comment.
When TRELLO_ALLOWED_WORKSPACES is configured, createBoard only validates if targetWorkspace is truthy, so a caller who omits idOrganization (and has no active workspace set) skips the restriction entirely. In that scenario, Trello will create a personal board (no organization), which violates the “only allowed workspaces” guarantee described in the README. Consider rejecting creation when restrictions are enabled and no workspace is specified/active, or forcing an allowed workspace ID.
Useful? React with 👍 / 👎.
![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 a valuable feature for restricting access to specific Trello workspaces via an environment variable. The implementation is well-structured, with robust parsing of the configuration and clear validation logic integrated into the TrelloClient. My review includes a few suggestions: one to fix a potential runtime error when filtering boards not associated with a workspace, another to correct a misleading example in the documentation, and a minor refactoring to improve code maintainability by removing a redundant property. Overall, this is a solid contribution that enhances the security and configurability of the tool.
|
|
||
| // Filter by allowed workspaces if restriction is enabled | ||
| if (this.hasWorkspaceRestriction) { | ||
| return boards.filter(board => this.isWorkspaceAllowed(board.idOrganization)); |
There was a problem hiding this comment.
The board.idOrganization property can be null for personal boards that don't belong to any workspace, but isWorkspaceAllowed expects a string. Passing null could lead to unexpected behavior or runtime errors. You should handle the null case to ensure personal boards are correctly filtered out when workspace restrictions are active.
It would also be beneficial to update the TrelloBoard type definition in src/types.ts to idOrganization: string | null; to reflect the Trello API response accurately, though that file is not part of this PR's changes.
| return boards.filter(board => this.isWorkspaceAllowed(board.idOrganization)); | |
| return boards.filter(board => board.idOrganization && this.isWorkspaceAllowed(board.idOrganization)); |
| Example configuration: | ||
| ```bash | ||
| # Only allow access to two specific workspaces | ||
| TRELLO_ALLOWED_WORKSPACES=697c549ce04dc460af133a75,5f8a3b2c1d4e5f6g7h8i9j0k |
There was a problem hiding this comment.
The example workspace ID 5f8a3b2c1d4e5f6g7h8i9j0k appears to be invalid. Trello IDs are typically 24-character hexadecimal strings, but this example is 23 characters long and contains non-hexadecimal characters (g, h, i, j, k). Using a realistic example will prevent confusion for users configuring this feature.
| TRELLO_ALLOWED_WORKSPACES=697c549ce04dc460af133a75,5f8a3b2c1d4e5f6g7h8i9j0k | |
| TRELLO_ALLOWED_WORKSPACES=697c549ce04dc460af133a75,5f8a3b2c1d4e5f6a7b8c9d0e |
| private rateLimiter; | ||
| private defaultBoardId?: string; | ||
| private activeConfig: TrelloConfig; | ||
| private allowedWorkspaceIds?: string[]; |
There was a problem hiding this comment.
The allowedWorkspaceIds property is redundant. The constructor already takes private config: TrelloConfig, which makes this.config available throughout the class. You can directly use this.config.allowedWorkspaceIds instead of storing this value in a separate property. This will make the code cleaner and avoid data duplication.
To apply this change:
- Remove this property (
allowedWorkspaceIds). - Remove its assignment in the constructor (line 55).
- Update
hasWorkspaceRestriction,isWorkspaceAllowed, andvalidateWorkspaceAccessto usethis.config.allowedWorkspaceIds.
|
Solid feature — workspace allowlisting is a real security knob for multi-tenant agent setups. Asking for:
Once rebased + a test, happy to merge. Thanks. |
Add optional workspace access restriction to limit which Trello workspaces can be accessed via MCP tools. This is useful for: - Security: Limiting AI agent access to only approved workspaces - Multi-tenant setups: Ensuring agents only access relevant workspaces - Testing: Isolating test environments from production data When TRELLO_ALLOWED_WORKSPACES is set (comma-separated IDs): - list_workspaces only returns workspaces in the allowed list - list_boards only returns boards from allowed workspaces - set_active_workspace rejects workspaces not in the allowed list - list_boards_in_workspace rejects non-allowed workspace IDs - create_board rejects creation in non-allowed workspaces If not set or empty, all workspaces remain accessible (default behaviour). Co-Authored-By: Claude <noreply@anthropic.com>
- Block createBoard when restrictions enabled but no workspace set (prevents bypassing restriction by creating personal boards) - Handle null idOrganization in listBoards filter (personal boards without a workspace could cause runtime errors) - Fix invalid example workspace ID in README (use valid 24-char hex string instead of non-hex characters) - Remove redundant allowedWorkspaceIds property (use this.config.allowedWorkspaceIds directly) Co-Authored-By: Claude <noreply@anthropic.com>
a9a22bc to
b27626a
Compare
2 participants
Summary
Add optional workspace access restriction via the
TRELLO_ALLOWED_WORKSPACESenvironment variable. This allows administrators to limit which Trello workspaces can be accessed via MCP tools.Use cases:
Changes
allowedWorkspaceIds?: string[]toTrelloConfiginterfaceTrelloClient:hasWorkspaceRestrictiongetterisWorkspaceAllowed(workspaceId)methodvalidateWorkspaceAccess(workspaceId)private methodlistWorkspaces()- filters results to allowed workspaces onlylistBoards()- filters boards by allowed workspacessetActiveWorkspace()- validates workspace ID before settinglistBoardsInWorkspace()- validates workspace ID before fetchingcreateBoard()- validates target workspace before creatingTRELLO_ALLOWED_WORKSPACESenv var (comma-separated) inindex.tsConfiguration
# Only allow access to specific workspaces (comma-separated IDs) TRELLO_ALLOWED_WORKSPACES=workspace-id-1,workspace-id-2If not set or empty, all workspaces the token has access to will be available (backwards compatible).
Test plan
bun run buildTRELLO_ALLOWED_WORKSPACESset to a single workspace IDlist_workspacesonly returns the allowed workspaceset_active_workspacerejects non-allowed workspace IDslist_boardsonly shows boards from allowed workspacesTRELLO_ALLOWED_WORKSPACESto confirm backwards compatibility🤖 Generated with Claude Code