Retain jobs.*.permissions for built-in safe_outputs and conclusion jobs

pkg/workflow/builtin_job_permissions_integration_test.go

@@ -0,0 +1,68 @@
+package workflow
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/goccy/go-yaml"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/github/gh-aw/pkg/testutil"
+)
+
+// TestBuiltinJobPermissionsAugmentation verifies that user-declared permissions under
+// jobs.<built-in>.permissions (e.g. safe_outputs, conclusion) are merged additively into the
+// compiled built-in jobs, so scopes such as id-token: write are retained in the lock file.
+func TestBuiltinJobPermissionsAugmentation(t *testing.T) {
+	tmpDir := testutil.TempDir(t, "builtin-job-permissions-augmentation")
+	compiler := NewCompiler()
+
+	workflowContent := `---
+on:
+  issue_comment:
+    types: [created]
+engine: copilot
+strict: false
+permissions:
+  contents: read
+  id-token: write
+safe-outputs:
+  add-comment:
+jobs:
+  safe_outputs:
+    permissions:
+      id-token: write
+      contents: read
+      issues: write
+  conclusion:
+    permissions:
+      id-token: write
+      contents: read
+      issues: write
+---
+Builtin job permissions augmentation
+`
+
+	workflowFile := filepath.Join(tmpDir, "builtin-job-permissions-augmentation.md")
+	require.NoError(t, os.WriteFile(workflowFile, []byte(workflowContent), 0644))
+	require.NoError(t, compiler.CompileWorkflow(workflowFile))
+
+	lockFile := filepath.Join(tmpDir, "builtin-job-permissions-augmentation.lock.yml")
+	lockBytes, err := os.ReadFile(lockFile)
+	require.NoError(t, err)
+
+	var lock map[string]any
+	require.NoError(t, yaml.Unmarshal(lockBytes, &lock))
+	jobs, ok := lock["jobs"].(map[string]any)
+	require.True(t, ok)
+
+	for _, jobName := range []string{"safe_outputs", "conclusion"} {
+		job, ok := jobs[jobName].(map[string]any)
+		require.True(t, ok, "expected %s job in compiled workflow", jobName)
+		perms, ok := job["permissions"].(map[string]any)
+		require.True(t, ok, "expected %s permissions to be a map", jobName)
+		assert.Equal(t, "write", perms["id-token"], "%s should retain id-token: write from jobs.%s.permissions", jobName, jobName)
+	}
+}

pkg/workflow/compiler_custom_jobs.go

@@ -775,7 +775,8 @@ func (c *Compiler) applyBuiltinJobAugmentations(data *WorkflowData) error {
 		if err != nil {
 			return err
 		}
-		if len(augmentedNeeds) == 0 && augmentedIf == "" {
+		_, hasPermissions := configMap["permissions"]
+		if len(augmentedNeeds) == 0 && augmentedIf == "" && !hasPermissions {
 			continue
 		}
 
@@ -784,13 +785,23 @@ func (c *Compiler) applyBuiltinJobAugmentations(data *WorkflowData) error {
 			// Report the actual field(s) the author configured so they can identify the problem.
 			augmentedField := configuredJobName + ".needs"
 			if len(augmentedNeeds) == 0 {
-				augmentedField = configuredJobName + ".if"
-			} else if augmentedIf != "" {
+				if augmentedIf != "" {
+					augmentedField = configuredJobName + ".if"
+				} else {
+					augmentedField = configuredJobName + ".permissions"
+				}
+			} else if augmentedIf != "" || hasPermissions {
 				augmentedField = configuredJobName
 			}
 			return fmt.Errorf("jobs.%s: cannot augment %q because this workflow does not generate that job", augmentedField, targetJobName)
 		}
 
+		if hasPermissions {
+			if err := applyBuiltinJobPermissionsAugmentation(configuredJobName, targetJobName, configMap, targetJob); err != nil {
+				return err
+			}
+		}
+
 		normalizedNeeds := make([]string, 0, len(augmentedNeeds))
 		for _, rawNeed := range augmentedNeeds {
 			need := normalizeBuiltinJobAlias(rawNeed)
@@ -840,7 +851,32 @@ func (c *Compiler) applyBuiltinJobAugmentations(data *WorkflowData) error {
 			compilerJobsLog.Printf("Applied jobs.%s.needs augmentation to %q: %v", configuredJobName, targetJobName, normalizedNeeds)
 		}
 	}
+	return nil
+}
+
+// applyBuiltinJobPermissionsAugmentation merges user-declared jobs.<built-in>.permissions
+// into a compiler-generated built-in job (e.g. safe_outputs, conclusion). The merge is
+// additive: the compiler-computed permissions are preserved and the user's declared scopes
+// are added on top, with write overriding read. This ensures scopes such as id-token: write
+// that authors declare under jobs.*.permissions are retained in the compiled lock file rather
+// than being dropped by the minimal least-privilege permission computation.
+func applyBuiltinJobPermissionsAugmentation(configuredJobName, targetJobName string, configMap map[string]any, targetJob *Job) error {
+	permissionsValue, exists := configMap["permissions"]
+	if !exists || permissionsValue == nil {
+		return nil
+	}
+
+	userPermissions := NewPermissionsParserFromValue(permissionsValue).ToPermissions()
+	if userPermissions == nil {
+		return nil
+	}
 
+	// Start from the compiler-computed permissions already rendered on the job, then merge
+	// the user-declared permissions additively so no compiler-required scope is lost.
+	merged := NewPermissionsParser(targetJob.Permissions).ToPermissions()
+	merged.Merge(userPermissions)
+	targetJob.Permissions = merged.RenderToYAML()
+	compilerJobsLog.Printf("Applied jobs.%s.permissions augmentation to %q", configuredJobName, targetJobName)
 	return nil
 }