Skip to content

Commit c0dfed9

Browse files
refactor: standardize empty array conventions in virtual key
1 parent e8a2605 commit c0dfed9

File tree

27 files changed

+334
-168
lines changed

27 files changed

+334
-168
lines changed

core/internal/mcptests/agent_test_helpers.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,10 @@ func SetupAgentTest(t *testing.T, config AgentTestConfig) (*mcp.MCPManager, *Dyn
132132
// Create context with filtering
133133
baseCtx := context.Background()
134134
if len(config.ClientFiltering) > 0 {
135-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeClients, config.ClientFiltering)
135+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeClients, config.ClientFiltering)
136136
}
137137
if len(config.ToolFiltering) > 0 {
138-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeTools, config.ToolFiltering)
138+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeTools, config.ToolFiltering)
139139
}
140140
ctx := schemas.NewBifrostContext(baseCtx, schemas.NoDeadline)
141141

@@ -193,10 +193,10 @@ func SetupAgentTestWithClients(t *testing.T, config AgentTestConfig, customClien
193193
// Create context with filtering
194194
baseCtx := context.Background()
195195
if len(config.ClientFiltering) > 0 {
196-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeClients, config.ClientFiltering)
196+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeClients, config.ClientFiltering)
197197
}
198198
if len(config.ToolFiltering) > 0 {
199-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeTools, config.ToolFiltering)
199+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeTools, config.ToolFiltering)
200200
}
201201
ctx := schemas.NewBifrostContext(baseCtx, schemas.NoDeadline)
202202

