feat(rbac): add tools.execute permission to team-scoped viewer role

[kimsehwan96]

🔗 Related Issue

Closes #3881

---

📝 Summary

Add tools.execute permission to the team-scoped viewer role so team members can invoke MCP tools without requiring the developer role (which also grants create/update/delete permissions). platform_viewer (global) is not modified.

---

🏷️ Type of Change

• Bug fix
• Feature / Enhancement
• Documentation
• Refactor
• Chore (deps, CI, tooling)
• Other (describe below)

---

🧪 Verification

Check	Command	Status
Lint suite	make lint	pass
Unit tests	make test	pass
Coverage ≥ 80%	make coverage	pass

I also checked e2e / playwright test in local environment.

---

✅ Checklist

• Code formatted (make black isort pre-commit)
• Tests added/updated for changes
• Documentation updated (if applicable)
• No secrets or credentials committed

---

📓 Notes (optional)

• Migration follows the same pattern established in fix(rbac): backfill admin.overview and servers.use permissions to viewer roles #3390 (abf8ac3b6008)
• E2E and Playwright tests verified on full docker-compose stack with fast_time_server

[crivetimihai]

Thanks @kimsehwan96 — this addresses #3878 cleanly. One concern: the viewer role is documented as read-only in CLAUDE.md and docs/docs/manage/rbac.md. Adding tools.execute changes the security model for all deployments. Consider:

1. Should this be a new role (e.g., executor) instead of modifying viewer?
2. If modifying viewer is the right call, the docs need updating.
3. DCO Signed-off-by line is required.

What are your thoughts?

[kimsehwan96]

@crivetimihai Thanks, good point.

Docs update: Both docs/docs/manage/rbac.md and CLAUDE.md are already updated in this PR — the viewer role table includestools.execute.
DCO Signed-off-by: Already present in the commit, and the DCO check is passing

I still think there's a reasonable argument for viewer having tools.execute — most MCP tools carry their own auth (OAuth, etc.), so the gateway-level restriction can feel like double-gating. But I understand the concern about breaking the read-only contract across all deployments.

What's your take — would you prefer keeping viewer strictly read-only, or is there room to reconsider?

If you'd rather keep it as-is, I can close this PR.

[marekdano]

Summary

This PR addresses issue #3881 by adding tools.execute permission to the team-scoped viewer role. This enables team members to execute MCP tools without requiring the developer role, which grants full CRUD permissions.

---

✅ Technical Implementation: Excellent

✅ Security Model: Correct Design

This is a least-privilege security enhancement, not a breaking change. Here's why:

Context from Issue #3881

PR #3390 added servers.use to viewer roles, allowing them to connect to Virtual Servers. However, without tools.execute, this permission was effectively useless for MCP workflows. The only alternative was granting the developer role, which includes full CRUD permissions—creating unnecessary operational risk.

This PR's Approach

• Team-scoped only: viewer (team members) gains tools.execute
• Global unchanged: platform_viewer (auto-assigned, no team membership) remains read-only
• Visibility filtering enforced: Team viewers can only execute:
  • Own team's tools ✅
  • Public tools ✅
  • NOT other teams' private tools (blocked by Layer 1 scoping) ❌

Operational Benefits

• Prevents forcing admins to grant developer role just for tool execution
• Reduces risk of accidental configuration changes
• Enables safe MCP tool usage for team members
• Addresses real multi-team deployment needs

---

✅ Test Coverage: Comprehensive

Updated tests verify:

• Team-scoped viewer can execute tools via /rpc tools/call ✅
• Cookie-based session tokens work correctly ✅
• Cross-team isolation maintained (Layer 1 scoping) ✅
• Permission matrix tests updated ✅
• E2E MCP RBAC transport tests updated ✅

---

📝 Minor Suggestion

Consider adding a note in CHANGELOG.md for release notes:

