Make unsupported workflow settings actionable

docs/compatibility.md

@@ -394,11 +394,11 @@ Supported values are `read`, `write`, and `none`. Supported repository permissio
 
 An omitted map defaults to exactly `contents: read` when a token is needed. This deterministic default does not inherit GitHub repository or organization settings. Hosted token issuance uses only the top-level map. Job-level repository permission maps do not narrow or expand `GITHUB_TOKEN`; the separate `id-token` permission retains job-level behavior. Write access therefore requires an explicit top-level map.
 
-The top-level scalars `permissions: read-all` and `permissions: write-all` expand during compilation to explicit maps containing the 13 supported repository permissions listed above. Plans and workflow-token requests contain those maps, not the aliases. Both expansions exclude `id-token`, `models`, `repository-projects`, `code-quality`, `metadata`, and `vulnerability-alerts`.
+Put `permissions: read-all` or `permissions: write-all` at the top level to apply the same access to all 13 supported repository permissions listed above. These shorthands exclude `id-token`, `models`, `repository-projects`, `code-quality`, `metadata`, and `vulnerability-alerts`.
 
 Jobs expanded from reusable workflows use the top-level requesting workflow's repository permissions for `GITHUB_TOKEN`. Only this immutable top-level map is enforced server-side; permission maps in called workflows do not narrow `GITHUB_TOKEN`. The separate `id-token` permission retains called-workflow narrowing. Warnings identify job-level repository maps that differ from the applied top-level permissions and called-workflow maps that would have narrowed the token scope.
 
-Job-level aliases and noncanonical permission names are unsupported. An empty top-level map, or a top-level map containing only `none`, creates no token.
+At job level, list each permission you need instead of using `permissions: read-all` or `permissions: write-all`. Move a shorthand to the top level only when every job should get that access. An empty top-level permissions map, or one that contains only `none`, creates no token. Use GitHub's exact permission names.
 
 ### Environment and defaults
 

internal/cli/validate_test.go

@@ -61,6 +61,55 @@ func TestValidateReportsIndependentWorkflowAndEventSyntaxFailures(t *testing.T)
 	}
 }
 
