Skip to content

Commit 5caea2d

Browse files
sd2kclaude
andauthored
feat: declare required RBAC permissions on each tool (#1306)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 7514746 commit 5caea2d

48 files changed

Lines changed: 636 additions & 114 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎CONTRIBUTING.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ Run `make lint` and `make test-unit` before pushing. The specifics:
102102
- **Pass `context.Context` through Grafana API client calls** — enforced by `make lint-openapi`.
103103
- **Write tools must respect `--disable-write`.** Any tool that creates, updates or deletes anything must be registered so that it disappears when the server runs read-only. Take the `enableWriteTools` parameter in your category's `Add*Tools` function.
104104
- **Query tools must respect `--disable-query`.** Any tool that executes a query against a datasource must disappear when query execution is disabled. Take the `enableQueryTools` parameter in your category's `Add*Tools` function, and leave metadata and discovery tools registered unconditionally. If the tool passes the query through unfiltered — raw SQL, InfluxQL — it can write, so it counts as a write tool too: add its category to `mutatingQueryCategories` in `cmd/mcp-grafana/main.go` so `--disable-write` removes it.
105-
- **New tools need a row in the README's tool table**, including the category and the required RBAC permissions and scopes.
105+
- **New tools declare their RBAC permissions** with `mcpgrafana.RequiresPermissions(...)` on the `MustTool` call (see `tools/rbac.go` for shared groups such as `datasourceQuery`), and **need a row in the README's tool table** with the category and required scopes. The permissions column is generated: run `go test ./cmd/mcp-grafana -run TestReadmeToolPermissions -update-readme`.
106106

107107
The checks that must pass to merge are `license/cla`, `Test Unit`, `Test Integration`, `Lint Go`, `Lint JSON Schemas`, `docker` and `docker-alpine`. `Test Cloud` and the Python E2E suites are informational — `Test Cloud` cannot pass from a fork at all, because it needs credentials, so please don't worry about it being red.
108108

‎README.md‎

Lines changed: 124 additions & 104 deletions
Large diffs are not rendered by default.

‎cmd/mcp-grafana/main_test.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,6 +1283,17 @@ var metadataToolNames = []string{
12831283
// and returns the set of advertised tool names.
12841284
func registerAllCategories(t *testing.T, dt disabledTools) map[string]bool {
12851285
t.Helper()
1286+
names := make(map[string]bool)
1287+
for name := range listAllCategoryTools(t, dt) {
1288+
names[name] = true
1289+
}
1290+
return names
1291+
}
1292+
1293+
// listAllCategoryTools enables every tool category on top of dt and returns
1294+
// the tools a client sees in tools/list.
1295+
func listAllCategoryTools(t *testing.T, dt disabledTools) map[string]*mcp.Tool {
1296+
t.Helper()
12861297

12871298
categories := make([]string, 0, len(dt.toolEntries()))
12881299
for _, e := range dt.toolEntries() {
@@ -1297,11 +1308,11 @@ func registerAllCategories(t *testing.T, dt disabledTools) map[string]bool {
12971308
result, err := session.ListTools(context.Background(), nil)
12981309
require.NoError(t, err)
12991310

1300-
names := make(map[string]bool, len(result.Tools))
1311+
tools := make(map[string]*mcp.Tool, len(result.Tools))
13011312
for _, tool := range result.Tools {
1302-
names[tool.Name] = true
1313+
tools[tool.Name] = tool
13031314
}
1304-
return names
1315+
return tools
13051316
}
13061317

13071318
func TestProcessTools_QueryToolsRegisteredByDefault(t *testing.T) {

‎cmd/mcp-grafana/rbac_test.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package main
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
8+
mcpgrafana "github.com/grafana/mcp-grafana/v2"
9+
)
10+
11+
// TestEveryToolDeclaresPermissions fails when a tool is registered without
12+
// mcpgrafana.RequiresPermissions. It checks every combination of the flags
13+
// that pick between separate MustTool variants of a tool.
14+
func TestEveryToolDeclaresPermissions(t *testing.T) {
15+
var variants []disabledTools
16+
for _, write := range []bool{false, true} {
17+
for _, query := range []bool{false, true} {
18+
for _, enableQuery := range []bool{false, true} {
19+
variants = append(variants, disabledTools{write: write, query: query, enableQuery: enableQuery})
20+
}
21+
}
22+
}
23+
for _, dt := range variants {
24+
for name, tool := range listAllCategoryTools(t, dt) {
25+
_, ok := mcpgrafana.ToolRequiredPermissions(tool)
26+
assert.True(t, ok, "tool %q declares no RBAC permissions: add mcpgrafana.RequiresPermissions(...) "+
27+
"to its MustTool call, e.g. RequiresPermissions(datasourceQuery...) for a datasource query tool "+
28+
"(see tools/rbac.go), or RequiresPermissions() if it needs none", name)
29+
}
30+
}
31+
}

‎cmd/mcp-grafana/readme_test.go‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
package main
2+
3+
import (
4+
"flag"
5+
"os"
6+
"slices"
7+
"strings"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
13+
mcpgrafana "github.com/grafana/mcp-grafana/v2"
14+
)
15+
16+
var updateReadme = flag.Bool("update-readme", false, "rewrite the README tool table's permissions column")
17+
18+
const readmePath = "../../README.md"
19+
20+
// TestReadmeToolPermissions keeps the README tool table's "Required RBAC
21+
// Permissions" column in step with each tool's RequiresPermissions. Run
22+
// `go test ./cmd/mcp-grafana -run TestReadmeToolPermissions -update-readme`
23+
// to regenerate it. The other columns are written by hand.
24+
func TestReadmeToolPermissions(t *testing.T) {
25+
declared := make(map[string]string)
26+
readOnly := listAllCategoryTools(t, disabledTools{write: true})
27+
for name, tool := range listAllCategoryTools(t, disabledTools{}) {
28+
perms, _ := mcpgrafana.ToolRequiredPermissions(tool)
29+
var roPerms []string
30+
if ro, ok := readOnly[name]; ok {
31+
roPerms, _ = mcpgrafana.ToolRequiredPermissions(ro)
32+
}
33+
declared[name] = formatPermissions(perms, roPerms)
34+
}
35+
for name, tool := range readOnly {
36+
if _, ok := declared[name]; !ok {
37+
perms, _ := mcpgrafana.ToolRequiredPermissions(tool)
38+
declared[name] = formatPermissions(perms, perms)
39+
}
40+
}
41+
42+
raw, err := os.ReadFile(readmePath)
43+
require.NoError(t, err)
44+
lines := strings.Split(string(raw), "\n")
45+
46+
start := slices.IndexFunc(lines, func(l string) bool { return strings.HasPrefix(l, "| Tool ") })
47+
require.NotEqual(t, -1, start, "README tool table not found")
48+
end := start
49+
for end < len(lines) && strings.HasPrefix(lines[end], "|") {
50+
end++
51+
}
52+
53+
var rows [][]string
54+
listed := make(map[string]bool)
55+
for i, line := range lines[start:end] {
56+
cells := splitTableRow(line)
57+
if i > 0 && !assert.Len(t, cells, len(rows[0]), "README tool table row has the wrong number of cells: %s", line) {
58+
continue
59+
}
60+
if i > 1 {
61+
name := strings.Trim(cells[0], "`")
62+
perms, ok := declared[name]
63+
if !assert.True(t, ok, "README lists %q, which is not a registered tool", name) {
64+
continue
65+
}
66+
listed[name] = true
67+
cells[3] = perms
68+
}
69+
rows = append(rows, cells)
70+
}
71+
for name := range declared {
72+
assert.True(t, listed[name], "tool %q has no row in the README tool table", name)
73+
}
74+
75+
want := strings.Join(slices.Concat(lines[:start], formatTable(rows), lines[end:]), "\n")
76+
if t.Failed() {
77+
return
78+
}
79+
if *updateReadme {
80+
require.NoError(t, os.WriteFile(readmePath, []byte(want), 0o644))
81+
return
82+
}
83+
assert.Equal(t, string(raw), want, "README tool permissions are out of date; regenerate with "+
84+
"`go test ./cmd/mcp-grafana -run TestReadmeToolPermissions -update-readme`")
85+
}
86+
87+
// splitTableRow splits a markdown table row into trimmed cells, leaving
88+
// escaped pipes (\|) inside their cell.
89+
func splitTableRow(line string) []string {
90+
var cells []string
91+
var cell strings.Builder
92+
line = strings.TrimSpace(line)
93+
line = strings.TrimSuffix(strings.TrimPrefix(line, "|"), "|")
94+
for i := 0; i < len(line); i++ {
95+
switch {
96+
case line[i] == '\\' && i+1 < len(line) && line[i+1] == '|':
97+
cell.WriteString(`\|`)
98+
i++
99+
case line[i] == '|':
100+
cells = append(cells, strings.TrimSpace(cell.String()))
101+
cell.Reset()
102+
default:
103+
cell.WriteByte(line[i])
104+
}
105+
}
106+
return append(cells, strings.TrimSpace(cell.String()))
107+
}
108+
109+
// formatPermissions renders a tool's actions. When the read-only variant
110+
// (--disable-write) needs fewer, the extra write actions are listed apart so
111+
// read-only deployments don't over-grant.
112+
func formatPermissions(readWrite, readOnly []string) string {
113+
writeOnly := slices.DeleteFunc(slices.Clone(readWrite), func(p string) bool { return slices.Contains(readOnly, p) })
114+
if readOnly == nil || len(writeOnly) == 0 || len(readOnly)+len(writeOnly) != len(readWrite) {
115+
return quotePermissions(readWrite)
116+
}
117+
return quotePermissions(readOnly) + "; with writes enabled also " + quotePermissions(writeOnly)
118+
}
119+
120+
func quotePermissions(perms []string) string {
121+
if len(perms) == 0 {
122+
return "None"
123+
}
124+
quoted := make([]string, len(perms))
125+
for i, p := range perms {
126+
quoted[i] = "`" + p + "`"
127+
}
128+
return strings.Join(quoted, ", ")
129+
}
130+
131+
// formatTable pads a markdown table so its columns line up. Row 1 is the
132+
// separator row.
133+
func formatTable(rows [][]string) []string {
134+
widths := make([]int, len(rows[0]))
135+
for i, row := range rows {
136+
for j, cell := range row {
137+
if i != 1 {
138+
widths[j] = max(widths[j], len([]rune(cell)))
139+
}
140+
}
141+
}
142+
out := make([]string, len(rows))
143+
for i, row := range rows {
144+
var b strings.Builder
145+
b.WriteString("|")
146+
for j, cell := range row {
147+
if i == 1 {
148+
cell = strings.Repeat("-", widths[j])
149+
}
150+
b.WriteString(" " + cell + strings.Repeat(" ", widths[j]-len([]rune(cell))) + " |")
151+
}
152+
out[i] = b.String()
153+
}
154+
return out
155+
}
156+
157+
func TestReadmeTableHelpers(t *testing.T) {
158+
assert.Equal(t, []string{"`a`", `x \| y`, ""}, splitTableRow("| `a` | x \\| y | |"))
159+
assert.Equal(t, "None", formatPermissions(nil, nil))
160+
assert.Equal(t, "`a`, `b`", formatPermissions([]string{"a", "b"}, []string{"a", "b"}))
161+
assert.Equal(t, "`a`; with writes enabled also `b`", formatPermissions([]string{"a", "b"}, []string{"a"}))
162+
}

‎permissions.go‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package mcpgrafana
2+
3+
import (
4+
"slices"
5+
6+
"github.com/modelcontextprotocol/go-sdk/mcp"
7+
)
8+
9+
// RequiredPermissionsMetaKey is the tool _meta key that lists the Grafana RBAC
10+
// actions a tool's calls need. Set it with RequiresPermissions.
11+
const RequiredPermissionsMetaKey = "grafana.com/requiredPermissions"
12+
13+
// RequiresPermissions declares Grafana RBAC actions the tool's calls need. A
14+
// caller missing any of them gets a 403 from the tool. Every tool must declare
15+
// its actions, including tools that need none (call it with no arguments); a
16+
// test fails otherwise. Repeated calls add to the list, so a shared group can
17+
// be combined with extra actions:
18+
//
19+
// mcpgrafana.RequiresPermissions(datasourceQuery...),
20+
// mcpgrafana.RequiresPermissions("dashboards:read"),
21+
//
22+
// Calls to /apis endpoints are checked twice: the user's legacy action (the
23+
// verb as mapped by Grafana, e.g. dashboards:read) and, for on-behalf-of
24+
// tokens, the token's group/resource:verb permission (e.g.
25+
// dashboard.grafana.app/dashboards:get). Tools that use /apis declare both.
26+
//
27+
// Declare the actions for every operation the tool can perform, including
28+
// write operations behind a read-write variant.
29+
func RequiresPermissions(actions ...string) ToolOption {
30+
return func(t *mcp.Tool) {
31+
if t.Meta == nil {
32+
t.Meta = mcp.Meta{}
33+
}
34+
perms, _ := ToolRequiredPermissions(t)
35+
perms = append(append([]string{}, perms...), actions...)
36+
slices.Sort(perms)
37+
t.Meta[RequiredPermissionsMetaKey] = slices.Compact(perms)
38+
}
39+
}
40+
41+
// ToolRequiredPermissions returns the actions declared with
42+
// RequiresPermissions and whether the tool declared any. It accepts both the
43+
// in-process value and one decoded from a tools/list response.
44+
func ToolRequiredPermissions(t *mcp.Tool) ([]string, bool) {
45+
switch v := t.Meta[RequiredPermissionsMetaKey].(type) {
46+
case []string:
47+
return slices.Clone(v), true
48+
case []any:
49+
perms := make([]string, 0, len(v))
50+
for _, p := range v {
51+
s, ok := p.(string)
52+
if !ok {
53+
return nil, false
54+
}
55+
perms = append(perms, s)
56+
}
57+
return perms, true
58+
}
59+
return nil, false
60+
}

‎permissions_test.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package mcpgrafana
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/modelcontextprotocol/go-sdk/mcp"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestRequiresPermissions(t *testing.T) {
13+
tool := &mcp.Tool{}
14+
RequiresPermissions("b:read", "a:read")(tool)
15+
RequiresPermissions("a:read", "c:write")(tool)
16+
perms, ok := ToolRequiredPermissions(tool)
17+
require.True(t, ok)
18+
assert.Equal(t, []string{"a:read", "b:read", "c:write"}, perms)
19+
20+
// An empty declaration still counts, including after a tools/list round trip.
21+
empty := &mcp.Tool{Name: "none"}
22+
RequiresPermissions()(empty)
23+
b, err := json.Marshal(empty)
24+
require.NoError(t, err)
25+
var decoded mcp.Tool
26+
require.NoError(t, json.Unmarshal(b, &decoded))
27+
perms, ok = ToolRequiredPermissions(&decoded)
28+
require.True(t, ok)
29+
assert.Empty(t, perms)
30+
31+
_, ok = ToolRequiredPermissions(&mcp.Tool{})
32+
assert.False(t, ok)
33+
}

0 commit comments

Comments
 (0)