Skip to content

Commit 58c2648

Browse files
authored
fix: move validateTrustedBots to validation.go and enforce on stdin path (#2217)
`validateTrustedBots` lived in `config_core.go` while all other validation helpers live in `validation.go`. The stdin JSON path also silently dropped `trustedBots: []` instead of rejecting it per spec §4.1.3.4. ## Changes - **`validation.go`**: Move `validateTrustedBots` here; call it from `validateGatewayConfig` so the stdin path enforces the same constraint as TOML - **`config_core.go`**: Remove `validateTrustedBots` and unused `strings` import - **`config_stdin.go`**: Call `validateTrustedBots` in `convertStdinConfig` when `TrustedBots != nil` (defense-in-depth); restructured to eliminate redundant length check - **`config_stdin_test.go`**: Update `"empty trustedBots not propagated"` sub-test to expect an error - **`config_test.go`**: Add `TestLoadFromStdin_WithEmptyTrustedBots` covering the full `LoadFromStdin` path Both config sources now consistently reject `trustedBots: []`: ```toml # TOML — already rejected [gateway] trusted_bots = [] # → error: trusted_bots must be a non-empty array when present ``` ```json // JSON stdin — now also rejected { "gateway": { "trustedBots": [] } } // → error: minimum 1 items required (schema) / trusted_bots must be non-empty (custom validator) ``` <!-- START COPILOT CODING AGENT TIPS --> --- ⌨️ Start Copilot coding agent tasks without leaving your editor — available in [VS Code](https://gh.io/cca-vs-code-docs), [Visual Studio](https://gh.io/cca-visual-studio-docs), [JetBrains IDEs](https://gh.io/cca-jetbrains-docs) and [Eclipse](https://gh.io/cca-eclipse-docs).
2 parents 9d49442 + 7b49503 commit 58c2648

5 files changed

Lines changed: 65 additions & 24 deletions

File tree

internal/config/config_core.go

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"io"
3030
"log"
3131
"os"
32-
"strings"
3332

3433
"github.com/BurntSushi/toml"
3534
"github.com/github/gh-aw-mcpg/internal/logger"
@@ -283,23 +282,6 @@ func LoadFromFile(path string) (*Config, error) {
283282
return &cfg, nil
284283
}
285284

286-
// validateTrustedBots checks that the trusted_bots list conforms to spec §4.1.3.4:
287-
// when present, it must be a non-empty array of non-empty strings.
288-
func validateTrustedBots(bots []string) error {
289-
if bots == nil {
290-
return nil
291-
}
292-
if len(bots) == 0 {
293-
return fmt.Errorf("trusted_bots must be a non-empty array when present (spec §4.1.3.4)")
294-
}
295-
for i, bot := range bots {
296-
if strings.TrimSpace(bot) == "" {
297-
return fmt.Errorf("trusted_bots[%d] must be a non-empty string", i)
298-
}
299-
}
300-
return nil
301-
}
302-
303285
// logger for config package
304286
var logConfig = log.New(io.Discard, "[CONFIG] ", log.LstdFlags)
305287

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: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,3 +1738,38 @@ func TestLoadFromStdin_WithTrustedBots(t *testing.T) {
17381738

17391739
assert.Equal(t, []string{"github-actions[bot]", "copilot-swe-agent[bot]"}, cfg.Gateway.TrustedBots)
17401740
}
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)