Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 12 additions & 17 deletions internal/githubhttp/visibility.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ func FetchRepoVisibility(ctx context.Context, apiBaseURL, nwo, authHeader string
}

// 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.
// actual repository visibility from the GitHub API. Only overrides the
// configured value in the security-critical case: repo is actually public
// but config doesn't say "public".
//
// Returns:
// - The effective visibility to use (may override configured value)
// - The effective visibility to use (configured value unless overridden to "public")
// - 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) {
Expand All @@ -105,23 +105,18 @@ func VerifySinkVisibility(ctx context.Context, apiBaseURL, nwo, authHeader, conf
return configuredVisibility, false, err
}

actualStr := string(actual)
configured := strings.ToLower(strings.TrimSpace(configuredVisibility))

// If actual is "public" but configured is not "public" (or unset),
// If actual is "public" but configured is not "public",
// 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
return "public", 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
// All other cases: return the configured value unchanged.
// This includes:
// - configured="public" but actual=private → keep "public" (more restrictive)
// - configured="private" and actual=private → no change needed
// - configured="internal" and actual=internal → no change needed
return configured, false, nil
}
2 changes: 1 addition & 1 deletion internal/githubhttp/visibility_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func TestVerifySinkVisibility(t *testing.T) {
configured: "private",
actualVis: "internal",
actualPrivate: true,
wantEffective: "internal",
wantEffective: "private",
wantOverridden: false,
},
}
Expand Down
8 changes: 8 additions & 0 deletions internal/server/guard_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,14 @@ func (us *UnifiedServer) getTrustedBots() []string {
// 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 {
// Skip runtime check when sink-visibility is not explicitly configured.
// This preserves backward-compatible behavior where omitted sink-visibility
// uses accept patterns without any override.
if configuredVisibility == "" {
logGuardInit.Printf("sink-visibility runtime check skipped: sink-visibility not configured (serverID=%s)", serverID)
return configuredVisibility
}

nwo := os.Getenv("GITHUB_REPOSITORY")
if nwo == "" {
logGuardInit.Printf("sink-visibility runtime check skipped: GITHUB_REPOSITORY not set (serverID=%s)", serverID)
Expand Down
Loading