Annotate imported workflow_call permissions in generated call jobs

pkg/workflow/call_workflow_permissions.go

@@ -3,14 +3,31 @@ package workflow
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"sort"
+	"strings"
 
+	"github.com/github/gh-aw/pkg/constants"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/parser"
 )
 
 var callWorkflowPermissionsLog = logger.New("workflow:call_workflow_permissions")
 
+type workflowSourceKind string
+
+const (
+	workflowSourceKindLock     workflowSourceKind = "lock"
+	workflowSourceKindYAML     workflowSourceKind = "yaml"
+	workflowSourceKindMarkdown workflowSourceKind = "markdown"
+)
+
+type callWorkflowPermissionImport struct {
+	permissions *Permissions
+	sourcePath  string
+	sourceKind  workflowSourceKind
+}
+
 // permissionLevelRank maps a permission level to a comparable rank where a higher
 // number grants strictly more access (none < read < write). Used to determine
 // whether one permission set covers another. Unknown or empty levels rank as 0.
@@ -101,6 +118,10 @@ func extractJobPermissionsFromParsedWorkflow(workflow map[string]any) *Permissio
 	return merged
 }
 
+// extractCallWorkflowPermissions is a compatibility helper used by existing tests.
+// New production code should prefer extractCallWorkflowPermissionImport when it
+// needs both the permissions and their review source metadata.
+//
 // extractCallWorkflowPermissions returns the permission superset required by the worker
 // workflow identified by workflowName. It resolves the file in priority order:
 // .lock.yml > .yml > .md (same-batch compilation target).
@@ -121,29 +142,90 @@ func extractJobPermissionsFromParsedWorkflow(workflow map[string]any) *Permissio
 // extractJobPermissionsFromParsedWorkflow initialises a fresh Permissions map
 // regardless of whether any jobs declare a permissions block.
 func extractCallWorkflowPermissions(workflowName, markdownPath string) (*Permissions, error) {
+	imported, err := extractCallWorkflowPermissionImport(workflowName, markdownPath)
+	if err != nil || imported == nil {
+		return nil, err
+	}
+	return imported.permissions, nil
+}
+
+func extractCallWorkflowPermissionImport(workflowName, markdownPath string) (*callWorkflowPermissionImport, error) {
 	fileResult, err := findWorkflowFile(workflowName, markdownPath)
 	if err != nil {
 		return nil, fmt.Errorf("failed to find workflow file for '%s': %w", workflowName, err)
 	}
 
 	// Priority: .lock.yml > .yml > .md
 	if fileResult.lockExists {
-		return extractPermissionsFromYAMLFile(fileResult.lockPath)
+		perms, err := extractPermissionsFromYAMLFile(fileResult.lockPath)
+		if err != nil {
+			return nil, err
+		}
+		return &callWorkflowPermissionImport{
+			permissions: perms,
+			sourcePath:  fileResult.lockPath,
+			sourceKind:  workflowSourceKindLock,
+		}, nil
 	}
 
 	if fileResult.ymlExists {
-		return extractPermissionsFromYAMLFile(fileResult.ymlPath)
+		perms, err := extractPermissionsFromYAMLFile(fileResult.ymlPath)
+		if err != nil {
+			return nil, err
+		}
+		return &callWorkflowPermissionImport{
+			permissions: perms,
+			sourcePath:  fileResult.ymlPath,
+			sourceKind:  workflowSourceKindYAML,
+		}, nil
 	}
 
 	if fileResult.mdExists {
-		return extractPermissionsFromMDFile(fileResult.mdPath)
+		perms, err := extractPermissionsFromMDFile(fileResult.mdPath)
+		if err != nil {
+			return nil, err
+		}
+		if perms == nil {
+			return nil, nil
+		}
+		return &callWorkflowPermissionImport{
+			permissions: perms,
+			sourcePath:  fileResult.mdPath,
+			sourceKind:  workflowSourceKindMarkdown,
+		}, nil
 	}
 
 	// No file found — return nil so the caller omits the permissions block.
 	callWorkflowPermissionsLog.Printf("No workflow file found for '%s', skipping permissions", workflowName)
 	return nil, nil
 }
 
