Skip to content

Commit 5800e0e

Browse files
authored
feat(mcp): add account capability policies (#914)
1 parent f69b2f8 commit 5800e0e

10 files changed

Lines changed: 479 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## 0.34.1 - Unreleased
44

5+
- MCP: add optional global and per-account capability ceilings in `config.json`, with narrow persistent write authorization, runtime-only restriction, and fail-closed selector validation. (#913) — thanks @mcaldas.
6+
57
## 0.34.0 - 2026-07-11
68

79
- Calendar: add `--timezone` / `--tz` display overrides to `calendar events` and `calendar event`, including uniform multi-calendar output without changing range parsing. (#908) — thanks @bxxd.

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,8 @@ gog --account [email protected] \
625625
```
626626

627627
See [docs/mcp.md](docs/mcp.md) for client config, tool selection, safety
628-
behavior, mcporter examples, and troubleshooting.
628+
behavior, persistent global/per-account capability ceilings, mcporter examples,
629+
and troubleshooting.
629630

630631
## Auth and Accounts
631632

docs/mcp.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ gog --account [email protected] mcp \
4646
`--allow-write` is always required for write tools. A write tool that matches
4747
`--allow-tool` is still hidden until `--allow-write` is present.
4848

49+
The exception is an explicit persistent MCP policy. It can authorize a narrow
50+
write surface without repeating `--allow-write` in every client definition;
51+
runtime flags can only reduce that configured surface.
52+
4953
## Why this is not `gog_exec`
5054

5155
MCP clients are often LLM-driven. A generic "run this command" tool would expose
@@ -105,6 +109,48 @@ gog mcp --allow-tool calendar,sheets
105109
gog mcp --allow-write --allow-tool write
106110
```
107111

112+
## Persistent capability policy
113+
114+
For several MCP clients or accounts, put the maximum registered tool surface in
115+
`config.json` instead of duplicating capability arguments. Without an `mcp`
116+
block, behavior is unchanged: all read tools are available, writes require
117+
`--allow-write`, and `--allow-tool` filters the result.
118+
119+
```json5
120+
{
121+
"mcp": {
122+
"allow_tools": ["read"],
123+
"allow_write": false,
124+
"accounts": {
125+
126+
"allow_tools": ["read", "docs.*", "calendar.*"],
127+
"allow_write": true
128+
},
129+
130+
"allow_tools": ["read"],
131+
"allow_write": false
132+
}
133+
}
134+
}
135+
}
136+
```
137+
138+
An account entry is a complete replacement for the global policy, not a partial
139+
merge. Account keys are matched case-insensitively after aliases and automatic
140+
account selection are resolved, then that resolved account is pinned for every
141+
MCP child command. Per-account policies require stored account credentials;
142+
direct access tokens and ADC can use only the global policy because an account
143+
label does not prove the authenticated principal. An omitted `allow_tools` value defaults to
144+
`["read"]`; an explicitly empty list is rejected. `allow_write: true` requires
145+
an explicit tool list so a typo cannot accidentally expose every write tool.
146+
147+
The configured policy is a ceiling. `--allow-tool` can intersect it with a
148+
smaller runtime set, `--readonly` removes all writes, and `--allow-write` cannot
149+
widen a read-only policy. Baked safety profiles remain the outer immutable
150+
ceiling. Unknown configured selectors and attempted write widening fail before
151+
the MCP server starts. Use `gog mcp --list-tools` with the same account and flags
152+
to inspect the final registered surface.
153+
108154
## Initial tools
109155

110156
Read tools:

internal/cmd/mcp.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,13 @@ func (c *McpCmd) Run(ctx context.Context, flags *RootFlags) error {
6262
return usage("--max-output-bytes must be greater than zero")
6363
}
6464

65-
tools := mcpEnabledTools(*c)
65+
tools, resolvedAccount, err := mcpEnabledToolsForRun(ctx, *c, flags)
66+
if err != nil {
67+
return err
68+
}
69+
if resolvedAccount != "" && flags != nil {
70+
flags.Account = resolvedAccount
71+
}
6672
if len(tools) == 0 {
6773
return usage("no MCP tools enabled")
6874
}

internal/cmd/mcp_policy.go

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package cmd
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"strings"
7+
8+
"github.com/steipete/gogcli/internal/config"
9+
)
10+
11+
func mcpEnabledToolsForRun(ctx context.Context, cmd McpCmd, flags *RootFlags) ([]mcpToolSpec, string, error) {
12+
store, err := commandConfigStore(ctx)
13+
if err != nil {
14+
return nil, "", err
15+
}
16+
cfg, err := store.Read()
17+
if err != nil {
18+
return nil, "", err
19+
}
20+
if cfg.MCP == nil {
21+
return mcpEnabledTools(cmd), "", nil
22+
}
23+
24+
account := ""
25+
if len(cfg.MCP.Accounts) > 0 {
26+
account, err = resolveMCPPolicyAccount(flags)
27+
if err != nil {
28+
return nil, "", fmt.Errorf("resolve account for MCP policy: %w", err)
29+
}
30+
}
31+
policy, err := selectMCPPolicy(*cfg.MCP, account)
32+
if err != nil {
33+
return nil, "", err
34+
}
35+
tools, err := mcpEnabledToolsWithPolicy(cmd, flags, policy)
36+
return tools, account, err
37+
}
38+
39+
func resolveMCPPolicyAccount(flags *RootFlags) (string, error) {
40+
if hasDirectAccessToken(flags) || isADCAuthMode(flags) {
41+
// Account values are only labels in these modes, so they must never select
42+
// a per-account authorization policy. The global policy still applies.
43+
return "", nil
44+
}
45+
return requireAccount(flags)
46+
}
47+
48+
func selectMCPPolicy(cfg config.MCPConfig, account string) (config.MCPPolicy, error) {
49+
policy, err := normalizeMCPPolicy(cfg.MCPPolicy)
50+
if err != nil {
51+
return config.MCPPolicy{}, fmt.Errorf("global MCP policy: %w", err)
52+
}
53+
normalizedAccount := normalizeMCPAccount(account)
54+
seen := make(map[string]string, len(cfg.Accounts))
55+
for configuredAccount, accountPolicy := range cfg.Accounts {
56+
normalized := normalizeMCPAccount(configuredAccount)
57+
if normalized == "" {
58+
return config.MCPPolicy{}, usage("MCP policy account must not be empty")
59+
}
60+
if previous, ok := seen[normalized]; ok {
61+
return config.MCPPolicy{}, usagef("duplicate MCP policy accounts %q and %q", previous, configuredAccount)
62+
}
63+
seen[normalized] = configuredAccount
64+
normalizedPolicy, err := normalizeMCPPolicy(accountPolicy)
65+
if err != nil {
66+
return config.MCPPolicy{}, fmt.Errorf("MCP policy account %q: %w", configuredAccount, err)
67+
}
68+
if normalized == normalizedAccount {
69+
// Account entries are complete policies, not inherited patches.
70+
policy = normalizedPolicy
71+
}
72+
}
73+
return policy, nil
74+
}
75+
76+
func normalizeMCPPolicy(policy config.MCPPolicy) (config.MCPPolicy, error) {
77+
explicitSelectors := splitCommaValues(policy.AllowTools)
78+
selectorsProvided := policy.AllowTools != nil
79+
if selectorsProvided && len(explicitSelectors) == 0 {
80+
return config.MCPPolicy{}, usage("MCP policy allow_tools must contain at least one selector")
81+
}
82+
if policy.AllowWrite && !selectorsProvided {
83+
return config.MCPPolicy{}, usage("MCP policy allow_write requires an explicit allow_tools list")
84+
}
85+
if !selectorsProvided {
86+
explicitSelectors = []string{string(mcpRiskRead)}
87+
}
88+
for _, selector := range explicitSelectors {
89+
if !mcpSelectorMatchesAnyTool(selector) {
90+
return config.MCPPolicy{}, usagef("MCP policy allow_tools selector %q matches no tool", selector)
91+
}
92+
}
93+
policy.AllowTools = explicitSelectors
94+
return policy, nil
95+
}
96+
97+
func mcpEnabledToolsWithPolicy(cmd McpCmd, flags *RootFlags, policy config.MCPPolicy) ([]mcpToolSpec, error) {
98+
if cmd.AllowWrite && !policy.AllowWrite {
99+
return nil, usage("--allow-write cannot widen the configured MCP policy")
100+
}
101+
102+
allowWrite := policy.AllowWrite
103+
if flags != nil && flags.ReadOnly {
104+
allowWrite = false
105+
}
106+
runtimeAllow := splitCommaValues(cmd.AllowTool)
107+
tools := make([]mcpToolSpec, 0, len(mcpAllTools()))
108+
for _, tool := range mcpAllTools() {
109+
if tool.Risk == mcpRiskWrite && !allowWrite {
110+
continue
111+
}
112+
if !mcpToolAllowed(tool, policy.AllowTools) {
113+
continue
114+
}
115+
if len(runtimeAllow) > 0 && !mcpToolAllowed(tool, runtimeAllow) {
116+
continue
117+
}
118+
tools = append(tools, tool)
119+
}
120+
return tools, nil
121+
}
122+
123+
func mcpSelectorMatchesAnyTool(selector string) bool {
124+
for _, tool := range mcpAllTools() {
125+
if mcpToolAllowed(tool, []string{selector}) {
126+
return true
127+
}
128+
}
129+
return false
130+
}
131+
132+
func normalizeMCPAccount(account string) string {
133+
return strings.ToLower(strings.TrimSpace(account))
134+
}

internal/cmd/mcp_test.go

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import (
99

1010
mcpclient "github.com/mark3labs/mcp-go/client"
1111
"github.com/mark3labs/mcp-go/mcp"
12+
13+
"github.com/steipete/gogcli/internal/config"
14+
"github.com/steipete/gogcli/internal/googleapi"
1215
)
1316

1417
func TestMCPEnabledToolsDefaultReadOnly(t *testing.T) {
@@ -39,6 +42,137 @@ func TestMCPEnabledToolsAllowWriteAndFilter(t *testing.T) {
3942
}
4043
}
4144

45+
func TestMCPPolicyDefaultsToReadOnly(t *testing.T) {
46+
policy, err := selectMCPPolicy(config.MCPConfig{}, "")
47+
if err != nil {
48+
t.Fatalf("selectMCPPolicy: %v", err)
49+
}
50+
tools, err := mcpEnabledToolsWithPolicy(McpCmd{}, &RootFlags{}, policy)
51+
if err != nil {
52+
t.Fatalf("mcpEnabledToolsWithPolicy: %v", err)
53+
}
54+
if !hasMCPTool(tools, "gmail_search") || hasMCPTool(tools, "docs_write") {
55+
t.Fatalf("unexpected default policy tools: %#v", toolNames(tools))
56+
}
57+
}
58+
59+
func TestMCPPolicyAccountReplacesGlobalAndEnablesNarrowWrites(t *testing.T) {
60+
cfg := config.MCPConfig{
61+
MCPPolicy: config.MCPPolicy{AllowTools: []string{"read"}},
62+
Accounts: map[string]config.MCPPolicy{
63+
" [email protected] ": {AllowTools: []string{"docs.*"}, AllowWrite: true},
64+
},
65+
}
66+
policy, err := selectMCPPolicy(cfg, "[email protected]")
67+
if err != nil {
68+
t.Fatalf("selectMCPPolicy: %v", err)
69+
}
70+
tools, err := mcpEnabledToolsWithPolicy(McpCmd{}, &RootFlags{}, policy)
71+
if err != nil {
72+
t.Fatalf("mcpEnabledToolsWithPolicy: %v", err)
73+
}
74+
if !hasMCPTool(tools, "docs_get") || !hasMCPTool(tools, "docs_write") {
75+
t.Fatalf("expected configured Docs tools: %#v", toolNames(tools))
76+
}
77+
if hasMCPTool(tools, "gmail_search") {
78+
t.Fatalf("global policy leaked into account replacement: %#v", toolNames(tools))
79+
}
80+
}
81+
82+
func TestMCPPolicyRuntimeCanOnlyNarrow(t *testing.T) {
83+
policy, err := normalizeMCPPolicy(config.MCPPolicy{AllowTools: []string{"docs.*"}, AllowWrite: true})
84+
if err != nil {
85+
t.Fatalf("normalizeMCPPolicy: %v", err)
86+
}
87+
tools, err := mcpEnabledToolsWithPolicy(McpCmd{AllowTool: []string{"docs_get"}}, &RootFlags{}, policy)
88+
if err != nil {
89+
t.Fatalf("mcpEnabledToolsWithPolicy: %v", err)
90+
}
91+
if got := toolNames(tools); len(got) != 1 || got[0] != "docs_get" {
92+
t.Fatalf("runtime narrowed tools = %#v", got)
93+
}
94+
95+
_, err = mcpEnabledToolsWithPolicy(McpCmd{AllowWrite: true}, &RootFlags{}, config.MCPPolicy{AllowTools: []string{"read"}})
96+
if err == nil || !strings.Contains(err.Error(), "cannot widen") {
97+
t.Fatalf("allow-write widening error = %v", err)
98+
}
99+
}
100+
101+
func TestMCPPolicyReadOnlyRootHidesConfiguredWrites(t *testing.T) {
102+
policy, err := normalizeMCPPolicy(config.MCPPolicy{AllowTools: []string{"docs.*"}, AllowWrite: true})
103+
if err != nil {
104+
t.Fatalf("normalizeMCPPolicy: %v", err)
105+
}
106+
tools, err := mcpEnabledToolsWithPolicy(McpCmd{}, &RootFlags{ReadOnly: true}, policy)
107+
if err != nil {
108+
t.Fatalf("mcpEnabledToolsWithPolicy: %v", err)
109+
}
110+
if !hasMCPTool(tools, "docs_get") || hasMCPTool(tools, "docs_write") {
111+
t.Fatalf("readonly tools = %#v", toolNames(tools))
112+
}
113+
}
114+
115+
func TestMCPPolicyRejectsUnsafeOrUnknownConfig(t *testing.T) {
116+
for _, policy := range []config.MCPPolicy{
117+
{AllowWrite: true},
118+
{AllowTools: []string{}},
119+
{AllowTools: []string{"not_a_tool"}},
120+
} {
121+
if _, err := normalizeMCPPolicy(policy); err == nil {
122+
t.Fatalf("expected policy error for %#v", policy)
123+
}
124+
}
125+
126+
duplicateAccount := " [email protected] "
127+
_, err := selectMCPPolicy(config.MCPConfig{Accounts: map[string]config.MCPPolicy{
128+
129+
duplicateAccount: {},
130+
131+
if err == nil || !strings.Contains(err.Error(), "duplicate") {
132+
t.Fatalf("duplicate account error = %v", err)
133+
}
134+
135+
_, err = selectMCPPolicy(config.MCPConfig{
136+
Accounts: map[string]config.MCPPolicy{
137+
"[email protected]": {AllowTools: []string{"read"}},
138+
"[email protected]": {AllowTools: []string{"not_a_tool"}},
139+
},
140+
141+
if err == nil || !strings.Contains(err.Error(), "[email protected]") || !strings.Contains(err.Error(), "matches no tool") {
142+
t.Fatalf("unselected account validation error = %v", err)
143+
}
144+
}
145+
146+
func TestMCPPolicyAccountResolutionPinsAliasAndRejectsUnverifiableIdentity(t *testing.T) {
147+
store := config.NewConfigStore(config.Layout{ConfigDir: t.TempDir()})
148+
if err := store.Write(config.File{AccountAliases: map[string]string{"personal": "[email protected]"}}); err != nil {
149+
t.Fatalf("write config: %v", err)
150+
}
151+
flags := &RootFlags{
152+
Account: "personal",
153+
configStoreResolver: func() (*config.ConfigStore, error) {
154+
return store, nil
155+
},
156+
}
157+
account, err := resolveMCPPolicyAccount(flags)
158+
if err != nil {
159+
t.Fatalf("resolveMCPPolicyAccount: %v", err)
160+
}
161+
if account != "[email protected]" {
162+
t.Fatalf("resolved account = %q", account)
163+
}
164+
165+
for _, unverifiable := range []*RootFlags{
166+
{AccessToken: "token", Account: "[email protected]"},
167+
{authMode: googleapi.AuthModeADC, Account: "[email protected]"},
168+
} {
169+
account, err := resolveMCPPolicyAccount(unverifiable)
170+
if err != nil || account != "" {
171+
t.Fatalf("unverifiable identity resolution = %q, %v", account, err)
172+
}
173+
}
174+
}
175+
42176
func TestMCPListToolsUsesRuntimeStdout(t *testing.T) {
43177
var output bytes.Buffer
44178
err := (&McpCmd{

0 commit comments

Comments
 (0)