diff --git a/docs/compatibility.md b/docs/compatibility.md index 70bffe83..658173e2 100644 --- a/docs/compatibility.md +++ b/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. +To grant repository access, list each required permission at the top level, such as `contents: write`. Every job that receives `GITHUB_TOKEN` gets that access. You cannot give different repository permissions to individual jobs. Use top-level `read-all` or `write-all` only when every job should get read or write access for every supported repository permission. An empty top-level permissions map, or one that contains only `none`, creates no token. Use GitHub's exact permission names. ### Environment and defaults diff --git a/internal/cli/validate_test.go b/internal/cli/validate_test.go index 546140ae..32f53ca9 100644 --- a/internal/cli/validate_test.go +++ b/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", you cannot set separate repository permissions. At the workflow top level, declare each needed repository permission, such as contents: write and pull-requests: write. These permissions apply to every job that receives GITHUB_TOKEN. Use permissions: write-all at the workflow top level only when every supported repository permission should have write access. If you need different repository permissions for individual jobs, 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) diff --git a/internal/workflow/parse.go b/internal/workflow/parse.go index 94f983be..a6f209a1 100644 --- a/internal/workflow/parse.go +++ b/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,12 +849,25 @@ 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 in.All.Value != "read-all" && in.All.Value != "write-all" { + message := fmt.Sprintf("invalid permissions scalar %q; declare each needed permission in a map", in.All.Value) + if jobID == "" { + message = fmt.Sprintf("invalid permissions scalar %q; use read-all, write-all, or a permissions map", in.All.Value) + } + return nil, permissionError(in.All.Pos, message) + } + if jobID == "" { access := strings.TrimSuffix(in.All.Value, "-all") scopes := make(map[string]string, len(topLevelAllPermissionNames)) for _, name := range topLevelAllPermissionNames { @@ -862,7 +875,8 @@ func adaptPermissions(path string, in *actionlint.Permissions, allowAll bool) (* } 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, you cannot set separate repository permissions. At the workflow top level, declare each needed repository permission, such as contents: %s and pull-requests: %s. These permissions apply to every job that receives GITHUB_TOKEN. Use permissions: %s at the workflow top level only when every supported repository permission should have %s access. If you need different repository permissions for individual jobs, 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, access) } scopes := make(map[string]string, len(in.Scopes)) names := make([]string, 0, len(in.Scopes)) @@ -873,17 +887,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 diff --git a/internal/workflow/parse_test.go b/internal/workflow/parse_test.go index cc6aba4b..881ef26e 100644 --- a/internal/workflow/parse_test.go +++ b/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,31 @@ 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 TestParseRejectsInvalidPermissionScalars(t *testing.T) { + for _, test := range []struct { + name, source, want string + }{ + {name: "workflow", source: "on: push\npermissions: write\njobs:\n test:\n runs-on: ubuntu-latest\n steps: [{run: true}]\n", want: `permissions.yml:2:14: workflow permissions: invalid permissions scalar "write"; use read-all, write-all, or a permissions map`}, + {name: "job", source: "on: push\njobs:\n test:\n permissions: write\n runs-on: ubuntu-latest\n steps: [{run: true}]\n", want: `permissions.yml:4:18: job "test": invalid permissions scalar "write"; declare each needed permission in a map`}, + } { + t.Run(test.name, func(t *testing.T) { + _, err := Parse("permissions.yml", []byte(test.source)) + if err == nil || err.Error() != test.want { + t.Fatalf("Parse() error = %q, want %q", err, test.want) + } + }) + } +} + +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", you cannot set separate repository permissions. At the workflow top level, declare each needed repository permission, such as contents: ` + access + ` and pull-requests: ` + access + `. These permissions apply to every job that receives GITHUB_TOKEN. Use permissions: ` + shorthand + ` at the workflow top level only when every supported repository permission should have ` + access + ` access. If you need different repository permissions for individual jobs, 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) } }) }