feat: add registry HasNonNoopGuard and enforce guards mode via label_agent only#1726
feat: add registry HasNonNoopGuard and enforce guards mode via label_agent only#1726
Conversation
Allow each MCP server to specify its own enforcement mode via the guards-mode field, independent of the global guards_mode setting. Mode resolution priority (highest to lowest): 1. Guard's LabelAgent() DIFCMode response (runtime, per-session) 2. Server config guards-mode (config, per-server) 3. Global guards_mode (config/CLI, gateway-wide) This enables configurations like github using filter mode while jira uses propagate mode, each tailored to the server's security needs. Changes: - Add GuardsMode field to ServerConfig (TOML: guards_mode, JSON: guards-mode) - Add guards-mode to StdinServerConfig and known fields list - Validate guards-mode values in config validation (strict/filter/propagate) - Check per-server mode in ensureGuardInitialized before global fallback - Add tests: JSON/TOML parsing, invalid mode rejection, registry HasNonNoopGuard Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds support for a per-server guards-mode override field, allowing each MCP server in the gateway config to specify its own DIFC enforcement mode (strict, filter, or propagate), instead of only relying on the global --guards-mode setting. The per-server mode sits at priority 2 in the resolution chain: below the LabelAgent() runtime response, but above the global default.
Changes:
- Add
GuardsModefield toServerConfig(TOML:guards_mode, JSON:guards-mode) andStdinServerConfig - Validate the field value for JSON/stdin configs in
validateStandardServerConfig; apply the per-server mode inensureGuardInitializedbefore falling back to the global default - Add tests: JSON/TOML parsing of valid modes, rejection of invalid mode in JSON, and
registry.HasNonNoopGuardcoverage
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/config/config_core.go |
Adds GuardsMode field to ServerConfig |
internal/config/config_stdin.go |
Adds GuardsMode to StdinServerConfig, includes it in knownFields, and propagates it through both conversion paths |
internal/config/validation.go |
Validates guards-mode value in validateStandardServerConfig (JSON path only) |
internal/config/config_guardpolicies_test.go |
Tests JSON/TOML parsing of valid modes and rejection of invalid mode via LoadFromStdin |
internal/server/unified.go |
Reads per-server GuardsMode from config in ensureGuardInitialized, sets it as defaultMode before the LabelAgent call |
internal/server/unified_test.go |
Tests that GuardsMode is preserved in the server config |
internal/guard/guard_test.go |
Tests registry.HasNonNoopGuard with empty, noop-only, mixed, and all-non-noop registries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/server/unified_test.go
Outdated
| func TestPerServerGuardsMode(t *testing.T) { | ||
| t.Run("per-server mode applied from config", func(t *testing.T) { | ||
| cfg := &config.Config{ | ||
| Servers: map[string]*config.ServerConfig{ | ||
| "github": { | ||
| Command: "docker", | ||
| Args: []string{}, | ||
| GuardsMode: "filter", | ||
| }, | ||
| "jira": { | ||
| Command: "docker", | ||
| Args: []string{}, | ||
| GuardsMode: "propagate", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| ctx := context.Background() | ||
| us, err := NewUnified(ctx, cfg) | ||
| require.NoError(t, err) | ||
| defer us.Close() | ||
|
|
||
| // Verify the config is stored and accessible | ||
| github := us.cfg.Servers["github"] | ||
| assert.Equal(t, "filter", github.GuardsMode) | ||
|
|
||
| jira := us.cfg.Servers["jira"] | ||
| assert.Equal(t, "propagate", jira.GuardsMode) | ||
| }) | ||
|
|
||
| t.Run("empty mode falls through to global", func(t *testing.T) { | ||
| cfg := &config.Config{ | ||
| DIFCMode: "filter", | ||
| Servers: map[string]*config.ServerConfig{ | ||
| "github": { | ||
| Command: "docker", | ||
| Args: []string{}, | ||
| // No GuardsMode set — should use global | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| ctx := context.Background() | ||
| us, err := NewUnified(ctx, cfg) | ||
| require.NoError(t, err) | ||
| defer us.Close() | ||
|
|
||
| github := us.cfg.Servers["github"] | ||
| assert.Empty(t, github.GuardsMode, "should be empty, falling through to global") | ||
| }) |
There was a problem hiding this comment.
The TestPerServerGuardsMode test only verifies that the GuardsMode value is stored in us.cfg — it does not test that ensureGuardInitialized actually reads and applies the per-server mode override. Since ensureGuardInitialized is the core piece of new behavior, there should be a test that verifies the returned EnforcementMode reflects the per-server GuardsMode config (e.g., by creating a mock guard that returns an empty DIFCMode in its LabelAgent response and confirming the returned mode matches the server's configured GuardsMode).
There was a problem hiding this comment.
@copilot make the mode (filter, propagate, strict) only settable from the label_agent response of a guard. default is strict. remove mode option from toml and stdin configs completely. update tests and documentation accordingly.https://gh.io/copilot-coding-agent-docs make the mode (filter, propagate, strict) only settable from the label_agent response of a guard. default is strict. remove mode option from toml and stdin configs completely. update tests and documentation accordingly.
There was a problem hiding this comment.
Already done in 03664ed. The guards-mode/guards_mode fields were removed from both ServerConfig (TOML) and StdinServerConfig (JSON/stdin). Enforcement mode is now exclusively settable via the guard's LabelAgent() response at runtime, with strict as the default. All related tests and validation logic were removed.
internal/config/validation.go
Outdated
| validModes := map[string]bool{"strict": true, "filter": true, "propagate": true} | ||
| if !validModes[strings.ToLower(server.GuardsMode)] { | ||
| logValidation.Printf("Validation failed: invalid guards-mode %q for server %s", server.GuardsMode, name) | ||
| return fmt.Errorf("configuration error at %s: invalid guards-mode %q (must be one of: strict, filter, propagate)", jsonPath, server.GuardsMode) | ||
| } |
There was a problem hiding this comment.
The valid modes map map[string]bool{"strict": true, "filter": true, "propagate": true} is created locally and duplicates the mode constants already defined in internal/difc (difc.ModeStrict, difc.ModeFilter, difc.ModePropagate, and difc.ValidModes). While introducing an import of internal/difc into internal/config would require checking for circular dependencies (none exist currently), an alternative is to reuse the difc.ParseEnforcementMode function which already does case-insensitive mode parsing and validation.
If reusing difc.ParseEnforcementMode is not feasible due to package structure concerns, at minimum the hardcoded string literals should be extracted into named constants within this package to avoid diverging if new modes are ever added.
…l_agent Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
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:
If you need me to access, download, or install something from one of these locations, you can either:
|
## Summary
Adds a daily agentic workflow (`smoke-allowonly.md`) that validates
AllowOnly guard policy enforcement through the MCP Gateway.
## Guard Policy
- **repos**: `["github/gh-aw*"]` (wildcard match)
- **min-integrity**: `approved` (OWNER, MEMBER, COLLABORATOR only)
## Test Coverage
### In-Scope Access (expected: allowed)
- `list_issues`, `list_pull_requests`, `list_commits`,
`get_file_contents`, `list_branches`, `search_code` against
`github/gh-aw-mcpg`
- `list_issues`, `get_file_contents` against `github/gh-aw`
### Out-of-Scope Blocking (expected: blocked)
- All repo-scoped calls against `octocat/Hello-World`
### Global API Blocking (expected: blocked)
- `search_repositories`, `search_users`
### Integrity Filtering (expected: filtered)
- Validates that content from non-approved authors is filtered
## Frontmatter
Uses `repos` and `min-integrity` fields directly on `tools.github` per
the gh-aw compiler format:
```yaml
tools:
github:
toolsets: [repos, issues, pull_requests, search]
repos: ["github/gh-aw*"]
min-integrity: approved
```
## Related
- PR #1723 (guard flag cleanup)
- PR #1726 (per-server guard mode)
- Issue #1711 (guards tracking)
Summary
Enforcement mode (
strict,filter,propagate) is now exclusively settable via the guard'sLabelAgent()response at runtime. The default mode isstrict. Per-serverguards-mode/guards_modeconfig fields have been removed from both TOML and JSON/stdin server configurations.Depends on: #1723 (cleanup/guard-flags)
Mode Resolution
LabelAgent()responsestrict(via global--guards-mode, default: strict)Changes
GuardsModefield fromServerConfig(TOML) andStdinServerConfig(JSON/stdin)validateStandardServerConfigensureGuardInitialized; mode is now only set by theLabelAgent()responseregistry.HasNonNoopGuardwith tests🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.