### Enhanced
- **RBAC:** Added `tools.execute` permission to team-scoped `viewer` role, enabling team members to execute MCP tools without requiring `developer` role (which grants full CRUD permissions). Addresses #3881. `platform_viewer` (global scope) remains read-only.

Otherwise LGTM 🚀

[kimsehwan96]

@marekdano Thanks for the review!
Added the changelog entry under ### Changed > 👥 RBAC / Teams in the [1.0.0] section.

[marekdano]

LGTM 🚀

[kimsehwan96]

@marekdano Thanks!

I think CI fails with two issues

• Multiple alembic heads : Change down revision sync with latest upstream main commit head
• .secrets.baseline line number shift : Just needs detect-secrets scan --update

Would you like me to rebase and push the fixes, or will you handle it on your end?

[marekdano]

@kimsehwan96 - yes, I can confirm

1. The error "Multiple heads are present" occurs because the PR introduces a new migration cbedf4e580e0 that branches from the same parent 615af4ab94b4 as the existing head 225bde88217e. This creates two parallel migration heads, which Alembic cannot resolve automatically.
   The solution can be to have a merge migration e.g.
   6e09506295a9_merge_grant_source_and_viewer_tools_.py that resolves this by having both migrations.

"""merge grant_source and viewer_tools_execute heads

Revision ID: 6e09506295a9
Revises: 225bde88217e, cbedf4e580e0
Create Date: 2026-04-02 12:09:28.269751

"""
from typing import Sequence, Union

from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision: str = '6e09506295a9'
down_revision: Union[str, Sequence[str], None] = ('225bde88217e', 'cbedf4e580e0')
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
    """Upgrade schema."""
    pass


def downgrade() -> None:
    """Downgrade schema."""
    pass

2. To solve .secrets.baseline, please run make detect-secrets-scan && make detect-secrets-audit before your new commit.

[kimsehwan96]

@marekdano I rebased it from main branch and re-run make detect-secrets-scan && make detect-secrets-audit

Now on there is no conflict.

Thanks!

[kimsehwan96]

I've done rebase again because of conflicts in .secrets.baseline

[marekdano]

Summary

This PR adds tools.execute permission to the team-scoped viewer role, allowing team members to invoke MCP tools without requiring the developer role (which also grants mutation permissions). The platform_viewer role is intentionally NOT modified.

Architecture Alignment

The change correctly maintains the two-layer security model:

• Layer 1 (Token Scoping): Controls visibility - unchanged
• Layer 2 (RBAC): Controls actions - viewer now has tools.execute in team scope

✅ Appropriate Scope Limitation

• ✅ Team-scoped viewer gets tools.execute
• ✅ Global platform_viewer does NOT get tools.execute
• ✅ Maintains principle of least privilege for unauthenticated/public access

✅ Test Coverage

• Positive path: Developer and viewer can execute team tools
• Deny path: Cross-team isolation maintained
• Cookie session auth: Tested for both developer and viewer
• RPC endpoint: /rpc tools/call tested with proper error codes

Migration File

# mcpgateway/alembic/versions/cbedf4e580e0_add_tools_execute_to_viewer_role.py
revision: str = "cbedf4e580e0"
down_revision: Union[str, Sequence[str], None] = "a7f3c9e1b2d4"  # ✅ Correct head

ROLE_PERMISSION_ADDITIONS = {
    "viewer": ["tools.execute"],  # Only viewer, not platform_viewer
}

Bootstrap Changes

# mcpgateway/bootstrap_db.py
{
    "name": "viewer",
    "description": "Read access and tool execution within team scope",  # Updated
    "scope": "team",
    "permissions": [
        # ... existing permissions ...
        "tools.execute",  # ✅ Added
        # ...
    ],
}

Conclusion

This is a well-implemented feature that appropriately extends the viewer role capabilities while maintaining security boundaries. The change is:

• ✅ Backward-compatible
• ✅ Properly tested with comprehensive coverage
• ✅ Follows all project conventions
• ✅ Maintains security invariants
• ✅ Includes proper documentation updates
• ✅ Uses idempotent migration pattern