+func buildCallWorkflowPermissionsComment(workflowName string, imported *callWorkflowPermissionImport) string {
+	if imported == nil || imported.permissions == nil {
+		return ""
+	}
+	if imported.permissions.RenderToYAML() == "" {
+		return ""
+	}
+
+	reviewWhat := "job-level permissions"
+	if imported.sourceKind == workflowSourceKindMarkdown {
+		reviewWhat = "frontmatter permissions"
+	}
+
+	return strings.Join([]string{
+		fmt.Sprintf("# Imported from called workflow %q because GitHub requires the caller job to grant permissions requested by reusable workflow jobs.", workflowName),
+		fmt.Sprintf("# Review the called workflow's %s in %s.", reviewWhat, renderWorkflowReviewPath(imported.sourcePath)),
+	}, "\n")
+}
+
+// renderWorkflowReviewPath converts an absolute workflow path to the canonical
+// repo-relative display path used in generated review comments. This assumes
+// workflow files live directly in constants.GetWorkflowDir().
+func renderWorkflowReviewPath(sourcePath string) string {
+	return "./" + filepath.ToSlash(filepath.Join(constants.GetWorkflowDir(), filepath.Base(sourcePath)))
+}
+
 // extractPermissionsFromYAMLFile reads a .lock.yml or .yml workflow file, parses it,
 // and returns the merged permissions from all its jobs.
 func extractPermissionsFromYAMLFile(filePath string) (*Permissions, error) {

pkg/workflow/call_workflow_permissions_test.go

@@ -268,11 +268,75 @@ func TestExtractCallWorkflowPermissions_FileNotFound(t *testing.T) {
 	assert.Nil(t, perms, "Should return nil when no file exists")
 }
 
+func TestExtractCallWorkflowPermissionImport_MDWithoutPermissionsReturnsNil(t *testing.T) {
+	tmpDir := t.TempDir()
+	workflowsDir := filepath.Join(tmpDir, ".github", "workflows")
+	require.NoError(t, os.MkdirAll(workflowsDir, 0755), "Failed to create workflows directory")
+
+	mdContent := `---
+on:
+  workflow_call: {}
+engine: copilot
+---
+
+# Worker Without Permissions
+`
+	require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "worker-no-perms.md"), []byte(mdContent), 0644), "Failed to write worker-no-perms.md")
+
+	markdownPath := filepath.Join(workflowsDir, "gateway.md")
+
+	imported, err := extractCallWorkflowPermissionImport("worker-no-perms", markdownPath)
+	require.NoError(t, err, "Should not error when markdown worker has no permissions")
+	assert.Nil(t, imported, "Should treat markdown workers with no permissions like other missing-import cases")
+}
+
+func TestExtractCallWorkflowPermissionImport_TracksReviewSource(t *testing.T) {
+	t.Setenv("GH_AW_WORKFLOWS_DIR", "")
+
+	tmpDir := t.TempDir()
+	workflowsDir := filepath.Join(tmpDir, ".github", "workflows")
+	require.NoError(t, os.MkdirAll(workflowsDir, 0755), "Failed to create workflows directory")
+
+	lockContent := `name: Worker Lock
+on:
+  workflow_call: {}
+jobs:
+  work:
+    permissions:
+      contents: write
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo "lock"
+`
+	require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "worker-review.lock.yml"), []byte(lockContent), 0644), "Failed to write worker-review.lock.yml")
+
+	markdownPath := filepath.Join(workflowsDir, "gateway.md")
+
+	imported, err := extractCallWorkflowPermissionImport("worker-review", markdownPath)
+	require.NoError(t, err, "Should extract imported permissions without error")
+	require.NotNil(t, imported, "Should return import metadata")
+	require.NotNil(t, imported.permissions, "Should include permissions")
+	assert.Equal(t, workflowSourceKindLock, imported.sourceKind, "Should track lock workflow source kind")
+	assert.Equal(t, "./.github/workflows/worker-review.lock.yml", renderWorkflowReviewPath(imported.sourcePath),
+		"Should render a repo-relative review path for help comments")
+}
+
+func TestBuildCallWorkflowPermissionsComment_NilInputs(t *testing.T) {
+	assert.Empty(t, buildCallWorkflowPermissionsComment("worker", nil), "Nil import should not emit a comment")
+	assert.Empty(t, buildCallWorkflowPermissionsComment("worker", &callWorkflowPermissionImport{}), "Nil permissions should not emit a comment")
+	assert.Empty(t, buildCallWorkflowPermissionsComment("worker", &callWorkflowPermissionImport{
+		permissions: NewPermissions(),
+		sourceKind:  workflowSourceKindLock,
+	}), "Empty permissions should not emit a comment")
+}
+
 // TestBuildCallWorkflowJobs_SetsPermissionsFromLockYML tests that call-workflow jobs
 // carry the union of caller + worker permissions when a .lock.yml worker file is present.
 // When the caller already covers all of the worker's needs, the effective permissions
 // equal the caller's declared permissions.
 func TestBuildCallWorkflowJobs_SetsPermissionsFromLockYML(t *testing.T) {
+	t.Setenv("GH_AW_WORKFLOWS_DIR", "")
+
 	compiler := NewCompiler(WithVersion("1.0.0"))
 
 	tmpDir := t.TempDir()
@@ -326,6 +390,12 @@ jobs:
 
 	job, exists := compiler.jobManager.GetJob("call-worker-docs")
 	require.True(t, exists, "Job should exist in job manager")
+	assert.Contains(t, job.PermissionsComment,
+		`Imported from called workflow "worker-docs" because GitHub requires the caller job to grant permissions requested by reusable workflow jobs.`,
+		"Job should explain why worker permissions are imported")
+	assert.Contains(t, job.PermissionsComment,
+		"Review the called workflow's job-level permissions in ./.github/workflows/worker-docs.lock.yml.",
+		"Job should point reviewers to the compiled worker workflow")
 	assert.NotEmpty(t, job.Permissions, "Job should have permissions set")
 	assert.Contains(t, job.Permissions, "contents: write", "Permissions should include contents: write")
 	assert.Contains(t, job.Permissions, "issues: write", "Permissions should include issues: write")
@@ -337,6 +407,8 @@ jobs:
 // target. When caller and worker declare the same permissions, the effective permissions
 // equal the caller's declared permissions.
 func TestBuildCallWorkflowJobs_SetsPermissionsFromMD(t *testing.T) {
+	t.Setenv("GH_AW_WORKFLOWS_DIR", "")
+
 	compiler := NewCompiler(WithVersion("1.0.0"))
 
 	tmpDir := t.TempDir()
@@ -379,6 +451,9 @@ permissions:
 
 	job, exists := compiler.jobManager.GetJob("call-worker-e")
 	require.True(t, exists, "Job should exist in job manager")
+	assert.Contains(t, job.PermissionsComment,
+		"Review the called workflow's frontmatter permissions in ./.github/workflows/worker-e.md.",
+		"Job should point reviewers to the markdown worker when no compiled file exists yet")
 	assert.NotEmpty(t, job.Permissions, "Job should have permissions")
 	assert.Contains(t, job.Permissions, "contents: read", "Permissions should include contents: read")
 	assert.Contains(t, job.Permissions, "issues: write", "Permissions should include issues: write")
@@ -547,6 +622,12 @@ jobs:
 	assert.Contains(t, yamlOutput, "uses: ./.github/workflows/worker-a.lock.yml", "Should contain uses directive")
 	assert.Contains(t, yamlOutput, "secrets: inherit", "Should inherit secrets")
 	assert.Contains(t, yamlOutput, "permissions:", "Should include permissions block")
+	assert.Contains(t, yamlOutput,
+		`# Imported from called workflow "worker-a" because GitHub requires the caller job to grant permissions requested by reusable workflow jobs.`,
+		"Rendered YAML should explain imported workflow_call permissions")
+	assert.Contains(t, yamlOutput,
+		"# Review the called workflow's job-level permissions in ./.github/workflows/worker-a.lock.yml.",
+		"Rendered YAML should point to the worker workflow for review")
 	// The call-* job gets the union of caller + worker permissions. Since the caller
 	// already covers all of the worker's needs, the effective permissions equal the
 	// caller's declared permissions.

pkg/workflow/compiler_safe_output_jobs.go

@@ -169,10 +169,10 @@ func (c *Compiler) buildSafeOutputsJobs(data *WorkflowData, jobName, markdownPat
 //   - `payload` is forwarded as the raw transport only when the worker declares it
 //     (GitHub Actions rejects undeclared inputs)
 //   - inherits all caller secrets via `secrets: inherit`
-//   - includes a job-level `permissions:` block derived from the CALLER's own
-//     declared permissions (not the worker's). The caller controls its own
-//     permission surface; the compiler validates that the declared permissions
-//     cover what the worker requires and warns if they do not.
+//   - includes a job-level `permissions:` block equal to the union of the
+//     caller's declared permissions and the called worker's required permissions
+//   - adds a help comment explaining why imported worker permissions appear on
+//     the call job and where to review them in the worker workflow source
 //
 // Returns the names of all generated jobs so they can be added to the conclusion
 // job's `needs` list.
@@ -305,14 +305,16 @@ func (c *Compiler) buildCallWorkflowJobs(data *WorkflowData, markdownPath string
 		}
 
 		effectivePerms := callerPerms
+		var importedPerms *callWorkflowPermissionImport
+		var permErr error
 		if markdownPath != "" {
-			workerPerms, permErr := extractCallWorkflowPermissions(workflowName, markdownPath)
+			importedPerms, permErr = extractCallWorkflowPermissionImport(workflowName, markdownPath)
 			if permErr != nil {
 				// Non-fatal: log and continue. The worker file may not exist yet (it may be
 				// compiled in the same batch), in which case we fall back to the caller's
 				// own declared permissions.
 				compilerSafeOutputJobsLog.Printf("Could not extract worker permissions for call-workflow job '%s' (falling back to caller-only permissions): %v", jobName, permErr)
-			} else if workerPerms != nil {
+			} else if importedPerms != nil && importedPerms.permissions != nil {
 				// Compute the union by merging caller and worker permissions into a
 				// fresh map-based Permissions. Starting from a blank slate (rather
 				// than a clone of callerPerms) ensures shorthand values like
@@ -322,7 +324,7 @@ func (c *Compiler) buildCallWorkflowJobs(data *WorkflowData, markdownPath string
 				// expanding it, silently dropping the caller's baseline grant.
 				merged := NewPermissions()
 				merged.Merge(callerPerms)
-				merged.Merge(workerPerms)
+				merged.Merge(importedPerms.permissions)
 				effectivePerms = merged
 				compilerSafeOutputJobsLog.Printf("Merged caller and worker permissions for call-workflow job '%s'", jobName)
 			}
@@ -331,6 +333,7 @@ func (c *Compiler) buildCallWorkflowJobs(data *WorkflowData, markdownPath string
 		if effectivePerms != nil {
 			rendered := effectivePerms.RenderToYAML()
 			if rendered != "" {
+				callJob.PermissionsComment = buildCallWorkflowPermissionsComment(workflowName, importedPerms)
 				callJob.Permissions = rendered
 				compilerSafeOutputJobsLog.Printf("Set permissions on call-workflow job '%s': %s", jobName, rendered)
 			}

pkg/workflow/jobs.go

@@ -42,6 +42,7 @@ type Job struct {
 	RunsOn                     string
 	If                         string
 	HasWorkflowRunSafetyChecks bool // If true, the job's if condition includes workflow_run safety checks
+	PermissionsComment         string
 	Permissions                string
 	TimeoutMinutes             int
 	TimeoutMinutesExpression   string
@@ -292,6 +293,11 @@ func (jm *JobManager) renderJobTo(b *strings.Builder, job *Job) {
 	}
 
 	// Add permissions section
+	if job.PermissionsComment != "" {
+		for line := range strings.SplitSeq(strings.TrimRight(job.PermissionsComment, "\n"), "\n") {
+			fmt.Fprintf(b, "    %s\n", line)
+		}
+	}
 	if job.Permissions != "" {
 		fmt.Fprintf(b, "    %s\n", job.Permissions)
 	}