+func TestValidateReportsActionableWorkflowSyntaxDiagnostics(t *testing.T) {
+	for _, test := range []struct {
+		name, source, headline, message, job string
+		column                               int
+	}{
+		{
+			name:     "GitHub environment",
+			source:   "on: push\njobs:\n  deploy:\n    environment: production\n    runs-on: ubuntu-latest\n    steps: [{run: true}]\n",
+			headline: "GitHub environments and environment secrets are unsupported.",
+			message:  `GitHub environments and environment secrets are unsupported. Remove the environment key from job "deploy". Approvals, deployment records, and protection rules are unavailable. Move environment secrets into Buildkite secrets and reference them by name. If you need GitHub environments, open an issue in https://github.com/buildkite/buildkite-gha so we can prioritize support`,
+			job:      "deploy",
+			column:   5,
+		},
+		{
+			name:     "job-level write-all",
+			source:   "on: push\njobs:\n  publish:\n    permissions: write-all\n    runs-on: ubuntu-latest\n    steps: [{run: true}]\n",
+			headline: "permissions: write-all is unsupported as job-level shorthand.",
+			message:  `permissions: write-all is unsupported as job-level shorthand. In job "publish", declare each needed permission explicitly, such as contents: write and pull-requests: write. Move permissions: write-all to the workflow top level only when that broader authority is intended for every job. If you need job-level permissions shorthand, open an issue in https://github.com/buildkite/buildkite-gha so we can prioritize support`,
+			job:      "publish",
+			column:   18,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			workflowPath := filepath.Join(t.TempDir(), "workflow.yml")
+			if err := os.WriteFile(workflowPath, []byte(test.source), 0o600); err != nil {
+				t.Fatal(err)
+			}
+			var stdout, stderr bytes.Buffer
+			if code := Run([]string{"validate", "--format", "json", workflowPath}, &stdout, &stderr, "dev"); code != 1 {
+				t.Fatalf("Run() code = %d, want 1; stderr = %q", code, stderr.String())
+			}
+			var report compatibility.ProcessingReport
+			if err := json.Unmarshal(stdout.Bytes(), &report); err != nil {
+				t.Fatal(err)
+			}
+			if len(report.Diagnostics) != 1 {
+				t.Fatalf("diagnostics = %#v, want one", report.Diagnostics)
+			}
+			diagnostic := report.Diagnostics[0]
+			if diagnostic.Message != test.message || diagnostic.Job != test.job || diagnostic.Location == nil || diagnostic.Location.Path != workflowPath || diagnostic.Location.Line != 4 || diagnostic.Location.Column != test.column {
+				t.Fatalf("diagnostic = %#v, want message %q, job %q at %s:4:%d", diagnostic, test.message, test.job, workflowPath, test.column)
+			}
+			if headline, _ := annotationDiagnosticPresentation(diagnostic); headline != test.headline {
+				t.Fatalf("diagnostic headline = %q, want %q", headline, test.headline)
+			}
+		})
+	}
+}
+
 func TestValidatePublishesProcessingDiagnosticsInBuildkite(t *testing.T) {
 	t.Setenv("BUILDKITE", "true")
 	t.Setenv("BUILDKITE_JOB_ID", cliTestJobID)

internal/workflow/parse.go

@@ -94,7 +94,7 @@ func Parse(path string, source []byte) (*Workflow, error) {
 		owned.Concurrency.CancelInProgress = true
 		owned.Concurrency.CancelInProgressPosition = *workflowCancellation
 	}
-	owned.Permissions, err = adaptPermissions(path, parsed.Permissions, true)
+	owned.Permissions, err = adaptPermissions(path, parsed.Permissions, "")
 	if err != nil {
 		return nil, err
 	}
@@ -545,14 +545,14 @@ func validateRawContainer(path string, node *yaml.Node, service bool) (rawServic
 func adaptJob(path string, in *actionlint.Job, scalars map[Position]any, concurrency map[Position]stepConcurrency, serviceContainers map[string]rawServiceContainer) (Job, error) {
 	out := Job{ID: in.ID.Value, Span: pointSpan(in.Pos)}
 	if in.Environment != nil {
-		return Job{}, locatedError(path, in.Environment.Pos, fmt.Sprintf("job %q", in.ID.Value), "GitHub environments and environment secrets are unsupported")
+		return Job{}, fmt.Errorf("%s:%d:%d: GitHub environments and environment secrets are unsupported. Remove the environment key from job %q. Approvals, deployment records, and protection rules are unavailable. Move environment secrets into Buildkite secrets and reference them by name. If you need GitHub environments, open an issue in https://github.com/buildkite/buildkite-gha so we can prioritize support", path, in.Environment.Pos.Line, in.Environment.Pos.Col, in.ID.Value)
 	}
 	ownedConcurrency, err := adaptConcurrency(path, in.ID.Value, in.Concurrency)
 	if err != nil {
 		return Job{}, err
 	}
 	out.Concurrency = ownedConcurrency
-	permissions, err := adaptPermissions(path, in.Permissions, false)
+	permissions, err := adaptPermissions(path, in.Permissions, in.ID.Value)
 	if err != nil {
 		return Job{}, err
 	}
@@ -849,20 +849,27 @@ func adaptConcurrency(path, jobID string, in *actionlint.Concurrency) (*Concurre
 	}, nil
 }
 
-func adaptPermissions(path string, in *actionlint.Permissions, allowAll bool) (*Permissions, error) {
+func adaptPermissions(path string, in *actionlint.Permissions, jobID string) (*Permissions, error) {
 	if in == nil {
 		return nil, nil
 	}
+	permissionError := func(pos *actionlint.Pos, message string) error {
+		if jobID == "" {
+			return fmt.Errorf("%s:%d:%d: workflow permissions: %s", path, pos.Line, pos.Col, message)
+		}
+		return locatedError(path, pos, jobID, message)
+	}
 	if in.All != nil {
-		if allowAll && (in.All.Value == "read-all" || in.All.Value == "write-all") {
+		if jobID == "" && (in.All.Value == "read-all" || in.All.Value == "write-all") {
 			access := strings.TrimSuffix(in.All.Value, "-all")
 			scopes := make(map[string]string, len(topLevelAllPermissionNames))
 			for _, name := range topLevelAllPermissionNames {
 				scopes[name] = access
 			}
 			return &Permissions{Scopes: scopes, Span: pointSpan(in.Pos)}, nil
 		}
-		return nil, locatedError(path, in.All.Pos, "permissions", "permission aliases are unsupported; declare each required permission explicitly")
+		access := strings.TrimSuffix(in.All.Value, "-all")
+		return nil, fmt.Errorf("%s:%d:%d: permissions: %s is unsupported as job-level shorthand. In job %q, declare each needed permission explicitly, such as contents: %s and pull-requests: %s. Move permissions: %s to the workflow top level only when that broader authority is intended for every job. If you need job-level permissions shorthand, open an issue in https://github.com/buildkite/buildkite-gha so we can prioritize support", path, in.All.Pos.Line, in.All.Pos.Col, in.All.Value, jobID, access, access, in.All.Value)
 	}
 	scopes := make(map[string]string, len(in.Scopes))
 	names := make([]string, 0, len(in.Scopes))
@@ -873,17 +880,17 @@ func adaptPermissions(path string, in *actionlint.Permissions, allowAll bool) (*
 	for _, name := range names {
 		scope := in.Scopes[name]
 		if scope == nil || scope.Name == nil || scope.Value == nil {
-			return nil, locatedError(path, in.Pos, "permissions", "invalid permission declaration")
+			return nil, permissionError(in.Pos, "invalid permission declaration")
 		}
 		if name != "id-token" && !supportedGitHubTokenPermission(name) {
-			return nil, locatedError(path, scope.Name.Pos, "permissions", fmt.Sprintf("unsupported permission %q; use canonical GitHub permission names", name))
+			return nil, permissionError(scope.Name.Pos, fmt.Sprintf("unsupported permission %q; use canonical GitHub permission names", name))
 		}
 		switch scope.Value.Value {
 		case "read", "write":
 			scopes[name] = scope.Value.Value
 		case "none":
 		default:
-			return nil, locatedError(path, scope.Value.Pos, "permissions", fmt.Sprintf("invalid access %q for permission %q", scope.Value.Value, name))
+			return nil, permissionError(scope.Value.Pos, fmt.Sprintf("invalid access %q for permission %q", scope.Value.Value, name))
 		}
 	}
 	return &Permissions{Scopes: scopes, Span: pointSpan(in.Pos)}, nil

internal/workflow/parse_test.go

@@ -141,8 +141,9 @@ func TestParseKeepsWorkflowAndJobExpressionSurfacesSeparate(t *testing.T) {
 
 func TestParseRejectsGitHubEnvironment(t *testing.T) {
 	_, err := Parse("environment.yml", []byte("on: push\njobs:\n  deploy:\n    environment: production\n    runs-on: ubuntu-latest\n    steps: [{run: true}]\n"))
-	if err == nil || !strings.Contains(err.Error(), "GitHub environments and environment secrets are unsupported") {
-		t.Fatalf("Parse() error = %v", err)
+	want := `environment.yml:4:5: GitHub environments and environment secrets are unsupported. Remove the environment key from job "deploy". Approvals, deployment records, and protection rules are unavailable. Move environment secrets into Buildkite secrets and reference them by name. If you need GitHub environments, open an issue in https://github.com/buildkite/buildkite-gha so we can prioritize support`
+	if err == nil || err.Error() != want {
+		t.Fatalf("Parse() error = %q, want %q", err, want)
 	}
 }
 
@@ -225,7 +226,7 @@ func TestParseRejectsUnsupportedPermissionFormsWithLocation(t *testing.T) {
 	for _, test := range []struct {
 		name, declaration, want string
 	}{
-		{name: "non-canonical name", declaration: "permissions:\n  pull_requests: write\n", want: "permissions.yml:3:3: job \"permissions\": unsupported permission \"pull_requests\""},
+		{name: "non-canonical name", declaration: "permissions:\n  pull_requests: write\n", want: "permissions.yml:3:3: workflow permissions: unsupported permission \"pull_requests\""},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			source := []byte("on: push\n" + test.declaration + "jobs:\n  test:\n    runs-on: ubuntu-latest\n    steps: [{run: true}]\n")
@@ -237,14 +238,15 @@ func TestParseRejectsUnsupportedPermissionFormsWithLocation(t *testing.T) {
 	}
 }
 
-func TestParseRejectsJobPermissionAliases(t *testing.T) {
-	for _, alias := range []string{"read-all", "write-all"} {
-		t.Run(alias, func(t *testing.T) {
-			source := []byte("on: push\njobs:\n  test:\n    permissions: " + alias + "\n    runs-on: ubuntu-latest\n    steps: [{run: true}]\n")
+func TestParseRejectsJobPermissionShorthand(t *testing.T) {
+	for _, shorthand := range []string{"read-all", "write-all"} {
+		t.Run(shorthand, func(t *testing.T) {
+			source := []byte("on: push\njobs:\n  publish:\n    permissions: " + shorthand + "\n    runs-on: ubuntu-latest\n    steps: [{run: true}]\n")
 			_, err := Parse("permissions.yml", source)
-			want := "permissions.yml:4:18: job \"permissions\": permission aliases are unsupported"
-			if err == nil || !strings.Contains(err.Error(), want) {
-				t.Fatalf("Parse() error = %v, want %q", err, want)
+			access := strings.TrimSuffix(shorthand, "-all")
+			want := `permissions.yml:4:18: permissions: ` + shorthand + ` is unsupported as job-level shorthand. In job "publish", declare each needed permission explicitly, such as contents: ` + access + ` and pull-requests: ` + access + `. Move permissions: ` + shorthand + ` to the workflow top level only when that broader authority is intended for every job. If you need job-level permissions shorthand, open an issue in https://github.com/buildkite/buildkite-gha so we can prioritize support`
+			if err == nil || err.Error() != want {
+				t.Fatalf("Parse() error = %q, want %q", err, want)
 			}
 		})
 	}