core/internal/mcptests/codemode_stdio_test.go

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -56,27 +56,27 @@ func setupCodeModeWithSTDIOServers(t *testing.T, serverNames ...string) (*mcp.MC
5656
config = GetTemperatureMCPClientConfig(bifrostRoot)
5757
config.IsCodeModeClient = true
5858
config.ID = "temperature-client" // Match test expectations
59-
config.Name = "temperature" // Use lowercase to match test code
59+
config.Name = "temperature" // Use lowercase to match test code
6060
config.ToolsToAutoExecute = []string{"executeToolCode", "listToolFiles", "readToolFile"}
6161
case "go-test-server":
6262
config = GetGoTestServerConfig(bifrostRoot)
6363
config.ID = "goTestServer-client" // Match test expectations
64-
config.Name = "goTestServer" // Use camelCase to match test code
64+
config.Name = "goTestServer" // Use camelCase to match test code
6565
config.ToolsToAutoExecute = []string{"executeToolCode", "listToolFiles", "readToolFile"}
6666
case "edge-case-server":
6767
config = GetEdgeCaseServerConfig(bifrostRoot)
6868
config.ID = "edgeCaseServer-client" // Match test expectations
69-
config.Name = "edgeCaseServer" // Use camelCase to match test code
69+
config.Name = "edgeCaseServer" // Use camelCase to match test code
7070
config.ToolsToAutoExecute = []string{"executeToolCode", "listToolFiles", "readToolFile"}
7171
case "error-test-server":
7272
config = GetErrorTestServerConfig(bifrostRoot)
7373
config.ID = "errorTestServer-client" // Match test expectations
74-
config.Name = "errorTestServer" // Use camelCase to match test code
74+
config.Name = "errorTestServer" // Use camelCase to match test code
7575
config.ToolsToAutoExecute = []string{"executeToolCode", "listToolFiles", "readToolFile"}
7676
case "parallel-test-server":
7777
config = GetParallelTestServerConfig(bifrostRoot)
7878
config.ID = "parallelTestServer-client" // Match test expectations
79-
config.Name = "parallelTestServer" // Use camelCase to match test code
79+
config.Name = "parallelTestServer" // Use camelCase to match test code
8080
config.ToolsToAutoExecute = []string{"executeToolCode", "listToolFiles", "readToolFile"}
8181
case "test-tools-server":
8282
// test-tools-server doesn't have a fixture, set up manually
@@ -367,32 +367,32 @@ func TestCodeMode_STDIO_ServerFiltering(t *testing.T) {
367367
expectedError string
368368
}{
369369
{
370-
name: "allow_only_test_tools_server",
371-
includeClients: []string{"testToolsServer"},
372-
code: `result = testToolsServer.echo(message="allowed")`,
370+
name: "allow_only_test_tools_server",
371+
includeClients: []string{"testToolsServer"},
372+
code: `result = testToolsServer.echo(message="allowed")`,
373373
shouldSucceed: true,
374374
expectedInResult: "allowed",
375375
},
376376
{
377377
name: "block_test_tools_server",
378378
includeClients: []string{"temperature"},
379379
code: `result = testToolsServer.echo(message="blocked")`,
380-
shouldSucceed: false,
381-
expectedError: "undefined: testToolsServer",
380+
shouldSucceed: false,
381+
expectedError: "undefined: testToolsServer",
382382
},
383383
{
384-
name: "allow_only_temperature_server",
385-
includeClients: []string{"temperature"},
386-
code: `result = temperature.get_temperature(location="Paris")`,
384+
name: "allow_only_temperature_server",
385+
includeClients: []string{"temperature"},
386+
code: `result = temperature.get_temperature(location="Paris")`,
387387
shouldSucceed: true,
388388
expectedInResult: "Paris",
389389
},
390390
{
391391
name: "block_temperature_server",
392392
includeClients: []string{"testToolsServer"},
393393
code: `result = temperature.get_temperature(location="blocked")`,
394-
shouldSucceed: false,
395-
expectedError: "undefined: temperature",
394+
shouldSucceed: false,
395+
expectedError: "undefined: temperature",
396396
},
397397
{
398398
name: "allow_both_servers",
@@ -409,7 +409,7 @@ result = {"echo": echo, "temp": temp}`,
409409
t.Run(tc.name, func(t *testing.T) {
410410
// Create context with client filtering
411411
baseCtx := context.Background()
412-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeClients, tc.includeClients)
412+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeClients, tc.includeClients)
413413
ctx := schemas.NewBifrostContext(baseCtx, schemas.NoDeadline)
414414

415415
// Verify filtering is applied at tool listing level
@@ -524,7 +524,7 @@ result = {"echo": echo, "calc": calc}`,
524524
t.Run(tc.name, func(t *testing.T) {
525525
// Create context with tool filtering
526526
baseCtx := context.Background()
527-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeTools, tc.includeTools)
527+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeTools, tc.includeTools)
528528
ctx := schemas.NewBifrostContext(baseCtx, schemas.NoDeadline)
529529

530530
// Verify filtering is applied
@@ -622,10 +622,10 @@ result = {"echo": echo, "temp": temp}`,
622622
// Create context with both client and tool filtering
623623
baseCtx := context.Background()
624624
if tc.includeClients != nil {
625-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeClients, tc.includeClients)
625+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeClients, tc.includeClients)
626626
}
627627
if tc.includeTools != nil {
628-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeTools, tc.includeTools)
628+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeTools, tc.includeTools)
629629
}
630630
ctx := schemas.NewBifrostContext(baseCtx, schemas.NoDeadline)
631631

@@ -1692,7 +1692,7 @@ result = {"count": 3}`,
16921692
for _, tc := range tests {
16931693
t.Run(tc.name, func(t *testing.T) {
16941694
baseCtx := context.Background()
1695-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeClients, tc.includeClients)
1695+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeClients, tc.includeClients)
16961696
ctx := schemas.NewBifrostContext(baseCtx, schemas.NoDeadline)
16971697

16981698
toolCall := CreateExecuteToolCodeCall(fmt.Sprintf("call-%s", tc.name), tc.code)

core/internal/mcptests/concurrency_advanced_test.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"testing"
1111
"time"
1212

13-
"github.com/maximhq/bifrost/core/mcp"
1413
"github.com/maximhq/bifrost/core/schemas"
1514
"github.com/stretchr/testify/assert"
1615
"github.com/stretchr/testify/require"
@@ -533,14 +532,14 @@ func TestConcurrent_FilteringChanges(t *testing.T) {
533532
if id%2 == 0 {
534533
// Even: allow all tools
535534
baseCtx := context.Background()
536-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeClients, []string{"*"})
537-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeTools, []string{"bifrostInternal-*"})
535+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeClients, []string{"*"})
536+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeTools, []string{"bifrostInternal-*"})
538537
ctx = schemas.NewBifrostContext(baseCtx, schemas.NoDeadline)
539538
} else {
540539
// Odd: allow only echo
541540
baseCtx := context.Background()
542-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeClients, []string{"*"})
543-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeTools, []string{"bifrostInternal-echo"})
541+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeClients, []string{"*"})
542+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeTools, []string{"bifrostInternal-echo"})
544543
ctx = schemas.NewBifrostContext(baseCtx, schemas.NoDeadline)
545544
}
546545

