Constrain tool results by project/datasource/workbook #131
Conversation
| process.env = { | ||
| ...originalEnv, | ||
| DATASOURCE_CREDENTIALS: undefined, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
I refactored these tests to mock getConfig instead of relying on setting and resetting environment variables.
| ), | ||
| })); | ||
|
|
||
| describe('queryDatasourceTool', () => { |
There was a problem hiding this comment.
I refactored these tests to mock getConfig instead of relying on setting and resetting environment variables.
| workbookId, | ||
| siteId: restApi.siteId, | ||
| }); | ||
| // Notice that we already have the workbook if it had been allowed by a project scope. |
There was a problem hiding this comment.
Leaving a note here since this does something a bit special. Checking whether the workbook is allowed could potentially have called restApi.workbooksMethods.getWorkbook(), the same method as below. No need to call it twice. See https://github.com/tableau/tableau-mcp/pull/131/files#r2458957992
| getErrorText?: (error: E) => string; | ||
|
|
||
| // A function that constrains the success result of the tool | ||
| constrainSuccessResult: (result: T) => ConstrainedResult<T> | Promise<ConstrainedResult<T>>; |
There was a problem hiding this comment.
I decided not to make this function optional, so we don't forget to implement it for any new tools
There was a problem hiding this comment.
Pull Request Overview
This PR adds tool scoping capabilities to the Tableau MCP server, allowing administrators to constrain tool results and arguments by projects, data sources, or workbooks through environment variables.
Key changes:
- Introduced bounded context filtering via
INCLUDE_PROJECT_IDS,INCLUDE_DATASOURCE_IDS, andINCLUDE_WORKBOOK_IDSenvironment variables - Implemented
ResourceAccessCheckersingleton for validating and caching access permissions - Added constraint functions to filter results in list tools (workbooks, views, datasources, Pulse metrics/definitions/subscriptions, and search content)
Reviewed Changes
Copilot reviewed 65 out of 67 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/config.ts | Added bounded context configuration parsing and validation |
| src/tools/resourceAccessChecker.ts | New singleton class for checking resource access with caching logic |
| src/tools/tool.ts | Extended tool execution to support result constraining |
| src/tools/workbooks/listWorkbooks.ts | Added workbook filtering by bounded context |
| src/tools/views/listViews.ts | Added view filtering by bounded context |
| src/tools/listDatasources/listDatasources.ts | Added datasource filtering by bounded context |
| src/tools/pulse/*.ts | Added Pulse metric/definition/subscription filtering |
| src/tools/contentExploration/searchContentUtils.ts | Added search content filtering by bounded context |
| src/tools/workbooks/getWorkbook.ts | Added workbook access validation before retrieval |
| src/tools/views/getViewImage.ts | Added view access validation before image retrieval |
| src/tools/views/getViewData.ts | Added view access validation before data retrieval |
| src/tools/queryDatasource/queryDatasource.ts | Added datasource access validation before querying |
| src/tools/getDatasourceMetadata/getDatasourceMetadata.ts | Added datasource access validation before metadata retrieval |
| src/tools/pulse/generateMetricValueInsightBundle/*.ts | Added datasource validation for insight bundle generation |
| src/sdks/tableau/types/pulse.ts | Added datasource_luid field to pulse metric schema |
| src/sdks/tableau/apis/*.ts | Added new REST API endpoints for view and datasource queries |
| docs/** | Documentation updates for tool scoping feature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...ols/pulse/generateMetricValueInsightBundle/generatePulseMetricValueInsightBundleTool.test.ts
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| return { allowed: true, content: workbook }; |
There was a problem hiding this comment.
Here's that "special" thing I mentioned in comment: https://github.com/tableau/tableau-mcp/pull/131/files#r2458160645
This case doesn't apply for the other content types which is why you don't see it for data sources or views.
| Enabling tool scoping can cause: | ||
|
|
||
| 1. Tools to return an error if they are called with arguments that are not within the allowed scope, | ||
| and |
There was a problem hiding this comment.
is this "and" intentional or did you mean to add more to this sentence?
There was a problem hiding this comment.
It was intentional but I can remove it. The 2nd bullet is the continuation of the sentence.
| - Data source IDs can be determined using the | ||
| [Query Data Sources](https://help.tableau.com/current/api/rest_api/en-us/REST/rest_api_ref_data_sources.htm#query_data_sources) | ||
| REST API or the [List Data Sources](../../tools/data-qna/list-datasources.md) tool (assuming tool | ||
| scoping is disabled). |
There was a problem hiding this comment.
Perhaps note that limiting a datasource does not limit workbooks or views (assuming that's how that works)
2 participants
These changes allow server administrators to limit the scope of its tools to a set of data sources, workbooks, or projects.
Enabling tool scoping can cause:
and
scope.
Examples use-cases