Skip to content

Commit 10f1341

Browse files
CopilotCopilot
authored
Remove top-level safe-jobs support, enforce safe-outputs.jobs (#12439)
1 parent 893541b commit 10f1341

3 files changed

Lines changed: 80 additions & 92 deletions

File tree

‎pkg/workflow/safe_jobs.go‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,24 +34,18 @@ func HasSafeJobsEnabled(safeJobs map[string]*SafeJobConfig) bool {
3434
return len(safeJobs) > 0
3535
}
3636

37-
// parseSafeJobsConfig parses safe-jobs configuration from a frontmatter map.
38-
// This is an internal helper function that expects a map with a "safe-jobs" key.
39-
// User workflows should use "safe-outputs.jobs" syntax; the top-level "safe-jobs" key is NOT supported.
40-
func (c *Compiler) parseSafeJobsConfig(frontmatter map[string]any) map[string]*SafeJobConfig {
41-
safeJobsSection, exists := frontmatter["safe-jobs"]
42-
if !exists {
37+
// parseSafeJobsConfig parses safe-jobs configuration from a jobs map.
38+
// This function expects a map of job configurations directly (from safe-outputs.jobs).
39+
// The top-level "safe-jobs" key is NOT supported - only "safe-outputs.jobs" is valid.
40+
func (c *Compiler) parseSafeJobsConfig(jobsMap map[string]any) map[string]*SafeJobConfig {
41+
if jobsMap == nil {
4342
return nil
4443
}
4544

46-
safeJobsMap, ok := safeJobsSection.(map[string]any)
47-
if !ok {
48-
return nil
49-
}
50-
51-
safeJobsLog.Printf("Parsing %d safe-jobs from frontmatter", len(safeJobsMap))
45+
safeJobsLog.Printf("Parsing %d safe-jobs from jobs map", len(jobsMap))
5246
result := make(map[string]*SafeJobConfig)
5347

54-
for jobName, jobValue := range safeJobsMap {
48+
for jobName, jobValue := range jobsMap {
5549
jobConfig, ok := jobValue.(map[string]any)
5650
if !ok {
5751
continue
@@ -300,16 +294,15 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool
300294
}
301295

302296
// extractSafeJobsFromFrontmatter extracts safe-jobs configuration from frontmatter.
303-
// Only checks the safe-outputs.jobs location. The old top-level "safe-jobs" syntax is NOT supported.
297+
// Only checks the safe-outputs.jobs location. The top-level "safe-jobs" syntax is NOT supported.
304298
func extractSafeJobsFromFrontmatter(frontmatter map[string]any) map[string]*SafeJobConfig {
305299
// Check location: safe-outputs.jobs
306300
if safeOutputs, exists := frontmatter["safe-outputs"]; exists {
307301
if safeOutputsMap, ok := safeOutputs.(map[string]any); ok {
308302
if jobs, exists := safeOutputsMap["jobs"]; exists {
309303
if jobsMap, ok := jobs.(map[string]any); ok {
310304
c := &Compiler{} // Create a temporary compiler instance for parsing
311-
frontmatterCopy := map[string]any{"safe-jobs": jobsMap}
312-
return c.parseSafeJobsConfig(frontmatterCopy)
305+
return c.parseSafeJobsConfig(jobsMap)
313306
}
314307
}
315308
}

‎pkg/workflow/safe_jobs_test.go‎

Lines changed: 69 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -10,48 +10,46 @@ import (
1010
func TestParseSafeJobsConfig(t *testing.T) {
1111
c := NewCompiler()
1212

13-
// Test parseSafeJobsConfig internal function which expects a "safe-jobs" key.
13+
// Test parseSafeJobsConfig internal function which now expects a jobs map directly.
1414
// Note: User workflows should use "safe-outputs.jobs" syntax; this test validates
1515
// the internal parsing logic used by extractSafeJobsFromFrontmatter and safe_outputs.go.
16-
frontmatter := map[string]any{
17-
"safe-jobs": map[string]any{
18-
"deploy": map[string]any{
19-
"runs-on": "ubuntu-latest",
20-
"if": "github.event.issue.number",
21-
"needs": []any{"task"},
22-
"env": map[string]any{
23-
"DEPLOY_ENV": "production",
24-
},
25-
"permissions": map[string]any{
26-
"contents": "write",
27-
"issues": "read",
16+
jobsMap := map[string]any{
17+
"deploy": map[string]any{
18+
"runs-on": "ubuntu-latest",
19+
"if": "github.event.issue.number",
20+
"needs": []any{"task"},
21+
"env": map[string]any{
22+
"DEPLOY_ENV": "production",
23+
},
24+
"permissions": map[string]any{
25+
"contents": "write",
26+
"issues": "read",
27+
},
28+
"github-token": "${{ secrets.CUSTOM_TOKEN }}",
29+
"inputs": map[string]any{
30+
"environment": map[string]any{
31+
"description": "Target deployment environment",
32+
"required": true,
33+
"type": "choice",
34+
"options": []any{"staging", "production"},
2835
},
29-
"github-token": "${{ secrets.CUSTOM_TOKEN }}",
30-
"inputs": map[string]any{
31-
"environment": map[string]any{
32-
"description": "Target deployment environment",
33-
"required": true,
34-
"type": "choice",
35-
"options": []any{"staging", "production"},
36-
},
37-
"force": map[string]any{
38-
"description": "Force deployment even if tests fail",
39-
"required": false,
40-
"type": "boolean",
41-
"default": "false",
42-
},
36+
"force": map[string]any{
37+
"description": "Force deployment even if tests fail",
38+
"required": false,
39+
"type": "boolean",
40+
"default": "false",
4341
},
44-
"steps": []any{
45-
map[string]any{
46-
"name": "Deploy application",
47-
"run": "echo 'Deploying to ${{ inputs.environment }}'",
48-
},
42+
},
43+
"steps": []any{
44+
map[string]any{
45+
"name": "Deploy application",
46+
"run": "echo 'Deploying to ${{ inputs.environment }}'",
4947
},
5048
},
5149
},
5250
}
5351

54-
result := c.parseSafeJobsConfig(frontmatter)
52+
result := c.parseSafeJobsConfig(jobsMap)
5553

5654
if result == nil {
5755
t.Fatal("Expected safe-jobs config to be parsed, got nil")
@@ -680,52 +678,50 @@ func TestMergeSafeJobsFromIncludedConfigs(t *testing.T) {
680678
func TestSafeJobsInputTypes(t *testing.T) {
681679
c := NewCompiler()
682680

683-
frontmatter := map[string]any{
684-
"safe-jobs": map[string]any{
685-
"test-job": map[string]any{
686-
"runs-on": "ubuntu-latest",
687-
"inputs": map[string]any{
688-
"message": map[string]any{
689-
"description": "String input",
690-
"type": "string",
691-
"default": "Hello World",
692-
"required": true,
693-
},
694-
"debug": map[string]any{
695-
"description": "Boolean input",
696-
"type": "boolean",
697-
"default": false,
698-
"required": false,
699-
},
700-
"count": map[string]any{
701-
"description": "Number input",
702-
"type": "number",
703-
"default": 100,
704-
"required": true,
705-
},
706-
"environment": map[string]any{
707-
"description": "Choice input",
708-
"type": "choice",
709-
"default": "staging",
710-
"options": []any{"dev", "staging", "prod"},
711-
},
712-
"deploy_env": map[string]any{
713-
"description": "Environment input",
714-
"type": "environment",
715-
"required": false,
716-
},
681+
jobsMap := map[string]any{
682+
"test-job": map[string]any{
683+
"runs-on": "ubuntu-latest",
684+
"inputs": map[string]any{
685+
"message": map[string]any{
686+
"description": "String input",
687+
"type": "string",
688+
"default": "Hello World",
689+
"required": true,
717690
},
718-
"steps": []any{
719-
map[string]any{
720-
"name": "Test step",
721-
"run": "echo 'Testing inputs'",
722-
},
691+
"debug": map[string]any{
692+
"description": "Boolean input",
693+
"type": "boolean",
694+
"default": false,
695+
"required": false,
696+
},
697+
"count": map[string]any{
698+
"description": "Number input",
699+
"type": "number",
700+
"default": 100,
701+
"required": true,
702+
},
703+
"environment": map[string]any{
704+
"description": "Choice input",
705+
"type": "choice",
706+
"default": "staging",
707+
"options": []any{"dev", "staging", "prod"},
708+
},
709+
"deploy_env": map[string]any{
710+
"description": "Environment input",
711+
"type": "environment",
712+
"required": false,
713+
},
714+
},
715+
"steps": []any{
716+
map[string]any{
717+
"name": "Test step",
718+
"run": "echo 'Testing inputs'",
723719
},
724720
},
725721
},
726722
}
727723

728-
result := c.parseSafeJobsConfig(frontmatter)
724+
result := c.parseSafeJobsConfig(jobsMap)
729725

730726
if result == nil {
731727
t.Fatal("Expected safe-jobs config to be parsed, got nil")

‎pkg/workflow/safe_outputs_config.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,12 +360,11 @@ func (c *Compiler) extractSafeOutputsConfig(frontmatter map[string]any) *SafeOut
360360
config.Mentions = parseMentionsConfig(mentions)
361361
}
362362

363-
// Handle jobs (safe-jobs moved under safe-outputs)
363+
// Handle jobs (safe-jobs must be under safe-outputs)
364364
if jobs, exists := outputMap["jobs"]; exists {
365365
if jobsMap, ok := jobs.(map[string]any); ok {
366366
c := &Compiler{} // Create a temporary compiler instance for parsing
367-
jobsFrontmatter := map[string]any{"safe-jobs": jobsMap}
368-
config.Jobs = c.parseSafeJobsConfig(jobsFrontmatter)
367+
config.Jobs = c.parseSafeJobsConfig(jobsMap)
369368
}
370369
}
371370

0 commit comments

Comments
 (0)