fix: move validateTrustedBots to validation.go and enforce on stdin path by Copilot · Pull Request #2217 · github/gh-aw-mcpg

Copilot · 2026-03-20T05:15:54Z

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 — already rejected
[gateway]
trusted_bots = []  # → error: trusted_bots must be a non-empty array when present

// 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 tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.

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

Copilot · 2026-03-20T05:23:13Z

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

example.com
- Triggering command: /tmp/go-build2530664659/b333/launcher.test /tmp/go-build2530664659/b333/launcher.test -test.testlogfile=/tmp/go-build2530664659/b333/testlog.txt -test.paniconexit0 -test.timeout=10m0s go1.25.8 -c=4 -nolocalimports -importcfg /tmp/go-build2530664659/b329/importcfg -pack /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/internal/guard/context.go JYL3lOygJhqU ap lp.go ap x_amd64/compile -I ache/go/1.25.8/x-test.testlogfile=/tmp/go-build2530664659/b324/testlog.txt -I x_amd64/compile (dns block)
invalid-host-that-does-not-exist-12345.com
- Triggering command: /tmp/go-build2530664659/b318/config.test /tmp/go-build2530664659/b318/config.test -test.testlogfile=/tmp/go-build2530664659/b318/testlog.txt -test.paniconexit0 -test.timeout=10m0s --no�� ache/go/1.25.8/x-c=4 (dns block)
nonexistent.local
- Triggering command: /tmp/go-build2530664659/b333/launcher.test /tmp/go-build2530664659/b333/launcher.test -test.testlogfile=/tmp/go-build2530664659/b333/testlog.txt -test.paniconexit0 -test.timeout=10m0s go1.25.8 -c=4 -nolocalimports -importcfg /tmp/go-build2530664659/b329/importcfg -pack /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/internal/guard/context.go JYL3lOygJhqU ap lp.go ap x_amd64/compile -I ache/go/1.25.8/x-test.testlogfile=/tmp/go-build2530664659/b324/testlog.txt -I x_amd64/compile (dns block)
slow.example.com
- Triggering command: /tmp/go-build2530664659/b333/launcher.test /tmp/go-build2530664659/b333/launcher.test -test.testlogfile=/tmp/go-build2530664659/b333/testlog.txt -test.paniconexit0 -test.timeout=10m0s go1.25.8 -c=4 -nolocalimports -importcfg /tmp/go-build2530664659/b329/importcfg -pack /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/internal/guard/context.go JYL3lOygJhqU ap lp.go ap x_amd64/compile -I ache/go/1.25.8/x-test.testlogfile=/tmp/go-build2530664659/b324/testlog.txt -I x_amd64/compile (dns block)
this-host-does-not-exist-12345.com
- Triggering command: /tmp/go-build2530664659/b342/mcp.test /tmp/go-build2530664659/b342/mcp.test -test.testlogfile=/tmp/go-build2530664659/b342/testlog.txt -test.paniconexit0 -test.timeout=10m0s swit�� /opt/hostedtoolcache/go/1.25.8/x-D cfg x_amd64/vet --gdwarf-5 tR/QfsGBRwG2DjReinspect -o x_amd64/vet bis 64/src/crypto/internal/fips140/nistec/p256_asm_a-test.v=true cfg x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Copilot

Pull request overview

This PR aligns config validation behavior across TOML and JSON-stdin sources by centralizing trustedBots validation and ensuring trustedBots: [] is rejected consistently per spec §4.1.3.4.

Changes:

Moved validateTrustedBots into internal/config/validation.go and invoked it from validateGatewayConfig.
Enforced trustedBots validation on the stdin JSON conversion path (convertStdinConfig) when the field is present.
Updated/added tests so the stdin path now expects an error for trustedBots: [] (including a full LoadFromStdin integration-style test).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/config/validation.go	Adds `trustedBots` validation to gateway validation and relocates the helper.
internal/config/config_core.go	Removes the old helper and drops an unused import after the move.
internal/config/config_stdin.go	Validates `trustedBots` when present during stdin config conversion.
internal/config/config_stdin_test.go	Updates conversion tests to expect an error for empty `trustedBots`.
internal/config/config_test.go	Adds an end-to-end stdin test expecting rejection of `trustedBots: []`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-20T05:26:40Z

