Skip to content

Commit 24416ef

Browse files
authored
Support wildcard ["*"] in allowed-tools filtering (#3445)
`buildAllowedToolSets()` treats every entry in the `Tools` list as a literal tool name. When the gh-aw compiler emits `tools: ["*"]`, no tool matches the literal string `"*"`, so all tools get filtered out. ### Changes - **`internal/server/unified.go`**: `buildAllowedToolSets` now detects `"*"` anywhere in the tools list via a `hasWildcard()` helper. Wildcard servers are skipped (not added to the filter map), same as servers with no `Tools` config. Logs `[allowed-tools] Wildcard "*" configured for <serverID>: allowing all tools` when triggered. - **Tests**: Wildcard cases added to `TestIsToolAllowed` table, plus new `TestBuildAllowedToolSets_WildcardStar`, `TestBuildAllowedToolSets_WildcardMixed`, `TestIsToolAllowed_Wildcard`, and `TestRegisterToolsFromBackend_WildcardAllowsAll`. ```go // Before: ["*"] builds a set with literal "*" key — nothing matches // After: if hasWildcard(serverCfg.Tools) { logger.LogInfo("backend", "[allowed-tools] Wildcard \"*\" configured for %s: allowing all tools", serverID) continue } ``` > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `example.com` > - Triggering command: `/tmp/go-build1981761529/b514/launcher.test /tmp/go-build1981761529/b514/launcher.test -test.testlogfile=/tmp/go-build1981761529/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s 9420�� 9420830/b336/_pkg_.a pkg/mod/github.com/tetratelabs/wazero@v1.11.0/in-ifaceassert x_amd64/vet --gdwarf-5 .io/otel/sdk/res-V=full -o x_amd64/vet` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build1981761529/b496/config.test /tmp/go-build1981761529/b496/config.test -test.testlogfile=/tmp/go-build1981761529/b496/testlog.txt -test.paniconexit0 -test.timeout=10m0s -w PXs3z5Xin .cfg 64/pkg/tool/linux_amd64/vet -c .io/otel/attribu/tmp/go-build2334098856/b305/vet.cfg /tmp/go-build3709420830/b148/ 64/pkg/tool/linux_amd64/vet ache�� olang.org/grpc@v1.80.0/channelz/channelz.go .cfg 64/pkg/tool/linux_amd64/vet --gdwarf-5 --64 -o 64/pkg/tool/linux_amd64/vet` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build1981761529/b514/launcher.test /tmp/go-build1981761529/b514/launcher.test -test.testlogfile=/tmp/go-build1981761529/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s 9420�� 9420830/b336/_pkg_.a pkg/mod/github.com/tetratelabs/wazero@v1.11.0/in-ifaceassert x_amd64/vet --gdwarf-5 .io/otel/sdk/res-V=full -o x_amd64/vet` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build1981761529/b514/launcher.test /tmp/go-build1981761529/b514/launcher.test -test.testlogfile=/tmp/go-build1981761529/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s 9420�� 9420830/b336/_pkg_.a pkg/mod/github.com/tetratelabs/wazero@v1.11.0/in-ifaceassert x_amd64/vet --gdwarf-5 .io/otel/sdk/res-V=full -o x_amd64/vet` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build1981761529/b523/mcp.test /tmp/go-build1981761529/b523/mcp.test -test.testlogfile=/tmp/go-build1981761529/b523/testlog.txt -test.paniconexit0 -test.timeout=10m0s` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent) (admins only) > > </details>
2 parents a8567c2 + 4e0dc14 commit 24416ef

File tree

3 files changed

+101
-1
lines changed

3 files changed

+101
-1
lines changed

internal/server/allowed_tools_integration_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,88 @@ func TestIsToolAllowed_Integration(t *testing.T) {
463463
assert.True(t, us.isToolAllowed("unknown", "tool"))
464464
}
465465

466+
// TestBuildAllowedToolSets_WildcardStar verifies that Tools: ["*"] results in no
467+
// entry in the sets map, meaning all tools are allowed (same as an empty list).
468+
func TestBuildAllowedToolSets_WildcardStar(t *testing.T) {
469+
cfg := &config.Config{
470+
Servers: map[string]*config.ServerConfig{
471+
"wildcard": {Tools: []string{"*"}},
472+
"restricted": {Tools: []string{"a", "b"}},
473+
"open": {},
474+
},
475+
}
476+
sets := buildAllowedToolSets(cfg)
477+
478+
_, hasWildcardServer := sets["wildcard"]
479+
assert.False(t, hasWildcardServer, "server with wildcard must not be in the set map")
480+
481+
_, hasRestricted := sets["restricted"]
482+
assert.True(t, hasRestricted, "restricted server should still be in the set map")
483+
484+
_, hasOpen := sets["open"]
485+
assert.False(t, hasOpen, "open server must not be in the set map")
486+
}
487+
488+
// TestBuildAllowedToolSets_WildcardMixed verifies that a "*" anywhere in the
489+
// Tools list causes the server to be treated as unrestricted.
490+
func TestBuildAllowedToolSets_WildcardMixed(t *testing.T) {
491+
cfg := &config.Config{
492+
Servers: map[string]*config.ServerConfig{
493+
"mixed": {Tools: []string{"tool_a", "*", "tool_b"}},
494+
},
495+
}
496+
sets := buildAllowedToolSets(cfg)
497+
498+
_, hasMixed := sets["mixed"]
499+
assert.False(t, hasMixed, "server with wildcard in mixed list must not be in the set map")
500+
}
501+
502+
// TestIsToolAllowed_Wildcard verifies that isToolAllowed returns true for any
503+
// tool name when the server is configured with Tools: ["*"].
504+
func TestIsToolAllowed_Wildcard(t *testing.T) {
505+
cfg := &config.Config{
506+
Servers: map[string]*config.ServerConfig{
507+
"wildcard": {Tools: []string{"*"}},
508+
},
509+
}
510+
us := &UnifiedServer{allowedToolSets: buildAllowedToolSets(cfg)}
511+
512+
assert.True(t, us.isToolAllowed("wildcard", "any_tool"))
513+
assert.True(t, us.isToolAllowed("wildcard", "another_tool"))
514+
assert.True(t, us.isToolAllowed("wildcard", "delete_everything"))
515+
}
516+
517+
// TestRegisterToolsFromBackend_WildcardAllowsAll verifies that when a backend
518+
// is configured with Tools: ["*"], all tools from the backend are registered.
519+
func TestRegisterToolsFromBackend_WildcardAllowsAll(t *testing.T) {
520+
require := require.New(t)
521+
assert := assert.New(t)
522+
523+
backend := newMockMCPBackendWithTools(t, "elastic-docs", []string{"search_code", "get_file_contents", "delete_repo"})
524+
defer backend.Close()
525+
526+
cfg := &config.Config{
527+
Servers: map[string]*config.ServerConfig{
528+
"elastic-docs": {
529+
Type: "http",
530+
URL: backend.URL,
531+
Tools: []string{"*"}, // wildcard — all tools allowed
532+
},
533+
},
534+
}
535+
536+
us, err := NewUnified(context.Background(), cfg)
537+
require.NoError(err)
538+
defer us.Close()
539+
540+
us.toolsMu.RLock()
541+
defer us.toolsMu.RUnlock()
542+
543+
assert.Contains(us.tools, "elastic-docs___search_code", "search_code should be registered")
544+
assert.Contains(us.tools, "elastic-docs___get_file_contents", "get_file_contents should be registered")
545+
assert.Contains(us.tools, "elastic-docs___delete_repo", "delete_repo should be registered with wildcard")
546+
}
547+
466548
// ----- helpers -----------------------------------------------------------
467549

468550
// toolNameSet converts a []ToolInfo slice into a name -> bool map for easy lookup.

internal/server/call_backend_tool_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,8 @@ func TestIsToolAllowed(t *testing.T) {
399399
{"empty list allows anything", []string{}, "any_tool", true},
400400
{"tool in list", []string{"a", "b"}, "a", true},
401401
{"tool not in list", []string{"a", "b"}, "c", false},
402+
{"wildcard allows anything", []string{"*"}, "any_tool", true},
403+
{"wildcard in mixed list allows anything", []string{"a", "*"}, "z", true},
402404
}
403405

404406
for _, tc := range tests {

internal/server/unified.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,14 +378,20 @@ func newErrorCallToolResult(err error) (*sdk.CallToolResult, interface{}, error)
378378

379379
// buildAllowedToolSets converts the per-server Tools lists from the config into pre-computed
380380
// map[string]bool sets for O(1) lookup. Servers with no Tools list are not added to the map,
381-
// which signals that all tools are permitted.
381+
// which signals that all tools are permitted. If the Tools list contains a "*" entry anywhere,
382+
// the server is treated the same as having no list (all tools allowed).
382383
func buildAllowedToolSets(cfg *config.Config) map[string]map[string]bool {
383384
sets := make(map[string]map[string]bool)
384385
if cfg == nil {
385386
return sets
386387
}
387388
for serverID, serverCfg := range cfg.Servers {
388389
if len(serverCfg.Tools) > 0 {
390+
// Treat "*" anywhere in the list as "allow all" — skip adding to the filter map
391+
if hasWildcard(serverCfg.Tools) {
392+
logger.LogInfo("backend", "[allowed-tools] Wildcard \"*\" configured for %s: allowing all tools", serverID)
393+
continue
394+
}
389395
set := make(map[string]bool, len(serverCfg.Tools))
390396
for _, t := range serverCfg.Tools {
391397
set[t] = true
@@ -396,6 +402,16 @@ func buildAllowedToolSets(cfg *config.Config) map[string]map[string]bool {
396402
return sets
397403
}
398404

405+
// hasWildcard reports whether the tools list contains a "*" entry.
406+
func hasWildcard(tools []string) bool {
407+
for _, t := range tools {
408+
if t == "*" {
409+
return true
410+
}
411+
}
412+
return false
413+
}
414+
399415
// isToolAllowed reports whether toolName is permitted by the server's configured
400416
// allowed-tools list. When no list is configured (empty), all tools are allowed.
401417
// Uses the pre-computed allowedToolSets map for O(1) lookup.

0 commit comments

Comments
 (0)