feat: check and cache permission remote MCP calls - #10930
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the ensureRole utility to verify and automatically bind the required IAM role (roles/mcp.toolUser) for a user or service account on a GCP project, caching positive results in configstore to reduce latency. The review feedback focuses on improving the robustness and user experience of this implementation: adding a 24-hour TTL to the cache to prevent stale permissions, printing a user-facing message during the 10-second propagation delay to avoid the appearance of a frozen CLI, avoiding the use of any in the catch block by using the getError utility, and updating the unit tests to reflect these changes.
Verifies that the authenticated account has the 'roles/mcp.toolUser' role before making remote calls for 'tools/call'. To avoid ResourceManager API query latency and quota burn, positive validations are cached indefinitely in configstore. If validation fails, it attempts to auto-bind the role using setIamPolicy, falling back to clear error instructions and manual gcloud repair commands on permission failures. - Unit tests for local caching logic, direct API checking (via force flag), prefix resolution, and auto-binding flows. - Unit tests verifying OneMcpServer callTool correctly invokes the validation logic.
54c122b to
d9ae369
Compare
joehan
left a comment
There was a problem hiding this comment.
Spoke in chat - this one might be uneeded?
… ensurePermissionsOrSetRole
e55112b to
458d2a5
Compare
Description
Verifies that the authenticated account has the right permission to make OneMCP
tools/callbefore making remote calls. To avoid ResourceManager API query latency and quota burn, positive validations are cached with TTL in configstore. If validation fails, it attempts to auto-bind the roleroles/mcp.toolUserusingsetIamPolicy, falling back to clear error instructions and manualgcloudrepair commands on permission failures.Scenarios Tested
Sample Commands
N/A. It only affect MCP