Skip to content

Commit 78b603e

Browse files
authored
Merge pull request #427 from docker/bugfix-preserve-empty-tools
fix: Preserve empty tools after applying policy
2 parents c577d49 + 844a0a9 commit 78b603e

File tree

2 files changed

+39
-0
lines changed

2 files changed

+39
-0
lines changed

pkg/gateway/configuration.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,10 @@ func (c *Configuration) FilterByPolicy(ctx context.Context, pc policy.Client) er
232232

233233
// Filter tools for this server if any.
234234
if tools, ok := c.tools.ServerTools[name]; ok {
235+
// Preserve the entry even when empty (explicit "disable all tools" signal).
236+
if _, exists := filteredTools.ServerTools[name]; !exists {
237+
filteredTools.ServerTools[name] = []string{}
238+
}
235239
for _, t := range tools {
236240
if allowedTools[name][t] {
237241
filteredTools.ServerTools[name] = append(filteredTools.ServerTools[name], t)

pkg/gateway/policy_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,41 @@ func TestFilterByPolicy(t *testing.T) {
140140
"blocked tool should be removed, allowed tool should remain")
141141
})
142142

143+
t.Run("preserves_empty_tools_list_disable_all", func(t *testing.T) {
144+
// Setup: server with empty tools list (all tools disabled via --disable-all).
145+
// FilterByPolicy must preserve the empty entry so that isToolEnabled
146+
// sees exists=true and blocks every tool.
147+
mock := newMockPolicyClient()
148+
149+
cfg := &Configuration{
150+
serverNames: []string{"test-server"},
151+
servers: map[string]catalog.Server{
152+
"test-server": {Image: "test-image"},
153+
},
154+
config: make(map[string]map[string]any),
155+
tools: config.ToolsConfig{
156+
ServerTools: map[string][]string{
157+
"test-server": {}, // empty = disable all tools
158+
},
159+
},
160+
}
161+
162+
// Execute
163+
err := cfg.FilterByPolicy(context.Background(), mock)
164+
165+
// Assert
166+
require.NoError(t, err)
167+
assert.Equal(t, []string{"test-server"}, cfg.serverNames, "server should remain")
168+
require.Contains(t, cfg.tools.ServerTools, "test-server",
169+
"empty tools entry must be preserved after policy filtering")
170+
assert.Empty(t, cfg.tools.ServerTools["test-server"],
171+
"tools list should remain empty (disable all)")
172+
173+
// Verify isToolEnabled respects the preserved empty list
174+
assert.False(t, isToolEnabled(*cfg, "test-server", "test-image", "any-tool", nil),
175+
"no tool should be enabled when tools list is empty")
176+
})
177+
143178
t.Run("nil_policy_client_allows_all", func(t *testing.T) {
144179
// Setup: nil policy client should be a no-op
145180
cfg := &Configuration{

0 commit comments

Comments
 (0)