From 6776eba817f5d22c89aa751949e6fc9b02a0adbd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 19 Aug 2026 11:40:34 +0000 Subject: [PATCH 1/6] Initial plan From adb32dbf3f0952410cf50dc32e8836bc7e986c01 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 19 Aug 2026 11:54:21 +0000 Subject: [PATCH 2/6] Support runner groups in safe jobs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .changeset/major-safe-job-runs-on.md | 5 + .../content/docs/reference/safe-outputs.md | 18 +++ .../docs/reference/self-hosted-runners.md | 2 + pkg/cli/codemod_safe_job_runner.go | 143 ++++++++++++++++++ pkg/cli/codemod_safe_job_runner_test.go | 121 +++++++++++++++ pkg/cli/fix_codemods.go | 1 + pkg/cli/fix_codemods_test.go | 2 + pkg/parser/schemas/main_workflow_schema.json | 30 +--- pkg/workflow/frontmatter_parsing.go | 9 ++ pkg/workflow/runs_on_validation.go | 10 ++ pkg/workflow/runs_on_validation_test.go | 18 +++ pkg/workflow/safe_jobs.go | 15 +- pkg/workflow/safe_jobs_syntax_test.go | 32 ++++ pkg/workflow/safe_jobs_test.go | 79 ++++++++++ 14 files changed, 457 insertions(+), 28 deletions(-) create mode 100644 .changeset/major-safe-job-runs-on.md create mode 100644 pkg/cli/codemod_safe_job_runner.go create mode 100644 pkg/cli/codemod_safe_job_runner_test.go diff --git a/.changeset/major-safe-job-runs-on.md b/.changeset/major-safe-job-runs-on.md new file mode 100644 index 00000000000..c9c86093c2d --- /dev/null +++ b/.changeset/major-safe-job-runs-on.md @@ -0,0 +1,5 @@ +"gh-aw": major + +Support runner-group objects in custom safe-job `runs-on` fields. + +The deprecated `safe-outputs.jobs..runner` alias has been removed. Run `gh aw fix` to migrate existing workflows to `safe-outputs.jobs..runs-on`. diff --git a/docs/src/content/docs/reference/safe-outputs.md b/docs/src/content/docs/reference/safe-outputs.md index 11c2deec077..b09466d33dc 100644 --- a/docs/src/content/docs/reference/safe-outputs.md +++ b/docs/src/content/docs/reference/safe-outputs.md @@ -1895,6 +1895,24 @@ safe-outputs: `safe-outputs.runs-on` overrides `runs-on-slim:` for safe-output jobs specifically. To override the runner for all framework jobs at once, use the top-level [`runs-on-slim:`](/gh-aw/reference/self-hosted-runners/#configuring-the-framework-job-runner) field instead. +Custom safe-jobs can select their own runner with `safe-outputs.jobs..runs-on`. This field supports runner labels, label arrays, and runner-group objects: + +```aw +--- +safe-outputs: + jobs: + notify: + runs-on: + group: safe-job-runners + labels: [linux] + inputs: + message: + description: Notification message + steps: + - run: echo "Notify" +--- +``` + ### Safe Outputs Job Concurrency (`concurrency-group:`) Control concurrency for the compiled `safe_outputs` job. When set, the job uses this group with `cancel-in-progress: false` (queuing semantics — in-progress runs are never cancelled). diff --git a/docs/src/content/docs/reference/self-hosted-runners.md b/docs/src/content/docs/reference/self-hosted-runners.md index 676183415b9..b22d5da6fd2 100644 --- a/docs/src/content/docs/reference/self-hosted-runners.md +++ b/docs/src/content/docs/reference/self-hosted-runners.md @@ -79,6 +79,8 @@ runs-on: --- ``` +The string, array, and object forms are supported by the top-level `runs-on`, `runs-on-slim`, `safe-outputs.runs-on`, `safe-outputs.threat-detection.runs-on`, and custom `safe-outputs.jobs..runs-on` fields. + ## Sharing configuration via imports `runs-on` must be set in each workflow — it is not merged from imports. Other settings like `network` and `tools` can be shared: diff --git a/pkg/cli/codemod_safe_job_runner.go b/pkg/cli/codemod_safe_job_runner.go new file mode 100644 index 00000000000..dc2be2c23a4 --- /dev/null +++ b/pkg/cli/codemod_safe_job_runner.go @@ -0,0 +1,143 @@ +package cli + +import ( + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var safeJobRunnerCodemodLog = logger.New("cli:codemod_safe_job_runner") + +func getSafeJobRunnerCodemod() Codemod { + return Codemod{ + ID: "safe-job-runner-to-runs-on", + Name: "Rename safe-outputs.jobs runner to runs-on", + Description: "Renames deprecated safe-outputs.jobs..runner fields to runs-on.", + IntroducedIn: "1.5.0", + Apply: func(content string, _ map[string]any) (string, bool, error) { + newContent, applied, err := applyFrontmatterLineTransform(content, renameSafeJobRunnerKeys) + if applied { + safeJobRunnerCodemodLog.Print("Renamed safe-job runner fields to runs-on") + } + return newContent, applied, err + }, + } +} + +func renameSafeJobRunnerKeys(lines []string) ([]string, bool) { + result := append([]string(nil), lines...) + modified := false + + for i := range lines { + if strings.TrimSpace(lines[i]) != "safe-outputs:" { + continue + } + + safeOutputsIndent := len(getIndentation(lines[i])) + childIndent := -1 + for j := i + 1; j < len(lines); j++ { + trimmed := strings.TrimSpace(lines[j]) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + + indent := len(getIndentation(lines[j])) + if indent <= safeOutputsIndent { + break + } + if childIndent == -1 { + childIndent = indent + } + if indent != childIndent || trimmed != "jobs:" { + continue + } + + if renameSafeJobRunnerKeysInJobsBlock(result, lines, j) { + modified = true + } + break + } + } + + return result, modified +} + +func renameSafeJobRunnerKeysInJobsBlock(result, lines []string, jobsLine int) bool { + jobsIndent := len(getIndentation(lines[jobsLine])) + jobIndent := -1 + jobStarts := []int{} + blockEnd := jobsLine + 1 + + for i := jobsLine + 1; i < len(lines); i++ { + trimmed := strings.TrimSpace(lines[i]) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + blockEnd = i + 1 + continue + } + + indent := len(getIndentation(lines[i])) + if indent <= jobsIndent { + blockEnd = i + break + } + blockEnd = i + 1 + if jobIndent == -1 { + jobIndent = indent + } + if indent == jobIndent { + jobStarts = append(jobStarts, i) + } + } + + modified := false + for i, start := range jobStarts { + end := blockEnd + if i+1 < len(jobStarts) { + end = jobStarts[i+1] + } + if renameSafeJobRunnerKeyInJob(result, lines, start, end) { + modified = true + } + } + return modified +} + +func renameSafeJobRunnerKeyInJob(result, lines []string, start, end int) bool { + jobIndent := len(getIndentation(lines[start])) + fieldIndent := -1 + runnerLine := -1 + hasRunsOn := false + + for i := start + 1; i < end; i++ { + trimmed := strings.TrimSpace(lines[i]) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + + indent := len(getIndentation(lines[i])) + if indent <= jobIndent { + break + } + if fieldIndent == -1 { + fieldIndent = indent + } + if indent != fieldIndent { + continue + } + if strings.HasPrefix(trimmed, "runs-on:") { + hasRunsOn = true + } + if strings.HasPrefix(trimmed, "runner:") { + runnerLine = i + } + } + + if runnerLine == -1 || hasRunsOn { + return false + } + replacement, replaced := findAndReplaceInLine(lines[runnerLine], "runner", "runs-on") + if replaced { + result[runnerLine] = replacement + } + return replaced +} diff --git a/pkg/cli/codemod_safe_job_runner_test.go b/pkg/cli/codemod_safe_job_runner_test.go new file mode 100644 index 00000000000..4fc68a7894d --- /dev/null +++ b/pkg/cli/codemod_safe_job_runner_test.go @@ -0,0 +1,121 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSafeJobRunnerCodemod(t *testing.T) { + codemod := getSafeJobRunnerCodemod() + + t.Run("metadata", func(t *testing.T) { + assert.Equal(t, "safe-job-runner-to-runs-on", codemod.ID) + assert.Equal(t, "Rename safe-outputs.jobs runner to runs-on", codemod.Name) + assert.Equal(t, "Renames deprecated safe-outputs.jobs..runner fields to runs-on.", codemod.Description) + assert.Equal(t, "1.5.0", codemod.IntroducedIn) + require.NotNil(t, codemod.Apply) + }) + + tests := []struct { + name string + content string + want string + wantApplied bool + }{ + { + name: "renames scalar runner", + content: `--- +safe-outputs: + jobs: + notify: + runner: ubuntu-latest + steps: + - run: echo hi +---`, + want: `--- +safe-outputs: + jobs: + notify: + runs-on: ubuntu-latest + steps: + - run: echo hi +---`, + wantApplied: true, + }, + { + name: "preserves runner group block", + content: `--- +safe-outputs: + jobs: + notify: + runner: # runner group + group: larger-runners + labels: [linux] +---`, + want: `--- +safe-outputs: + jobs: + notify: + runs-on: # runner group + group: larger-runners + labels: [linux] +---`, + wantApplied: true, + }, + { + name: "skips job with canonical field", + content: `--- +safe-outputs: + jobs: + notify: + runner: old-runner + runs-on: ubuntu-latest +--- +`, + want: `--- +safe-outputs: + jobs: + notify: + runner: old-runner + runs-on: ubuntu-latest +--- +`, + wantApplied: false, + }, + { + name: "ignores runner outside safe jobs", + content: `--- +runner: top-level +safe-outputs: + create-issue: {} +jobs: + build: + runner: custom +--- +`, + want: `--- +runner: top-level +safe-outputs: + create-issue: {} +jobs: + build: + runner: custom +--- +`, + wantApplied: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, applied, err := codemod.Apply(tt.content, map[string]any{}) + require.NoError(t, err) + assert.Equal(t, tt.wantApplied, applied) + assert.Equal(t, tt.want, result) + }) + } +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index b16c50a60b1..127ce53ed3d 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -91,6 +91,7 @@ func GetAllCodemods() []Codemod { getSafeOutputMergePRConstraintsCodemod(), // Rename deprecated merge-pull-request allowed-labels/allowed-branches getSafeOutputAddReviewerAllowlistsCodemod(), // Rename deprecated add-reviewer reviewers/team-reviewers getSafeOutputDispatchRepositoryKeyCodemod(), // Rename deprecated safe-outputs.dispatch_repository key + getSafeJobRunnerCodemod(), // Rename deprecated safe-outputs.jobs runner fields getSafeInputsToMCPScriptsCodemod(), // Rename safe-inputs to mcp-scripts getRateLimitToUserRateLimitCodemod(), // Rename rate-limit to user-rate-limit with max key migration getEffectiveTokensToAICreditsCodemod(), // Migrate obsolete effective-token budget keys to AI credits keys diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 7c9dda1a545..43ef7735c82 100644 --- a/pkg/cli/fix_codemods_test.go +++ b/pkg/cli/fix_codemods_test.go @@ -99,6 +99,7 @@ func TestGetAllCodemods_ContainsExpectedCodemods(t *testing.T) { "safe-output-merge-pr-constraints", "safe-output-add-reviewer-allowlists", "safe-output-dispatch-repository-key", + "safe-job-runner-to-runs-on", "safe-inputs-to-mcp-scripts", "rate-limit-to-user-rate-limit", "effective-tokens-to-ai-credits", @@ -228,6 +229,7 @@ func expectedCodemodOrder() []string { "safe-output-merge-pr-constraints", "safe-output-add-reviewer-allowlists", "safe-output-dispatch-repository-key", + "safe-job-runner-to-runs-on", "safe-inputs-to-mcp-scripts", "rate-limit-to-user-rate-limit", "effective-tokens-to-ai-credits", diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 5dbce6371a6..1d25b4f4118 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -11041,16 +11041,14 @@ "description": "Description of the safe-job (used in MCP tool registration)" }, "runs-on": { - "description": "Runner specification for this job", - "oneOf": [ - { - "type": "string" - }, + "$ref": "#/$defs/github_actions_runs_on", + "description": "Runner specification for this job. Supports string, array, or runner-group object forms. Defaults to 'ubuntu-latest'.", + "examples": [ + "ubuntu-latest", + ["self-hosted", "linux", "x64"], { - "type": "array", - "items": { - "type": "string" - } + "group": "larger-runners", + "labels": ["ubuntu-latest-8-cores"] } ] }, @@ -11156,20 +11154,6 @@ "$ref": "#/$defs/githubActionsStep" } }, - "runner": { - "description": "Runner specification for this job (alias for runs-on)", - "oneOf": [ - { - "type": "string" - }, - { - "type": "array", - "items": { - "type": "string" - } - } - ] - }, "agent-output": { "type": "string", "description": "Agent output field to use as input for this safe job (alias for output)" diff --git a/pkg/workflow/frontmatter_parsing.go b/pkg/workflow/frontmatter_parsing.go index 0cc0d3d279b..b59b8d66a62 100644 --- a/pkg/workflow/frontmatter_parsing.go +++ b/pkg/workflow/frontmatter_parsing.go @@ -51,6 +51,15 @@ func ParseFrontmatterConfig(frontmatter map[string]any) (*FrontmatterConfig, err return nil, err } } + if jobsRaw, ok := safeOutputsRaw["jobs"].(map[string]any); ok { + for _, jobRaw := range jobsRaw { + if job, ok := jobRaw.(map[string]any); ok { + if err := validateRunsOnValue(job["runs-on"]); err != nil { + return nil, err + } + } + } + } } // Parse typed Runtimes field if runtimes exist diff --git a/pkg/workflow/runs_on_validation.go b/pkg/workflow/runs_on_validation.go index 75323a41f14..04dbc559c17 100644 --- a/pkg/workflow/runs_on_validation.go +++ b/pkg/workflow/runs_on_validation.go @@ -57,6 +57,16 @@ func validateRunsOn(frontmatter map[string]any, markdownPath string) error { if threatDetection, ok := safeOutputs["threat-detection"].(map[string]any); ok { runsOnFields = append(runsOnFields, runnerField{name: "safe-outputs.threat-detection.runs-on", value: threatDetection["runs-on"]}) } + if jobs, ok := safeOutputs["jobs"].(map[string]any); ok { + for jobName, jobValue := range jobs { + if job, ok := jobValue.(map[string]any); ok { + runsOnFields = append(runsOnFields, runnerField{ + name: fmt.Sprintf("safe-outputs.jobs.%s.runs-on", jobName), + value: job["runs-on"], + }) + } + } + } } for _, field := range runsOnFields { diff --git a/pkg/workflow/runs_on_validation_test.go b/pkg/workflow/runs_on_validation_test.go index 44276d2cff2..b36b5b47bff 100644 --- a/pkg/workflow/runs_on_validation_test.go +++ b/pkg/workflow/runs_on_validation_test.go @@ -191,6 +191,24 @@ func TestValidateRunsOn(t *testing.T) { wantErr: false, description: "threat-detection runs-on with a Linux runner should be accepted", }, + { + name: "macos in custom safe-job runs-on labels", + frontmatter: map[string]any{ + "safe-outputs": map[string]any{ + "jobs": map[string]any{ + "notify": map[string]any{ + "runs-on": map[string]any{ + "group": "runner-group", + "labels": []any{"linux", "macos-latest"}, + }, + }, + }, + }, + }, + wantErr: true, + errorInMsg: "safe-outputs.jobs.notify.runs-on", + description: "custom safe-job runs-on labels containing macos should be rejected", + }, } for _, tt := range tests { diff --git a/pkg/workflow/safe_jobs.go b/pkg/workflow/safe_jobs.go index 29bb412e20a..946bf173407 100644 --- a/pkg/workflow/safe_jobs.go +++ b/pkg/workflow/safe_jobs.go @@ -32,6 +32,7 @@ type SafeJobConfig struct { Output string `yaml:"output,omitempty"` Max int `yaml:"max,omitempty"` // Maximum number of times this output type may be emitted per run (default: 1) runsOnArray bool `yaml:"-"` + runsOnMap map[string]any `yaml:"-"` } // parseSafeJobsConfig parses safe-jobs configuration from a jobs map. @@ -67,13 +68,11 @@ func (c *Compiler) parseSafeJobsConfig(jobsMap map[string]any) map[string]*SafeJ } } - // Parse runs-on (also accept "runner" as alias) + // Parse runs-on if runsOn, exists := jobConfig["runs-on"]; exists { safeJob.RunsOn = toRunsOnValue(runsOn) safeJob.runsOnArray = isRunsOnArrayValue(runsOn) - } else if runner, exists := jobConfig["runner"]; exists { - safeJob.RunsOn = toRunsOnValue(runner) - safeJob.runsOnArray = isRunsOnArrayValue(runner) + safeJob.runsOnMap, _ = runsOn.(map[string]any) } // Parse if condition @@ -239,7 +238,13 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool // Set runs-on. Preserve list-shaped input from safe-outputs.jobs as a // YAML array; formatSafeJobRunsOn centralizes the array-vs-scalar // rendering decision shared with other runs-on parsers. - job.RunsOn = c.indentYAMLLines(formatSafeJobRunsOn(jobConfig.RunsOn, jobConfig.runsOnArray, defaultRunsOn), " ") + runsOn := formatSafeJobRunsOn(jobConfig.RunsOn, jobConfig.runsOnArray, defaultRunsOn) + if jobConfig.runsOnMap != nil { + if snippet := renderRunsOnSnippet(jobConfig.runsOnMap); snippet != "" { + runsOn = snippet + } + } + job.RunsOn = c.indentYAMLLines(runsOn, " ") // Set if condition - combine safe output type check with user-provided condition // Custom safe jobs should only run if the agent output contains the job name (tool call) diff --git a/pkg/workflow/safe_jobs_syntax_test.go b/pkg/workflow/safe_jobs_syntax_test.go index e2e1cbdfe11..3e1c17efc4c 100644 --- a/pkg/workflow/safe_jobs_syntax_test.go +++ b/pkg/workflow/safe_jobs_syntax_test.go @@ -102,3 +102,35 @@ Test new syntax t.Error("Expected 'deploy' job to exist in SafeOutputs.Jobs") } } + +func TestSafeOutputsJobsRunnerAliasRejected(t *testing.T) { + c := NewCompiler() + tmpDir := testutil.TempDir(t, "test-*") + workflowPath := filepath.Join(tmpDir, "test-runner-alias.md") + content := `--- +on: issues +permissions: + contents: read +safe-outputs: + jobs: + deploy: + runner: ubuntu-latest + steps: + - run: echo test +--- + +# Test workflow +` + err := os.WriteFile(workflowPath, []byte(content), 0o644) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + _, err = c.ParseWorkflowFile(workflowPath) + if err == nil { + t.Fatal("Expected runner alias to be rejected") + } + if !strings.Contains(err.Error(), "runner") { + t.Errorf("Expected error to mention 'runner', got: %v", err) + } +} diff --git a/pkg/workflow/safe_jobs_test.go b/pkg/workflow/safe_jobs_test.go index 07259978c1d..6264e306fe1 100644 --- a/pkg/workflow/safe_jobs_test.go +++ b/pkg/workflow/safe_jobs_test.go @@ -402,6 +402,85 @@ func TestParseAndBuildSafeJobsRunsOnList(t *testing.T) { require.Equal(t, "runs-on:\n - self-hosted\n - linux", job.RunsOn) } +func TestParseAndBuildSafeJobsRunsOnObject(t *testing.T) { + tests := []struct { + name string + runsOn map[string]any + expected string + }{ + { + name: "group only", + runsOn: map[string]any{"group": "safe-job-runners"}, + expected: "runs-on:\n group: safe-job-runners", + }, + { + name: "group and labels", + runsOn: map[string]any{ + "group": "safe-job-runners", + "labels": []any{"linux", "x64"}, + }, + expected: "runs-on:\n group: safe-job-runners\n labels:\n - linux\n - x64", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := NewCompiler() + safeJobs := c.parseSafeJobsConfig(map[string]any{ + "deploy": map[string]any{ + "runs-on": tt.runsOn, + "steps": []any{map[string]any{"run": "echo 'Deploying'"}}, + }, + }) + + _, err := c.buildSafeJobs(&WorkflowData{ + Name: "test-workflow", + SafeOutputs: &SafeOutputsConfig{Jobs: safeJobs}, + }, false) + require.NoError(t, err) + + jobs := c.jobManager.GetAllJobs() + require.Len(t, jobs, 1) + for _, job := range jobs { + require.Equal(t, tt.expected, job.RunsOn) + } + }) + } +} + +func TestCompileSafeJobRunsOnObject(t *testing.T) { + tmpDir := testutil.TempDir(t, "safe-job-runs-on-object") + workflowPath := filepath.Join(tmpDir, "safe-job-runs-on-object.md") + content := `--- +on: workflow_dispatch +permissions: read-all +engine: copilot +safe-outputs: + jobs: + notify: + runs-on: + group: safe-job-runners + labels: [linux] + inputs: + message: + description: Message + steps: + - run: echo hi +--- + +# Test +` + require.NoError(t, os.WriteFile(workflowPath, []byte(content), 0o644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(workflowPath)) + + compiled, err := os.ReadFile(filepath.Join(tmpDir, "safe-job-runs-on-object.lock.yml")) + require.NoError(t, err) + notifyJob := extractJobSection(string(compiled), "notify") + require.Contains(t, notifyJob, " runs-on:\n group: safe-job-runners\n labels:\n - linux") +} + func TestParseAndBuildSafeJobsSingleRunsOnList(t *testing.T) { c := NewCompiler() From 94f99ace736366757fb9f571d3ab65cb6788e3e4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 19 Aug 2026 11:59:33 +0000 Subject: [PATCH 3/6] Reject empty safe-job runner objects Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/runs_on_validation.go | 4 ++++ pkg/workflow/runs_on_validation_test.go | 6 ++++++ pkg/workflow/safe_jobs.go | 5 +++-- pkg/workflow/safe_jobs_test.go | 16 ++++++++++++++++ 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/runs_on_validation.go b/pkg/workflow/runs_on_validation.go index 04dbc559c17..b528244634b 100644 --- a/pkg/workflow/runs_on_validation.go +++ b/pkg/workflow/runs_on_validation.go @@ -24,6 +24,7 @@ package workflow import ( + "errors" "fmt" "strings" @@ -104,6 +105,9 @@ func validateRunsOnValue(value any) error { } return nil case map[string]any: + if len(v) == 0 { + return errors.New("runs-on object is empty. Expected an object with 'group' or 'labels'. Example: runs-on:\n group: my-runner-group") + } for key, value := range v { switch key { case "group": diff --git a/pkg/workflow/runs_on_validation_test.go b/pkg/workflow/runs_on_validation_test.go index b36b5b47bff..162d07a95d1 100644 --- a/pkg/workflow/runs_on_validation_test.go +++ b/pkg/workflow/runs_on_validation_test.go @@ -317,6 +317,12 @@ func TestValidateRunsOnValue(t *testing.T) { wantErr: true, errContain: "runs-on object key 'runner' is not supported", }, + { + name: "empty object is invalid", + value: map[string]any{}, + wantErr: true, + errContain: "runs-on object is empty", + }, { name: "unsupported type is invalid", value: 123, diff --git a/pkg/workflow/safe_jobs.go b/pkg/workflow/safe_jobs.go index 946bf173407..f9b731cb5c5 100644 --- a/pkg/workflow/safe_jobs.go +++ b/pkg/workflow/safe_jobs.go @@ -240,8 +240,9 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool // rendering decision shared with other runs-on parsers. runsOn := formatSafeJobRunsOn(jobConfig.RunsOn, jobConfig.runsOnArray, defaultRunsOn) if jobConfig.runsOnMap != nil { - if snippet := renderRunsOnSnippet(jobConfig.runsOnMap); snippet != "" { - runsOn = snippet + runsOn = renderRunsOnSnippet(jobConfig.runsOnMap) + if runsOn == "" { + return nil, fmt.Errorf("runs-on field for safe-job '%s' is empty. Expected an object with 'group' or 'labels'. Example: runs-on:\n group: my-runner-group", normalizedJobName) } } job.RunsOn = c.indentYAMLLines(runsOn, " ") diff --git a/pkg/workflow/safe_jobs_test.go b/pkg/workflow/safe_jobs_test.go index 6264e306fe1..b2eeb0cdf79 100644 --- a/pkg/workflow/safe_jobs_test.go +++ b/pkg/workflow/safe_jobs_test.go @@ -448,6 +448,22 @@ func TestParseAndBuildSafeJobsRunsOnObject(t *testing.T) { } } +func TestBuildSafeJobsRejectsEmptyRunsOnObject(t *testing.T) { + c := NewCompiler() + safeJobs := c.parseSafeJobsConfig(map[string]any{ + "deploy": map[string]any{ + "runs-on": map[string]any{}, + "steps": []any{map[string]any{"run": "echo 'Deploying'"}}, + }, + }) + + _, err := c.buildSafeJobs(&WorkflowData{ + Name: "test-workflow", + SafeOutputs: &SafeOutputsConfig{Jobs: safeJobs}, + }, false) + require.ErrorContains(t, err, "runs-on field for safe-job 'deploy' is empty") +} + func TestCompileSafeJobRunsOnObject(t *testing.T) { tmpDir := testutil.TempDir(t, "safe-job-runs-on-object") workflowPath := filepath.Join(tmpDir, "safe-job-runs-on-object.md") From 96f450125d98e827c4f77625e901bcb444c3b7d5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 19 Aug 2026 12:34:31 +0000 Subject: [PATCH 4/6] Reuse shared runs-on parser for safe jobs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../compiler_custom_job_properties.go | 3 ++ pkg/workflow/runs_on_snippet.go | 39 ------------------- pkg/workflow/safe_jobs.go | 34 ++++++++-------- pkg/workflow/safe_jobs_test.go | 35 ++++++++--------- pkg/workflow/safe_outputs_import_test.go | 2 +- 5 files changed, 37 insertions(+), 76 deletions(-) diff --git a/pkg/workflow/compiler_custom_job_properties.go b/pkg/workflow/compiler_custom_job_properties.go index 3bcdd1ca752..aedd7a2b57d 100644 --- a/pkg/workflow/compiler_custom_job_properties.go +++ b/pkg/workflow/compiler_custom_job_properties.go @@ -80,6 +80,9 @@ func (c *Compiler) extractCustomJobRunsOn(job *Job, jobName string, configMap ma if !hasRunsOn { return nil } + if err := validateRunsOnValue(runsOn); err != nil { + return fmt.Errorf("runs-on field for job '%s' is invalid: %w", jobName, err) + } if runsOnStr, ok := runsOn.(string); ok { job.RunsOn = "runs-on: " + runsOnStr return nil diff --git a/pkg/workflow/runs_on_snippet.go b/pkg/workflow/runs_on_snippet.go index 05c0509c8d3..9d43febb90a 100644 --- a/pkg/workflow/runs_on_snippet.go +++ b/pkg/workflow/runs_on_snippet.go @@ -37,45 +37,6 @@ func (r *RunsOnValue) UnmarshalJSON(data []byte) error { return nil } -// toRunsOnValue converts a YAML-decoded runs-on value (a string or a list of -// strings) into a RunsOnValue. Values with an unsupported shape, and non-string -// list entries, are ignored. -func toRunsOnValue(value any) RunsOnValue { - switch v := value.(type) { - case string: - return RunsOnValue{v} - case []any: - labels := make(RunsOnValue, 0, len(v)) - for _, item := range v { - if itemStr, ok := item.(string); ok { - labels = append(labels, itemStr) - } - } - if len(labels) == 0 { - return nil - } - return labels - case []string: - if len(v) == 0 { - return nil - } - return RunsOnValue(v) - default: - return nil - } -} - -// isRunsOnArrayValue reports whether a YAML-decoded runs-on value has array -// shape (as opposed to a single string label). -func isRunsOnArrayValue(value any) bool { - switch value.(type) { - case []any, []string: - return true - default: - return false - } -} - // FormatRunsOn serialises a RunsOnValue to a YAML-compatible string that can // be inlined directly after "runs-on: " in a generated workflow. // diff --git a/pkg/workflow/safe_jobs.go b/pkg/workflow/safe_jobs.go index f9b731cb5c5..39fd91069b7 100644 --- a/pkg/workflow/safe_jobs.go +++ b/pkg/workflow/safe_jobs.go @@ -18,7 +18,7 @@ type SafeJobConfig struct { // Standard GitHub Actions job properties Name string `yaml:"name,omitempty"` Description string `yaml:"description,omitempty"` - RunsOn RunsOnValue `yaml:"runs-on,omitempty"` + RunsOn string `yaml:"runs-on,omitempty"` If string `yaml:"if,omitempty"` Needs []string `yaml:"needs,omitempty"` Steps []any `yaml:"steps,omitempty"` @@ -31,8 +31,7 @@ type SafeJobConfig struct { GitHubToken string `yaml:"github-token,omitempty"` Output string `yaml:"output,omitempty"` Max int `yaml:"max,omitempty"` // Maximum number of times this output type may be emitted per run (default: 1) - runsOnArray bool `yaml:"-"` - runsOnMap map[string]any `yaml:"-"` + runsOnError error `yaml:"-"` } // parseSafeJobsConfig parses safe-jobs configuration from a jobs map. @@ -68,11 +67,12 @@ func (c *Compiler) parseSafeJobsConfig(jobsMap map[string]any) map[string]*SafeJ } } - // Parse runs-on - if runsOn, exists := jobConfig["runs-on"]; exists { - safeJob.RunsOn = toRunsOnValue(runsOn) - safeJob.runsOnArray = isRunsOnArrayValue(runsOn) - safeJob.runsOnMap, _ = runsOn.(map[string]any) + // Parse runs-on using the shared custom-job parser. + runsOnJob := &Job{} + if err := c.extractCustomJobRunsOn(runsOnJob, jobName, jobConfig); err != nil { + safeJob.runsOnError = err + } else { + safeJob.RunsOn = runsOnJob.RunsOn } // Parse if condition @@ -235,17 +235,15 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool const defaultRunsOn = "ubuntu-latest" - // Set runs-on. Preserve list-shaped input from safe-outputs.jobs as a - // YAML array; formatSafeJobRunsOn centralizes the array-vs-scalar - // rendering decision shared with other runs-on parsers. - runsOn := formatSafeJobRunsOn(jobConfig.RunsOn, jobConfig.runsOnArray, defaultRunsOn) - if jobConfig.runsOnMap != nil { - runsOn = renderRunsOnSnippet(jobConfig.runsOnMap) - if runsOn == "" { - return nil, fmt.Errorf("runs-on field for safe-job '%s' is empty. Expected an object with 'group' or 'labels'. Example: runs-on:\n group: my-runner-group", normalizedJobName) - } + // Set runs-on, defaulting to ubuntu-latest when omitted. + if jobConfig.runsOnError != nil { + return nil, fmt.Errorf("invalid runs-on for safe-job '%s': %w", normalizedJobName, jobConfig.runsOnError) + } + runsOn := jobConfig.RunsOn + if runsOn == "" { + runsOn = "runs-on: " + defaultRunsOn } - job.RunsOn = c.indentYAMLLines(runsOn, " ") + job.RunsOn = runsOn // Set if condition - combine safe output type check with user-provided condition // Custom safe jobs should only run if the agent output contains the job name (tool call) diff --git a/pkg/workflow/safe_jobs_test.go b/pkg/workflow/safe_jobs_test.go index b2eeb0cdf79..ecdd5170ee1 100644 --- a/pkg/workflow/safe_jobs_test.go +++ b/pkg/workflow/safe_jobs_test.go @@ -70,7 +70,7 @@ func TestParseSafeJobsConfig(t *testing.T) { } // Test runs-on - if len(deployJob.RunsOn) != 1 || deployJob.RunsOn[0] != "ubuntu-latest" { + if deployJob.RunsOn != "runs-on: ubuntu-latest" { t.Errorf("Expected runs-on to be 'ubuntu-latest', got %v", deployJob.RunsOn) } @@ -232,7 +232,7 @@ func TestBuildSafeJobs(t *testing.T) { SafeOutputs: &SafeOutputsConfig{ Jobs: map[string]*SafeJobConfig{ "deploy": { - RunsOn: RunsOnValue{"ubuntu-latest"}, + RunsOn: "runs-on: ubuntu-latest", If: "github.event.issue.number", Env: map[string]string{ "DEPLOY_ENV": "production", @@ -378,8 +378,7 @@ func TestParseAndBuildSafeJobsRunsOnList(t *testing.T) { }, }) - require.Equal(t, RunsOnValue{"self-hosted", "linux"}, safeJobs["deploy"].RunsOn) - require.True(t, safeJobs["deploy"].runsOnArray) + require.Equal(t, "runs-on:\n - self-hosted\n - linux", safeJobs["deploy"].RunsOn) workflowData := &WorkflowData{ Name: "test-workflow", @@ -419,7 +418,7 @@ func TestParseAndBuildSafeJobsRunsOnObject(t *testing.T) { "group": "safe-job-runners", "labels": []any{"linux", "x64"}, }, - expected: "runs-on:\n group: safe-job-runners\n labels:\n - linux\n - x64", + expected: "runs-on:\n group: safe-job-runners\n labels:\n - linux\n - x64", }, } @@ -461,7 +460,8 @@ func TestBuildSafeJobsRejectsEmptyRunsOnObject(t *testing.T) { Name: "test-workflow", SafeOutputs: &SafeOutputsConfig{Jobs: safeJobs}, }, false) - require.ErrorContains(t, err, "runs-on field for safe-job 'deploy' is empty") + require.ErrorContains(t, err, "invalid runs-on for safe-job 'deploy'") + require.ErrorContains(t, err, "runs-on object is empty") } func TestCompileSafeJobRunsOnObject(t *testing.T) { @@ -494,7 +494,7 @@ safe-outputs: compiled, err := os.ReadFile(filepath.Join(tmpDir, "safe-job-runs-on-object.lock.yml")) require.NoError(t, err) notifyJob := extractJobSection(string(compiled), "notify") - require.Contains(t, notifyJob, " runs-on:\n group: safe-job-runners\n labels:\n - linux") + require.Contains(t, notifyJob, " runs-on:\n group: safe-job-runners\n labels:\n - linux") } func TestParseAndBuildSafeJobsSingleRunsOnList(t *testing.T) { @@ -509,8 +509,7 @@ func TestParseAndBuildSafeJobsSingleRunsOnList(t *testing.T) { }, }) - require.Equal(t, RunsOnValue{"self-hosted"}, safeJobs["deploy"].RunsOn) - require.True(t, safeJobs["deploy"].runsOnArray) + require.Equal(t, "runs-on:\n - self-hosted", safeJobs["deploy"].RunsOn) workflowData := &WorkflowData{ Name: "test-workflow", @@ -569,7 +568,7 @@ func TestBuildSafeJobsWithoutCustomIfCondition(t *testing.T) { SafeOutputs: &SafeOutputsConfig{ Jobs: map[string]*SafeJobConfig{ "notify": { - RunsOn: RunsOnValue{"ubuntu-latest"}, + RunsOn: "runs-on: ubuntu-latest", // No custom 'if' condition Inputs: map[string]*InputDefinition{ "message": { @@ -620,7 +619,7 @@ func TestBuildSafeJobsWithDashesInName(t *testing.T) { SafeOutputs: &SafeOutputsConfig{ Jobs: map[string]*SafeJobConfig{ "send-notification": { - RunsOn: RunsOnValue{"ubuntu-latest"}, + RunsOn: "runs-on: ubuntu-latest", Steps: []any{ map[string]any{ "name": "Send notification", @@ -744,7 +743,7 @@ func TestExtractSafeJobsFromFrontmatter(t *testing.T) { t.Error("Expected 'deploy' job to exist") } - if len(deployJob.RunsOn) != 1 || deployJob.RunsOn[0] != "ubuntu-latest" { + if deployJob.RunsOn != "runs-on: ubuntu-latest" { t.Errorf("Expected runs-on to be 'ubuntu-latest', got '%v'", deployJob.RunsOn) } } @@ -752,13 +751,13 @@ func TestExtractSafeJobsFromFrontmatter(t *testing.T) { func TestMergeSafeJobs(t *testing.T) { base := map[string]*SafeJobConfig{ "deploy": { - RunsOn: RunsOnValue{"ubuntu-latest"}, + RunsOn: "runs-on: ubuntu-latest", }, } additional := map[string]*SafeJobConfig{ "test": { - RunsOn: RunsOnValue{"ubuntu-latest"}, + RunsOn: "runs-on: ubuntu-latest", }, } @@ -775,7 +774,7 @@ func TestMergeSafeJobs(t *testing.T) { // Test conflict detection conflicting := map[string]*SafeJobConfig{ "deploy": { - RunsOn: RunsOnValue{"windows-latest"}, + RunsOn: "runs-on: windows-latest", }, } @@ -797,7 +796,7 @@ func TestMergeSafeJobsFromIncludedConfigs(t *testing.T) { topSafeJobs := map[string]*SafeJobConfig{ "deploy": { Name: "Deploy Application", - RunsOn: RunsOnValue{"ubuntu-latest"}, + RunsOn: "runs-on: ubuntu-latest", }, } @@ -848,7 +847,7 @@ func TestMergeSafeJobsFromIncludedConfigs(t *testing.T) { t.Error("Expected 'test' job from includes to exist") } - if len(testJob.RunsOn) != 1 || testJob.RunsOn[0] != "ubuntu-latest" { + if testJob.RunsOn != "runs-on: ubuntu-latest" { t.Errorf("Expected test job runs-on to be 'ubuntu-latest', got '%v'", testJob.RunsOn) } @@ -893,7 +892,7 @@ func TestBuildSafeJobsEnvExpressionHoisting(t *testing.T) { SafeOutputs: &SafeOutputsConfig{ Jobs: map[string]*SafeJobConfig{ "publish": { - RunsOn: RunsOnValue{"ubuntu-latest"}, + RunsOn: "runs-on: ubuntu-latest", Env: map[string]string{ "GH_TOKEN": "${{ github.token }}", "STATIC_VAR": "literal-value", diff --git a/pkg/workflow/safe_outputs_import_test.go b/pkg/workflow/safe_outputs_import_test.go index eba75476d9b..1faf793a1d6 100644 --- a/pkg/workflow/safe_outputs_import_test.go +++ b/pkg/workflow/safe_outputs_import_test.go @@ -1397,7 +1397,7 @@ func TestMergeSafeOutputsJobsNotMerged(t *testing.T) { Jobs: map[string]*SafeJobConfig{ "existing-job": { Name: "Existing Job", - RunsOn: RunsOnValue{"ubuntu-latest"}, + RunsOn: "runs-on: ubuntu-latest", }, }, } From c6a83528960b02b641a302017c12e41e527a747e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 00:11:37 +0000 Subject: [PATCH 5/6] docs(adr): add draft ADR-53977 for runner-group support in custom safe jobs Co-Authored-By: Claude Sonnet 4.6 --- ...-support-runner-groups-custom-safe-jobs.md | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 docs/adr/53977-support-runner-groups-custom-safe-jobs.md diff --git a/docs/adr/53977-support-runner-groups-custom-safe-jobs.md b/docs/adr/53977-support-runner-groups-custom-safe-jobs.md new file mode 100644 index 00000000000..5df36f2a6b6 --- /dev/null +++ b/docs/adr/53977-support-runner-groups-custom-safe-jobs.md @@ -0,0 +1,45 @@ +# ADR-53977: Extend Custom Safe-Job `runs-on` to Support Runner-Group Objects and Remove `runner` Alias + +**Date**: 2026-08-20 +**Status**: Draft +**Deciders**: pelikhan, copilot-swe-agent + +--- + +### Context + +Custom safe jobs (`safe-outputs.jobs.`) had their own limited `runs-on` parser that only accepted strings and label arrays. Runner-group object form (`{group: ..., labels: [...]}`) was not supported, leaving custom safe jobs unable to run on self-hosted runner groups. All other `runs-on` configuration surfaces in the framework (top-level `runs-on`, `safe-outputs.runs-on`, `safe-outputs.threat-detection.runs-on`) already accepted all three forms via the shared `extractCustomJobRunsOn` parser. The legacy `runner` key was a deprecated alias for `runs-on` that duplicated the configuration surface and required a separate code path. + +### Decision + +We will reuse the shared `extractCustomJobRunsOn` parser for custom safe jobs, making their `runs-on` field accept the same three forms (string, label array, runner-group object) as every other runner configuration surface. The deprecated `runner` alias will be removed as a breaking change (major version bump), and a `gh aw fix` codemod (`safe-job-runner-to-runs-on`) will be provided to automatically migrate existing workflows. + +### Alternatives Considered + +#### Alternative 1: Extend the Custom Safe-Job Parser In-Place + +Extend the existing `toRunsOnValue`/`isRunsOnArrayValue` helpers to also handle `map[string]any` (runner-group objects) without delegating to the shared parser. This avoids a code-sharing dependency, but duplicates validation logic (macOS label rejection, empty-object rejection, unknown key rejection) that is already tested and maintained in `extractCustomJobRunsOn`. Any future change to runner-group validation would need to be applied in two places. + +#### Alternative 2: Keep the `runner` Alias as a Deprecated No-Op + +Retain `runner` as a tolerated (but warned) alias rather than removing it outright, making the change non-breaking. This avoids the need for a migration codemod and a major version bump. However, it perpetuates two parallel configuration keys for the same concept and increases schema surface area indefinitely. Given that `gh aw fix` can automate the rename, the migration cost is low enough to justify a clean removal. + +### Consequences + +#### Positive +- Custom safe jobs now have full parity with all other runner configuration surfaces, enabling use of runner groups. +- Validation logic (macOS rejection, empty-object rejection, unknown-key rejection) is exercised from a single code path, reducing the risk of inconsistencies. +- The schema is simplified: one canonical key (`runs-on`) replaces two (`runs-on` and `runner`). +- The automated codemod minimizes user effort for migration. + +#### Negative +- This is a breaking change: workflows using `safe-outputs.jobs..runner` will fail validation until migrated. Users must run `gh aw fix` or manually rename the key. +- The major version bump signals a broader API break even though only one deprecated alias is removed, which may cause friction for teams managing dependency pins. + +#### Neutral +- The `SafeJobConfig.RunsOn` field type changes from `RunsOnValue` (a `[]string`) to `string` (the pre-serialized YAML snippet), aligning with how the shared compiler represents runner configuration internally. +- Two helper functions (`toRunsOnValue`, `isRunsOnArrayValue`) are deleted from `runs_on_snippet.go` as they are now unused. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 63caff07a73eab62442c64d8ed8dd7f9046e3c35 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Aug 2026 00:17:39 +0000 Subject: [PATCH 6/6] Address safe job runner review feedback Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .changeset/major-safe-job-runs-on.md | 2 ++ pkg/cli/codemod_safe_job_runner.go | 12 +++++--- pkg/cli/codemod_safe_job_runner_test.go | 16 ++++++++++ pkg/workflow/compiler_safe_outputs.go | 3 ++ pkg/workflow/runs_on_validation.go | 11 ++++++- pkg/workflow/safe_jobs.go | 9 ++++++ pkg/workflow/safe_jobs_test.go | 38 ++++++++++++++++++++++++ pkg/workflow/safe_outputs_import_test.go | 2 +- 8 files changed, 87 insertions(+), 6 deletions(-) diff --git a/.changeset/major-safe-job-runs-on.md b/.changeset/major-safe-job-runs-on.md index c9c86093c2d..880a8b8fad9 100644 --- a/.changeset/major-safe-job-runs-on.md +++ b/.changeset/major-safe-job-runs-on.md @@ -1,4 +1,6 @@ +--- "gh-aw": major +--- Support runner-group objects in custom safe-job `runs-on` fields. diff --git a/pkg/cli/codemod_safe_job_runner.go b/pkg/cli/codemod_safe_job_runner.go index dc2be2c23a4..68a0613aa31 100644 --- a/pkg/cli/codemod_safe_job_runner.go +++ b/pkg/cli/codemod_safe_job_runner.go @@ -29,7 +29,7 @@ func renameSafeJobRunnerKeys(lines []string) ([]string, bool) { modified := false for i := range lines { - if strings.TrimSpace(lines[i]) != "safe-outputs:" { + if !hasYAMLKey(strings.TrimSpace(lines[i]), "safe-outputs") { continue } @@ -48,7 +48,7 @@ func renameSafeJobRunnerKeys(lines []string) ([]string, bool) { if childIndent == -1 { childIndent = indent } - if indent != childIndent || trimmed != "jobs:" { + if indent != childIndent || !hasYAMLKey(trimmed, "jobs") { continue } @@ -124,10 +124,10 @@ func renameSafeJobRunnerKeyInJob(result, lines []string, start, end int) bool { if indent != fieldIndent { continue } - if strings.HasPrefix(trimmed, "runs-on:") { + if hasYAMLKey(trimmed, "runs-on") { hasRunsOn = true } - if strings.HasPrefix(trimmed, "runner:") { + if hasYAMLKey(trimmed, "runner") { runnerLine = i } } @@ -141,3 +141,7 @@ func renameSafeJobRunnerKeyInJob(result, lines []string, start, end int) bool { } return replaced } + +func hasYAMLKey(line, key string) bool { + return strings.HasPrefix(line, key+":") +} diff --git a/pkg/cli/codemod_safe_job_runner_test.go b/pkg/cli/codemod_safe_job_runner_test.go index 4fc68a7894d..caf22502ce3 100644 --- a/pkg/cli/codemod_safe_job_runner_test.go +++ b/pkg/cli/codemod_safe_job_runner_test.go @@ -63,6 +63,22 @@ safe-outputs: runs-on: # runner group group: larger-runners labels: [linux] +---`, + wantApplied: true, + }, + { + name: "matches keys with trailing comments", + content: `--- +safe-outputs: # security settings + jobs: # custom output jobs + notify: + runner: ubuntu-latest # legacy field +---`, + want: `--- +safe-outputs: # security settings + jobs: # custom output jobs + notify: + runs-on: ubuntu-latest # legacy field ---`, wantApplied: true, }, diff --git a/pkg/workflow/compiler_safe_outputs.go b/pkg/workflow/compiler_safe_outputs.go index d55a5495073..1cdd475104f 100644 --- a/pkg/workflow/compiler_safe_outputs.go +++ b/pkg/workflow/compiler_safe_outputs.go @@ -28,6 +28,9 @@ func (c *Compiler) mergeSafeJobsFromIncludedConfigs(topSafeJobs map[string]*Safe compilerSafeOutputsLog.Printf("Warning: skipping included safe-outputs config with invalid JSON: %v", err) continue } + if err := validateRunsOn(map[string]any{"safe-outputs": safeOutputsConfig}, c.markdownPath); err != nil { + return nil, err + } // Extract safe-jobs from the safe-outputs.jobs field includedSafeJobs := extractSafeJobsFromFrontmatter(map[string]any{ diff --git a/pkg/workflow/runs_on_validation.go b/pkg/workflow/runs_on_validation.go index b528244634b..2404d104ac9 100644 --- a/pkg/workflow/runs_on_validation.go +++ b/pkg/workflow/runs_on_validation.go @@ -141,7 +141,16 @@ func isEmptyRunsOnValue(value any) bool { case string: return strings.TrimSpace(v) == "" case []any: - return len(v) == 0 + if len(v) == 0 { + return true + } + for _, label := range v { + label, ok := label.(string) + if !ok || strings.TrimSpace(label) != "" { + return false + } + } + return true case map[string]any: if len(v) == 0 { return true diff --git a/pkg/workflow/safe_jobs.go b/pkg/workflow/safe_jobs.go index 39fd91069b7..f98d2b5aad5 100644 --- a/pkg/workflow/safe_jobs.go +++ b/pkg/workflow/safe_jobs.go @@ -71,6 +71,8 @@ func (c *Compiler) parseSafeJobsConfig(jobsMap map[string]any) map[string]*SafeJ runsOnJob := &Job{} if err := c.extractCustomJobRunsOn(runsOnJob, jobName, jobConfig); err != nil { safeJob.runsOnError = err + } else if isEmptySafeJobRunsOn(jobConfig["runs-on"]) { + safeJob.RunsOn = "" } else { safeJob.RunsOn = runsOnJob.RunsOn } @@ -183,6 +185,13 @@ func (c *Compiler) parseSafeJobsConfig(jobsMap map[string]any) map[string]*SafeJ return result } +func isEmptySafeJobRunsOn(value any) bool { + if _, isObject := value.(map[string]any); isObject { + return false + } + return isEmptyRunsOnValue(value) +} + // buildSafeJobs creates custom safe-output jobs defined in SafeOutputs.Jobs func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool) ([]string, error) { if data.SafeOutputs == nil || len(data.SafeOutputs.Jobs) == 0 { diff --git a/pkg/workflow/safe_jobs_test.go b/pkg/workflow/safe_jobs_test.go index ecdd5170ee1..f42446c6894 100644 --- a/pkg/workflow/safe_jobs_test.go +++ b/pkg/workflow/safe_jobs_test.go @@ -148,6 +148,30 @@ func TestParseSafeJobsConfig(t *testing.T) { } } +func TestBuildSafeJobsDefaultsEmptyRunsOn(t *testing.T) { + for name, runsOn := range map[string]any{ + "empty scalar": "", + "empty array": []any{}, + "empty label": []any{""}, + } { + t.Run(name, func(t *testing.T) { + c := NewCompiler() + safeJobs := c.parseSafeJobsConfig(map[string]any{ + "notify": map[string]any{"runs-on": runsOn}, + }) + _, err := c.buildSafeJobs(&WorkflowData{ + Name: "test-workflow", + SafeOutputs: &SafeOutputsConfig{Jobs: safeJobs}, + }, false) + + require.NoError(t, err) + job, exists := c.jobManager.GetJob("notify") + require.True(t, exists) + require.Equal(t, "runs-on: ubuntu-latest", job.RunsOn) + }) + } +} + func TestParseSafeJobsConfigMax(t *testing.T) { c := NewCompiler() @@ -1075,3 +1099,17 @@ func TestSafeJobsInputTypes(t *testing.T) { t.Errorf("Expected type 'environment', got %s", envInput.Type) } } + +func TestMergeSafeJobsFromIncludedConfigsRejectsMacOSRunner(t *testing.T) { + c := NewCompiler() + + _, err := c.mergeSafeJobsFromIncludedConfigs(nil, []string{`{ +"jobs": { +"notify": { +"runs-on": "macos-latest" +} +} +}`}) + + require.ErrorContains(t, err, "safe-outputs.jobs.notify.runs-on") +} diff --git a/pkg/workflow/safe_outputs_import_test.go b/pkg/workflow/safe_outputs_import_test.go index 1faf793a1d6..6f14d845953 100644 --- a/pkg/workflow/safe_outputs_import_test.go +++ b/pkg/workflow/safe_outputs_import_test.go @@ -1010,7 +1010,7 @@ This workflow imports safe-jobs from a shared workflow. // Verify job configuration job := workflowData.SafeOutputs.Jobs["my-custom-job"] assert.Equal(t, "My Custom Job", job.Name, "Job name should match") - assert.Equal(t, RunsOnValue{"ubuntu-latest"}, job.RunsOn, "Job runs-on should match") + assert.Equal(t, "runs-on: ubuntu-latest", job.RunsOn, "Job runs-on should match") assert.Len(t, job.Steps, 1, "Job should have 1 step") assert.Contains(t, job.Permissions, "contents", "Job should have contents permission") assert.Contains(t, job.Permissions, "issues", "Job should have issues permission")