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
9 changes: 8 additions & 1 deletion pkg/workflow/github_tool_to_toolset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 15 additions & 2 deletions pkg/workflow/github_toolsets.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
25 changes: 18 additions & 7 deletions pkg/workflow/github_toolsets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
{
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
},
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/workflow/permissions_toolset_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +112 to +113
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment here implies these GitHub App-only scopes are "validated separately in validateGitHubAppOnlyPermissions", but that validator only checks explicitly declared workflow permissions and doesn’t validate toolset-derived requirements. Consider rewording to clarify the real behavior (App-only scopes are not valid for GITHUB_TOKEN/job permissions, are filtered from rendered workflow permissions, and are only validated when explicitly declared for GitHub App token minting).

Suggested change
// Skip GitHub App-only permission scopes; these cannot be set via GITHUB_TOKEN
// and are validated separately in validateGitHubAppOnlyPermissions.
// Skip GitHub App-only permission scopes; these are not valid for GITHUB_TOKEN/job
// permissions, are excluded from toolset-derived workflow permissions, and are only
// validated when explicitly declared for GitHub App token minting.

Copilot uses AI. Check for mistakes.
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
Expand Down
5 changes: 2 additions & 3 deletions pkg/workflow/permissions_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
{
Expand Down
Loading