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

docs/adr/50642-additive-permissions-merge-for-builtin-jobs.md

@@ -0,0 +1,43 @@
+# ADR-50642: Additive Permissions Merge for Built-in Jobs
+
+**Date**: 2026-08-05
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+### Context
+
+The workflow compiler generates built-in jobs (`safe_outputs`, `conclusion`) with a least-privilege permission set computed automatically. Authors sometimes need scopes beyond this baseline — most notably `id-token: write` for OIDC token minting — and express these by declaring a `permissions` block under `jobs.<built-in>` in their workflow file. Prior to this change, `applyBuiltinJobAugmentations` only consulted `needs` and `if` fields from a built-in job's config entry; a `permissions` block was silently skipped, causing the declared scopes to be absent from the compiled lock file. This caused OIDC token minting to fail at runtime with "Missing id-token permission" errors despite the author having explicitly declared `id-token: write`.
+
+### Decision
+
+We will support user-declared `permissions` blocks under `jobs.<built-in>` entries in `applyBuiltinJobAugmentations`, merging them **additively** with the compiler-computed least-privilege permissions. The merge applies "write overrides read" semantics and preserves all compiler-required scopes. A new helper `applyBuiltinJobPermissionsAugmentation` encapsulates the merge logic and is invoked before the existing `needs`/`if` augmentation path.
+
+### Alternatives Considered
+
+#### Alternative 1: Reject permissions blocks on built-in jobs (error on detection)
+
+Treat any `permissions:` key under a built-in job config entry as a validation error. Authors would be required to rely solely on the compiler's least-privilege computation and could not extend it. This is the safest option for minimizing permission surface, but it makes OIDC-dependent workflows impossible to author without the compiler growing built-in knowledge of every OIDC-related scope.
+
+#### Alternative 2: Full replacement of compiler-computed permissions
+
+Allow the user's declared permissions to entirely replace the compiler-computed set, giving authors complete control. This is simpler to implement but risks silently dropping required scopes (e.g., `issues: write` that `safe_outputs` needs to post comments), which would cause different runtime failures. The additive approach avoids this regression.
+
+### Consequences
+
+#### Positive
+- OIDC-dependent workflows that declare `id-token: write` under `jobs.safe_outputs.permissions` or `jobs.conclusion.permissions` now compile correctly and retain the scope in the lock file.
+- Additive merge preserves all compiler-required scopes; no existing workflow is affected by this change (the new path is only entered when a `permissions` block is present).
+
+#### Negative
+- Authors can widen the permission surface of compiler-generated built-in jobs beyond the least-privilege baseline, potentially granting scopes that are not strictly needed at runtime.
+- The compiler gains a new augmentation code path for permissions that must be kept in sync with future changes to built-in job generation and the `Permissions.Merge` / `RenderToYAML` helpers.
+
+#### Neutral
+- The error message for invalid built-in job augmentation now also covers the `.permissions` field, improving diagnostics when a user references a built-in job that the workflow does not generate.
+- The integration test (`builtin_job_permissions_integration_test.go`) compiles a real workflow fixture and asserts lock-file contents, establishing a regression guard for this behavior.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/actionpins/data/action_pins.json

@@ -165,6 +165,11 @@
     }
   },
   "containers": {
+    "ghcr.io/fabio-rovai/open-ontologies:latest": {
+      "image": "ghcr.io/fabio-rovai/open-ontologies:latest",
+      "digest": "sha256:2932c10682eac29ccf840a6bd6c4c7c82c5ce770ad9e94697d44057187452530",
+      "pinned_image": "ghcr.io/fabio-rovai/open-ontologies:latest@sha256:2932c10682eac29ccf840a6bd6c4c7c82c5ce770ad9e94697d44057187452530"
+    },
     "ghcr.io/github/gh-aw-firewall/agent:0.27.43": {
       "image": "ghcr.io/github/gh-aw-firewall/agent:0.27.43",
       "digest": "sha256:04e2d1987a565000a8f114b89d806ae7a3864dd4f944be65275b28c93d8690e6",
@@ -264,11 +269,6 @@
       "image": "python:alpine",
       "digest": "sha256:26730869004e2b9c4b9ad09cab8625e81d256d1ce97e72df5520e806b1709f92",
       "pinned_image": "python:alpine@sha256:26730869004e2b9c4b9ad09cab8625e81d256d1ce97e72df5520e806b1709f92"
-    },
-    "ghcr.io/fabio-rovai/open-ontologies:latest": {
-      "image": "ghcr.io/fabio-rovai/open-ontologies:latest",
-      "digest": "sha256:2932c10682eac29ccf840a6bd6c4c7c82c5ce770ad9e94697d44057187452530",
-      "pinned_image": "ghcr.io/fabio-rovai/open-ontologies:latest@sha256:2932c10682eac29ccf840a6bd6c4c7c82c5ce770ad9e94697d44057187452530"
     }
   }
 }

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
 }
 

pkg/workflow/data/action_pins.json

@@ -165,6 +165,11 @@
     }
   },
   "containers": {
+    "ghcr.io/fabio-rovai/open-ontologies:latest": {
+      "image": "ghcr.io/fabio-rovai/open-ontologies:latest",
+      "digest": "sha256:2932c10682eac29ccf840a6bd6c4c7c82c5ce770ad9e94697d44057187452530",
+      "pinned_image": "ghcr.io/fabio-rovai/open-ontologies:latest@sha256:2932c10682eac29ccf840a6bd6c4c7c82c5ce770ad9e94697d44057187452530"
+    },
     "ghcr.io/github/gh-aw-firewall/agent:0.27.43": {
       "image": "ghcr.io/github/gh-aw-firewall/agent:0.27.43",
       "digest": "sha256:04e2d1987a565000a8f114b89d806ae7a3864dd4f944be65275b28c93d8690e6",
@@ -264,11 +269,6 @@
       "image": "python:alpine",
       "digest": "sha256:26730869004e2b9c4b9ad09cab8625e81d256d1ce97e72df5520e806b1709f92",
       "pinned_image": "python:alpine@sha256:26730869004e2b9c4b9ad09cab8625e81d256d1ce97e72df5520e806b1709f92"
-    },
-    "ghcr.io/fabio-rovai/open-ontologies:latest": {
-      "image": "ghcr.io/fabio-rovai/open-ontologies:latest",
-      "digest": "sha256:2932c10682eac29ccf840a6bd6c4c7c82c5ce770ad9e94697d44057187452530",
-      "pinned_image": "ghcr.io/fabio-rovai/open-ontologies:latest@sha256:2932c10682eac29ccf840a6bd6c4c7c82c5ce770ad9e94697d44057187452530"
     }
   }
 }