LGTM 🚀

[kimsehwan96]

@marekdano The CI fails with

diff --git a/.secrets.baseline b/.secrets.baseline
index 23958c459..78c89d6df 100644
--- a/.secrets.baseline
+++ b/.secrets.baseline
@@ -11397,4 +11397,4 @@
     "file": null,
     "hash": null
   }
-}
\ No newline at end of file
+}
make: *** [Makefile:3956: pre-commit] Error 1

I added new line in the .secrets.baseline and pushed!

Another conflict was made.. I will rebase it.

[kimsehwan96]

Rebased onto latest main. Resolved conflicts in:

• tests/playwright/test_rbac_permissions.py: fix: Fix playwright gateway action tests #3995 (Fix playwright gateway action tests) added assertions expecting viewer to be denied tools.execute(== -32003). Since this PR grants tools.execute to the viewer role, updated those assertions to expect allowed (!= -32003) — 4 conflict sites, all resolved to our version.
• .secrets.baseline: test: add secrets detection hook regressions #3931 (secrets detection hook regressions) updated the baseline. Re-scanned and marked migration hashes as is_secret: false.

[crivetimihai]

Starting review, please don't push anymore.

[crivetimihai]

Nice work, @kimsehwan96 — clean implementation overall. The design makes sense: letting team-scoped viewers execute tools without promoting them to developer (which also grants mutation permissions) is the right least-privilege move, and keeping platform_viewer read-only is the correct call.

I rebased onto main (one commit behind, trivial .secrets.baseline conflict) and added a few commits on top during review:

What I changed

Critical fix — Playwright test payloads (visibility placement)

All six Playwright tests that create team-scoped tools via POST /tools (JSON API) were placing visibility: "team" at the request body top level instead of inside the nested tool object. The create_tool endpoint parses tool: ToolCreate as a nested body param, so top-level visibility is silently ignored and register_tool falls back to visibility="public". I verified this against the live deployment — top-level produces a public tool, inside-tool produces a team-scoped one. Moved visibility into the tool object for all six payloads (4 from this PR, 2 pre-existing).

Migration scope safety

The role lookup query used WHERE name = :name LIMIT 1 without filtering by scope. If an operator had created a custom role also named "viewer" at global scope, the migration could modify the wrong one. Added AND scope = :scope and changed the config from a dict to (name, scope, permissions) tuples.

Stronger viewer allow-path assertions

The viewer tests asserted error_code != -32003, which passes on any non-RBAC error (including tool-not-found or internal errors). Strengthened to assert visibility == "team" on the created tool, HTTP 200 on the RPC response, and "result" in body to prove successful execution.

Deny-path coverage for platform_viewer

Since tools.execute is now in VIEWER_PERMISSIONS, it drops out of MUTATION_PERMISSIONS (which excludes viewer perms). The parametrized test_platform_viewer_denies_mutation no longer covers it. Added an explicit test_platform_viewer_denied_tools_execute enforcement-level test.

Doc drift

Updated three docs that still described viewer as read-only / unable to execute tools: multitenancy.md, sso-entra-role-mapping.md, sso-microsoft-entra-id-tutorial.md. Also fixed stale "Read-Only" section headers in the permission matrix test.

What looks good as-is

• Migration is idempotent, uses parameterized queries, handles both SQLite and PostgreSQL, and downgrade correctly reverses the upgrade.
• bootstrap_db.py and migration agree on the change. All permission sources of truth (bootstrap, migration, AGENTS.md, rbac.md, test constants) are consistent.
• The e2e test test_viewer_denied_tools_execute_on_default_endpoint correctly preserves the deny-path on the global /mcp endpoint.
• The meta-test test_bootstrap_roles_match_test_constants now explicitly verifies viewer_set - platform_viewer_set == {"tools.execute"}.
• No privilege escalation path — tools.execute gates only tool invocation and does not chain into write access or admin capabilities.

282 unit tests pass. LGTM.