core/internal/mcptests/fixtures.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,10 +1984,10 @@ func AssertExecutionTimeUnder(t *testing.T, fn func(), maxDuration time.Duration
19841984
func CreateTestContextWithMCPFilter(includeClients []string, includeTools []string) *schemas.BifrostContext {
19851985
baseCtx := context.Background()
19861986
if includeClients != nil {
1987-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeClients, includeClients)
1987+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeClients, includeClients)
19881988
}
19891989
if includeTools != nil {
1990-
baseCtx = context.WithValue(baseCtx, mcp.MCPContextKeyIncludeTools, includeTools)
1990+
baseCtx = context.WithValue(baseCtx, schemas.MCPContextKeyIncludeTools, includeTools)
19911991
}
19921992
return schemas.NewBifrostContext(baseCtx, schemas.NoDeadline)
19931993
}

core/mcp/mcp.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,6 @@ const (
2121
BifrostMCPClientKey = "bifrostInternal" // Key for internal Bifrost client in clientMap
2222
MCPLogPrefix = "[Bifrost MCP]" // Consistent logging prefix
2323
MCPClientConnectionEstablishTimeout = 30 * time.Second // Timeout for MCP client connection establishment
24-
25-
// Context keys for client filtering in requests
26-
// NOTE: []string is used for both keys, and by default all clients/tools are included (when nil).
27-
// If "*" is present, all clients/tools are included, and [] means no clients/tools are included.
28-
// Request context filtering takes priority over client config - context can override client exclusions.
29-
MCPContextKeyIncludeClients schemas.BifrostContextKey = "mcp-include-clients" // Context key for whitelist client filtering
30-
MCPContextKeyIncludeTools schemas.BifrostContextKey = "mcp-include-tools" // Context key for whitelist tool filtering (Note: toolName should be in "clientName-toolName" format for individual tools, or "clientName-*" for wildcard)
3124
)
3225

3326
// ============================================================================

core/mcp/utils.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (m *MCPManager) GetToolPerClient(ctx context.Context) map[string][]schemas.
6565
var includeClients []string
6666

