diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4be7b8f..03c8596 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -44,6 +44,15 @@ APIs change between minors without a deprecation period.
redacted to the same text shared one answer — with the first caller's
address put back in it. Such requests are now never answered from the
cache nor kept in it.
+- **Granting a server's tools could take others away; revoking could close
+ a level to every tool.** A level with no tool list sees what the levels
+ above grant; giving it its first list made it see only that list. And a
+ level left with an empty list was closed to every tool, which the screens
+ did not show. Now a first list starts from the nearest list above, taking
+ a server away leaves the rest or goes back to no list, the screens say
+ when a level above keeps granted tools out of reach and which levels are
+ closed, and the console reads and writes the level at once rather than
+ from what it loaded.
- **Similar questions about different pictures got each other's answer.**
A question with a picture, a sound or a file in it is reused only when it
is the same request.
diff --git a/docs/console.md b/docs/console.md
index 7aed363..5d4daff 100644
--- a/docs/console.md
+++ b/docs/console.md
@@ -181,7 +181,8 @@ its name, which is its tools' prefix, its address, and a bearer token or
another header, kept sealed in the store. On a row: **Test, and list its
tools**, with its credential, as a call would; **Grant its tools to…** a
team, project, organization, person or key — all of them, `jira.*`, or some
-by name — beside whatever else that level is granted; **Revoke** a grant;
+by name — beside whatever else that level is granted, and saying when a
+level above keeps them out of reach; **Revoke** a grant;
**Change…** its address, timeout or rate; **Replace its credential…**;
**Turn off** (its tools are listed nowhere and every call refused) or on;
**Remove…**. A server's tools are seen nowhere until granted. The screen
diff --git a/docs/install.md b/docs/install.md
index 94aa1f0..de8cf47 100644
--- a/docs/install.md
+++ b/docs/install.md
@@ -2606,10 +2606,20 @@ llmgw keys limits --scope team --id Acme/support --tools 'jira.*'
- **Who may call what:** tools are named `server.tool` and granted as
patterns (`jira.*`, `db.query`) at any level of the tenancy tree, as
models are.
- - A level that names any narrows what is above it.
+ - A level that names any narrows what is above it: a tool is seen where
+ every level above that names tools names it too. A team under an
+ organization that names `hr.*` sees no `jira.*` whatever the team is
+ given, until the organization names it as well.
- Unlike models, **none granted anywhere is none**: a tool reaches into a
company system, so it is granted, never assumed.
- - The console's limits form has the same field.
+ - `keys limits --tools` writes a level's list whole, every server's
+ together; an empty list (`--tools ''`) closes the level to every tool.
+ The console's **MCP servers** screen and the `/admin/mcp` page grant
+ and take back one server's tools: a level given its first list starts
+ from the nearest list above it, so it keeps what it saw; one left
+ naming none goes back to naming none, or stays closed when a level
+ above names tools, so taking one server away opens it to no other. They
+ say when a level above keeps the granted tools out of reach.
- **The server is told who calls:** `X-Llmgw-Key`, and `X-Llmgw-User` and
`X-Llmgw-Project` where there are some. It is called with llmgw's
credentials, never the caller's key.
diff --git a/internal/cli/tui_mcp.go b/internal/cli/tui_mcp.go
index a02439f..ee46e13 100644
--- a/internal/cli/tui_mcp.go
+++ b/internal/cli/tui_mcp.go
@@ -82,13 +82,13 @@ func (m consoleModel) fetchMCP() tea.Cmd {
}
msg.grants = []mcpGrant{}
for _, l := range limits {
- if len(l.AllowedTools) == 0 {
+ if l.AllowedTools == nil {
continue
}
o := store.LimitOwner{Scope: l.Scope, ID: l.ScopeID}
label := msg.where[o.CounterKey()]
if label == "" {
- label = string(l.Scope) + " " + l.ScopeID
+ label = l.ScopeID
}
msg.grants = append(msg.grants, mcpGrant{owner: o, label: string(l.Scope) + " " + label, tools: l.AllowedTools})
}
@@ -100,7 +100,10 @@ func (m consoleModel) fetchMCP() tea.Cmd {
func (s *consoleMCPMsg) name(scope store.Scope, id, label string) {
o := store.LimitOwner{Scope: scope, ID: id}
s.where[o.CounterKey()] = label
- s.ids[string(scope)+":"+label] = o
+ s.ids[string(scope)+":"+strings.ToLower(label)] = o
+ if m := store.MemberScope(scope); m != "" {
+ s.where[store.LimitOwner{Scope: m, ID: id}.CounterKey()] = label
+ }
}
// mcpGrantsOf is who may use a server's tools, and which of them.
@@ -278,8 +281,8 @@ func (m consoleModel) mcpMenu(s adminapi.MCPServerView) *menuDialog {
items = append(items, menuItem{"Revoke from " + g.label + " (" + strings.Join(g.tools, ", ") + ")…",
func(m *consoleModel) tea.Cmd {
m.confirmD = &confirmDialog{title: "Revoke " + s.Name + " from " + g.label,
- text: "It and everything below it then no longer see " + strings.Join(g.tools, ", ") +
- ",\nunless another level grants them.",
+ text: "It then sees " + strings.Join(g.tools, ", ") + " only if the levels above grant them\n" +
+ "and it names no tools of its own. Its other grants stay.",
yes: func(m *consoleModel) tea.Cmd { return m.mcpSetTools(s.Name, g.owner, nil) }}
return nil
}})
@@ -397,8 +400,8 @@ func (m *consoleModel) mcpGrantForm(s adminapi.MCPServerView) {
return
}
m.openForm(&formDialog{title: "Grant " + s.Name + "'s tools",
- note: "It and everything below it then see them, unless a level below grants\n" +
- "tools and not these. Other grants at that level stay.",
+ note: "Seen there, and below, where every level above that names tools names these too.\n" +
+ "A level given its first grant keeps what it saw from above; other grants stay.",
fields: []formField{
choiceF("to", []string{"team", "project", "org", "user", "key"}, "team"),
textF("which", "", "ORG/TEAM/PROJECT, ORG/TEAM, ORG, an e-mail, or a key's ID"),
@@ -409,7 +412,7 @@ func (m *consoleModel) mcpGrantForm(s adminapi.MCPServerView) {
if which == "" {
return nil, fmt.Errorf("which %s?", v[0])
}
- owner, ok := m.mcp.ids[v[0]+":"+which]
+ owner, ok := m.mcp.ids[v[0]+":"+strings.ToLower(which)]
if v[0] == "key" {
owner, ok = store.LimitOwner{Scope: store.ScopeKey, ID: which}, true
}
@@ -432,25 +435,38 @@ func (m *consoleModel) mcpGrantForm(s adminapi.MCPServerView) {
}})
}
-// mcpSetTools makes a level's grants of a server's tools these, leaving its
-// grants of other servers' as they are. None revokes them.
+// mcpSetTools grants a level these of a server's tools, or with none takes
+// them away, in the store (store.GrantTools, store.RevokeTools): read and
+// written at once, so another administrator's change since this screen
+// was loaded is kept.
func (m *consoleModel) mcpSetTools(server string, owner store.LimitOwner, tools []string) tea.Cmd {
- var keep []string
- for _, g := range m.mcp.grants {
- if g.owner != owner {
- continue
- }
- for _, t := range g.tools {
- if !strings.HasPrefix(t, server+".") {
- keep = append(keep, t)
- }
- }
- }
- all := append(keep, tools...)
+ h, where := m.st, m.mcp.where
title := "Grant " + server
if len(tools) == 0 {
title = "Revoke " + server
}
- return m.runCLI(title, "keys", "limits", "--scope="+string(owner.Scope), "--id="+owner.ID,
- "--tools="+strings.Join(all, ","))
+ return func() tea.Msg {
+ ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+ defer cancel()
+ s, err := h.get(ctx)
+ if err != nil {
+ return actionDoneMsg{title: title, err: err}
+ }
+ if len(tools) == 0 {
+ if err := s.RevokeTools(ctx, owner.Scope, owner.ID, server); err != nil {
+ return actionDoneMsg{title: title, err: err}
+ }
+ return actionDoneMsg{title: title, text: server + "'s tools taken back there; its other grants stay."}
+ }
+ g, err := s.GrantTools(ctx, owner.Scope, owner.ID, server, tools)
+ if err != nil {
+ return actionDoneMsg{title: title, err: err}
+ }
+ text := "Granted " + strings.Join(tools, ", ") + "."
+ for _, b := range g.BlockedBy {
+ text += "\n\nBut " + string(b.Scope) + " " + or(where[b.CounterKey()], b.ID) + " names tools and not these:\n" +
+ server + "'s stay out of reach there until it grants them too."
+ }
+ return actionDoneMsg{title: title, text: text}
+ }
}
diff --git a/internal/store/toolgrants.go b/internal/store/toolgrants.go
new file mode 100644
index 0000000..68cf803
--- /dev/null
+++ b/internal/store/toolgrants.go
@@ -0,0 +1,159 @@
+package store
+
+import (
+ "context"
+ "errors"
+ "slices"
+ "strings"
+)
+
+// Granting one MCP server's tools at a level of the tree, and taking them
+// back, as the console and the admin page do. A tool is usable only where
+// every level above that names tools names it too (Effective.AllowsTool),
+// which makes adding a list where there was none a narrowing: a team that
+// saw its organization's jira.* through having no list of its own would see
+// only hr.* once given hr.*. So a level given its first list starts from
+// the nearest list above it, and keeps what it had.
+
+// ToolGrant is what granting came to: the levels above whose lists do not
+// name the tools granted, so that they stay out of reach until those grant
+// them too.
+type ToolGrant struct {
+ BlockedBy []LimitOwner
+}
+
+// GrantTools makes a level's grants of server's tools these, keeping its
+// grants of other servers' and the rest of its limits.
+func (s *DB) GrantTools(ctx context.Context, scope Scope, id, server string, tools []string) (ToolGrant, error) {
+ above, err := s.limitsAbove(ctx, scope, id)
+ if err != nil {
+ return ToolGrant{}, err
+ }
+ lim, err := s.LimitsFor(ctx, scope, id)
+ switch {
+ case errors.Is(err, ErrNotFound):
+ lim = Limits{Scope: scope, ScopeID: id}
+ case err != nil:
+ return ToolGrant{}, err
+ }
+ if lim.AllowedTools == nil {
+ // Its first list: what it saw through the levels above, kept.
+ for _, a := range above {
+ if a.AllowedTools != nil {
+ lim.AllowedTools = slices.Clone(a.AllowedTools)
+ break
+ }
+ }
+ }
+ keep := withoutServer(lim.AllowedTools, server)
+ for _, t := range tools {
+ if !slices.Contains(keep, t) {
+ keep = append(keep, t)
+ }
+ }
+ lim.AllowedTools = keep
+ if err := s.PutLimits(ctx, lim); err != nil {
+ return ToolGrant{}, err
+ }
+ var g ToolGrant
+ for _, a := range above {
+ if a.AllowedTools == nil {
+ continue
+ }
+ for _, t := range tools {
+ if !matchesTool(a.AllowedTools, sampleTool(t)) {
+ g.BlockedBy = append(g.BlockedBy, LimitOwner{Scope: a.Scope, ID: a.ScopeID})
+ break
+ }
+ }
+ }
+ return g, nil
+}
+
+// RevokeTools takes a level's grants of server's tools away, keeping the
+// rest. A level left naming no tools no longer names any — it then sees
+// what the levels above let it — unless a level above names tools: then
+// it is left naming none, closed to every tool, so that taking one
+// server's tools away does not open it to the others above.
+func (s *DB) RevokeTools(ctx context.Context, scope Scope, id, server string) error {
+ lim, err := s.LimitsFor(ctx, scope, id)
+ if errors.Is(err, ErrNotFound) {
+ return nil
+ }
+ if err != nil {
+ return err
+ }
+ lim.AllowedTools = withoutServer(lim.AllowedTools, server)
+ if len(lim.AllowedTools) == 0 {
+ above, err := s.limitsAbove(ctx, scope, id)
+ if err != nil {
+ return err
+ }
+ lim.AllowedTools = nil
+ for _, a := range above {
+ if a.AllowedTools != nil {
+ lim.AllowedTools = []string{}
+ break
+ }
+ }
+ }
+ return s.PutLimits(ctx, lim)
+}
+
+func withoutServer(tools []string, server string) []string {
+ return slices.DeleteFunc(slices.Clone(tools), func(t string) bool { return strings.HasPrefix(t, server+".") })
+}
+
+// sampleTool is a tool a pattern grants, for asking whether a list above
+// grants it too: jira.* stands for any of jira's.
+func sampleTool(pattern string) string {
+ if before, ok := strings.CutSuffix(pattern, "*"); ok {
+ return before + "x"
+ }
+ return pattern
+}
+
+// limitsAbove are the limits of the levels above one, nearest first: a
+// project's team and organization, a team's organization, a key's project
+// and those above it. None for an organization or a person.
+func (s *DB) limitsAbove(ctx context.Context, scope Scope, id string) ([]Limits, error) {
+ var owners []LimitOwner
+ if scope == ScopeKey {
+ k, err := s.KeyByID(ctx, id)
+ if err != nil {
+ return nil, nil
+ }
+ scope, id = ScopeProject, k.ProjectID
+ owners = append(owners, LimitOwner{Scope: ScopeProject, ID: id})
+ }
+ if scope == ScopeTeam || scope == ScopeProject {
+ tree, err := s.Tree(ctx)
+ if err != nil {
+ return nil, err
+ }
+ for _, o := range tree {
+ for _, t := range o.Teams {
+ if scope == ScopeTeam && t.ID == id {
+ owners = append(owners, LimitOwner{Scope: ScopeOrg, ID: o.ID})
+ }
+ for _, p := range t.Projects {
+ if scope == ScopeProject && p.ID == id {
+ owners = append(owners, LimitOwner{Scope: ScopeTeam, ID: t.ID}, LimitOwner{Scope: ScopeOrg, ID: o.ID})
+ }
+ }
+ }
+ }
+ }
+ var out []Limits
+ for _, o := range owners {
+ l, err := s.LimitsFor(ctx, o.Scope, o.ID)
+ if errors.Is(err, ErrNotFound) {
+ continue
+ }
+ if err != nil {
+ return nil, err
+ }
+ out = append(out, l)
+ }
+ return out, nil
+}
diff --git a/internal/store/toolgrants_test.go b/internal/store/toolgrants_test.go
new file mode 100644
index 0000000..d6dee0d
--- /dev/null
+++ b/internal/store/toolgrants_test.go
@@ -0,0 +1,100 @@
+package store
+
+import (
+ "context"
+ "path/filepath"
+ "slices"
+ "testing"
+)
+
+func TestGrantingToolsAtALevelKeepsWhatItSawFromAbove(t *testing.T) {
+ ctx := context.Background()
+ s, err := OpenSQLite(ctx, filepath.Join(t.TempDir(), "s.db"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ org, _ := s.CreateOrg(ctx, "Acme")
+ team, _ := s.CreateTeam(ctx, org, "eng")
+ tools := func(scope Scope, id string) []string {
+ l, _ := s.LimitsFor(ctx, scope, id)
+ return l.AllowedTools
+ }
+ sees := func(tool string) bool {
+ e := Effective{}
+ for _, l := range []struct {
+ sc Scope
+ id string
+ }{{ScopeOrg, org}, {ScopeTeam, team}} {
+ if ts := tools(l.sc, l.id); ts != nil {
+ e.ToolGrants = append(e.ToolGrants, ts)
+ }
+ }
+ return e.AllowsTool(tool)
+ }
+
+ // The organization grants hr's tools; the team names none and sees them.
+ if _, err := s.GrantTools(ctx, ScopeOrg, org, "hr", []string{"hr.*"}); err != nil {
+ t.Fatal(err)
+ }
+ if !sees("hr.search") {
+ t.Fatal("the team does not see what its organization grants")
+ }
+ // Granting jira's to the team keeps hr's, and says the organization
+ // still keeps jira's out of reach.
+ g, err := s.GrantTools(ctx, ScopeTeam, team, "jira", []string{"jira.*"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !sees("hr.search") {
+ t.Errorf("granting jira's tools to the team took hr's away: %v", tools(ScopeTeam, team))
+ }
+ if len(g.BlockedBy) != 1 || g.BlockedBy[0].ID != org {
+ t.Errorf("the organization, which does not grant jira's tools, should be named: %+v", g)
+ }
+ if sees("jira.search") {
+ t.Error("the team sees jira's tools its organization does not grant")
+ }
+ // The organization grants them too: now the team sees both.
+ _, _ = s.GrantTools(ctx, ScopeOrg, org, "jira", []string{"jira.*"})
+ if !sees("jira.search") || !sees("hr.get") {
+ t.Errorf("org %v team %v", tools(ScopeOrg, org), tools(ScopeTeam, team))
+ }
+ // Taking jira's back from the team leaves it hr's.
+ if err := s.RevokeTools(ctx, ScopeTeam, team, "jira"); err != nil {
+ t.Fatal(err)
+ }
+ if sees("jira.search") || !sees("hr.get") {
+ t.Errorf("after the revoke: team %v", tools(ScopeTeam, team))
+ }
+}
+
+func TestRevokingALevelsLastToolsNeitherClosesNorOpensIt(t *testing.T) {
+ ctx := context.Background()
+ s, err := OpenSQLite(ctx, filepath.Join(t.TempDir(), "s.db"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer s.Close()
+ org, _ := s.CreateOrg(ctx, "Acme")
+ team, _ := s.CreateTeam(ctx, org, "eng")
+
+ // Nothing above names tools: the team's last grant taken leaves it
+ // naming none at all, not closed to every tool.
+ _, _ = s.GrantTools(ctx, ScopeTeam, team, "jira", []string{"jira.*"})
+ _ = s.RevokeTools(ctx, ScopeTeam, team, "jira")
+ if l, _ := s.LimitsFor(ctx, ScopeTeam, team); l.AllowedTools != nil {
+ t.Errorf("left %v; want no list", l.AllowedTools)
+ }
+ // The organization grants every tool; the team was kept to jira's.
+ // Taking jira's away must not open it to everything above.
+ _ = s.PutLimits(ctx, Limits{Scope: ScopeOrg, ScopeID: org, AllowedTools: []string{"*"}})
+ _ = s.PutLimits(ctx, Limits{Scope: ScopeTeam, ScopeID: team, AllowedTools: []string{"jira.*"}})
+ _ = s.RevokeTools(ctx, ScopeTeam, team, "jira")
+ if l, _ := s.LimitsFor(ctx, ScopeTeam, team); l.AllowedTools == nil || len(l.AllowedTools) != 0 {
+ t.Errorf("left %v; want an empty list, closed", l.AllowedTools)
+ }
+ if !slices.Equal(withoutServer([]string{"a.x", "b.*", "a.*"}, "a"), []string{"b.*"}) {
+ t.Error("withoutServer")
+ }
+}
diff --git a/internal/web/admin_mcp.go b/internal/web/admin_mcp.go
index c111d65..9ae5574 100644
--- a/internal/web/admin_mcp.go
+++ b/internal/web/admin_mcp.go
@@ -21,7 +21,8 @@ import (
func init() {
// Named from the grant's scope in the template.
- registerKeys("mcp.scope.org", "mcp.scope.team", "mcp.scope.project", "mcp.scope.user", "mcp.scope.key")
+ registerKeys("mcp.scope.org", "mcp.scope.team", "mcp.scope.project", "mcp.scope.user", "mcp.scope.key",
+ "mcp.scope.org-member", "mcp.scope.team-member", "mcp.scope.project-member")
}
// MCPAdmin is where the servers, their sealed credentials and the tree's
@@ -31,6 +32,8 @@ type MCPAdmin interface {
AllLimits(ctx context.Context) (map[string]store.Limits, error)
LimitsFor(ctx context.Context, scope store.Scope, id string) (store.Limits, error)
PutLimits(ctx context.Context, l store.Limits) error
+ GrantTools(ctx context.Context, scope store.Scope, id, server string, tools []string) (store.ToolGrant, error)
+ RevokeTools(ctx context.Context, scope store.Scope, id, server string) error
Tree(ctx context.Context) ([]store.TreeOrg, error)
Users(ctx context.Context, state store.UserState) ([]store.User, error)
}
@@ -46,6 +49,9 @@ type mcpPageData struct {
// Tools are what a test found, for the server Tested.
Tested string
Tools []string
+ // Closed are the levels that name no tool at all, and so see none
+ // whatever is granted above them.
+ Closed []mcpGrantRow
Notice string
Error string
}
@@ -66,6 +72,9 @@ type mcpRow struct {
type mcpGrantRow struct {
Scope, ID, Label string
Tools []string
+ // Revocable is false for a grant of every tool, "*", which taking this
+ // server's away would not change: it is changed with the level's limits.
+ Revocable bool
}
// RegisterAdminMCP mounts /admin/mcp, for administrators only: a server's
@@ -81,6 +90,11 @@ func (p *AccountPages) RegisterAdminMCP(mux *http.ServeMux, s MCPAdmin, local fu
d.Error = l.Err(err)
}
grants := mcpGrants(ctx, s)
+ for _, g := range grants {
+ if len(g.Tools) == 0 {
+ d.Closed = append(d.Closed, g)
+ }
+ }
for _, m := range list {
row := mcpRow{MCPServerView: adminapi.MCPServerView{Name: m.Name, URL: m.URL, Description: m.Description,
Enabled: m.Enabled, TimeoutSeconds: int(m.Timeout.Seconds()), RatePerMinute: m.RatePerMinute},
@@ -95,13 +109,16 @@ func (p *AccountPages) RegisterAdminMCP(mux *http.ServeMux, s MCPAdmin, local fu
slices.Sort(row.Plain)
for _, g := range grants {
var mine []string
+ revocable := false
for _, t := range g.Tools {
if strings.HasPrefix(t, m.Name+".") || t == "*" {
mine = append(mine, t)
+ revocable = revocable || t != "*"
}
}
if len(mine) > 0 {
- row.Grants = append(row.Grants, mcpGrantRow{Scope: g.Scope, ID: g.ID, Label: g.Label, Tools: mine})
+ row.Grants = append(row.Grants, mcpGrantRow{Scope: g.Scope, ID: g.ID, Label: g.Label, Tools: mine,
+ Revocable: revocable})
}
}
d.Servers = append(d.Servers, row)
@@ -254,16 +271,25 @@ func (p *AccountPages) RegisterAdminMCP(mux *http.ServeMux, s MCPAdmin, local fu
if err == nil {
id, label, err = mcpResolve(r.Context(), s, l, scope, which)
}
+ notice := l.T("mcp.granted", name, label)
if err == nil {
- err = mcpSetTools(r.Context(), s, name, scope, id, tools)
+ var g store.ToolGrant
+ if g, err = s.GrantTools(r.Context(), scope, id, name, tools); err == nil && len(g.BlockedBy) > 0 {
+ names := mcpNames(r.Context(), s)
+ var by []string
+ for _, o := range g.BlockedBy {
+ by = append(by, l.T("mcp.scope."+string(o.Scope))+" "+or(names[o.CounterKey()], o.ID))
+ }
+ notice += " " + l.T("mcp.blocked_above", strings.Join(by, ", "), name)
+ }
}
- done(w, r, admin, "mcp grants "+name, l.T("mcp.granted", name, label), err)
+ done(w, r, admin, "mcp grants "+name, notice, err)
}))
mux.HandleFunc("POST /admin/mcp/{name}/revoke", adminsOnly(func(w http.ResponseWriter, r *http.Request, admin store.User) {
l, name := langOf(r), r.PathValue("name")
scope, id := store.Scope(r.FormValue("scope")), r.FormValue("id")
- err := mcpSetTools(r.Context(), s, name, scope, id, nil)
+ err := s.RevokeTools(r.Context(), scope, id, name)
done(w, r, admin, "mcp grants "+name, l.T("mcp.revoked", name), err)
}))
}
@@ -275,26 +301,10 @@ func mcpGrants(ctx context.Context, s MCPAdmin) []mcpGrantRow {
if err != nil {
return nil
}
- names := map[string]string{}
- if tree, err := s.Tree(ctx); err == nil {
- for _, o := range tree {
- names[store.LimitOwner{Scope: store.ScopeOrg, ID: o.ID}.CounterKey()] = o.Name
- for _, t := range o.Teams {
- names[store.LimitOwner{Scope: store.ScopeTeam, ID: t.ID}.CounterKey()] = o.Name + "/" + t.Name
- for _, pr := range t.Projects {
- names[store.LimitOwner{Scope: store.ScopeProject, ID: pr.ID}.CounterKey()] = o.Name + "/" + t.Name + "/" + pr.Name
- }
- }
- }
- }
- if users, err := s.Users(ctx, ""); err == nil {
- for _, u := range users {
- names[store.LimitOwner{Scope: store.ScopeUser, ID: u.ID}.CounterKey()] = u.Email
- }
- }
+ names := mcpNames(ctx, s)
var out []mcpGrantRow
for key, lim := range limits {
- if len(lim.AllowedTools) == 0 {
+ if lim.AllowedTools == nil {
continue
}
label := names[key]
@@ -307,6 +317,35 @@ func mcpGrants(ctx context.Context, s MCPAdmin) []mcpGrantRow {
return out
}
+// mcpNames are the tree's levels and the people by what they are called,
+// keyed as limits are; a member scope is named as the level it is set on.
+func mcpNames(ctx context.Context, s MCPAdmin) map[string]string {
+ names := map[string]string{}
+ name := func(scope store.Scope, id, label string) {
+ names[store.LimitOwner{Scope: scope, ID: id}.CounterKey()] = label
+ if m := store.MemberScope(scope); m != "" {
+ names[store.LimitOwner{Scope: m, ID: id}.CounterKey()] = label
+ }
+ }
+ if tree, err := s.Tree(ctx); err == nil {
+ for _, o := range tree {
+ name(store.ScopeOrg, o.ID, o.Name)
+ for _, t := range o.Teams {
+ name(store.ScopeTeam, t.ID, o.Name+"/"+t.Name)
+ for _, pr := range t.Projects {
+ name(store.ScopeProject, pr.ID, o.Name+"/"+t.Name+"/"+pr.Name)
+ }
+ }
+ }
+ }
+ if users, err := s.Users(ctx, ""); err == nil {
+ for _, u := range users {
+ name(store.ScopeUser, u.ID, u.Email)
+ }
+ }
+ return names
+}
+
// mcpResolve is the ID of the level a form names: a path in the tree, a
// person's e-mail, or a key's ID; and how to say it.
func mcpResolve(ctx context.Context, s MCPAdmin, l *Lang, scope store.Scope, which string) (string, string, error) {
@@ -369,21 +408,6 @@ func mcpCheckTools(l *Lang, server string, tools []string) error {
return nil
}
-// mcpSetTools makes a level's grants of a server's tools these, leaving its
-// other grants and its other limits as they are; none revokes them.
-func mcpSetTools(ctx context.Context, s MCPAdmin, server string, scope store.Scope, id string, tools []string) error {
- lim, err := s.LimitsFor(ctx, scope, id)
- switch {
- case errors.Is(err, store.ErrNotFound):
- lim = store.Limits{Scope: scope, ScopeID: id}
- case err != nil:
- return err
- }
- keep := slices.DeleteFunc(slices.Clone(lim.AllowedTools), func(t string) bool { return strings.HasPrefix(t, server+".") })
- lim.AllowedTools = append(keep, tools...)
- return s.PutLimits(ctx, lim)
-}
-
// mcpSecrets turns the form's credential into what the server is sent.
func mcpSecrets(l *Lang, kind, header, value string) (map[string]string, error) {
header, value = strings.TrimSpace(header), strings.TrimSpace(value)
diff --git a/internal/web/admin_mcp_test.go b/internal/web/admin_mcp_test.go
index bf024af..0989b91 100644
--- a/internal/web/admin_mcp_test.go
+++ b/internal/web/admin_mcp_test.go
@@ -165,3 +165,21 @@ func TestTheMCPPageSpeaksTurkishWhenAsked(t *testing.T) {
}
}
}
+
+func TestAGrantBelowALevelThatNamesOtherToolsSaysSoAndKeepsWhatItHad(t *testing.T) {
+ mux, st, _ := mcpPage(t, mcpAdmin)
+ ctx := context.Background()
+ org, _ := st.CreateOrg(ctx, "Acme")
+ team, _ := st.CreateTeam(ctx, org, "eng")
+ _ = st.PutLimits(ctx, store.Limits{Scope: store.ScopeOrg, ScopeID: org, AllowedTools: []string{"hr.*"}})
+ _ = st.PutMCPServer(ctx, store.MCPServer{Name: "jira", URL: "https://jira/mcp", Enabled: true})
+
+ body := mirrorPost(t, mux, "/admin/mcp/jira/grant", url.Values{"csrf": {"tok"}, "scope": {"team"}, "which": {"Acme/eng"},
+ "tools": {"jira.*"}}).Body.String()
+ if !strings.Contains(body, "But organization Acme names tools and not these") {
+ t.Errorf("the grant should say the organization keeps jira out of reach: %s", body)
+ }
+ if l, _ := st.LimitsFor(ctx, store.ScopeTeam, team); !slices.Equal(l.AllowedTools, []string{"hr.*", "jira.*"}) {
+ t.Errorf("the team's list is %v; it should keep hr's, which it saw from its organization", l.AllowedTools)
+ }
+}
diff --git a/internal/web/locales/en/mcp.json b/internal/web/locales/en/mcp.json
index 1d7a754..d8dae0d 100644
--- a/internal/web/locales/en/mcp.json
+++ b/internal/web/locales/en/mcp.json
@@ -30,7 +30,7 @@
"mcp.which_hint": "ORG/TEAM, ORG/TEAM/PROJECT, ORG, an e-mail or a key's ID",
"mcp.tools": "Tools",
"mcp.grant_button": "Grant",
- "mcp.confirm_revoke": "Revoke %s from %s? It and everything below it then no longer see those tools, unless another level grants them.",
+ "mcp.confirm_revoke": "Revoke %s from %s? It then sees them only if the levels above grant them and it names no tools of its own.",
"mcp.revoke": "Revoke from %s",
"mcp.change": "Change",
"mcp.address": "Address",
@@ -78,5 +78,11 @@
"mcp.err.header": "Which header carries the credential?",
"mcp.err.auth": "The credential is a bearer token, another header or none.",
"mcp.err.timeout": "The timeout is whole seconds.",
- "mcp.err.rate": "The rate is calls a minute."
+ "mcp.err.rate": "The rate is calls a minute.",
+ "mcp.scope.org-member": "each member of the organization",
+ "mcp.scope.team-member": "each member of the team",
+ "mcp.scope.project-member": "each member of the project",
+ "mcp.blocked_above": "But %s names tools and not these: %s's tools stay out of reach here until it grants them too.",
+ "mcp.closed": "Closed to every tool, whatever is granted above:",
+ "mcp.how_grants": "A tool is seen where it is granted and every level above that names tools names it too. A level given its first grant keeps what it saw from above; taking a server's tools away leaves the rest."
}
diff --git a/internal/web/locales/tr/mcp.json b/internal/web/locales/tr/mcp.json
index f9b3b89..cb491e5 100644
--- a/internal/web/locales/tr/mcp.json
+++ b/internal/web/locales/tr/mcp.json
@@ -30,7 +30,7 @@
"mcp.which_hint": "ORG/TAKIM, ORG/TAKIM/PROJE, ORG, bir e-posta ya da anahtarın kimliği",
"mcp.tools": "Araçlar",
"mcp.grant_button": "Yetki ver",
- "mcp.confirm_revoke": "%s yetkisi %s üzerinden geri alınsın mı? Başka bir seviye vermedikçe orası ve altındakiler bu araçları artık görmez.",
+ "mcp.confirm_revoke": "%s yetkisi %s üzerinden geri alınsın mı? Bundan sonra onları yalnızca üst seviyeler veriyorsa ve kendi araç listesi yoksa görür.",
"mcp.revoke": "%s üzerinden geri al",
"mcp.change": "Değiştir",
"mcp.address": "Adres",
@@ -77,5 +77,11 @@
"mcp.err.header": "Kimlik bilgisini hangi başlık taşıyor?",
"mcp.err.auth": "Kimlik bilgisi bir bearer token, başka bir başlık ya da hiçbiri olabilir.",
"mcp.err.timeout": "Zaman aşımı tam saniye olmalı.",
- "mcp.err.rate": "Hız dakikadaki çağrı sayısıdır."
+ "mcp.err.rate": "Hız dakikadaki çağrı sayısıdır.",
+ "mcp.scope.org-member": "organizasyonun her üyesi",
+ "mcp.scope.team-member": "takımın her üyesi",
+ "mcp.scope.project-member": "projenin her üyesi",
+ "mcp.blocked_above": "Ama %s araç listesi tanımlıyor ve bunlar onda yok: o da yetki verene kadar %s araçları burada erişilemez.",
+ "mcp.closed": "Yukarıda ne verilirse verilsin tüm araçlara kapalı:",
+ "mcp.how_grants": "Bir araç, yetki verildiği yerde ve araç listesi tanımlayan her üst seviye de onu tanımlıyorsa görünür. İlk kez yetki verilen bir seviye yukarıdan gördüklerini korur; bir sunucunun yetkisini almak diğerlerini bırakır."
}
diff --git a/internal/web/templates/admin_mcp.html b/internal/web/templates/admin_mcp.html
index 0028194..3d19c29 100644
--- a/internal/web/templates/admin_mcp.html
+++ b/internal/web/templates/admin_mcp.html
@@ -51,11 +51,11 @@
{{$name := .Name}}
- {{range .Grants}}
+ {{range .Grants}}{{if .Revocable}}
- {{end}}
+ {{end}}{{end}}
{{if not .Local}}