Skip to content

Commit 90cba00

Browse files
authored
fix: validate empty trusted_bots at config load time (#2215)
## Summary Adds `validateTrustedBots()` to `LoadFromFile` so that `trusted_bots = []` in TOML config is rejected at parse time per spec §4.1.3.4 (*"trustedBots MUST be a non-empty array of strings when present"*). Previously empty arrays were only caught downstream in `buildStrictLabelAgentPayload`. This fixes `TestLoadFromFile_WithEmptyTrustedBotsToml`. ## Changes - `internal/config/config_core.go`: Added `validateTrustedBots()` function and call in `LoadFromFile` ## Verification `make agent-finished` passes — all unit + integration tests green, 0 lint issues.
2 parents 47b0674 + 58c2648 commit 90cba00

5 files changed

Lines changed: 71 additions & 6 deletions

File tree

internal/config/config_core.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,11 @@ func LoadFromFile(path string) (*Config, error) {
263263
cfg.Gateway = &GatewayConfig{}
264264
}
265265

266+
// Validate trusted_bots per spec §4.1.3.4: must be non-empty array when present
267+
if err := validateTrustedBots(cfg.Gateway.TrustedBots); err != nil {
268+
return nil, err
269+
}
270+
266271
// Apply core gateway defaults
267272
applyGatewayDefaults(cfg.Gateway)
268273

internal/config/config_stdin.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,10 @@ func convertStdinConfig(stdinCfg *StdinConfig) (*Config, error) {
281281
if stdinCfg.Gateway.PayloadDir != "" {
282282
cfg.Gateway.PayloadDir = stdinCfg.Gateway.PayloadDir
283283
}
284-
if len(stdinCfg.Gateway.TrustedBots) > 0 {
284+
if stdinCfg.Gateway.TrustedBots != nil {
285+
if err := validateTrustedBots(stdinCfg.Gateway.TrustedBots); err != nil {
286+
return nil, err
287+
}
285288
cfg.Gateway.TrustedBots = stdinCfg.Gateway.TrustedBots
286289
}
287290
} else {

internal/config/config_stdin_test.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -988,18 +988,17 @@ func TestConvertStdinConfig_TrustedBots(t *testing.T) {
988988
assert.Equal(t, []string{"copilot-swe-agent[bot]", "my-org-bot"}, cfg.Gateway.TrustedBots)
989989
})
990990

991-
t.Run("empty trustedBots not propagated", func(t *testing.T) {
991+
t.Run("empty trustedBots rejected per spec §4.1.3.4", func(t *testing.T) {
992992
stdinCfg := &StdinConfig{
993993
MCPServers: map[string]*StdinServerConfig{},
994994
Gateway: &StdinGatewayConfig{
995995
TrustedBots: []string{},
996996
},
997997
}
998998

999-
cfg, err := convertStdinConfig(stdinCfg)
1000-
require.NoError(t, err)
1001-
require.NotNil(t, cfg.Gateway)
1002-
assert.Nil(t, cfg.Gateway.TrustedBots)
999+
_, err := convertStdinConfig(stdinCfg)
1000+
require.Error(t, err)
1001+
assert.Contains(t, err.Error(), "trusted_bots must be a non-empty array when present")
10031002
})
10041003

10051004
t.Run("nil trustedBots not propagated", func(t *testing.T) {

internal/config/config_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1701,6 +1701,7 @@ args = ["run", "--rm", "-i", "test/container:latest"]
17011701
_, err = LoadFromFile(tmpFile)
17021702
require.Error(t, err)
17031703
}
1704+
17041705
// TestLoadFromStdin_WithTrustedBots verifies JSON stdin parsing of trustedBots.
17051706
// Covers spec §4.1.3.4 (Trusted Bot Identity Configuration).
17061707
func TestLoadFromStdin_WithTrustedBots(t *testing.T) {
@@ -1737,3 +1738,38 @@ func TestLoadFromStdin_WithTrustedBots(t *testing.T) {
17371738

17381739
assert.Equal(t, []string{"github-actions[bot]", "copilot-swe-agent[bot]"}, cfg.Gateway.TrustedBots)
17391740
}
1741+
1742+
// TestLoadFromStdin_WithEmptyTrustedBots verifies JSON stdin parsing rejects trustedBots: [].
1743+
// Covers spec §4.1.3.4 (trustedBots MUST be a non-empty array when present).
1744+
func TestLoadFromStdin_WithEmptyTrustedBots(t *testing.T) {
1745+
stdinJSON := `{
1746+
"mcpServers": {
1747+
"test": {
1748+
"container": "test/container:latest",
1749+
"type": "stdio"
1750+
}
1751+
},
1752+
"gateway": {
1753+
"port": 8080,
1754+
"domain": "localhost",
1755+
"apiKey": "test-key",
1756+
"trustedBots": []
1757+
}
1758+
}`
1759+
1760+
oldStdin := os.Stdin
1761+
defer func() { os.Stdin = oldStdin }()
1762+
1763+
r, w, err := os.Pipe()
1764+
require.NoError(t, err)
1765+
os.Stdin = r
1766+
1767+
go func() {
1768+
defer w.Close()
1769+
_, _ = w.Write([]byte(stdinJSON))
1770+
}()
1771+
1772+
_, err = LoadFromStdin()
1773+
require.Error(t, err)
1774+
assert.Contains(t, err.Error(), "trustedBots")
1775+
}

internal/config/validation.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,10 +365,32 @@ func validateGatewayConfig(gateway *StdinGatewayConfig) error {
365365
}
366366
}
367367

368+
// Validate trustedBots per spec §4.1.3.4: must be non-empty array when present
369+
if err := validateTrustedBots(gateway.TrustedBots); err != nil {
370+
return err
371+
}
372+
368373
logValidation.Print("Gateway config validation passed")
369374
return nil
370375
}
371376

377+
// validateTrustedBots checks that the trusted_bots/trustedBots list conforms to spec §4.1.3.4:
378+
// when present, it must be a non-empty array of non-empty strings.
379+
func validateTrustedBots(bots []string) error {
380+
if bots == nil {
381+
return nil
382+
}
383+
if len(bots) == 0 {
384+
return fmt.Errorf("trusted_bots must be a non-empty array when present (spec §4.1.3.4)")
385+
}
386+
for i, bot := range bots {
387+
if strings.TrimSpace(bot) == "" {
388+
return fmt.Errorf("trusted_bots[%d] must be a non-empty string", i)
389+
}
390+
}
391+
return nil
392+
}
393+
372394
// validateTOMLStdioContainerization validates that TOML stdio servers use Docker for containerization.
373395
// This enforces MCP Gateway Specification Section 3.2.1: "Stdio-based MCP servers MUST be containerized."
374396
func validateTOMLStdioContainerization(servers map[string]*ServerConfig) error {

0 commit comments

Comments
 (0)