From b59e400fc9726b94ef16a4dad52f887e35d000aa Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Thu, 19 Mar 2026 22:11:53 -0700 Subject: [PATCH 1/5] fix: validate empty trusted_bots at config load time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add validateTrustedBots() to LoadFromFile to reject trusted_bots = [] at TOML parse time per spec §4.1.3.4. Previously, empty arrays were only caught later in buildStrictLabelAgentPayload. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/config/config_core.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/internal/config/config_core.go b/internal/config/config_core.go index 1630c4a9..eefa924d 100644 --- a/internal/config/config_core.go +++ b/internal/config/config_core.go @@ -29,6 +29,7 @@ import ( "io" "log" "os" + "strings" "github.com/BurntSushi/toml" "github.com/github/gh-aw-mcpg/internal/logger" @@ -263,6 +264,11 @@ func LoadFromFile(path string) (*Config, error) { cfg.Gateway = &GatewayConfig{} } + // Validate trusted_bots per spec §4.1.3.4: must be non-empty array when present + if err := validateTrustedBots(cfg.Gateway.TrustedBots); err != nil { + return nil, err + } + // Apply core gateway defaults applyGatewayDefaults(cfg.Gateway) @@ -277,6 +283,23 @@ func LoadFromFile(path string) (*Config, error) { return &cfg, nil } +// validateTrustedBots checks that the trusted_bots list conforms to spec §4.1.3.4: +// when present, it must be a non-empty array of non-empty strings. +func validateTrustedBots(bots []string) error { + if bots == nil { + return nil + } + if len(bots) == 0 { + return fmt.Errorf("trusted_bots must be a non-empty array when present (spec §4.1.3.4)") + } + for i, bot := range bots { + if strings.TrimSpace(bot) == "" { + return fmt.Errorf("trusted_bots[%d] must be a non-empty string", i) + } + } + return nil +} + // logger for config package var logConfig = log.New(io.Discard, "[CONFIG] ", log.LstdFlags) From ddc3b111656a9b388decdbc4c17b6a40d953dc3c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 05:12:56 +0000 Subject: [PATCH 2/5] Initial plan From 7d77827d5a5dbaceb4f7a9f623789cce2242353b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 05:14:32 +0000 Subject: [PATCH 3/5] fix: format internal/config/config_test.go with gofmt Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/81fe5009-0fee-487d-bf38-31c3d7f8731d --- internal/config/config_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c39d94e4..39e675ff 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1701,6 +1701,7 @@ args = ["run", "--rm", "-i", "test/container:latest"] _, err = LoadFromFile(tmpFile) require.Error(t, err) } + // TestLoadFromStdin_WithTrustedBots verifies JSON stdin parsing of trustedBots. // Covers spec §4.1.3.4 (Trusted Bot Identity Configuration). func TestLoadFromStdin_WithTrustedBots(t *testing.T) { From a89889df29fea993626378a0408378b2001b938b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 05:15:53 +0000 Subject: [PATCH 4/5] Initial plan From 7b495032773d5666fd18df094b035a2134af210b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 05:22:37 +0000 Subject: [PATCH 5/5] fix: move validateTrustedBots to validation.go and apply to stdin path Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/e8fa10a8-4831-4dee-b2dc-0dd2cd845d55 --- internal/config/config_core.go | 18 -------------- internal/config/config_stdin.go | 5 +++- internal/config/config_stdin_test.go | 9 ++++--- internal/config/config_test.go | 35 ++++++++++++++++++++++++++++ internal/config/validation.go | 22 +++++++++++++++++ 5 files changed, 65 insertions(+), 24 deletions(-) diff --git a/internal/config/config_core.go b/internal/config/config_core.go index eefa924d..f559a9a9 100644 --- a/internal/config/config_core.go +++ b/internal/config/config_core.go @@ -29,7 +29,6 @@ import ( "io" "log" "os" - "strings" "github.com/BurntSushi/toml" "github.com/github/gh-aw-mcpg/internal/logger" @@ -283,23 +282,6 @@ func LoadFromFile(path string) (*Config, error) { return &cfg, nil } -// validateTrustedBots checks that the trusted_bots list conforms to spec §4.1.3.4: -// when present, it must be a non-empty array of non-empty strings. -func validateTrustedBots(bots []string) error { - if bots == nil { - return nil - } - if len(bots) == 0 { - return fmt.Errorf("trusted_bots must be a non-empty array when present (spec §4.1.3.4)") - } - for i, bot := range bots { - if strings.TrimSpace(bot) == "" { - return fmt.Errorf("trusted_bots[%d] must be a non-empty string", i) - } - } - return nil -} - // logger for config package var logConfig = log.New(io.Discard, "[CONFIG] ", log.LstdFlags) diff --git a/internal/config/config_stdin.go b/internal/config/config_stdin.go index 70c8c866..03561388 100644 --- a/internal/config/config_stdin.go +++ b/internal/config/config_stdin.go @@ -281,7 +281,10 @@ func convertStdinConfig(stdinCfg *StdinConfig) (*Config, error) { if stdinCfg.Gateway.PayloadDir != "" { cfg.Gateway.PayloadDir = stdinCfg.Gateway.PayloadDir } - if len(stdinCfg.Gateway.TrustedBots) > 0 { + if stdinCfg.Gateway.TrustedBots != nil { + if err := validateTrustedBots(stdinCfg.Gateway.TrustedBots); err != nil { + return nil, err + } cfg.Gateway.TrustedBots = stdinCfg.Gateway.TrustedBots } } else { diff --git a/internal/config/config_stdin_test.go b/internal/config/config_stdin_test.go index b79b1ebe..4db148f8 100644 --- a/internal/config/config_stdin_test.go +++ b/internal/config/config_stdin_test.go @@ -988,7 +988,7 @@ func TestConvertStdinConfig_TrustedBots(t *testing.T) { assert.Equal(t, []string{"copilot-swe-agent[bot]", "my-org-bot"}, cfg.Gateway.TrustedBots) }) - t.Run("empty trustedBots not propagated", func(t *testing.T) { + t.Run("empty trustedBots rejected per spec §4.1.3.4", func(t *testing.T) { stdinCfg := &StdinConfig{ MCPServers: map[string]*StdinServerConfig{}, Gateway: &StdinGatewayConfig{ @@ -996,10 +996,9 @@ func TestConvertStdinConfig_TrustedBots(t *testing.T) { }, } - cfg, err := convertStdinConfig(stdinCfg) - require.NoError(t, err) - require.NotNil(t, cfg.Gateway) - assert.Nil(t, cfg.Gateway.TrustedBots) + _, err := convertStdinConfig(stdinCfg) + require.Error(t, err) + assert.Contains(t, err.Error(), "trusted_bots must be a non-empty array when present") }) t.Run("nil trustedBots not propagated", func(t *testing.T) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 39e675ff..46747326 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1738,3 +1738,38 @@ func TestLoadFromStdin_WithTrustedBots(t *testing.T) { assert.Equal(t, []string{"github-actions[bot]", "copilot-swe-agent[bot]"}, cfg.Gateway.TrustedBots) } + +// TestLoadFromStdin_WithEmptyTrustedBots verifies JSON stdin parsing rejects trustedBots: []. +// Covers spec §4.1.3.4 (trustedBots MUST be a non-empty array when present). +func TestLoadFromStdin_WithEmptyTrustedBots(t *testing.T) { + stdinJSON := `{ + "mcpServers": { + "test": { + "container": "test/container:latest", + "type": "stdio" + } + }, + "gateway": { + "port": 8080, + "domain": "localhost", + "apiKey": "test-key", + "trustedBots": [] + } + }` + + oldStdin := os.Stdin + defer func() { os.Stdin = oldStdin }() + + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stdin = r + + go func() { + defer w.Close() + _, _ = w.Write([]byte(stdinJSON)) + }() + + _, err = LoadFromStdin() + require.Error(t, err) + assert.Contains(t, err.Error(), "trustedBots") +} diff --git a/internal/config/validation.go b/internal/config/validation.go index f91eced4..57c156ff 100644 --- a/internal/config/validation.go +++ b/internal/config/validation.go @@ -365,10 +365,32 @@ func validateGatewayConfig(gateway *StdinGatewayConfig) error { } } + // Validate trustedBots per spec §4.1.3.4: must be non-empty array when present + if err := validateTrustedBots(gateway.TrustedBots); err != nil { + return err + } + logValidation.Print("Gateway config validation passed") return nil } +// validateTrustedBots checks that the trusted_bots/trustedBots list conforms to spec §4.1.3.4: +// when present, it must be a non-empty array of non-empty strings. +func validateTrustedBots(bots []string) error { + if bots == nil { + return nil + } + if len(bots) == 0 { + return fmt.Errorf("trusted_bots must be a non-empty array when present (spec §4.1.3.4)") + } + for i, bot := range bots { + if strings.TrimSpace(bot) == "" { + return fmt.Errorf("trusted_bots[%d] must be a non-empty string", i) + } + } + return nil +} + // validateTOMLStdioContainerization validates that TOML stdio servers use Docker for containerization. // This enforces MCP Gateway Specification Section 3.2.1: "Stdio-based MCP servers MUST be containerized." func validateTOMLStdioContainerization(servers map[string]*ServerConfig) error {