internal/config/validation.go

+		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)


validateTrustedBots returns plain fmt.Errorf messages, while the rest of validateGatewayConfig primarily returns *rules.ValidationError (with consistent Field/JSONPath/Suggestion formatting). This leads to inconsistent error formatting for trustedBots compared to port/timeouts/payloadDir. Consider returning rules.InvalidValue (and include an appropriate JSONPath like "gateway.trustedBots" / "gateway.trustedBots[i]") so error output is consistent across gateway validations (and still usable from the TOML path).

Suggested change

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 rules.InvalidValue(

"trusted_bots",

"gateway.trustedBots",

"trusted_bots must be a non-empty array when present (spec §4.1.3.4)",

"Remove the trusted_bots setting or provide at least one non-empty trusted bot identifier.",

)

}

for i, bot := range bots {

if strings.TrimSpace(bot) == "" {

return rules.InvalidValue(

"trusted_bots",

fmt.Sprintf("gateway.trustedBots[%d]", i),

"trusted_bots entries must be non-empty strings",

"Remove empty entries from trusted_bots or replace them with valid trusted bot identifiers.",

)

Copilot · 2026-03-20T05:26:40Z

internal/config/validation.go

+	// Validate trustedBots per spec §4.1.3.4: must be non-empty array when present
+	if err := validateTrustedBots(gateway.TrustedBots); err != nil {
+		return err
+	}


validateGatewayConfig now enforces the trustedBots constraint, but TestValidateGatewayConfig in validation_test.go doesn’t cover the new cases (empty slice, whitespace-only entries). Adding focused unit cases there would prevent regressions even if JSON schema behavior changes and would directly exercise the new validation branch.

Initial plan

a89889d

Copilot AI assigned Copilot and lpcox Mar 20, 2026

Copilot AI mentioned this pull request Mar 20, 2026

fix: validate empty trusted_bots at config load time #2215

Merged

Copilot started work on behalf of lpcox March 20, 2026 05:15 View session

fix: move validateTrustedBots to validation.go and apply to stdin path

7b49503

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

Copilot AI changed the title ~~[WIP] Fix: validate empty trusted_bots at config load time~~ fix: move validateTrustedBots to validation.go and enforce on stdin path Mar 20, 2026

Copilot AI requested a review from lpcox March 20, 2026 05:23

Copilot finished work on behalf of lpcox March 20, 2026 05:23

lpcox marked this pull request as ready for review March 20, 2026 05:24

Copilot AI review requested due to automatic review settings March 20, 2026 05:24

lpcox merged commit 58c2648 into fix/trusted-bot-empty-array-validation Mar 20, 2026

lpcox deleted the copilot/sub-pr-2215 branch March 20, 2026 05:24

Copilot started reviewing on behalf of lpcox March 20, 2026 05:24 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

This was referenced Mar 20, 2026

Smoke Test: Copilot - 23330321028 #2220

Closed

Smoke Test: Copilot - 23330376355 #2222

Closed

Smoke Test: Copilot - 23343789482 #2227

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: move validateTrustedBots to validation.go and enforce on stdin path#2217

fix: move validateTrustedBots to validation.go and enforce on stdin path#2217
lpcox merged 2 commits intofix/trusted-bot-empty-array-validationfrom
copilot/sub-pr-2215

Copilot AI commented Mar 20, 2026 •

edited

Loading

Uh oh!

Copilot AI commented Mar 20, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-		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 rules.InvalidValue(
+			"trusted_bots",
+			"gateway.trustedBots",
+			"trusted_bots must be a non-empty array when present (spec §4.1.3.4)",
+			"Remove the trusted_bots setting or provide at least one non-empty trusted bot identifier.",
+		)
+	}
+	for i, bot := range bots {
+		if strings.TrimSpace(bot) == "" {
+			return rules.InvalidValue(
+				"trusted_bots",
+				fmt.Sprintf("gateway.trustedBots[%d]", i),
+				"trusted_bots entries must be non-empty strings",
+				"Remove empty entries from trusted_bots or replace them with valid trusted bot identifiers.",
+			)

Conversation

Copilot AI commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

Copilot AI commented Mar 20, 2026

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Mar 20, 2026 •

edited

Loading