Skip to content

Commit a9a22bc

Browse files
Nathan Schramclaude
andcommitted
fix: address review feedback on workspace restriction
- 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>
1 parent 2b0683f commit a9a22bc

2 files changed

Lines changed: 13 additions & 9 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ When `TRELLO_ALLOWED_WORKSPACES` is set:
252252
Example configuration:
253253
```bash
254254
# Only allow access to two specific workspaces
255-
TRELLO_ALLOWED_WORKSPACES=697c549ce04dc460af133a75,5f8a3b2c1d4e5f6g7h8i9j0k
255+
TRELLO_ALLOWED_WORKSPACES=697c549ce04dc460af133a75,5f8a3b2c1d4e5f6a7b8c9d0e
256256
```
257257

258258
If `TRELLO_ALLOWED_WORKSPACES` is not set or empty, all workspaces the token has access to will be available (default behaviour).

‎src/trello-client.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,10 @@ export class TrelloClient {
4747
private rateLimiter;
4848
private defaultBoardId?: string;
4949
private activeConfig: TrelloConfig;
50-
private allowedWorkspaceIds?: string[];
5150

5251
constructor(private config: TrelloConfig) {
5352
this.defaultBoardId = config.defaultBoardId;
5453
this.activeConfig = { ...config };
55-
this.allowedWorkspaceIds = config.allowedWorkspaceIds;
5654
// If boardId is provided in config, use it as the active board
5755
if (config.boardId && !this.activeConfig.boardId) {
5856
this.activeConfig.boardId = config.boardId;
@@ -137,7 +135,7 @@ export class TrelloClient {
137135
* Check if workspace restriction is enabled
138136
*/
139137
get hasWorkspaceRestriction(): boolean {
140-
return this.allowedWorkspaceIds !== undefined && this.allowedWorkspaceIds.length > 0;
138+
return this.config.allowedWorkspaceIds !== undefined && this.config.allowedWorkspaceIds.length > 0;
141139
}
142140

143141
/**
@@ -147,7 +145,7 @@ export class TrelloClient {
147145
if (!this.hasWorkspaceRestriction) {
148146
return true;
149147
}
150-
return this.allowedWorkspaceIds!.includes(workspaceId);
148+
return this.config.allowedWorkspaceIds!.includes(workspaceId);
151149
}
152150

153151
/**
@@ -157,7 +155,7 @@ export class TrelloClient {
157155
if (!this.isWorkspaceAllowed(workspaceId)) {
158156
throw new McpError(
159157
ErrorCode.InvalidParams,
160-
`Access to workspace '${workspaceId}' is not allowed. Allowed workspaces: ${this.allowedWorkspaceIds!.join(', ')}`
158+
`Access to workspace '${workspaceId}' is not allowed. Allowed workspaces: ${this.config.allowedWorkspaceIds!.join(', ')}`
161159
);
162160
}
163161
}
@@ -225,7 +223,7 @@ export class TrelloClient {
225223

226224
// Filter by allowed workspaces if restriction is enabled
227225
if (this.hasWorkspaceRestriction) {
228-
return boards.filter(board => this.isWorkspaceAllowed(board.idOrganization));
226+
return boards.filter(board => board.idOrganization && this.isWorkspaceAllowed(board.idOrganization));
229227
}
230228
return boards;
231229
});
@@ -296,8 +294,14 @@ export class TrelloClient {
296294
// Determine the target workspace
297295
const targetWorkspace = params.idOrganization ?? this.activeConfig.workspaceId;
298296

299-
// Validate workspace access if a workspace is specified and restrictions are enabled
300-
if (targetWorkspace && this.hasWorkspaceRestriction) {
297+
// When workspace restrictions are enabled, require a valid workspace
298+
if (this.hasWorkspaceRestriction) {
299+
if (!targetWorkspace) {
300+
throw new McpError(
301+
ErrorCode.InvalidParams,
302+
`Workspace restrictions are enabled but no workspace was specified. Provide idOrganization or set an active workspace. Allowed workspaces: ${this.config.allowedWorkspaceIds!.join(', ')}`
303+
);
304+
}
301305
this.validateWorkspaceAccess(targetWorkspace);
302306
}
303307

0 commit comments

Comments
 (0)