diff --git a/pkg/workflow/github_tool_to_toolset_test.go b/pkg/workflow/github_tool_to_toolset_test.go index 8815d1082d..47b2dd969c 100644 --- a/pkg/workflow/github_tool_to_toolset_test.go +++ b/pkg/workflow/github_tool_to_toolset_test.go @@ -291,8 +291,15 @@ func expandToolsetsForTesting(toolsets []string) []string { } } case "all": - // Add all toolsets from the permissions map + // Add all toolsets from the permissions map, excluding those in GitHubToolsetsExcludedFromAll + excludedMap := make(map[string]bool, len(GitHubToolsetsExcludedFromAll)) + for _, ex := range GitHubToolsetsExcludedFromAll { + excludedMap[ex] = true + } for t := range toolsetPermissionsMap { + if excludedMap[t] { + continue + } if !seenToolsets[t] { expanded = append(expanded, t) seenToolsets[t] = true diff --git a/pkg/workflow/github_toolsets.go b/pkg/workflow/github_toolsets.go index fad70f13e3..cc916b4fef 100644 --- a/pkg/workflow/github_toolsets.go +++ b/pkg/workflow/github_toolsets.go @@ -18,6 +18,11 @@ var DefaultGitHubToolsets = []string{"context", "repos", "issues", "pull_request // Use this when the workflow will run in GitHub Actions with GITHUB_TOKEN. var ActionFriendlyGitHubToolsets = []string{"context", "repos", "issues", "pull_requests"} +// GitHubToolsetsExcludedFromAll defines toolsets that are NOT included when "all" is specified. +// These toolsets require GitHub App-only permissions (e.g., vulnerability-alerts) that +// cannot be granted via GITHUB_TOKEN, so they must be opted-in to explicitly. +var GitHubToolsetsExcludedFromAll = []string{"dependabot"} + // ParseGitHubToolsets parses the toolsets string and expands "default" and "all" // into their constituent toolsets. It handles comma-separated lists and deduplicates. func ParseGitHubToolsets(toolsetsStr string) []string { @@ -58,9 +63,17 @@ func ParseGitHubToolsets(toolsetsStr string) []string { } } case "all": - // Add all toolsets from the toolset permissions map - toolsetsLog.Printf("Expanding 'all' to %d toolsets from permissions map", len(toolsetPermissionsMap)) + // Add all toolsets from the toolset permissions map, excluding those that + // require GitHub App-only permissions (see GitHubToolsetsExcludedFromAll). + toolsetsLog.Printf("Expanding 'all' to toolsets from permissions map (excluding %v)", GitHubToolsetsExcludedFromAll) + excludedMap := make(map[string]bool, len(GitHubToolsetsExcludedFromAll)) + for _, ex := range GitHubToolsetsExcludedFromAll { + excludedMap[ex] = true + } for t := range toolsetPermissionsMap { + if excludedMap[t] { + continue + } if !seenToolsets[t] { expanded = append(expanded, t) seenToolsets[t] = true diff --git a/pkg/workflow/github_toolsets_test.go b/pkg/workflow/github_toolsets_test.go index fe4b5996e4..8fdb9ca433 100644 --- a/pkg/workflow/github_toolsets_test.go +++ b/pkg/workflow/github_toolsets_test.go @@ -70,9 +70,9 @@ func TestParseGitHubToolsets(t *testing.T) { expected: []string{"context", "repos", "issues", "pull_requests", "discussions"}, }, { - name: "All expands to all toolsets", + name: "All expands to all toolsets except excluded ones", input: "all", - // Should include all 19 toolsets - we'll check the count + // Should include all toolsets except those in GitHubToolsetsExcludedFromAll (e.g., dependabot) expected: nil, }, { @@ -111,10 +111,21 @@ func TestParseGitHubToolsets(t *testing.T) { t.Run(tt.name, func(t *testing.T) { result := ParseGitHubToolsets(tt.input) - if tt.name == "All expands to all toolsets" { - // Check that all toolsets are present - if len(result) != len(toolsetPermissionsMap) { - t.Errorf("Expected %d toolsets for 'all', got %d", len(toolsetPermissionsMap), len(result)) + if tt.name == "All expands to all toolsets except excluded ones" { + // Check that result is all toolsets minus excluded ones + expectedCount := len(toolsetPermissionsMap) - len(GitHubToolsetsExcludedFromAll) + if len(result) != expectedCount { + t.Errorf("Expected %d toolsets for 'all', got %d: %v", expectedCount, len(result), result) + } + // Verify excluded toolsets are not present + resultMap := make(map[string]bool) + for _, ts := range result { + resultMap[ts] = true + } + for _, ex := range GitHubToolsetsExcludedFromAll { + if resultMap[ex] { + t.Errorf("Excluded toolset %q should not be present in 'all' expansion", ex) + } } return } @@ -175,7 +186,7 @@ func TestParseGitHubToolsetsDeduplication(t *testing.T) { { name: "All with duplicates", input: "all,repos,issues", - expected: len(toolsetPermissionsMap), // All toolsets, duplicates ignored + expected: len(toolsetPermissionsMap) - len(GitHubToolsetsExcludedFromAll), // All toolsets minus excluded, duplicates ignored }, } diff --git a/pkg/workflow/permissions_toolset_data.go b/pkg/workflow/permissions_toolset_data.go index 80538014fa..60bf7534f7 100644 --- a/pkg/workflow/permissions_toolset_data.go +++ b/pkg/workflow/permissions_toolset_data.go @@ -109,6 +109,12 @@ func collectRequiredPermissions(toolsets []string, readOnly bool) map[Permission // Add read permissions only (write tools are not considered for permission requirements) for _, scope := range perms.ReadPermissions { + // Skip GitHub App-only permission scopes; these cannot be set via GITHUB_TOKEN + // and are validated separately in validateGitHubAppOnlyPermissions. + if IsGitHubAppOnlyScope(scope) { + permissionsValidationLog.Printf("Skipping GitHub App-only scope %s for toolset %s", scope, toolset) + continue + } // Always require at least read access if existing, found := required[scope]; !found || existing == PermissionNone { required[scope] = PermissionRead diff --git a/pkg/workflow/permissions_validator_test.go b/pkg/workflow/permissions_validator_test.go index b871783e32..a4da6b7b03 100644 --- a/pkg/workflow/permissions_validator_test.go +++ b/pkg/workflow/permissions_validator_test.go @@ -89,12 +89,11 @@ func TestCollectRequiredPermissions(t *testing.T) { }, }, { - name: "Dependabot toolset requires vulnerability-alerts and security-events", + name: "Dependabot toolset requires only security-events (vulnerability-alerts is GitHub App-only)", toolsets: []string{"dependabot"}, readOnly: false, expected: map[PermissionScope]PermissionLevel{ - PermissionSecurityEvents: PermissionRead, - PermissionVulnerabilityAlerts: PermissionRead, + PermissionSecurityEvents: PermissionRead, }, }, {