6767
// Extract client filtering from request context
68-
if existingIncludeClients, ok := ctx.Value(MCPContextKeyIncludeClients).([]string); ok && existingIncludeClients != nil {
68+
if existingIncludeClients, ok := ctx.Value(schemas.MCPContextKeyIncludeClients).([]string); ok && existingIncludeClients != nil {
6969
includeClients = existingIncludeClients
7070
}
7171

@@ -439,7 +439,7 @@ func canAutoExecuteTool(toolName string, config *schemas.MCPClientConfig) bool {
439439
// Context filtering can only NARROW the tools available, NOT expand beyond client configuration.
440440
// This is checked AFTER client-level filtering (shouldSkipToolForConfig).
441441
func shouldSkipToolForRequest(ctx context.Context, clientName, toolName string) bool {
442-
includeTools := ctx.Value(MCPContextKeyIncludeTools)
442+
includeTools := ctx.Value(schemas.MCPContextKeyIncludeTools)
443443

444444
if includeTools != nil {
445445
// Try []string first (preferred type)

core/schemas/bifrost.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,18 @@ type BifrostContextKey string
152152

153153
// BifrostContextKeyRequestType is a context key for the request type.
154154
const (
155-
BifrostContextKeyVirtualKey BifrostContextKey = "x-bf-vk" // string
156-
BifrostContextKeyAPIKeyName BifrostContextKey = "x-bf-api-key" // string (explicit key name selection)
157-
BifrostContextKeyRequestID BifrostContextKey = "request-id" // string
158-
BifrostContextKeyFallbackRequestID BifrostContextKey = "fallback-request-id" // string
159-
BifrostContextKeyDirectKey BifrostContextKey = "bifrost-direct-key" // Key struct
155+
BifrostContextKeyVirtualKey BifrostContextKey = "x-bf-vk" // string
156+
BifrostContextKeyAPIKeyName BifrostContextKey = "x-bf-api-key" // string (explicit key name selection)
157+
BifrostContextKeyRequestID BifrostContextKey = "request-id" // string
158+
BifrostContextKeyFallbackRequestID BifrostContextKey = "fallback-request-id" // string
159+
BifrostContextKeyDirectKey BifrostContextKey = "bifrost-direct-key" // Key struct
160+
161+
// NOTE: []string is used for both keys, and by default all clients/tools are included (when nil).
162+
// If "*" is present, all clients/tools are included, and [] means no clients/tools are included.
163+
// Request context filtering takes priority over client config - context can override client exclusions.
164+
MCPContextKeyIncludeClients BifrostContextKey = "mcp-include-clients" // Context key for whitelist client filtering
165+
MCPContextKeyIncludeTools BifrostContextKey = "mcp-include-tools" // Context key for whitelist tool filtering (Note: toolName should be in "clientName-toolName" format for individual tools, or "clientName-*" for wildcard)
166+
160167
BifrostContextKeySelectedKeyID BifrostContextKey = "bifrost-selected-key-id" // string (to store the selected key ID (set by bifrost governance plugin - DO NOT SET THIS MANUALLY))
161168
BifrostContextKeySelectedKeyName BifrostContextKey = "bifrost-selected-key-name" // string (to store the selected key name (set by bifrost governance plugin - DO NOT SET THIS MANUALLY))
162169
BifrostContextKeyGovernanceVirtualKeyID BifrostContextKey = "bifrost-governance-virtual-key-id" // string (to store the virtual key ID (set by bifrost governance plugin - DO NOT SET THIS MANUALLY))
@@ -211,9 +218,9 @@ const (
211218
BifrostContextKeySCIMClaims BifrostContextKey = "scim_claims"
212219
BifrostContextKeyUserID BifrostContextKey = "user_id"
213220
BifrostContextKeyTargetUserID BifrostContextKey = "target_user_id"
214-
BifrostContextKeyIsAzureUserAgent BifrostContextKey = "bifrost-is-azure-user-agent" // bool (set by bifrost - DO NOT SET THIS MANUALLY)) - whether the request is an Azure user agent (only used in gateway)
221+
BifrostContextKeyIsAzureUserAgent BifrostContextKey = "bifrost-is-azure-user-agent" // bool (set by bifrost - DO NOT SET THIS MANUALLY)) - whether the request is an Azure user agent (only used in gateway)
215222
BifrostContextKeyVideoOutputRequested BifrostContextKey = "bifrost-video-output-requested"
216-
BifrostContextKeyValidateKeys BifrostContextKey = "bifrost-validate-keys" // bool (triggers additional key validation during provider add/update)
223+
BifrostContextKeyValidateKeys BifrostContextKey = "bifrost-validate-keys" // bool (triggers additional key validation during provider add/update)
217224
BifrostContextKeyProviderResponseHeaders BifrostContextKey = "bifrost-provider-response-headers" // map[string]string (set by provider handlers for response header forwarding)
218225
)
219226

@@ -891,4 +898,3 @@ type BifrostErrorExtraFields struct {
891898
LiteLLMCompat bool `json:"litellm_compat,omitempty"`
892899
KeyStatuses []KeyStatus `json:"key_statuses,omitempty"`
893900
}
894-

docs/features/governance/mcp-tools.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ Make sure you have at least one MCP client set up. Read more about it [here](../
1515
The filtering logic is determined by the Virtual Key's configuration:
1616

1717
1. **No MCP Configuration on Virtual Key (Default)**
18-
- If a Virtual Key has no specific MCP configurations, all tools from all enabled MCP clients are available by default.
19-
- In this state, a user can still manually filter tools for a single request by passing the `x-bf-mcp-include-tools` header.
18+
- If a Virtual Key has no specific MCP configurations, **no MCP tools are available** (deny-by-default).
19+
- You must explicitly add MCP client configurations to allow tools.
2020

2121
2. **With MCP Configuration on Virtual Key**
2222
- When you configure MCP clients on a Virtual Key, its settings take full precedence.

docs/features/governance/routing.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ This powerful feature enables key use cases like:
2828
Virtual Keys can be restricted to use only specific provider/models. When provider/model restrictions are configured, the VK can only access those designated provider/models, providing fine-grained control over which provider/models different users or applications can utilize.
2929

3030
**How It Works:**
31-
- **No Restrictions** (default): VK can use any available provider/models based on global configuration
32-
- **With Restrictions**: VK limited to only the specified provider/models with weighted load balancing
31+
- **No Provider Configs** (default): VK **blocks all providers** (deny-by-default). You must add provider configurations to allow traffic.
32+
- **With Provider Configs**: VK limited to only the specified provider/models with weighted load balancing
3333

3434
**Model Validation:**
3535
When you configure provider restrictions on a Virtual Key, Bifrost validates that the requested model is allowed for the selected provider:

docs/mcp/filtering.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ This consistent naming convention ensures clear separation between tools from di
227227
Virtual Keys can have their own MCP tool access configuration, which **takes precedence** over request-level headers.
228228

229229
<Note>
230-
When a Virtual Key has MCP configurations, it generates the `x-bf-mcp-include-tools` header automatically, overriding any manually sent header.
230+
When a Virtual Key has no MCP configurations, **no MCP tools are available** (deny-by-default). You must explicitly add MCP client configurations to allow tools. When a Virtual Key has MCP configurations, it generates the `x-bf-mcp-include-tools` header automatically, overriding any manually sent header.
231231
</Note>
232232

233233
### Configuration

0 commit comments

Comments
 (0)