From a9b8bf9ada0db7bfe7dc047aaf424a611a036e44 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Tue, 7 Jul 2026 19:26:16 -0700 Subject: [PATCH] Add sink-visibility runtime verification and integration tests - Add runtime repo visibility check via GitHub API at gateway startup - Override configured sink-visibility to 'public' with warning if repo is actually public (defense-in-depth against stale compile-time config) - Gracefully skip check when GITHUB_REPOSITORY or token unavailable - Fall back to configured value on API errors (non-fatal) - Add FetchRepoVisibility and VerifySinkVisibility in githubhttp package - Add comprehensive integration tests covering: - Valid config acceptance (public/private/internal/omitted/case-insensitive) - Invalid values fall back to noop guard - Runtime override emits warning when repo more public than configured - Runtime check skipped without GITHUB_REPOSITORY - Runtime check skipped without GitHub token - API failure graceful fallback - Effective sink-visibility logged correctly Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/CONFIGURATION.md | 11 + internal/githubhttp/visibility.go | 127 +++++ internal/githubhttp/visibility_test.go | 222 ++++++++ internal/server/guard_init.go | 53 +- test/integration/sink_visibility_test.go | 665 +++++++++++++++++++++++ 5 files changed, 1076 insertions(+), 2 deletions(-) create mode 100644 internal/githubhttp/visibility.go create mode 100644 internal/githubhttp/visibility_test.go create mode 100644 test/integration/sink_visibility_test.go diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 0e68ef753..19d3e2cb9 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -446,6 +446,17 @@ to the agent via `label_agent`. The mapping depends on the `repos` configuration - Internal target repo → `sink-visibility: "internal"` — same as private - Without `sink-visibility: "public"`, an agent tricked into reading private data (via prompt injection) can exfiltrate it to a public repo comment (GitLost vulnerability) +**Runtime Verification (defense-in-depth)**: +- At startup, the gateway performs a runtime check against `GET /repos/{owner}/{repo}` using the `GITHUB_REPOSITORY` environment variable +- If the API reports the repo is **public** but the configured `sink-visibility` is not `"public"`, the gateway **overrides** the configured value to `"public"` and emits a warning: + ``` + SINK VISIBILITY OVERRIDE: configured="private" but runtime check shows repo owner/repo is "public" — overriding to "public" to prevent potential data exfiltration + ``` +- This catches cases where a repo was made public **after** the workflow was compiled +- If the API check fails (network error, 404, 403), the gateway falls back to the configured value and logs a warning +- The gateway never relaxes the setting: if configured as `"public"` but the repo is actually private, it keeps `"public"` (more restrictive) + + ## Custom Schemas (`customSchemas`) The `customSchemas` top-level field allows you to define custom server types beyond the built-in `"stdio"` and `"http"` types. Each custom type maps to an HTTPS schema URL that describes its configuration format. diff --git a/internal/githubhttp/visibility.go b/internal/githubhttp/visibility.go new file mode 100644 index 000000000..ecf9b2fb2 --- /dev/null +++ b/internal/githubhttp/visibility.go @@ -0,0 +1,127 @@ +package githubhttp + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "strings" + + "github.com/github/gh-aw-mcpg/internal/logger" +) + +var logVisibility = logger.New("githubhttp:visibility") + +// RepoVisibility represents the visibility of a GitHub repository. +type RepoVisibility string + +const ( + // RepoVisibilityPublic indicates a public repository. + RepoVisibilityPublic RepoVisibility = "public" + // RepoVisibilityPrivate indicates a private repository. + RepoVisibilityPrivate RepoVisibility = "private" + // RepoVisibilityInternal indicates an organization-internal repository. + RepoVisibilityInternal RepoVisibility = "internal" +) + +// repoResponse is the minimal subset of the GitHub repos API response we need. +type repoResponse struct { + Visibility string `json:"visibility"` + Private bool `json:"private"` +} + +// FetchRepoVisibility calls GET /repos/{owner}/{repo} and returns the +// repository's visibility. The nwo parameter should be in "owner/repo" format. +// apiBaseURL is the API root (e.g. "https://api.github.com") and authHeader +// is the full Authorization header value (e.g. "token xyz"). +// +// Returns RepoVisibilityPublic, RepoVisibilityPrivate, or RepoVisibilityInternal. +// On API errors (network, 404, 403) returns an error — callers should treat +// this as non-fatal and fall back to the configured value. +func FetchRepoVisibility(ctx context.Context, apiBaseURL, nwo, authHeader string) (RepoVisibility, error) { + parts := strings.SplitN(nwo, "/", 2) + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return "", fmt.Errorf("invalid repository nwo: %q (expected owner/repo)", nwo) + } + + path := fmt.Sprintf("/repos/%s/%s", parts[0], parts[1]) + logVisibility.Printf("Fetching repo visibility: nwo=%s, apiBaseURL=%s", nwo, apiBaseURL) + + resp, err := DoGitHubGET(ctx, apiBaseURL, path, authHeader) + if err != nil { + return "", fmt.Errorf("failed to fetch repo visibility for %s: %w", nwo, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + logVisibility.Printf("Repo visibility check failed: nwo=%s, status=%d", nwo, resp.StatusCode) + return "", fmt.Errorf("repo visibility check for %s returned status %d", nwo, resp.StatusCode) + } + + var repo repoResponse + if err := json.NewDecoder(resp.Body).Decode(&repo); err != nil { + return "", fmt.Errorf("failed to decode repo response for %s: %w", nwo, err) + } + + // The "visibility" field is available on GitHub.com and GHES 3.x+. + // Fall back to the boolean "private" field for older API versions. + var vis RepoVisibility + switch strings.ToLower(repo.Visibility) { + case "public": + vis = RepoVisibilityPublic + case "internal": + vis = RepoVisibilityInternal + case "private": + vis = RepoVisibilityPrivate + default: + // Fallback: use the "private" boolean + if repo.Private { + vis = RepoVisibilityPrivate + } else { + vis = RepoVisibilityPublic + } + } + + logVisibility.Printf("Repo visibility resolved: nwo=%s, visibility=%s", nwo, vis) + return vis, nil +} + +// VerifySinkVisibility compares the configured sink-visibility against the +// actual repository visibility from the GitHub API. If the actual visibility +// is more public than configured, it returns the actual (more restrictive) +// visibility to prevent exfiltration. +// +// Returns: +// - The effective visibility to use (may override configured value) +// - Whether an override occurred +// - Any error encountered (non-fatal — callers should log and use configured value) +func VerifySinkVisibility(ctx context.Context, apiBaseURL, nwo, authHeader, configuredVisibility string) (string, bool, error) { + if nwo == "" { + return configuredVisibility, false, fmt.Errorf("no repository configured for sink visibility verification") + } + + actual, err := FetchRepoVisibility(ctx, apiBaseURL, nwo, authHeader) + if err != nil { + return configuredVisibility, false, err + } + + actualStr := string(actual) + configured := strings.ToLower(strings.TrimSpace(configuredVisibility)) + + // If actual is "public" but configured is not "public" (or unset), + // override to "public" — this is the security-critical case. + if actual == RepoVisibilityPublic && configured != "public" { + logger.LogWarn("difc", "Sink visibility override: configured=%q but repo %s is actually PUBLIC — overriding to \"public\" to prevent exfiltration", configured, nwo) + return actualStr, true, nil + } + + // If configured says public but actual says private/internal, + // keep "public" (the more restrictive setting) — defense in depth. + if configured == "public" && actual != RepoVisibilityPublic { + logVisibility.Printf("Sink visibility: configured=public but repo %s is %s — keeping public (more restrictive)", nwo, actual) + return "public", false, nil + } + + logVisibility.Printf("Sink visibility verified: configured=%q, actual=%s — no override needed", configured, actual) + return actualStr, false, nil +} diff --git a/internal/githubhttp/visibility_test.go b/internal/githubhttp/visibility_test.go new file mode 100644 index 000000000..70eed79f0 --- /dev/null +++ b/internal/githubhttp/visibility_test.go @@ -0,0 +1,222 @@ +package githubhttp + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFetchRepoVisibility(t *testing.T) { + tests := []struct { + name string + nwo string + response repoResponse + statusCode int + wantVis RepoVisibility + wantErr bool + }{ + { + name: "public repo via visibility field", + nwo: "octo/public-repo", + response: repoResponse{Visibility: "public", Private: false}, + statusCode: http.StatusOK, + wantVis: RepoVisibilityPublic, + }, + { + name: "private repo via visibility field", + nwo: "octo/private-repo", + response: repoResponse{Visibility: "private", Private: true}, + statusCode: http.StatusOK, + wantVis: RepoVisibilityPrivate, + }, + { + name: "internal repo via visibility field", + nwo: "octo/internal-repo", + response: repoResponse{Visibility: "internal", Private: true}, + statusCode: http.StatusOK, + wantVis: RepoVisibilityInternal, + }, + { + name: "fallback to private boolean when visibility empty", + nwo: "octo/old-ghes-repo", + response: repoResponse{Visibility: "", Private: true}, + statusCode: http.StatusOK, + wantVis: RepoVisibilityPrivate, + }, + { + name: "fallback to public when visibility empty and not private", + nwo: "octo/old-public-repo", + response: repoResponse{Visibility: "", Private: false}, + statusCode: http.StatusOK, + wantVis: RepoVisibilityPublic, + }, + { + name: "404 returns error", + nwo: "octo/missing-repo", + statusCode: http.StatusNotFound, + wantErr: true, + }, + { + name: "403 returns error", + nwo: "octo/forbidden-repo", + statusCode: http.StatusForbidden, + wantErr: true, + }, + { + name: "invalid nwo", + nwo: "no-slash", + wantErr: true, + }, + { + name: "empty nwo", + nwo: "", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.nwo == "" || tt.nwo == "no-slash" { + // These fail before making a request + _, err := FetchRepoVisibility(context.Background(), "http://unused", tt.nwo, "token test") + require.Error(t, err) + return + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tt.statusCode) + if tt.statusCode == http.StatusOK { + json.NewEncoder(w).Encode(tt.response) + } + })) + defer server.Close() + + vis, err := FetchRepoVisibility(context.Background(), server.URL, tt.nwo, "token test-token") + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantVis, vis) + }) + } +} + +func TestVerifySinkVisibility(t *testing.T) { + tests := []struct { + name string + configured string + actualVis string // JSON visibility field from API + actualPrivate bool + wantEffective string + wantOverridden bool + wantErr bool + }{ + { + name: "configured private but repo is public — override to public", + configured: "private", + actualVis: "public", + actualPrivate: false, + wantEffective: "public", + wantOverridden: true, + }, + { + name: "configured empty but repo is public — override to public", + configured: "", + actualVis: "public", + actualPrivate: false, + wantEffective: "public", + wantOverridden: true, + }, + { + name: "configured internal but repo is public — override to public", + configured: "internal", + actualVis: "public", + actualPrivate: false, + wantEffective: "public", + wantOverridden: true, + }, + { + name: "configured public and repo is public — no override", + configured: "public", + actualVis: "public", + actualPrivate: false, + wantEffective: "public", + wantOverridden: false, + }, + { + name: "configured public but repo is private — keep public (more restrictive)", + configured: "public", + actualVis: "private", + actualPrivate: true, + wantEffective: "public", + wantOverridden: false, + }, + { + name: "configured private and repo is private — no override", + configured: "private", + actualVis: "private", + actualPrivate: true, + wantEffective: "private", + wantOverridden: false, + }, + { + name: "configured private and repo is internal — no override", + configured: "private", + actualVis: "internal", + actualPrivate: true, + wantEffective: "internal", + wantOverridden: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + json.NewEncoder(w).Encode(repoResponse{ + Visibility: tt.actualVis, + Private: tt.actualPrivate, + }) + })) + defer server.Close() + + effective, overridden, err := VerifySinkVisibility( + context.Background(), server.URL, "octo/test-repo", "token xyz", tt.configured, + ) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantEffective, effective) + assert.Equal(t, tt.wantOverridden, overridden) + }) + } +} + +func TestVerifySinkVisibility_EmptyNWO(t *testing.T) { + _, _, err := VerifySinkVisibility(context.Background(), "http://unused", "", "token xyz", "private") + assert.Error(t, err) + assert.Contains(t, err.Error(), "no repository configured") +} + +func TestVerifySinkVisibility_APIError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + effective, overridden, err := VerifySinkVisibility( + context.Background(), server.URL, "octo/broken", "token xyz", "private", + ) + assert.Error(t, err) + // On error, returns configured value unchanged + assert.Equal(t, "private", effective) + assert.False(t, overridden) +} diff --git a/internal/server/guard_init.go b/internal/server/guard_init.go index 852768ad5..0babf4d3e 100644 --- a/internal/server/guard_init.go +++ b/internal/server/guard_init.go @@ -4,10 +4,14 @@ import ( "context" "encoding/json" "fmt" + "os" "path/filepath" + "time" "github.com/github/gh-aw-mcpg/internal/config" "github.com/github/gh-aw-mcpg/internal/difc" + "github.com/github/gh-aw-mcpg/internal/envutil" + "github.com/github/gh-aw-mcpg/internal/githubhttp" "github.com/github/gh-aw-mcpg/internal/guard" "github.com/github/gh-aw-mcpg/internal/logger" "github.com/github/gh-aw-mcpg/internal/util" @@ -61,8 +65,9 @@ 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 { - g = guard.NewWriteSinkGuardWithVisibility(ws.Accept, ws.SinkVisibility) - logger.LogInfoToServer(serverID, "difc", "Created write-sink guard with %d accept patterns, sink-visibility=%q", len(ws.Accept), ws.SinkVisibility) + effectiveVisibility := us.verifySinkVisibilityAtRuntime(serverID, ws.SinkVisibility) + 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) } } @@ -376,3 +381,47 @@ func (us *UnifiedServer) getTrustedBots() []string { } return us.cfg.Gateway.TrustedBots } + +// verifySinkVisibilityAtRuntime checks the actual repository visibility via the +// GitHub API and overrides the configured sink-visibility if the repo is more +// public than declared. This is a defense-in-depth measure: even if the compile- +// time config says "private", a runtime check catches cases where the repo was +// made public after the workflow was compiled. +// +// Emits a warning when overriding the configured value. +// Falls back to the configured value on any API error (non-fatal). +func (us *UnifiedServer) verifySinkVisibilityAtRuntime(serverID, configuredVisibility string) string { + nwo := os.Getenv("GITHUB_REPOSITORY") + if nwo == "" { + logGuardInit.Printf("sink-visibility runtime check skipped: GITHUB_REPOSITORY not set (serverID=%s)", serverID) + return configuredVisibility + } + + token := envutil.LookupGitHubToken() + if token == "" { + logGuardInit.Printf("sink-visibility runtime check skipped: no GitHub token available (serverID=%s)", serverID) + 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) + return configuredVisibility + } + + if overridden { + 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) + } else { + logger.LogInfoToServer(serverID, "difc", "Sink visibility runtime verification passed: repo=%s, visibility=%q", nwo, effective) + } + + return effective +} diff --git a/test/integration/sink_visibility_test.go b/test/integration/sink_visibility_test.go new file mode 100644 index 000000000..82b07ae33 --- /dev/null +++ b/test/integration/sink_visibility_test.go @@ -0,0 +1,665 @@ +package integration + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// startMockBackend starts a simple HTTP server that returns 500 for all requests. +// This allows the gateway to start without needing Docker. +func startMockBackend(t *testing.T) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte(`{"error":"mock backend"}`)) + })) +} + +// startMockGitHubAPI starts a mock GitHub API that returns the specified visibility. +func startMockGitHubAPI(t *testing.T, visibility string, private bool) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/repos/") { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]interface{}{ + "visibility": visibility, + "private": private, + }) + return + } + w.WriteHeader(http.StatusNotFound) + })) +} + +// TestSinkVisibility_ConfigAccepted verifies the gateway starts successfully +// with various sink-visibility configurations in the write-sink guard policy. +func TestSinkVisibility_ConfigAccepted(t *testing.T) { + binary := binaryPath(t) + + tests := []struct { + name string + sinkVisibility string + accept string + wantInLog string + }{ + { + name: "public visibility", + sinkVisibility: `"public"`, + accept: `["*"]`, + wantInLog: "write-sink guard", + }, + { + name: "private visibility", + sinkVisibility: `"private"`, + accept: `["private:owner/repo"]`, + wantInLog: "write-sink guard", + }, + { + name: "internal visibility", + sinkVisibility: `"internal"`, + accept: `["private:owner/repo"]`, + wantInLog: "write-sink guard", + }, + { + name: "omitted visibility (backward compat)", + sinkVisibility: "", // will not include the field + accept: `["*"]`, + wantInLog: "write-sink guard", + }, + { + name: "public visibility case insensitive", + sinkVisibility: `"PUBLIC"`, + accept: `["*"]`, + wantInLog: "write-sink guard", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + var writeSinkJSON string + if tt.sinkVisibility == "" { + writeSinkJSON = fmt.Sprintf(`{"accept": %s}`, tt.accept) + } else { + writeSinkJSON = fmt.Sprintf(`{"accept": %s, "sink-visibility": %s}`, tt.accept, tt.sinkVisibility) + } + + config := fmt.Sprintf(`{ + "mcpServers": { + "safe-outputs": { + "type": "http", + "url": "%s", + "guard-policies": { + "write-sink": %s + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, writeSinkJSON, port) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + // Remove GITHUB_REPOSITORY to skip runtime check + 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", 12*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, tt.wantInLog, + "Log should contain write-sink guard registration") + t.Logf("✓ sink-visibility=%s accepted", tt.sinkVisibility) + }) + } +} + +// TestSinkVisibility_InvalidValue verifies the gateway falls back to noop guard +// when an invalid sink-visibility value is provided. +func TestSinkVisibility_InvalidValue(t *testing.T) { + binary := binaryPath(t) + + tests := []struct { + name string + sinkVisibility string + }{ + { + name: "invalid value foo", + sinkVisibility: `"foo"`, + }, + { + name: "invalid value restricted", + sinkVisibility: `"restricted"`, + }, + { + name: "invalid numeric value", + sinkVisibility: `"123"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "safe-outputs": { + "type": "http", + "url": "%s", + "guard-policies": { + "write-sink": { + "accept": ["*"], + "sink-visibility": %s + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, tt.sinkVisibility, port) + + ctx, cancel := context.WithTimeout(context.Background(), 15*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") + 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", 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) + }) + } +} + +// TestSinkVisibility_RuntimeOverride verifies that the gateway overrides +// sink-visibility when the GitHub API reports the repo is public but config +// says private. Uses a mock GitHub API server. +func TestSinkVisibility_RuntimeOverride(t *testing.T) { + binary := binaryPath(t) + + tests := []struct { + name string + configuredVis string + apiVisibility string + apiPrivate bool + wantOverrideLog string + wantNoOverrideLog string + }{ + { + name: "private config but public repo — override with warning", + configuredVis: "private", + apiVisibility: "public", + apiPrivate: false, + wantOverrideLog: "SINK VISIBILITY OVERRIDE", + }, + { + name: "internal config but public repo — override with warning", + configuredVis: "internal", + apiVisibility: "public", + apiPrivate: false, + wantOverrideLog: "SINK VISIBILITY OVERRIDE", + }, + { + name: "public config and public repo — no override", + configuredVis: "public", + apiVisibility: "public", + apiPrivate: false, + wantNoOverrideLog: "runtime verification passed", + }, + { + name: "private config and private repo — no override", + configuredVis: "private", + apiVisibility: "private", + apiPrivate: true, + wantNoOverrideLog: "runtime verification passed", + }, + { + name: "public config but private repo — keep public (more restrictive)", + configuredVis: "public", + apiVisibility: "private", + apiPrivate: true, + wantNoOverrideLog: "runtime verification passed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + mockAPI := startMockGitHubAPI(t, tt.apiVisibility, tt.apiPrivate) + defer mockAPI.Close() + + writeSinkJSON := fmt.Sprintf(`{"accept": ["*"], "sink-visibility": "%s"}`, tt.configuredVis) + + config := fmt.Sprintf(`{ + "mcpServers": { + "safe-outputs": { + "type": "http", + "url": "%s", + "guard-policies": { + "write-sink": %s + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, writeSinkJSON, port) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + // Remove all GitHub token vars and set our mock values + 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", 12*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + + if tt.wantOverrideLog != "" { + assert.Contains(t, logContent, tt.wantOverrideLog, + "Log should contain override warning when repo is more public than configured") + t.Logf("✓ Override warning emitted: configured=%q, actual=%s", tt.configuredVis, tt.apiVisibility) + } + if tt.wantNoOverrideLog != "" { + assert.Contains(t, logContent, tt.wantNoOverrideLog, + "Log should confirm verification passed when no override needed") + assert.NotContains(t, logContent, "SINK VISIBILITY OVERRIDE", + "Should NOT contain override warning") + t.Logf("✓ No override: configured=%q, actual=%s", tt.configuredVis, tt.apiVisibility) + } + }) + } +} + +// TestSinkVisibility_RuntimeCheckSkippedWithoutRepo verifies that when +// GITHUB_REPOSITORY is not set, the runtime check is skipped gracefully. +func TestSinkVisibility_RuntimeCheckSkippedWithoutRepo(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "safe-outputs": { + "type": "http", + "url": "%s", + "guard-policies": { + "write-sink": { + "accept": ["*"], + "sink-visibility": "private" + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + + // Explicitly remove GITHUB_REPOSITORY from env + 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", 12*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, "write-sink guard", + "Write-sink guard should still be created without GITHUB_REPOSITORY") + assert.NotContains(t, logContent, "SINK VISIBILITY OVERRIDE", + "Should not override when no runtime check was performed") + t.Log("✓ Runtime check gracefully skipped without GITHUB_REPOSITORY") +} + +// TestSinkVisibility_RuntimeCheckSkippedWithoutToken verifies that when +// no GitHub token is available, the runtime check is skipped gracefully. +func TestSinkVisibility_RuntimeCheckSkippedWithoutToken(t *testing.T) { + binary := binaryPath(t) + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "safe-outputs": { + "type": "http", + "url": "%s", + "guard-policies": { + "write-sink": { + "accept": ["*"], + "sink-visibility": "private" + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + + // Remove all GitHub token env vars but keep GITHUB_REPOSITORY + filteredEnv := filterEnv(os.Environ(), + "GITHUB_TOKEN", "GITHUB_MCP_SERVER_TOKEN", + "GITHUB_PERSONAL_ACCESS_TOKEN", "GH_TOKEN", + ) + filteredEnv = append(filteredEnv, + "GITHUB_REPOSITORY=test-owner/test-repo", + "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", 12*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, "write-sink guard", + "Write-sink guard should still be created without a token") + assert.NotContains(t, logContent, "SINK VISIBILITY OVERRIDE", + "Should not override when no runtime check was performed") + t.Log("✓ Runtime check gracefully skipped without GitHub token") +} + +// TestSinkVisibility_RuntimeCheckAPIFailure verifies that when the GitHub API +// returns an error, the gateway falls back to the configured value with a warning. +func TestSinkVisibility_RuntimeCheckAPIFailure(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 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer mockAPI.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "safe-outputs": { + "type": "http", + "url": "%s", + "guard-policies": { + "write-sink": { + "accept": ["*"], + "sink-visibility": "private" + } + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, port) + + ctx, cancel := context.WithTimeout(context.Background(), 15*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", + 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", 12*time.Second) + require.Truef(t, ok, "timeout waiting for startup; stderr:\n%s", stderr.String()) + + cmd.Process.Kill() + cmd.Wait() + + logContent := readUnifiedLog(logDir) + // Should log a warning about the failed check but still start + assert.Contains(t, logContent, "write-sink guard", + "Write-sink guard should still be created on API failure") + assert.Contains(t, logContent, "runtime verification failed", + "Should log warning about failed verification") + assert.NotContains(t, logContent, "SINK VISIBILITY OVERRIDE", + "Should not override on API failure — keeps configured value") + t.Log("✓ API failure handled gracefully — falls back to configured value with warning") +} + +// TestSinkVisibility_WriteSinkGuardLogsSinkVisibility verifies that the +// write-sink guard logs include the effective sink-visibility value. +func TestSinkVisibility_WriteSinkGuardLogsSinkVisibility(t *testing.T) { + binary := binaryPath(t) + + tests := []struct { + name string + config string + wantInLog string + }{ + { + name: "public visibility logged", + config: `{"accept": ["*"], "sink-visibility": "public"}`, + wantInLog: `sink-visibility="public"`, + }, + { + name: "private visibility logged", + config: `{"accept": ["private:org/repo"], "sink-visibility": "private"}`, + wantInLog: `sink-visibility="private"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + port := getFreePort(t) + logDir := t.TempDir() + + backend := startMockBackend(t) + defer backend.Close() + + config := fmt.Sprintf(`{ + "mcpServers": { + "safe-outputs": { + "type": "http", + "url": "%s", + "guard-policies": { + "write-sink": %s + } + } + }, + "gateway": { + "port": %d, + "domain": "localhost", + "agentId": "test-key" + } + }`, backend.URL, tt.config, port) + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, binary, "--config-stdin", "--log-dir", logDir) + cmd.Stdin = strings.NewReader(config) + // Remove GITHUB_REPOSITORY to skip runtime check + 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", 12*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, tt.wantInLog, + "Log should contain the effective sink-visibility") + }) + } +} + +// filterEnv returns a copy of env with the specified keys removed. +func filterEnv(env []string, keys ...string) []string { + keySet := make(map[string]bool, len(keys)) + for _, k := range keys { + keySet[k] = true + } + filtered := make([]string, 0, len(env)) + for _, e := range env { + key := strings.SplitN(e, "=", 2)[0] + if !keySet[key] { + filtered = append(filtered, e) + } + } + return filtered +}