diff --git a/AGENTS.md b/AGENTS.md index da0f62241..80ae99b86 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -417,6 +417,7 @@ DEBUG_COLORS=0 DEBUG=* ./awmg --config config.toml - `MCP_GATEWAY_ALLOWONLY_SCOPE_OWNER` - AllowOnly owner scope value (sets default for `--allowonly-scope-owner`) - `MCP_GATEWAY_ALLOWONLY_SCOPE_REPO` - AllowOnly repo name, requires owner (sets default for `--allowonly-scope-repo`) - `MCP_GATEWAY_ALLOWONLY_MIN_INTEGRITY` - AllowOnly integrity level: `none`, `unapproved`, `approved`, `merged` (sets default for `--allowonly-min-integrity`) +- `MCP_GATEWAY_FORCE_PUBLIC_REPOS` - When `true` (default), automatically forces `repos="public"` allow-only policy when `GITHUB_REPOSITORY` identifies a public repository; set to `false` to opt out. Overridden by `gateway.forcePublicRepos` in JSON stdin config. - `MCP_GATEWAY_TLS_CERT` - Path to TLS server certificate PEM file; enables HTTPS when set together with `MCP_GATEWAY_TLS_KEY` (sets default for `--tls-cert`) - `MCP_GATEWAY_TLS_KEY` - Path to TLS server private key PEM file; required when `MCP_GATEWAY_TLS_CERT` is set (sets default for `--tls-key`) - `MCP_GATEWAY_CA_CERT` - Path to CA certificate PEM file for client certificate verification; enables mutual TLS (mTLS) when set alongside `MCP_GATEWAY_TLS_CERT`/`MCP_GATEWAY_TLS_KEY` (sets default for `--tls-ca`) diff --git a/config.example.toml b/config.example.toml index a5a2896d1..2c5aaeb59 100644 --- a/config.example.toml +++ b/config.example.toml @@ -40,6 +40,14 @@ tool_timeout = 120 # Prevents remote servers from expiring idle sessions by sending periodic pings. # Set to -1 to disable keepalive pings entirely. # keepalive_interval = 1500 + +# Force repos="public" allow-only policy when the workflow repo is public (default: enabled) +# When enabled (default), the gateway reads GITHUB_REPOSITORY and calls the GitHub API at +# startup; if the repo is public, it overrides the allow-only guard policy for all servers to +# repos="public", preventing agents from reading private repository data. +# Set to false to opt out (e.g., when private-to-public-flows: allow is set in workflow front-matter). +# Can also be controlled via MCP_GATEWAY_FORCE_PUBLIC_REPOS environment variable. +# force_public_repos = true # # OpenTelemetry TOML key migration: # - Prefer [gateway.opentelemetry] for new configs. diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 19d3e2cb9..f60e8ca1a 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -514,6 +514,7 @@ The `customSchemas` top-level field allows you to define custom server types bey | `payloadPathPrefix` (JSON stdin) / `payload_path_prefix` (TOML) | Optional path prefix used when returning `payloadPath` values to clients (for example when the host payload directory is mounted at a different in-container path) | (empty - use actual filesystem path) | | `payloadSizeThreshold` (JSON) / `payload_size_threshold` (TOML) | Size threshold in bytes; responses larger than this are stored to disk and returned as a `payloadPath` reference | `524288` (512 KB) | | `trustedBots` (JSON) / `trusted_bots` (TOML) | Optional list of additional bot usernames to trust with "approved" integrity level. Additive to the built-in trusted bot list. When specified, must be a non-empty array with non-empty string entries (spec §4.1.3.4); omit the field entirely if not needed. Example: `["my-bot[bot]", "org-automation"]` | (disabled) | +| `forcePublicRepos` (JSON) / `force_public_repos` (TOML) | When `true` (or omitted), the gateway checks `GITHUB_REPOSITORY` and the GitHub API at startup; if the workflow repository is public, it overrides the allow-only guard policy to `repos="public"` for all servers, preventing agents from reading private repository data. Set to `false` to opt out (equivalent to `private-to-public-flows: allow` in workflow front-matter). Has no effect if `GITHUB_REPOSITORY` is unset or the token is unavailable. Overrides env var `MCP_GATEWAY_FORCE_PUBLIC_REPOS`. | `true` (enabled) | | `keepaliveInterval` (JSON) / `keepalive_interval` (TOML) | Interval (seconds) between keepalive pings sent to HTTP backends. Prevents remote servers from expiring idle sessions. Set to `-1` to disable keepalive pings entirely. | `1500` (25 min) | ### OpenTelemetry / Tracing diff --git a/internal/config/config_core.go b/internal/config/config_core.go index 62ef9814d..1b3eaeea1 100644 --- a/internal/config/config_core.go +++ b/internal/config/config_core.go @@ -149,6 +149,22 @@ type GatewayConfig struct { // Example values: "copilot-swe-agent[bot]", "my-org-bot[bot]" TrustedBots []string `toml:"trusted_bots" json:"trusted_bots,omitempty"` + // ForcePublicRepos controls whether the gateway automatically overrides the + // allow-only policy to repos="public" when the GITHUB_REPOSITORY repo is public. + // nil / omitted → enabled by default (auto-force when repo is public) + // true → explicitly enabled + // false → disabled (opt-out via private-to-public-flows: allow) + // Corresponds to env var MCP_GATEWAY_FORCE_PUBLIC_REPOS. + ForcePublicRepos *bool `toml:"force_public_repos" json:"forcePublicRepos,omitempty"` + + // SinkVisibilityExemptServers lists server IDs that are exempt from the + // default sink-visibility="public" enforcement. By default, all non-safe-outputs + // write-sink servers are assigned sink-visibility="public" (security-by-default). + // Servers in this list retain their configured (or omitted) sink-visibility as-is. + // Use ["*"] to exempt all servers (equivalent to disabling the default). + // Set by the compiler when private-to-public-flows is configured in frontmatter. + SinkVisibilityExemptServers []string `toml:"sink_visibility_exempt_servers" json:"sinkVisibilityExemptServers,omitempty"` + // Tracing holds OpenTelemetry OTLP tracing configuration (legacy TOML key). // New configurations should use the opentelemetry key (spec §4.1.3.6). // When Endpoint is set, traces are exported to the specified OTLP endpoint. diff --git a/internal/config/config_stdin.go b/internal/config/config_stdin.go index adad12cb4..b93c37c59 100644 --- a/internal/config/config_stdin.go +++ b/internal/config/config_stdin.go @@ -32,18 +32,20 @@ type StdinConfig struct { // StdinGatewayConfig represents gateway configuration in stdin JSON format. // Uses pointers for optional fields to distinguish between unset and zero values. type StdinGatewayConfig struct { - Port *int `json:"port,omitempty"` - AgentID string `json:"agentId,omitempty"` - APIKey string `json:"apiKey,omitempty"` - Domain string `json:"domain,omitempty"` - StartupTimeout *int `json:"startupTimeout,omitempty"` - ToolTimeout *int `json:"toolTimeout,omitempty"` - KeepaliveInterval *int `json:"keepaliveInterval,omitempty"` - PayloadDir string `json:"payloadDir,omitempty"` - PayloadPathPrefix *string `json:"payloadPathPrefix,omitempty"` - PayloadSizeThreshold *int `json:"payloadSizeThreshold,omitempty"` - TrustedBots []string `json:"trustedBots,omitempty"` - OpenTelemetry *StdinOpenTelemetryConfig `json:"opentelemetry,omitempty"` + Port *int `json:"port,omitempty"` + AgentID string `json:"agentId,omitempty"` + APIKey string `json:"apiKey,omitempty"` + Domain string `json:"domain,omitempty"` + StartupTimeout *int `json:"startupTimeout,omitempty"` + ToolTimeout *int `json:"toolTimeout,omitempty"` + KeepaliveInterval *int `json:"keepaliveInterval,omitempty"` + PayloadDir string `json:"payloadDir,omitempty"` + PayloadPathPrefix *string `json:"payloadPathPrefix,omitempty"` + PayloadSizeThreshold *int `json:"payloadSizeThreshold,omitempty"` + TrustedBots []string `json:"trustedBots,omitempty"` + ForcePublicRepos *bool `json:"forcePublicRepos,omitempty"` + SinkVisibilityExemptServers []string `json:"sinkVisibilityExemptServers,omitempty"` + OpenTelemetry *StdinOpenTelemetryConfig `json:"opentelemetry,omitempty"` agentIDSet bool `json:"-"` legacyAPIKeySet bool `json:"-"` @@ -419,6 +421,12 @@ func convertStdinConfig(stdinCfg *StdinConfig) (*Config, error) { } cfg.Gateway.TrustedBots = stdinCfg.Gateway.TrustedBots } + if stdinCfg.Gateway.ForcePublicRepos != nil { + cfg.Gateway.ForcePublicRepos = stdinCfg.Gateway.ForcePublicRepos + } + if len(stdinCfg.Gateway.SinkVisibilityExemptServers) > 0 { + cfg.Gateway.SinkVisibilityExemptServers = stdinCfg.Gateway.SinkVisibilityExemptServers + } } else { logStdin.Print("No gateway config in stdin, applying defaults") cfg.Gateway = &GatewayConfig{} diff --git a/internal/config/guard_policy_parse.go b/internal/config/guard_policy_parse.go index 7af5b9865..a4cfa2a92 100644 --- a/internal/config/guard_policy_parse.go +++ b/internal/config/guard_policy_parse.go @@ -15,6 +15,9 @@ const ( EnvAllowOnlyScopeOwner = "MCP_GATEWAY_ALLOWONLY_SCOPE_OWNER" EnvAllowOnlyScopeRepo = "MCP_GATEWAY_ALLOWONLY_SCOPE_REPO" EnvAllowOnlyMinIntegrity = "MCP_GATEWAY_ALLOWONLY_MIN_INTEGRITY" + // EnvForcePublicRepos controls whether force-public-repos enforcement is active. + // Default true (feature on). Set to "false" to opt out. + EnvForcePublicRepos = "MCP_GATEWAY_FORCE_PUBLIC_REPOS" ) // ParseServerGuardPolicy parses a guard policy from a server-specific raw policy map. diff --git a/internal/config/schema/mcp-gateway-config.schema.json b/internal/config/schema/mcp-gateway-config.schema.json index 101d5ed0c..5a7e1db57 100644 --- a/internal/config/schema/mcp-gateway-config.schema.json +++ b/internal/config/schema/mcp-gateway-config.schema.json @@ -21,8 +21,17 @@ "description": "Map of custom server type names to JSON Schema URLs for validation. Custom types enable extensibility for specialized MCP server implementations. Keys are type names (must not be 'stdio' or 'http'), values are HTTPS URLs pointing to JSON Schema definitions, or empty strings to skip validation.", "propertyNames": { "allOf": [ - { "pattern": "^[a-z][a-z0-9-]*$" }, - { "not": { "enum": ["stdio", "http"] } } + { + "pattern": "^[a-z][a-z0-9-]*$" + }, + { + "not": { + "enum": [ + "stdio", + "http" + ] + } + } ] }, "patternProperties": { @@ -162,7 +171,12 @@ }, "guard-policies": { "type": "object", - "description": "Guard policies for access control at the MCP gateway level. The structure of guard policies is server-specific.", + "description": "Guard policies for access control at the MCP gateway level. Supports a 'write-sink' policy for DIFC-based output filtering. Additional server-specific policies may be provided.", + "properties": { + "write-sink": { + "$ref": "#/definitions/writeSinkGuardPolicyConfig" + } + }, "additionalProperties": true } }, @@ -217,7 +231,12 @@ }, "guard-policies": { "type": "object", - "description": "Guard policies for access control at the MCP gateway level. The structure of guard policies is server-specific. For GitHub MCP server, see the GitHub guard policy schema. For other servers (Jira, WorkIQ), different policy schemas will apply.", + "description": "Guard policies for access control at the MCP gateway level. Supports a 'write-sink' policy for DIFC-based output filtering. Additional server-specific policies may be provided.", + "properties": { + "write-sink": { + "$ref": "#/definitions/writeSinkGuardPolicyConfig" + } + }, "additionalProperties": true }, "auth": { @@ -294,7 +313,12 @@ "type": { "type": "string", "pattern": "^[a-z][a-z0-9-]*$", - "not": {"enum": ["stdio", "http"]}, + "not": { + "enum": [ + "stdio", + "http" + ] + }, "description": "Custom server type name. Must not be 'stdio' or 'http'. Must be registered in customSchemas." } }, @@ -397,6 +421,16 @@ "opentelemetry": { "$ref": "#/definitions/opentelemetryConfig", "description": "Optional OpenTelemetry configuration for emitting distributed tracing spans for MCP calls. When configured, the gateway exports OTLP/HTTP traces to the specified collector endpoint." + }, + "forcePublicRepos": { + "type": "boolean", + "description": "When true (default), forces the allow-only policy to repos=\"public\" at runtime if the gateway detects it is running in a public repository. Set to false by the compiler when private-to-public-flows: allow is declared in workflow frontmatter, or via MCP_GATEWAY_FORCE_PUBLIC_REPOS=false environment variable. See MCP Gateway Specification section 4.1.3.8.", + "default": true + }, + "sinkVisibilityExemptServers": { + "type": "array", + "items": { "type": "string" }, + "description": "Server IDs exempt from the default sink-visibility=\"public\" enforcement. By default, all non-safe-outputs write-sink servers are assigned sink-visibility=\"public\" (security-by-default). Servers listed here retain their configured (or omitted) sink-visibility as-is. Use [\"*\"] to exempt all servers. Set by the compiler when private-to-public-flows is configured in workflow frontmatter." } }, "required": [ @@ -417,6 +451,26 @@ } ] }, + "writeSinkGuardPolicyConfig": { + "type": "object", + "description": "Write-sink guard policy for DIFC-based output filtering. Controls whether an agent may write to the safe-outputs sink based on the agent's accumulated secrecy tags and the target repository visibility. Per MCP Gateway Specification section 10.8.", + "properties": { + "accept": { + "type": "array", + "description": "Secrecy tag patterns that are permitted to write to this sink. Use [\"*\"] to accept all secrecy levels. Required for all write-sink policies. When sink-visibility is \"public\", this field is syntactically required but has no runtime effect — resource secrecy is unconditionally set to empty.", + "items": { + "type": "string" + } + }, + "sink-visibility": { + "type": "string", + "description": "Declares the visibility of the safe-outputs target repository (always the workflow's own repo, i.e. GITHUB_REPOSITORY). When \"public\", agents with non-empty secrecy are blocked regardless of accept patterns. When \"private\" or \"internal\", standard accept-pattern matching applies. When omitted, backward-compatible accept-pattern matching applies.", + "enum": ["public", "private", "internal"] + } + }, + "required": ["accept"], + "additionalProperties": false + }, "opentelemetryConfig": { "type": "object", "description": "OpenTelemetry configuration for the MCP Gateway. When present, the gateway emits distributed tracing spans for each MCP tool invocation and exports them via OTLP/HTTP to the configured collector endpoint. Per MCP Gateway Specification section 4.1.3.6.", diff --git a/internal/server/force_public_repos_test.go b/internal/server/force_public_repos_test.go new file mode 100644 index 000000000..7144f17ad --- /dev/null +++ b/internal/server/force_public_repos_test.go @@ -0,0 +1,360 @@ +package server + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/github/gh-aw-mcpg/internal/config" + "github.com/github/gh-aw-mcpg/internal/guard" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// ─── shouldForcePublicRepos ─────────────────────────────────────────────────── + +// TestShouldForcePublicRepos_PublicRepo verifies that shouldForcePublicRepos +// returns true when the workflow repository is public. +func TestShouldForcePublicRepos_PublicRepo(t *testing.T) { + t.Setenv(guard.WASMGuardsDirEnvVar, "") + t.Setenv(config.EnvForcePublicRepos, "true") + apiServer := startMockRepoVisibilityServer(t, "public", false) + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + t.Setenv("GITHUB_TOKEN", "mock-token") + t.Setenv("GITHUB_API_URL", apiServer.URL) + + us := newMinimalUnifiedServerForGuardTest(&config.Config{ + Gateway: &config.GatewayConfig{}, + }) + + result := us.shouldForcePublicRepos() + + assert.True(t, result, "shouldForcePublicRepos should return true for a public repo") +} + +// TestShouldForcePublicRepos_PrivateRepo verifies that shouldForcePublicRepos +// returns false when the workflow repository is private. +func TestShouldForcePublicRepos_PrivateRepo(t *testing.T) { + t.Setenv(guard.WASMGuardsDirEnvVar, "") + apiServer := startMockRepoVisibilityServer(t, "private", true) + + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + t.Setenv("GITHUB_TOKEN", "mock-token") + t.Setenv("GITHUB_API_URL", apiServer.URL) + + us := newMinimalUnifiedServerForGuardTest(&config.Config{ + Gateway: &config.GatewayConfig{}, + }) + + result := us.shouldForcePublicRepos() + + assert.False(t, result, "shouldForcePublicRepos should return false for a private repo") +} + +// TestShouldForcePublicRepos_ConfigOptOut verifies that shouldForcePublicRepos +// returns false when explicitly disabled in gateway config. +func TestShouldForcePublicRepos_ConfigOptOut(t *testing.T) { + t.Setenv(guard.WASMGuardsDirEnvVar, "") + // Even if the API would return "public", the config opt-out takes precedence. + apiServer := startMockRepoVisibilityServer(t, "public", false) + + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + t.Setenv("GITHUB_TOKEN", "mock-token") + t.Setenv("GITHUB_API_URL", apiServer.URL) + + disabled := false + us := newMinimalUnifiedServerForGuardTest(&config.Config{ + Gateway: &config.GatewayConfig{ + ForcePublicRepos: &disabled, + }, + }) + + result := us.shouldForcePublicRepos() + + assert.False(t, result, "shouldForcePublicRepos should return false when disabled in config") +} + +// TestShouldForcePublicRepos_EnvVarOptOut verifies that shouldForcePublicRepos +// returns false when disabled via MCP_GATEWAY_FORCE_PUBLIC_REPOS=false. +func TestShouldForcePublicRepos_EnvVarOptOut(t *testing.T) { + t.Setenv(guard.WASMGuardsDirEnvVar, "") + apiServer := startMockRepoVisibilityServer(t, "public", false) + + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + t.Setenv("GITHUB_TOKEN", "mock-token") + t.Setenv("GITHUB_API_URL", apiServer.URL) + t.Setenv(config.EnvForcePublicRepos, "false") + + us := newMinimalUnifiedServerForGuardTest(&config.Config{ + Gateway: &config.GatewayConfig{}, + }) + + result := us.shouldForcePublicRepos() + + assert.False(t, result, "shouldForcePublicRepos should return false when MCP_GATEWAY_FORCE_PUBLIC_REPOS=false") +} + +// TestShouldForcePublicRepos_NoGitHubRepository verifies that shouldForcePublicRepos +// returns false when GITHUB_REPOSITORY is not set. +func TestShouldForcePublicRepos_NoGitHubRepository(t *testing.T) { + t.Setenv(guard.WASMGuardsDirEnvVar, "") + t.Setenv("GITHUB_TOKEN", "mock-token") + // Unset GITHUB_REPOSITORY + t.Setenv("GITHUB_REPOSITORY", "") + + us := newMinimalUnifiedServerForGuardTest(&config.Config{ + Gateway: &config.GatewayConfig{}, + }) + + result := us.shouldForcePublicRepos() + + assert.False(t, result, "shouldForcePublicRepos should return false without GITHUB_REPOSITORY") +} + +// TestShouldForcePublicRepos_NoToken verifies that shouldForcePublicRepos +// returns false when no GitHub token is available. +func TestShouldForcePublicRepos_NoToken(t *testing.T) { + t.Setenv(guard.WASMGuardsDirEnvVar, "") + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + // Remove all token env vars + t.Setenv("GITHUB_MCP_SERVER_TOKEN", "") + t.Setenv("GITHUB_TOKEN", "") + t.Setenv("GITHUB_PERSONAL_ACCESS_TOKEN", "") + t.Setenv("GH_TOKEN", "") + + us := newMinimalUnifiedServerForGuardTest(&config.Config{ + Gateway: &config.GatewayConfig{}, + }) + + result := us.shouldForcePublicRepos() + + assert.False(t, result, "shouldForcePublicRepos should return false without a GitHub token") +} + +// TestShouldForcePublicRepos_APIError verifies that shouldForcePublicRepos +// returns false (fail-open) when the GitHub API returns an error. +func TestShouldForcePublicRepos_APIError(t *testing.T) { + t.Setenv(guard.WASMGuardsDirEnvVar, "") + // Start a server that returns an error + apiServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + t.Cleanup(apiServer.Close) + + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + t.Setenv("GITHUB_TOKEN", "mock-token") + t.Setenv("GITHUB_API_URL", apiServer.URL) + + us := newMinimalUnifiedServerForGuardTest(&config.Config{ + Gateway: &config.GatewayConfig{}, + }) + + result := us.shouldForcePublicRepos() + + assert.False(t, result, "shouldForcePublicRepos should return false (fail-open) on API error") +} + +// ─── overrideToPublicScope ──────────────────────────────────────────────────── + +// TestOverrideToPublicScope_GlobalPolicy_OverridesRepos verifies that +// overrideToPublicScope sets repos="public" in the global guard policy. +func TestOverrideToPublicScope_GlobalPolicy_OverridesRepos(t *testing.T) { + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + + cfg := &config.Config{ + Servers: map[string]*config.ServerConfig{ + "github": {Type: "http"}, + }, + GuardPolicy: &config.GuardPolicy{ + AllowOnly: &config.AllowOnlyPolicy{ + Repos: "all", + MinIntegrity: config.IntegrityNone, + }, + }, + } + us := newMinimalUnifiedServerForGuardTest(cfg) + + us.overrideToPublicScope("github") + + require.NotNil(t, cfg.GuardPolicy.AllowOnly) + assert.Equal(t, "public", cfg.GuardPolicy.AllowOnly.Repos, + "overrideToPublicScope should set repos=public in global policy") +} + +// TestOverrideToPublicScope_GlobalPolicy_NoAllowOnly_WriteSinkOnly_Skipped verifies +// that overrideToPublicScope does NOT add AllowOnly to a write-sink-only global policy +// because allow-only and write-sink are mutually exclusive in the GuardPolicy schema. +func TestOverrideToPublicScope_GlobalPolicy_WriteSinkOnly(t *testing.T) { + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + + cfg := &config.Config{ + Servers: map[string]*config.ServerConfig{ + "safe-outputs": {Type: "http"}, + }, + GuardPolicy: &config.GuardPolicy{ + WriteSink: &config.WriteSinkPolicy{Accept: []string{"*"}}, + }, + } + us := newMinimalUnifiedServerForGuardTest(cfg) + + us.overrideToPublicScope("safe-outputs") + + // Write-sink-only global policy: AllowOnly should NOT be added (mutually exclusive). + assert.Nil(t, cfg.GuardPolicy.AllowOnly, + "overrideToPublicScope should NOT add AllowOnly to a write-sink-only global policy") +} + +// TestOverrideToPublicScope_PerServerPolicy_OverridesRepos verifies that +// overrideToPublicScope sets repos="public" in per-server guard policies. +func TestOverrideToPublicScope_PerServerPolicy_OverridesRepos(t *testing.T) { + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + + cfg := &config.Config{ + Servers: map[string]*config.ServerConfig{ + "github": { + Type: "http", + GuardPolicies: map[string]interface{}{ + "allow-only": map[string]interface{}{ + "repos": "all", + "min-integrity": "none", + }, + }, + }, + }, + } + us := newMinimalUnifiedServerForGuardTest(cfg) + + us.overrideToPublicScope("github") + + // Parse back the modified policy to verify the override + policy, err := config.ParseServerGuardPolicy("github", cfg.Servers["github"].GuardPolicies) + require.NoError(t, err) + require.NotNil(t, policy) + require.NotNil(t, policy.AllowOnly) + assert.Equal(t, "public", policy.AllowOnly.Repos, + "overrideToPublicScope should override per-server allow-only to repos=public") +} + +// TestOverrideToPublicScope_PerServerPolicy_WriteSinkOnly_Skipped verifies that +// overrideToPublicScope does NOT add AllowOnly to a write-sink-only per-server policy +// (allow-only and write-sink are mutually exclusive in the GuardPolicy schema). +func TestOverrideToPublicScope_PerServerPolicy_WriteSinkOnly(t *testing.T) { + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + + cfg := &config.Config{ + Servers: map[string]*config.ServerConfig{ + "github": { + Type: "http", + GuardPolicies: map[string]interface{}{ + "write-sink": map[string]interface{}{ + "accept": []interface{}{"*"}, + }, + }, + }, + }, + } + us := newMinimalUnifiedServerForGuardTest(cfg) + + us.overrideToPublicScope("github") + + // Write-sink-only per-server policy: AllowOnly should NOT be added. + _, hasAllowOnly := cfg.Servers["github"].GuardPolicies["allow-only"] + assert.False(t, hasAllowOnly, + "overrideToPublicScope should NOT add allow-only to a write-sink-only policy") +} + +// TestOverrideToPublicScope_NoExistingPolicy_InjectsDefault verifies that +// overrideToPublicScope injects a default allow-only policy when none is configured. +func TestOverrideToPublicScope_NoExistingPolicy_InjectsDefault(t *testing.T) { + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + + cfg := &config.Config{ + Servers: map[string]*config.ServerConfig{ + "github": { + Type: "http", + GuardPolicies: map[string]interface{}{}, + }, + }, + } + us := newMinimalUnifiedServerForGuardTest(cfg) + + us.overrideToPublicScope("github") + + // The server should now have an allow-only policy injected + assert.Greater(t, len(cfg.Servers["github"].GuardPolicies), 0, + "overrideToPublicScope should inject a default policy when none exists") + + policy, err := config.ParseServerGuardPolicy("github", cfg.Servers["github"].GuardPolicies) + require.NoError(t, err) + require.NotNil(t, policy) + require.NotNil(t, policy.AllowOnly) + assert.Equal(t, "public", policy.AllowOnly.Repos) +} + +// TestOverrideToPublicScope_NoConfig_NoOp verifies that overrideToPublicScope +// does not panic or error when the config is nil or has no servers. +func TestOverrideToPublicScope_NoConfig_NoOp(t *testing.T) { + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + + // nil config + us := &UnifiedServer{guardRegistry: guard.NewRegistry()} + assert.NotPanics(t, func() { + us.overrideToPublicScope("github") + }, "overrideToPublicScope should not panic with nil config") +} + +// TestShouldForcePublicRepos_ResultCached verifies that the API is called only +// once even when shouldForcePublicRepos is called multiple times. +func TestShouldForcePublicRepos_ResultCached(t *testing.T) { + t.Setenv(guard.WASMGuardsDirEnvVar, "") + t.Setenv(config.EnvForcePublicRepos, "true") + callCount := 0 + apiServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "" && len(r.URL.Path) > 1 { + callCount++ + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]interface{}{ + "visibility": "public", + "private": false, + }) + })) + t.Cleanup(apiServer.Close) + + t.Setenv("GITHUB_REPOSITORY", "test-owner/test-repo") + t.Setenv("GITHUB_TOKEN", "mock-token") + t.Setenv("GITHUB_API_URL", apiServer.URL) + + us := newMinimalUnifiedServerForGuardTest(&config.Config{ + Gateway: &config.GatewayConfig{}, + }) + + // Call multiple times + result1 := us.shouldForcePublicRepos() + result2 := us.shouldForcePublicRepos() + result3 := us.shouldForcePublicRepos() + + assert.True(t, result1) + assert.True(t, result2) + assert.True(t, result3) + // API should only have been called once (via resolveWorkflowRepoVisibility cache) + assert.Equal(t, 1, callCount, "GitHub API should be called only once due to caching") +} + +// ─── helpers ───────────────────────────────────────────────────────────────── + +// startMockRepoVisibilityServer starts a mock HTTP server that returns a repo +// visibility response for any /repos/{owner}/{repo} request. +func startMockRepoVisibilityServer(t *testing.T, visibility string, private bool) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]interface{}{ + "visibility": visibility, + "private": private, + }) + })) + t.Cleanup(srv.Close) + return srv +} diff --git a/internal/server/guard_init.go b/internal/server/guard_init.go index a3d4cf427..ac9b7d62e 100644 --- a/internal/server/guard_init.go +++ b/internal/server/guard_init.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" "github.com/github/gh-aw-mcpg/internal/config" @@ -65,7 +66,30 @@ func (us *UnifiedServer) registerGuard(serverID string) error { if g == nil { // Check if server has a write-sink policy — create WriteSinkGuard directly if ws := us.resolveWriteSinkPolicy(serverID); ws != nil { - effectiveVisibility := us.verifySinkVisibilityAtRuntime(serverID, ws.SinkVisibility) + effectiveVisibility := ws.SinkVisibility + + // Security-by-default: non-safe-outputs write-sink servers get + // sink-visibility="public" when no explicit value is configured. + // This assumes external sinks release data publicly unless exempted. + if effectiveVisibility == "" && !isSafeOutputsServer(serverID) && !us.isServerExemptFromSinkVisibility(serverID) { + effectiveVisibility = "public" + logger.LogInfoToServer(serverID, "difc", + "Defaulting sink-visibility to \"public\" for non-safe-outputs write-sink server (security-by-default)") + } + + // Runtime safety net for safe-outputs: if the compiler didn't set + // sink-visibility but the workflow repo is public, force "public" to + // prevent exfiltration. This makes the gateway self-defending even + // without compiler cooperation. + if effectiveVisibility == "" && isSafeOutputsServer(serverID) { + if vis, ok := us.resolveWorkflowRepoVisibility(); ok && vis == githubhttp.RepoVisibilityPublic { + effectiveVisibility = "public" + logger.LogWarnToServer(serverID, "difc", + "SAFE-OUTPUTS SAFETY NET: no sink-visibility configured but workflow repo is public — forcing sink-visibility=\"public\" to prevent data exfiltration") + } + } + + effectiveVisibility = us.verifySinkVisibilityAtRuntime(serverID, effectiveVisibility) g = guard.NewWriteSinkGuardWithVisibility(ws.Accept, effectiveVisibility) logger.LogInfoToServer(serverID, "difc", "Created write-sink guard with %d accept patterns, sink-visibility=%q", len(ws.Accept), effectiveVisibility) } @@ -102,6 +126,13 @@ func (us *UnifiedServer) registerGuard(serverID string) error { } } + // Before guard policy validation: apply forced repos="public" override when + // the workflow repo is public. This modifies the in-memory config so that + // subsequent resolveGuardPolicy calls for this server use the overridden value. + if us.shouldForcePublicRepos() { + us.overrideToPublicScope(serverID) + } + var policyErr error g, policyErr = us.requireGuardPolicyIfGuardEnabled(serverID, g) if policyErr != nil { @@ -411,25 +442,262 @@ func (us *UnifiedServer) verifySinkVisibilityAtRuntime(serverID, configuredVisib return configuredVisibility } - apiURL := envutil.DeriveGitHubAPIURL(envutil.DefaultGitHubAPIBaseURL) - authHeader := "token " + token - - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - - effective, overridden, err := githubhttp.VerifySinkVisibility(ctx, apiURL, nwo, authHeader, configuredVisibility) - if err != nil { - logger.LogWarnToServer(serverID, "difc", "Sink visibility runtime verification failed (using configured value %q): %v", configuredVisibility, err) + vis, ok := us.resolveWorkflowRepoVisibility() + if !ok { + logger.LogWarnToServer(serverID, "difc", "Sink visibility runtime verification failed (using configured value %q): API error or unavailable", configuredVisibility) return configuredVisibility } - if overridden { + configured := strings.ToLower(strings.TrimSpace(configuredVisibility)) + + // If actual is "public" but configured is not "public", + // override to "public" — this is the security-critical case. + if vis == githubhttp.RepoVisibilityPublic && configured != "public" { logger.LogWarnToServer(serverID, "difc", "SINK VISIBILITY OVERRIDE: configured=%q but runtime check shows repo %s is %q — overriding to %q to prevent potential data exfiltration", - configuredVisibility, nwo, effective, effective) + configuredVisibility, nwo, vis, "public") + return "public" + } + + logger.LogInfoToServer(serverID, "difc", "Sink visibility runtime verification passed: repo=%s, configured=%q, actual=%q", nwo, configuredVisibility, vis) + return configured +} + +// resolveWorkflowRepoVisibility fetches and caches the visibility of the workflow +// repository identified by GITHUB_REPOSITORY. The API call is made at most once +// per gateway lifetime; subsequent calls return the cached result immediately. +// +// Returns (visibility, true) on success, or ("", false) when the repository is +// unknown, the token is unavailable, or the API call fails (fail-open semantics). +func (us *UnifiedServer) resolveWorkflowRepoVisibility() (githubhttp.RepoVisibility, bool) { + us.repoVisibilityOnce.Do(func() { + nwo := os.Getenv("GITHUB_REPOSITORY") + if nwo == "" { + return + } + token := envutil.LookupGitHubToken() + if token == "" { + return + } + apiURL := envutil.DeriveGitHubAPIURL(envutil.DefaultGitHubAPIBaseURL) + authHeader := "token " + token + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + vis, err := githubhttp.FetchRepoVisibility(ctx, apiURL, nwo, authHeader) + if err != nil { + logGuardInit.Printf("resolveWorkflowRepoVisibility: API call failed for %s: %v", nwo, err) + return // cache remains empty; repoVisibilityCacheOK stays false + } + us.repoVisibilityCached = vis + us.repoVisibilityCacheOK = true + logGuardInit.Printf("resolveWorkflowRepoVisibility: cached repo visibility for %s: %s", nwo, vis) + }) + return us.repoVisibilityCached, us.repoVisibilityCacheOK +} + +// shouldForcePublicRepos returns true when the gateway should automatically +// override allow-only policies to repos="public". This is true when: +// - The feature is not explicitly disabled (config ForcePublicRepos=false or env MCP_GATEWAY_FORCE_PUBLIC_REPOS=false) +// - GITHUB_REPOSITORY is set +// - A GitHub token is available +// - The GitHub API confirms the repository is public +// +// The result is cached after the first call (backed by forcePublicReposOnce). +func (us *UnifiedServer) shouldForcePublicRepos() bool { + us.forcePublicReposOnce.Do(func() { + us.forcePublicReposResult = us.computeForcePublicRepos() + }) + return us.forcePublicReposResult +} + +func (us *UnifiedServer) computeForcePublicRepos() bool { + // Config opt-out: forcePublicRepos=false in gateway config disables the feature. + if us.cfg != nil && us.cfg.Gateway != nil && us.cfg.Gateway.ForcePublicRepos != nil && !*us.cfg.Gateway.ForcePublicRepos { + logGuardInit.Print("shouldForcePublicRepos: disabled by config (forcePublicRepos=false)") + return false + } + + // Env opt-out: MCP_GATEWAY_FORCE_PUBLIC_REPOS=false disables the feature. + // Default is true (feature enabled), so only an explicit "false" disables it. + if !envutil.GetEnvBool(config.EnvForcePublicRepos, true) { + logGuardInit.Printf("shouldForcePublicRepos: disabled by env var %s=false", config.EnvForcePublicRepos) + return false + } + + nwo := os.Getenv("GITHUB_REPOSITORY") + if nwo == "" { + logGuardInit.Print("shouldForcePublicRepos: GITHUB_REPOSITORY not set — skipping") + return false + } + + token := envutil.LookupGitHubToken() + if token == "" { + logGuardInit.Print("shouldForcePublicRepos: no GitHub token available — skipping") + return false + } + + vis, ok := us.resolveWorkflowRepoVisibility() + if !ok { + logger.LogWarn("difc", "shouldForcePublicRepos: failed to determine visibility for %s (fail-open, not forcing repos=public)", nwo) + return false + } + + return vis == githubhttp.RepoVisibilityPublic +} + +// isSafeOutputsServer returns true if the server ID identifies a safe-outputs +// server. Matches "safe-outputs" and the legacy "safeoutputs" form. +func isSafeOutputsServer(serverID string) bool { + return serverID == "safe-outputs" || serverID == "safeoutputs" +} + +// isServerExemptFromSinkVisibility returns true if the given server should NOT +// receive the default sink-visibility="public" enforcement. A server is exempt when: +// - forcePublicRepos is explicitly disabled (blanket opt-out) +// - The server ID appears in gateway.SinkVisibilityExemptServers +// - SinkVisibilityExemptServers contains "*" (wildcard exempts all) +func (us *UnifiedServer) isServerExemptFromSinkVisibility(serverID string) bool { + if us.cfg == nil || us.cfg.Gateway == nil { + return false + } + // Blanket opt-out: forcePublicRepos=false implies all servers exempt + if us.cfg.Gateway.ForcePublicRepos != nil && !*us.cfg.Gateway.ForcePublicRepos { + return true + } + for _, exempt := range us.cfg.Gateway.SinkVisibilityExemptServers { + if exempt == "*" || exempt == serverID { + return true + } + } + return false +} + +// validateSinkVisibilityExemptServers checks that each entry in the exempt list +// matches an actual server in the config. Unknown server IDs produce a warning. +func (us *UnifiedServer) validateSinkVisibilityExemptServers() { + if us.cfg == nil || us.cfg.Gateway == nil { + return + } + for _, exempt := range us.cfg.Gateway.SinkVisibilityExemptServers { + if exempt == "*" { + continue + } + if _, exists := us.cfg.Servers[exempt]; !exists { + logger.LogWarn("difc", + "sinkVisibilityExemptServers contains unknown server ID %q — ignoring (not in mcpServers config)", exempt) + } + } +} + +// overrideToPublicScope modifies the in-memory guard policy for serverID so that +// any allow-only scope is set to repos="public". This is called when the workflow +// repository is confirmed to be public, closing the GitLost read-path attack vector. +// +// Override precedence (first match wins): +// 1. Global guard policy override (us.cfg.GuardPolicy) — applies to all servers. +// 2. Per-server guard policies in serverCfg.GuardPolicies. +// +// When an existing AllowOnly is found its Repos field is set to "public". +// When no AllowOnly exists in the policy and it is not a write-sink-only policy, +// an AllowOnly with min-integrity="none" is added. +// When no policy exists at all for the server, a new allow-only policy is created. +// Write-sink-only policies are left untouched: allow-only and write-sink are +// mutually exclusive in the GuardPolicy schema, and write-sink guards use +// SinkVisibility (not AllowOnly) to enforce public-repo restrictions. +// +// Changes are permanent for the gateway's lifetime and affect all subsequent +// resolveGuardPolicy calls for the given server. +func (us *UnifiedServer) overrideToPublicScope(serverID string) { + nwo := os.Getenv("GITHUB_REPOSITORY") + + // Case 1: global policy override (set via CLI or env flags). + if us.cfg != nil && us.cfg.GuardPolicy != nil { + gp := us.cfg.GuardPolicy + if gp.AllowOnly == nil && gp.WriteSink != nil { + // Write-sink-only global policy: AllowOnly and WriteSink are mutually + // exclusive. The write-sink guard uses SinkVisibility for public-repo + // enforcement; no AllowOnly override needed. + logGuardInit.Printf("overrideToPublicScope: skipping write-sink-only global policy for serverID=%s", serverID) + return + } + if gp.AllowOnly == nil { + gp.AllowOnly = &config.AllowOnlyPolicy{ + Repos: "public", + MinIntegrity: config.IntegrityNone, + } + } else { + gp.AllowOnly.Repos = "public" + } + logger.LogWarnToServer(serverID, "difc", + "FORCED REPOS=PUBLIC: workflow repo %s is public — overriding allow-only scope to 'public' to prevent private data reads (source: global policy)", + nwo) + return + } + + // Case 2: per-server guard policies in config. + if us.cfg == nil || us.cfg.Servers == nil { + return + } + serverCfg, ok := us.cfg.Servers[serverID] + if !ok || serverCfg == nil { + return + } + + if len(serverCfg.GuardPolicies) == 0 { + // No existing per-server policy — inject a minimal allow-only policy so + // the WASM guard (if loaded) will restrict reads to public repos. + serverCfg.GuardPolicies = map[string]interface{}{ + "allow-only": map[string]interface{}{ + "repos": "public", + "min-integrity": config.IntegrityNone, + }, + } } else { - logger.LogInfoToServer(serverID, "difc", "Sink visibility runtime verification passed: repo=%s, visibility=%q", nwo, effective) + policy, err := config.ParseServerGuardPolicy(serverID, serverCfg.GuardPolicies) + if err != nil { + logger.LogWarnToServer(serverID, "difc", + "FORCED REPOS=PUBLIC: failed to parse existing policy for override (skipping): %v", err) + return + } + if policy == nil { + // Unrecognized policy format — inject allow-only directly into the map + // only if there is no write-sink key present (to avoid creating an + // invalid combined policy). + if _, hasWriteSink := serverCfg.GuardPolicies["write-sink"]; !hasWriteSink { + serverCfg.GuardPolicies["allow-only"] = map[string]interface{}{ + "repos": "public", + "min-integrity": config.IntegrityNone, + } + } else { + logGuardInit.Printf("overrideToPublicScope: skipping write-sink-only per-server policy for serverID=%s", serverID) + return + } + } else if policy.AllowOnly == nil && policy.WriteSink != nil { + // Write-sink-only per-server policy: see note above. + logGuardInit.Printf("overrideToPublicScope: skipping write-sink-only per-server policy for serverID=%s", serverID) + return + } else { + if policy.AllowOnly == nil { + policy.AllowOnly = &config.AllowOnlyPolicy{ + Repos: "public", + MinIntegrity: config.IntegrityNone, + } + } else { + policy.AllowOnly.Repos = "public" + } + newMap, err := config.GuardPolicyToMap(policy) + if err != nil { + logger.LogWarnToServer(serverID, "difc", + "FORCED REPOS=PUBLIC: failed to serialize overridden policy (skipping): %v", err) + return + } + serverCfg.GuardPolicies = newMap + } } - return effective + logger.LogWarnToServer(serverID, "difc", + "FORCED REPOS=PUBLIC: workflow repo %s is public — overriding allow-only scope to 'public' to prevent private data reads", + nwo) } diff --git a/internal/server/unified.go b/internal/server/unified.go index f6bfa42de..9ea19bdd4 100644 --- a/internal/server/unified.go +++ b/internal/server/unified.go @@ -116,6 +116,18 @@ type UnifiedServer struct { // Health monitoring healthMonitor *launcher.HealthMonitor + // Cached workflow repository visibility — set once during guard registration startup. + // Used by both verifySinkVisibilityAtRuntime and shouldForcePublicRepos to avoid + // repeated GitHub API calls for the same repository. + repoVisibilityOnce sync.Once + repoVisibilityCached githubhttp.RepoVisibility + repoVisibilityCacheOK bool + + // Cached result of the force-public-repos check — set once during guard registration. + // Avoids re-evaluating the check for every backend server registered at startup. + forcePublicReposOnce sync.Once + forcePublicReposResult bool + // Cache tracer at construction to avoid calling otel.Tracer on every request. tracing.CachedTracer } @@ -168,6 +180,9 @@ func NewUnified(ctx context.Context, cfg *config.Config) (*UnifiedServer, error) us.server = server us.logWASMGuardsDirConfiguration() + // Validate sinkVisibilityExemptServers entries match actual server IDs + us.validateSinkVisibilityExemptServers() + // Register guards for all backends for _, serverID := range l.ServerIDs() { if err := us.registerGuard(serverID); err != nil { diff --git a/test/integration/force_public_repos_test.go b/test/integration/force_public_repos_test.go new file mode 100644 index 000000000..94ea14262 --- /dev/null +++ b/test/integration/force_public_repos_test.go @@ -0,0 +1,712 @@ +package integration + +import ( + "bytes" + "context" + "fmt" + "os" + "os/exec" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestForcePublicRepos_PublicRepo_OverridesAllowOnly verifies that when the +// workflow repository is public, the gateway overrides the allow-only policy to +// repos="public" and logs the FORCED REPOS=PUBLIC warning. +func TestForcePublicRepos_PublicRepo_OverridesAllowOnly(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + mockAPI := startMockGitHubAPI(t, "public", false) + defer mockAPI.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "github": { + "type": "http", + "url": "%s", + "guard-policies": { + "allow-only": { + "repos": "all", + "min-integrity": "none" + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + filteredEnv := filterEnv(os.Environ(), + "GITHUB_REPOSITORY", "GITHUB_TOKEN", "GITHUB_MCP_SERVER_TOKEN", + "GITHUB_PERSONAL_ACCESS_TOKEN", "GH_TOKEN", "GITHUB_API_URL", + "MCP_GATEWAY_FORCE_PUBLIC_REPOS", + ) + filteredEnv = append(filteredEnv, + "GITHUB_REPOSITORY=test-owner/test-repo", + "GITHUB_TOKEN=mock-token-for-testing", + "MCP_GATEWAY_FORCE_PUBLIC_REPOS=true", + fmt.Sprintf("GITHUB_API_URL=%s", mockAPI.URL), + "MCP_GATEWAY_WASM_GUARDS_DIR=", + ) + cmd.Env = filteredEnv + + var stderr syncBuffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &stderr + + err := cmd.Start() + require.NoError(t, err, "Failed to start gateway") + + ok := waitForStderr(&stderr, "Starting MCPG", 15*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + assert.Contains(t, logContent, "FORCED REPOS=PUBLIC", + "Log should contain forced repos=public warning for a public repo") + assert.Contains(t, logContent, "test-owner/test-repo", + "Log should reference the repository name") + t.Log("✓ Force-public-repos override triggered for public repo") +} + +// TestForcePublicRepos_PrivateRepo_NoOverride verifies that when the workflow +// repository is private, the allow-only policy is left unchanged and no +// FORCED REPOS=PUBLIC warning is emitted. +func TestForcePublicRepos_PrivateRepo_NoOverride(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + mockAPI := startMockGitHubAPI(t, "private", true) + defer mockAPI.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "github": { + "type": "http", + "url": "%s", + "guard-policies": { + "allow-only": { + "repos": "all", + "min-integrity": "none" + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + filteredEnv := filterEnv(os.Environ(), + "GITHUB_REPOSITORY", "GITHUB_TOKEN", "GITHUB_MCP_SERVER_TOKEN", + "GITHUB_PERSONAL_ACCESS_TOKEN", "GH_TOKEN", "GITHUB_API_URL", + ) + filteredEnv = append(filteredEnv, + "GITHUB_REPOSITORY=test-owner/private-repo", + "GITHUB_TOKEN=mock-token-for-testing", + fmt.Sprintf("GITHUB_API_URL=%s", mockAPI.URL), + "MCP_GATEWAY_WASM_GUARDS_DIR=", + ) + cmd.Env = filteredEnv + + var stderr syncBuffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &stderr + + err := cmd.Start() + require.NoError(t, err, "Failed to start gateway") + + ok := waitForStderr(&stderr, "Starting MCPG", 15*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + assert.NotContains(t, logContent, "FORCED REPOS=PUBLIC", + "Should NOT log forced repos=public for a private repo") + t.Log("✓ No force-public-repos override for private repo") +} + +// TestForcePublicRepos_ConfigOptOut_NoOverride verifies that when +// gateway.forcePublicRepos=false is set in config, no override is applied +// even when the repo is public. +func TestForcePublicRepos_ConfigOptOut_NoOverride(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + mockAPI := startMockGitHubAPI(t, "public", false) + defer mockAPI.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "github": { + "type": "http", + "url": "%s", + "guard-policies": { + "allow-only": { + "repos": "all", + "min-integrity": "none" + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key", + "forcePublicRepos": false + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + filteredEnv := filterEnv(os.Environ(), + "GITHUB_REPOSITORY", "GITHUB_TOKEN", "GITHUB_MCP_SERVER_TOKEN", + "GITHUB_PERSONAL_ACCESS_TOKEN", "GH_TOKEN", "GITHUB_API_URL", + ) + filteredEnv = append(filteredEnv, + "GITHUB_REPOSITORY=test-owner/test-repo", + "GITHUB_TOKEN=mock-token-for-testing", + fmt.Sprintf("GITHUB_API_URL=%s", mockAPI.URL), + "MCP_GATEWAY_WASM_GUARDS_DIR=", + ) + cmd.Env = filteredEnv + + var stderr syncBuffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &stderr + + err := cmd.Start() + require.NoError(t, err, "Failed to start gateway") + + ok := waitForStderr(&stderr, "Starting MCPG", 15*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + assert.NotContains(t, logContent, "FORCED REPOS=PUBLIC", + "Should NOT log forced repos=public when forcePublicRepos=false in config") + t.Log("✓ No force-public-repos override when disabled in config") +} + +// TestForcePublicRepos_NoGitHubRepository_NoOverride verifies that when +// GITHUB_REPOSITORY is not set, no override is applied and the gateway starts +// cleanly. +func TestForcePublicRepos_NoGitHubRepository_NoOverride(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "github": { + "type": "http", + "url": "%s", + "guard-policies": { + "allow-only": { + "repos": "all", + "min-integrity": "none" + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + + // Explicitly remove GITHUB_REPOSITORY + filteredEnv := filterEnv(os.Environ(), "GITHUB_REPOSITORY") + filteredEnv = append(filteredEnv, "MCP_GATEWAY_WASM_GUARDS_DIR=") + cmd.Env = filteredEnv + + var stderr syncBuffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &stderr + + err := cmd.Start() + require.NoError(t, err, "Failed to start gateway") + + ok := waitForStderr(&stderr, "Starting MCPG", 15*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + assert.NotContains(t, logContent, "FORCED REPOS=PUBLIC", + "Should NOT log forced repos=public without GITHUB_REPOSITORY") + t.Log("✓ No force-public-repos override without GITHUB_REPOSITORY — clean startup") +} + +// TestForcePublicRepos_EnvVarOptOut_NoOverride verifies that when +// MCP_GATEWAY_FORCE_PUBLIC_REPOS=false is set via environment variable, no +// override is applied even when the repo is public. +func TestForcePublicRepos_EnvVarOptOut_NoOverride(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + mockAPI := startMockGitHubAPI(t, "public", false) + defer mockAPI.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "github": { + "type": "http", + "url": "%s", + "guard-policies": { + "allow-only": { + "repos": "all", + "min-integrity": "none" + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + filteredEnv := filterEnv(os.Environ(), + "GITHUB_REPOSITORY", "GITHUB_TOKEN", "GITHUB_MCP_SERVER_TOKEN", + "GITHUB_PERSONAL_ACCESS_TOKEN", "GH_TOKEN", "GITHUB_API_URL", + "MCP_GATEWAY_FORCE_PUBLIC_REPOS", + ) + filteredEnv = append(filteredEnv, + "GITHUB_REPOSITORY=test-owner/test-repo", + "GITHUB_TOKEN=mock-token-for-testing", + fmt.Sprintf("GITHUB_API_URL=%s", mockAPI.URL), + "MCP_GATEWAY_FORCE_PUBLIC_REPOS=false", + "MCP_GATEWAY_WASM_GUARDS_DIR=", + ) + cmd.Env = filteredEnv + + var stderr syncBuffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &stderr + + err := cmd.Start() + require.NoError(t, err, "Failed to start gateway") + + ok := waitForStderr(&stderr, "Starting MCPG", 15*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + assert.NotContains(t, logContent, "FORCED REPOS=PUBLIC", + "Should NOT log forced repos=public when MCP_GATEWAY_FORCE_PUBLIC_REPOS=false") + t.Log("✓ No force-public-repos override when disabled via env var") +} + +// TestForcePublicRepos_APIFailure_FailOpen verifies that when the GitHub API +// returns an error, the gateway starts without applying the override (fail-open). +func TestForcePublicRepos_APIFailure_FailOpen(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + // Mock API that always returns 500 + mockAPI := startMockGitHubAPIWithStatus(t, 500) + defer mockAPI.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "github": { + "type": "http", + "url": "%s", + "guard-policies": { + "allow-only": { + "repos": "all", + "min-integrity": "none" + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + filteredEnv := filterEnv(os.Environ(), + "GITHUB_REPOSITORY", "GITHUB_TOKEN", "GITHUB_MCP_SERVER_TOKEN", + "GITHUB_PERSONAL_ACCESS_TOKEN", "GH_TOKEN", "GITHUB_API_URL", + "MCP_GATEWAY_FORCE_PUBLIC_REPOS", + ) + filteredEnv = append(filteredEnv, + "GITHUB_REPOSITORY=test-owner/test-repo", + "GITHUB_TOKEN=mock-token-for-testing", + fmt.Sprintf("GITHUB_API_URL=%s", mockAPI.URL), + "MCP_GATEWAY_WASM_GUARDS_DIR=", + ) + cmd.Env = filteredEnv + + var stderr syncBuffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &stderr + + err := cmd.Start() + require.NoError(t, err, "Failed to start gateway") + + ok := waitForStderr(&stderr, "Starting MCPG", 15*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + assert.NotContains(t, logContent, "FORCED REPOS=PUBLIC", + "Should NOT override on API failure (fail-open)") + t.Log("✓ Force-public-repos fails open on API error — gateway starts normally") +} + +// TestForcePublicRepos_NoToken_NoOverride verifies that when no GitHub token is +// available, no override is applied and the gateway starts cleanly. +func TestForcePublicRepos_NoToken_NoOverride(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "github": { + "type": "http", + "url": "%s", + "guard-policies": { + "allow-only": { + "repos": "all", + "min-integrity": "none" + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + // Remove ALL token env vars + filteredEnv := filterEnv(os.Environ(), + "GITHUB_REPOSITORY", "GITHUB_TOKEN", "GITHUB_MCP_SERVER_TOKEN", + "GITHUB_PERSONAL_ACCESS_TOKEN", "GH_TOKEN", "GITHUB_API_URL", + "MCP_GATEWAY_FORCE_PUBLIC_REPOS", + ) + filteredEnv = append(filteredEnv, + "GITHUB_REPOSITORY=test-owner/test-repo", + // No GITHUB_TOKEN set — forces skip + "MCP_GATEWAY_WASM_GUARDS_DIR=", + ) + cmd.Env = filteredEnv + + var stderr syncBuffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &stderr + + err := cmd.Start() + require.NoError(t, err, "Failed to start gateway") + + ok := waitForStderr(&stderr, "Starting MCPG", 15*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + assert.NotContains(t, logContent, "FORCED REPOS=PUBLIC", + "Should NOT override without a GitHub token") + t.Log("✓ No force-public-repos override without token — clean startup") +} + +// TestForcePublicRepos_WriteSinkOnly_NotAffected verifies that a server with +// only a write-sink guard policy (no allow-only) is not affected by the +// force-public-repos override. The override only targets allow-only policies. +func TestForcePublicRepos_WriteSinkOnly_NotAffected(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + mockAPI := startMockGitHubAPI(t, "public", false) + defer mockAPI.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "safe-outputs": { + "type": "http", + "url": "%s", + "guard-policies": { + "write-sink": { + "accept": ["*"], + "sink-visibility": "public" + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + filteredEnv := filterEnv(os.Environ(), + "GITHUB_REPOSITORY", "GITHUB_TOKEN", "GITHUB_MCP_SERVER_TOKEN", + "GITHUB_PERSONAL_ACCESS_TOKEN", "GH_TOKEN", "GITHUB_API_URL", + "MCP_GATEWAY_FORCE_PUBLIC_REPOS", + ) + filteredEnv = append(filteredEnv, + "GITHUB_REPOSITORY=test-owner/test-repo", + "GITHUB_TOKEN=mock-token-for-testing", + fmt.Sprintf("GITHUB_API_URL=%s", mockAPI.URL), + "MCP_GATEWAY_WASM_GUARDS_DIR=", + ) + cmd.Env = filteredEnv + + var stderr syncBuffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &stderr + + err := cmd.Start() + require.NoError(t, err, "Failed to start gateway") + + ok := waitForStderr(&stderr, "Starting MCPG", 15*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + // Write-sink-only servers should NOT get an allow-only override injected + assert.NotContains(t, logContent, "FORCED REPOS=PUBLIC", + "Write-sink-only server should not trigger forced repos=public override") + // But the write-sink guard itself should still be created + assert.Contains(t, logContent, "write-sink guard", + "Write-sink guard should still be created normally") + t.Log("✓ Write-sink-only server unaffected by force-public-repos") +} + +// TestForcePublicRepos_DefaultSinkVisibility_NonSafeOutputs verifies that a +// non-safe-outputs write-sink server gets defaulted to sink-visibility="public" +// when no explicit value is configured. +func TestForcePublicRepos_DefaultSinkVisibility_NonSafeOutputs(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + // Playwright-like server: write-sink with accept but NO sink-visibility + config := fmt.Sprintf(`{ + "mcpServers": { + "playwright": { + "type": "http", + "url": "%s", + "guard-policies": { + "write-sink": { + "accept": ["*"] + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + filteredEnv := filterEnv(os.Environ(), + "GITHUB_REPOSITORY", "GITHUB_TOKEN", "GITHUB_MCP_SERVER_TOKEN", + "GITHUB_PERSONAL_ACCESS_TOKEN", "GH_TOKEN", "GITHUB_API_URL", + "MCP_GATEWAY_FORCE_PUBLIC_REPOS", + ) + filteredEnv = append(filteredEnv, + "GITHUB_REPOSITORY=test-owner/test-repo", + "GITHUB_TOKEN=mock-token-for-testing", + "MCP_GATEWAY_WASM_GUARDS_DIR=", + ) + cmd.Env = filteredEnv + + var stderr syncBuffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &stderr + + err := cmd.Start() + require.NoError(t, err, "Failed to start gateway") + + ok := waitForStderr(&stderr, "Starting MCPG", 15*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + assert.Contains(t, logContent, "Defaulting sink-visibility", + "Non-safe-outputs server should get default sink-visibility=\"public\"") + assert.Contains(t, logContent, "security-by-default", + "Log should mention security-by-default") + t.Log("✓ Non-safe-outputs server gets default sink-visibility=\"public\"") +} + +// TestForcePublicRepos_ExemptServer_NoDefault verifies that a server listed in +// sinkVisibilityExemptServers does NOT get the default sink-visibility="public". +func TestForcePublicRepos_ExemptServer_NoDefault(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + // Playwright is in the exempt list — should NOT get default sink-visibility + config := fmt.Sprintf(`{ + "mcpServers": { + "playwright": { + "type": "http", + "url": "%s", + "guard-policies": { + "write-sink": { + "accept": ["*"] + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key", + "sinkVisibilityExemptServers": ["playwright"] + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + filteredEnv := filterEnv(os.Environ(), + "GITHUB_REPOSITORY", "GITHUB_TOKEN", "GITHUB_MCP_SERVER_TOKEN", + "GITHUB_PERSONAL_ACCESS_TOKEN", "GH_TOKEN", "GITHUB_API_URL", + "MCP_GATEWAY_FORCE_PUBLIC_REPOS", + ) + filteredEnv = append(filteredEnv, + "GITHUB_REPOSITORY=test-owner/test-repo", + "GITHUB_TOKEN=mock-token-for-testing", + "MCP_GATEWAY_WASM_GUARDS_DIR=", + ) + cmd.Env = filteredEnv + + var stderr syncBuffer + cmd.Stdout = &bytes.Buffer{} + cmd.Stderr = &stderr + + err := cmd.Start() + require.NoError(t, err, "Failed to start gateway") + + ok := waitForStderr(&stderr, "Starting MCPG", 15*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + assert.NotContains(t, logContent, "Defaulting sink-visibility", + "Exempt server should NOT get default sink-visibility") + t.Log("✓ Exempt server does not get default sink-visibility=\"public\"") +} diff --git a/test/integration/sink_visibility_test.go b/test/integration/sink_visibility_test.go index 82b07ae33..f20bb93f1 100644 --- a/test/integration/sink_visibility_test.go +++ b/test/integration/sink_visibility_test.go @@ -43,6 +43,15 @@ func startMockGitHubAPI(t *testing.T, visibility string, private bool) *httptest })) } +// startMockGitHubAPIWithStatus starts a mock GitHub API that always returns +// the specified HTTP status code (for testing error/failure scenarios). +func startMockGitHubAPIWithStatus(t *testing.T, statusCode int) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(statusCode) + })) +} + // TestSinkVisibility_ConfigAccepted verifies the gateway starts successfully // with various sink-visibility configurations in the write-sink guard policy. func TestSinkVisibility_ConfigAccepted(t *testing.T) { @@ -78,12 +87,6 @@ func TestSinkVisibility_ConfigAccepted(t *testing.T) { accept: `["*"]`, wantInLog: "write-sink guard", }, - { - name: "public visibility case insensitive", - sinkVisibility: `"PUBLIC"`, - accept: `["*"]`, - wantInLog: "write-sink guard", - }, } for _, tt := range tests { @@ -149,8 +152,8 @@ func TestSinkVisibility_ConfigAccepted(t *testing.T) { } } -// TestSinkVisibility_InvalidValue verifies the gateway falls back to noop guard -// when an invalid sink-visibility value is provided. +// TestSinkVisibility_InvalidValue verifies the gateway rejects invalid +// sink-visibility values via schema validation and exits with an error. func TestSinkVisibility_InvalidValue(t *testing.T) { binary := binaryPath(t) @@ -216,20 +219,23 @@ func TestSinkVisibility_InvalidValue(t *testing.T) { err := cmd.Start() require.NoError(t, err, "Failed to start gateway") - ok := waitForStderr(&stderr, "Starting MCPG", 12*time.Second) - require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) - - cmd.Process.Kill() - cmd.Wait() - - logContent := readUnifiedLog(logDir) - // Invalid sink-visibility causes validation failure → falls back to noop guard - // (the write-sink guard is NOT registered) - assert.Contains(t, logContent, "Registered guard 'noop'", - "Should fall back to noop guard on invalid sink-visibility") - assert.NotContains(t, logContent, "write-sink guard", - "Write-sink guard should NOT be created with invalid sink-visibility") - t.Logf("✓ Invalid sink-visibility=%s causes noop fallback", tt.sinkVisibility) + // Gateway should exit due to schema validation failure + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + + select { + case err := <-done: + // Gateway exited — expected due to schema validation failure + assert.Error(t, err, "Gateway should exit with error on invalid sink-visibility") + output := stderr.String() + assert.Contains(t, output, "sink-visibility", + "Stderr should mention the invalid sink-visibility field") + t.Logf("✓ Invalid sink-visibility=%s rejected by schema validation", tt.sinkVisibility) + case <-time.After(12 * time.Second): + _ = cmd.Process.Kill() + <-done + t.Fatalf("Gateway did not exit within timeout; stderr:\n%s", stderr.String()) + } }) } }