fix: validate empty trusted_bots at config load time#2215
Merged
Conversation
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>
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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enforces the MCP Gateway spec requirement that trusted_bots must be a non-empty list when explicitly present in TOML configs, by validating it during LoadFromFile rather than only failing later during label-agent payload construction.
Changes:
- Add
validateTrustedBots()and invoke it fromLoadFromFile. - Reject
trusted_bots = [](and blank/whitespace entries) at TOML config load time.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/config/config_core.go
Outdated
Comment on lines
+286
to
+293
| // 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)") |
There was a problem hiding this comment.
validateTrustedBots is the only validation helper in config_core.go, while the rest of the config validations live in internal/config/validation.go (and use logValidation / rules.*). Consider moving this helper into the validation module for consistency and easier reuse across config entrypoints.
Comment on lines
+267
to
+270
| // 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 | ||
| } |
There was a problem hiding this comment.
This spec validation is only applied on the TOML load path. The stdin JSON load path currently treats trustedBots: [] as “absent” (it’s not propagated) and does not validate blank/whitespace entries, which is inconsistent with spec §4.1.3.4 as enforced here. Consider calling the same validation from the stdin conversion/validation path so all config sources reject empty arrays when the field is present.
- [x] Investigate CI lint failure (job 67859560031) - [x] Identify root cause: `internal/config/config_test.go` was not formatted with gofmt - [x] Fix: run `gofmt -w internal/config/config_test.go` to format the file - [x] Verify lint passes locally <!-- START COPILOT CODING AGENT SUFFIX --> <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > Fix the failing GitHub Actions workflow lint > Analyze the workflow logs, identify the root cause of the failure, and implement a fix. > Job ID: 67859560031 > Job URL: https://github.com/github/gh-aw-mcpg/actions/runs/23330039619/job/67859560031 </details> <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/github/gh-aw-mcpg/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.
Collaborator
Author
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Contributor
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
…ath (#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).
This was referenced Mar 20, 2026
This was referenced Mar 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
validateTrustedBots()toLoadFromFileso thattrusted_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 fixesTestLoadFromFile_WithEmptyTrustedBotsToml.Changes
internal/config/config_core.go: AddedvalidateTrustedBots()function and call inLoadFromFileVerification
make agent-finishedpasses — all unit + integration tests green, 0 lint issues.