Skip to content

Remove top-level safe-jobs support, enforce safe-outputs.jobs #12439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 7 commits into from
Jan 29, 2026
Merged

Remove top-level safe-jobs support, enforce safe-outputs.jobs #12439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d9bfae9
Initial plan
Copilot Jan 29, 2026
3d11109
Initial investigation: understanding safe-jobs support
Copilot Jan 29, 2026
b34a978
Refactor safe-jobs parsing and add validation to reject top-level saf…
Copilot Jan 29, 2026
985ff29
Fix linting issues in validation code and tests
Copilot Jan 29, 2026
e0be4d5
Merge branch 'main' into copilot/remove-safe-jobs-support
pelikhan Jan 29, 2026
feacfe2
Remove redundant safe-jobs validation - schema validation already cat…
Copilot Jan 29, 2026
77c9d3b
Merge branch 'main' into copilot/remove-safe-jobs-support
pelikhan Jan 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 9 additions & 16 deletions pkg/workflow/safe_jobs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,18 @@ func HasSafeJobsEnabled(safeJobs map[string]*SafeJobConfig) bool {
return len(safeJobs) > 0
}

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

safeJobsMap, ok := safeJobsSection.(map[string]any)
if !ok {
return nil
}

safeJobsLog.Printf("Parsing %d safe-jobs from frontmatter", len(safeJobsMap))
safeJobsLog.Printf("Parsing %d safe-jobs from jobs map", len(jobsMap))
result := make(map[string]*SafeJobConfig)

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

// extractSafeJobsFromFrontmatter extracts safe-jobs configuration from frontmatter.
// Only checks the safe-outputs.jobs location. The old top-level "safe-jobs" syntax is NOT supported.
// Only checks the safe-outputs.jobs location. The top-level "safe-jobs" syntax is NOT supported.
func extractSafeJobsFromFrontmatter(frontmatter map[string]any) map[string]*SafeJobConfig {
// Check location: safe-outputs.jobs
if safeOutputs, exists := frontmatter["safe-outputs"]; exists {
if safeOutputsMap, ok := safeOutputs.(map[string]any); ok {
if jobs, exists := safeOutputsMap["jobs"]; exists {
if jobsMap, ok := jobs.(map[string]any); ok {
c := &Compiler{} // Create a temporary compiler instance for parsing
frontmatterCopy := map[string]any{"safe-jobs": jobsMap}
return c.parseSafeJobsConfig(frontmatterCopy)
return c.parseSafeJobsConfig(jobsMap)
}
}
}
Expand Down
142 changes: 69 additions & 73 deletions pkg/workflow/safe_jobs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,48 +10,46 @@ import (
func TestParseSafeJobsConfig(t *testing.T) {
c := NewCompiler()

// Test parseSafeJobsConfig internal function which expects a "safe-jobs" key.
// Test parseSafeJobsConfig internal function which now expects a jobs map directly.
// Note: User workflows should use "safe-outputs.jobs" syntax; this test validates
// the internal parsing logic used by extractSafeJobsFromFrontmatter and safe_outputs.go.
frontmatter := map[string]any{
"safe-jobs": map[string]any{
"deploy": map[string]any{
"runs-on": "ubuntu-latest",
"if": "github.event.issue.number",
"needs": []any{"task"},
"env": map[string]any{
"DEPLOY_ENV": "production",
},
"permissions": map[string]any{
"contents": "write",
"issues": "read",
jobsMap := map[string]any{
"deploy": map[string]any{
"runs-on": "ubuntu-latest",
"if": "github.event.issue.number",
"needs": []any{"task"},
"env": map[string]any{
"DEPLOY_ENV": "production",
},
"permissions": map[string]any{
"contents": "write",
"issues": "read",
},
"github-token": "${{ secrets.CUSTOM_TOKEN }}",
"inputs": map[string]any{
"environment": map[string]any{
"description": "Target deployment environment",
"required": true,
"type": "choice",
"options": []any{"staging", "production"},
},
"github-token": "${{ secrets.CUSTOM_TOKEN }}",
"inputs": map[string]any{
"environment": map[string]any{
"description": "Target deployment environment",
"required": true,
"type": "choice",
"options": []any{"staging", "production"},
},
"force": map[string]any{
"description": "Force deployment even if tests fail",
"required": false,
"type": "boolean",
"default": "false",
},
"force": map[string]any{
"description": "Force deployment even if tests fail",
"required": false,
"type": "boolean",
"default": "false",
},
"steps": []any{
map[string]any{
"name": "Deploy application",
"run": "echo 'Deploying to ${{ inputs.environment }}'",
},
},
"steps": []any{
map[string]any{
"name": "Deploy application",
"run": "echo 'Deploying to ${{ inputs.environment }}'",
},
},
},
}

result := c.parseSafeJobsConfig(frontmatter)
result := c.parseSafeJobsConfig(jobsMap)

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

frontmatter := map[string]any{
"safe-jobs": map[string]any{
"test-job": map[string]any{
"runs-on": "ubuntu-latest",
"inputs": map[string]any{
"message": map[string]any{
"description": "String input",
"type": "string",
"default": "Hello World",
"required": true,
},
"debug": map[string]any{
"description": "Boolean input",
"type": "boolean",
"default": false,
"required": false,
},
"count": map[string]any{
"description": "Number input",
"type": "number",
"default": 100,
"required": true,
},
"environment": map[string]any{
"description": "Choice input",
"type": "choice",
"default": "staging",
"options": []any{"dev", "staging", "prod"},
},
"deploy_env": map[string]any{
"description": "Environment input",
"type": "environment",
"required": false,
},
jobsMap := map[string]any{
"test-job": map[string]any{
"runs-on": "ubuntu-latest",
"inputs": map[string]any{
"message": map[string]any{
"description": "String input",
"type": "string",
"default": "Hello World",
"required": true,
},
"steps": []any{
map[string]any{
"name": "Test step",
"run": "echo 'Testing inputs'",
},
"debug": map[string]any{
"description": "Boolean input",
"type": "boolean",
"default": false,
"required": false,
},
"count": map[string]any{
"description": "Number input",
"type": "number",
"default": 100,
"required": true,
},
"environment": map[string]any{
"description": "Choice input",
"type": "choice",
"default": "staging",
"options": []any{"dev", "staging", "prod"},
},
"deploy_env": map[string]any{
"description": "Environment input",
"type": "environment",
"required": false,
},
},
"steps": []any{
map[string]any{
"name": "Test step",
"run": "echo 'Testing inputs'",
},
},
},
}

result := c.parseSafeJobsConfig(frontmatter)
result := c.parseSafeJobsConfig(jobsMap)

if result == nil {
t.Fatal("Expected safe-jobs config to be parsed, got nil")
Expand Down
5 changes: 2 additions & 3 deletions pkg/workflow/safe_outputs_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,11 @@ func (c *Compiler) extractSafeOutputsConfig(frontmatter map[string]any) *SafeOut
config.Mentions = parseMentionsConfig(mentions)
}

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

Expand Down
Loading