diff --git a/pkg/cli/compile_permissions_integration_test.go b/pkg/cli/compile_permissions_integration_test.go new file mode 100644 index 00000000000..9d9cf5a186d --- /dev/null +++ b/pkg/cli/compile_permissions_integration_test.go @@ -0,0 +1,92 @@ +//go:build integration + +package cli + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + goyaml "github.com/goccy/go-yaml" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestCompileVulnerabilityAlertsPermissionFiltered compiles the canonical +// test-vulnerability-alerts-permission.md workflow file and verifies that +// the GitHub App-only `vulnerability-alerts` scope does NOT appear in any +// job-level permissions block of the compiled lock file. +// +// GitHub Actions rejects workflows that declare App-only permissions at the +// job level (e.g. vulnerability-alerts, members, administration). These scopes +// must only appear as `permission-*` inputs to actions/create-github-app-token. +func TestCompileVulnerabilityAlertsPermissionFiltered(t *testing.T) { + setup := setupIntegrationTest(t) + defer setup.cleanup() + + // Copy the canonical workflow file into the test's .github/workflows dir + srcPath := filepath.Join(projectRoot, "pkg/cli/workflows/test-vulnerability-alerts-permission.md") + dstPath := filepath.Join(setup.workflowsDir, "test-vulnerability-alerts-permission.md") + + srcContent, err := os.ReadFile(srcPath) + require.NoError(t, err, "Failed to read source workflow file %s", srcPath) + require.NoError(t, os.WriteFile(dstPath, srcContent, 0644), "Failed to write workflow to test dir") + + // Compile the workflow using the pre-built binary + cmd := exec.Command(setup.binaryPath, "compile", dstPath) + output, err := cmd.CombinedOutput() + require.NoError(t, err, "CLI compile command failed:\n%s", string(output)) + + // Read the compiled lock file + lockFilePath := filepath.Join(setup.workflowsDir, "test-vulnerability-alerts-permission.lock.yml") + lockContent, err := os.ReadFile(lockFilePath) + require.NoError(t, err, "Failed to read lock file") + lockContentStr := string(lockContent) + + // The App token minting step must receive `permission-vulnerability-alerts: read` as input. + assert.Contains(t, lockContentStr, "permission-vulnerability-alerts: read", + "App token minting step should include permission-vulnerability-alerts: read") + assert.Contains(t, lockContentStr, "id: github-mcp-app-token", + "GitHub App token minting step should be generated") + + // Critically: vulnerability-alerts must NOT appear inside any job-level permissions block. + // Parse the YAML and walk every job's permissions map to check. + var workflow map[string]any + require.NoError(t, goyaml.Unmarshal(lockContent, &workflow), + "Lock file should be valid YAML") + + jobs, ok := workflow["jobs"].(map[string]any) + require.True(t, ok, "Lock file should have a jobs section") + + for jobName, jobConfig := range jobs { + jobMap, ok := jobConfig.(map[string]any) + if !ok { + continue + } + perms, hasPerms := jobMap["permissions"] + if !hasPerms { + continue + } + permsMap, ok := perms.(map[string]any) + if !ok { + // Shorthand permissions (read-all / write-all / none) — nothing to check + continue + } + assert.NotContains(t, permsMap, "vulnerability-alerts", + "Job %q must not have vulnerability-alerts in job-level permissions block "+ + "(it is a GitHub App-only scope and not a valid GitHub Actions permission)", jobName) + } + + // Extra belt-and-suspenders: the string "vulnerability-alerts: read" must not appear + // anywhere other than inside the App token step inputs. + // We verify by counting occurrences: exactly one occurrence for the App token step. + occurrences := strings.Count(lockContentStr, "vulnerability-alerts: read") + // The permission-vulnerability-alerts: read line contains "vulnerability-alerts: read" + // as a substring, so we count that and only that occurrence. + appTokenOccurrences := strings.Count(lockContentStr, "permission-vulnerability-alerts: read") + assert.Equal(t, appTokenOccurrences, occurrences, + "vulnerability-alerts: read should appear only inside the App token step inputs, not elsewhere in the lock file\nLock file:\n%s", + lockContentStr) +} diff --git a/pkg/cli/workflows/test-vulnerability-alerts-permission.md b/pkg/cli/workflows/test-vulnerability-alerts-permission.md new file mode 100644 index 00000000000..0bb221efb3c --- /dev/null +++ b/pkg/cli/workflows/test-vulnerability-alerts-permission.md @@ -0,0 +1,25 @@ +--- +on: + issues: + types: [opened] +permissions: + contents: read + pull-requests: read + security-events: read + vulnerability-alerts: read +strict: false +engine: copilot +tools: + github: + toolsets: [dependabot] + github-app: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} +--- + +# Vulnerability Alerts with GitHub App Token + +This workflow uses the Dependabot toolset with a GitHub App token. +The `vulnerability-alerts: read` permission is a GitHub App-only scope and must NOT +appear in the compiled job-level `permissions:` block. It should only appear as a +`permission-vulnerability-alerts: read` input to the `create-github-app-token` step. diff --git a/pkg/workflow/compiler_main_job.go b/pkg/workflow/compiler_main_job.go index b2189c04f6c..84ea46fe483 100644 --- a/pkg/workflow/compiler_main_job.go +++ b/pkg/workflow/compiler_main_job.go @@ -251,7 +251,12 @@ func (c *Compiler) buildMainJob(data *WorkflowData, activationJobCreated bool) ( // Set up permissions for the agent job // In dev/script mode, automatically add contents: read if the actions folder checkout is needed // In release mode, use the permissions as specified by the user (no automatic augmentation) - permissions := data.Permissions + // + // GitHub App-only permissions (e.g., vulnerability-alerts) must be filtered out before + // rendering to the job-level permissions block. These scopes are not valid GitHub Actions + // workflow permissions and cause a parse error when queued. They are handled separately + // when minting GitHub App installation access tokens (as permission-* inputs). + permissions := filterJobLevelPermissions(data.Permissions) needsContentsRead := (c.actionMode.IsDev() || c.actionMode.IsScript()) && len(c.generateCheckoutActionsFolder(data)) > 0 if needsContentsRead { if permissions == "" { diff --git a/pkg/workflow/github_mcp_app_token_test.go b/pkg/workflow/github_mcp_app_token_test.go index a2dc2719921..609cd2d48f5 100644 --- a/pkg/workflow/github_mcp_app_token_test.go +++ b/pkg/workflow/github_mcp_app_token_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + goyaml "github.com/goccy/go-yaml" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -359,4 +360,28 @@ Test that permission-vulnerability-alerts is emitted in the App token minting st assert.Contains(t, lockContent, "permission-security-events: read", "Should also include security-events read permission in App token") // Verify the token minting step is present assert.Contains(t, lockContent, "id: github-mcp-app-token", "GitHub App token step should be generated") + // Verify that vulnerability-alerts does NOT appear in any job-level permissions block. + // It is a GitHub App-only permission and not a valid GitHub Actions workflow permission; + // GitHub Actions rejects workflows that declare it at the job level. + var workflow map[string]any + require.NoError(t, goyaml.Unmarshal(content, &workflow), "Lock file should be valid YAML") + jobs, ok := workflow["jobs"].(map[string]any) + require.True(t, ok, "Should have jobs section") + for jobName, jobConfig := range jobs { + jobMap, ok := jobConfig.(map[string]any) + if !ok { + continue + } + perms, hasPerms := jobMap["permissions"] + if !hasPerms { + continue + } + permsMap, ok := perms.(map[string]any) + if !ok { + continue + } + if _, found := permsMap["vulnerability-alerts"]; found { + t.Errorf("Job %q should not have vulnerability-alerts in job-level permissions block (it is a GitHub App-only permission)", jobName) + } + } } diff --git a/pkg/workflow/permissions_operations.go b/pkg/workflow/permissions_operations.go index 00ab7f5af81..534ea0e113a 100644 --- a/pkg/workflow/permissions_operations.go +++ b/pkg/workflow/permissions_operations.go @@ -18,6 +18,52 @@ func SortPermissionScopes(s []PermissionScope) { }) } +// filterJobLevelPermissions takes a raw permissions YAML string (as stored in WorkflowData.Permissions) +// and returns a version suitable for use in a GitHub Actions job-level permissions block. +// +// GitHub App-only permission scopes (e.g., vulnerability-alerts, members, administration) are not +// valid GitHub Actions workflow permissions and cause a parse error when GitHub Actions tries to +// queue the workflow. Those scopes must only appear as permission-* inputs when minting GitHub App +// installation access tokens via actions/create-github-app-token, not in the job-level block. +// +// RenderToYAML already skips App-only scopes; this function converts the raw YAML string through +// the Permissions struct so that filtering is applied before job-level rendering. +// The returned string uses 2-space indentation so that the caller's subsequent +// indentYAMLLines(" ") call adds 4 spaces, producing the correct 6-space job-level +// indentation in the final YAML (matching the renderJob format). +// +// If the input YAML is malformed or contains only App-only scopes, an empty string is returned +// so the caller omits the permissions block entirely rather than emitting invalid YAML. +func filterJobLevelPermissions(rawPermissionsYAML string) string { + if rawPermissionsYAML == "" { + return "" + } + + filtered := NewPermissionsParser(rawPermissionsYAML).ToPermissions() + rendered := filtered.RenderToYAML() + if rendered == "" { + return "" + } + + // RenderToYAML hard-codes 6-space indentation for permission values so that shorthand + // callers that embed the output directly into a job block get the right alignment: + // permissions: ← first line, 4 spaces added by renderJob's fmt.Fprintf + // contents: read ← 6 spaces from RenderToYAML → total 10 would be wrong + // Here we normalise back to 2-space indentation. The caller will then run + // indentYAMLLines(" "), adding 4 spaces to lines 1+, yielding 6 spaces total. + const renderYAMLIndent = 6 // spaces used by RenderToYAML for permission value lines + const targetIndent = 2 // spaces we want here so indentYAMLLines(" ") gives 6 + prefix := strings.Repeat(" ", renderYAMLIndent) + replacement := strings.Repeat(" ", targetIndent) + lines := strings.Split(rendered, "\n") + for i := 1; i < len(lines); i++ { + if strings.HasPrefix(lines[i], prefix) { + lines[i] = replacement + lines[i][renderYAMLIndent:] + } + } + return strings.Join(lines, "\n") +} + // Set sets a permission for a specific scope func (p *Permissions) Set(scope PermissionScope, level PermissionLevel) { permissionsOpsLog.Printf("Setting permission: scope=%s, level=%s", scope, level) diff --git a/pkg/workflow/permissions_operations_test.go b/pkg/workflow/permissions_operations_test.go index 05518baac88..6b15cd28b9e 100644 --- a/pkg/workflow/permissions_operations_test.go +++ b/pkg/workflow/permissions_operations_test.go @@ -3,6 +3,7 @@ package workflow import ( + "strings" "testing" ) @@ -602,3 +603,96 @@ func TestPermissions_AllRead(t *testing.T) { }) } } + +func TestFilterJobLevelPermissions(t *testing.T) { + tests := []struct { + name string + input string + expectEmpty bool + contains []string + excludes []string + }{ + { + name: "empty input returns empty", + input: "", + expectEmpty: true, + contains: []string{}, + excludes: []string{}, + }, + { + name: "standard permissions are preserved", + input: "permissions:\n contents: read\n issues: write", + contains: []string{ + "permissions:", + " contents: read", + " issues: write", + }, + excludes: []string{}, + }, + { + name: "vulnerability-alerts is filtered out", + input: "permissions:\n contents: read\n pull-requests: read\n security-events: read\n vulnerability-alerts: read", + contains: []string{ + "permissions:", + " contents: read", + " pull-requests: read", + " security-events: read", + }, + excludes: []string{"vulnerability-alerts"}, + }, + { + name: "multiple GitHub App-only scopes are filtered out", + input: "permissions:\n contents: read\n issues: write\n administration: read\n members: read\n vulnerability-alerts: read", + contains: []string{ + "permissions:", + " contents: read", + " issues: write", + }, + excludes: []string{"administration", "members", "vulnerability-alerts"}, + }, + { + name: "only GitHub App-only scopes returns empty string", + input: "permissions:\n vulnerability-alerts: read\n members: read", + expectEmpty: true, + contains: []string{}, + excludes: []string{"vulnerability-alerts", "members"}, + }, + { + name: "shorthand read-all is preserved unchanged", + input: "permissions: read-all", + contains: []string{"permissions: read-all"}, + excludes: []string{}, + }, + { + name: "shorthand write-all is preserved unchanged", + input: "permissions: write-all", + contains: []string{"permissions: write-all"}, + excludes: []string{}, + }, + { + name: "shorthand none is preserved unchanged", + input: "permissions: none", + contains: []string{"permissions: none"}, + excludes: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := filterJobLevelPermissions(tt.input) + if tt.expectEmpty && result != "" { + t.Errorf("filterJobLevelPermissions() should return empty string, but got:\n%q", result) + } + for _, expected := range tt.contains { + if !strings.Contains(result, expected) { + t.Errorf("filterJobLevelPermissions() should contain %q, but got:\n%q", expected, result) + } + } + for _, excluded := range tt.excludes { + if strings.Contains(result, excluded) { + t.Errorf("filterJobLevelPermissions() should NOT contain %q, but got:\n%q", excluded, result) + } + } + }) + } +}