Skip to content

Conversation

@roomote
Copy link
Collaborator

@roomote roomote commented Jul 1, 2025

Summary

Fixes #5300 by implementing missing auto-approval logic for the access_mcp_resource tool.

Problem

The access_mcp_resource tool was not respecting auto-approval settings, always prompting users for approval even when alwaysAllowMcp was enabled. This was because the Task.ask() method had no auto-approval logic implemented.

Solution

  • Added auto-approval logic to Task.ask() method: Check autoApprovalEnabled setting and specific tool permissions before prompting user
  • Implemented shouldAutoApprove() method: Maps different ask types to their corresponding auto-approval settings
  • Fixed use_mcp_server ask type: Now properly respects the alwaysAllowMcp setting like use_mcp_tool does

Changes

src/core/task/Task.ts

  • Modified ask() method (lines 415-440) to check for auto-approval before user interaction
  • Added shouldAutoApprove() private method (lines 533-547) that handles different ask types:
    • use_mcp_serveralwaysAllowMcp
    • commandalwaysAllowExecute
    • browser_action_launchalwaysAllowBrowser
    • toolalwaysAllowReadOnly

Testing

  • Verified that access_mcp_resource tool calls askApproval("use_mcp_server", ...)
  • Confirmed the new logic correctly maps use_mcp_server to alwaysAllowMcp setting
  • All existing tests pass
  • Linting and type checking pass

Impact

  • access_mcp_resource tool now respects auto-approval settings
  • Consistent behavior between use_mcp_tool and access_mcp_resource
  • No breaking changes to existing functionality

Important

Implements auto-approval logic for access_mcp_resource tool in Task.ts, ensuring it respects alwaysAllowMcp setting.

  • Behavior:
    • Implements auto-approval logic in Task.ask() in Task.ts to check autoApprovalEnabled and tool permissions before user prompts.
    • Adds shouldAutoApprove() method to map ask types to auto-approval settings.
    • Ensures use_mcp_server respects alwaysAllowMcp setting.
  • Testing:
    • Verified access_mcp_resource tool calls askApproval("use_mcp_server", ...).
    • Confirmed correct mapping of use_mcp_server to alwaysAllowMcp.
    • All existing tests pass.
    • Linting and type checking pass.

This description was created by Ellipsis for 3e478df. You can customize this summary. It will automatically update as commits are pushed.

- Add auto-approval logic to Task.ask() method
- Check autoApprovalEnabled setting and specific tool permissions
- Implement shouldAutoApprove() method to handle different ask types
- Fix missing auto-approval for use_mcp_server ask type used by both MCP tools
- Ensure access_mcp_resource respects alwaysAllowMcp setting like use_mcp_tool
@roomote roomote requested review from cte, jr and mrubens as code owners July 1, 2025 06:27
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Jul 1, 2025
@delve-auditor
Copy link

delve-auditor bot commented Jul 1, 2025

No security or compliance issues detected. Reviewed everything up to 3e478df.

Security Overview
  • 🔎 Scanned files: 1 changed file(s)
Detected Code Changes

The diff is too large to display a summary of code changes.

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 1, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 7, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Jul 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[bug] Auto-approval is failing for access_mcp_resource

3 participants