diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 201301c39f..ed9bc9cbda 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -1130,7 +1130,7 @@ func (c *Compiler) ParseWorkflowFile(markdownPath string) (*WorkflowData, error) // Use the already extracted output configuration workflowData.SafeOutputs = safeOutputs - // Extract safe-jobs from the new location (safe-outputs.jobs) or old location (safe-jobs) for backwards compatibility + // Extract safe-jobs from safe-outputs.jobs location topSafeJobs := extractSafeJobsFromFrontmatter(result.Frontmatter) // Process @include directives to extract additional safe-outputs configurations diff --git a/pkg/workflow/safe_jobs.go b/pkg/workflow/safe_jobs.go index 7c256e7422..98649a63ae 100644 --- a/pkg/workflow/safe_jobs.go +++ b/pkg/workflow/safe_jobs.go @@ -39,7 +39,9 @@ func HasSafeJobsEnabled(safeJobs map[string]*SafeJobConfig) bool { return len(safeJobs) > 0 } -// parseSafeJobsConfig parses the safe-jobs configuration from top-level frontmatter +// 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 { @@ -328,8 +330,8 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool return nil } -// extractSafeJobsFromFrontmatter extracts safe-jobs section from frontmatter map -// Only checks the location under safe-outputs.jobs +// extractSafeJobsFromFrontmatter extracts safe-jobs configuration from frontmatter. +// Only checks the safe-outputs.jobs location. The old 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 { diff --git a/pkg/workflow/safe_jobs_syntax_test.go b/pkg/workflow/safe_jobs_syntax_test.go new file mode 100644 index 0000000000..2d10750c91 --- /dev/null +++ b/pkg/workflow/safe_jobs_syntax_test.go @@ -0,0 +1,100 @@ +package workflow + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestOldSafeJobsSyntaxRejected verifies that the old top-level safe-jobs syntax is rejected +func TestOldSafeJobsSyntaxRejected(t *testing.T) { + c := NewCompiler(false, "", "test") + + // Create a temporary workflow file with old safe-jobs syntax + tmpDir := t.TempDir() + workflowPath := filepath.Join(tmpDir, "test-old-syntax.md") + content := `--- +on: issues +permissions: + contents: read +safe-jobs: + deploy: + runs-on: ubuntu-latest + steps: + - name: Test + run: echo "test" +--- + +# Test workflow +Test old syntax +` + err := os.WriteFile(workflowPath, []byte(content), 0644) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + // Attempt to parse the workflow + _, err = c.ParseWorkflowFile(workflowPath) + + // Should fail with schema validation error + if err == nil { + t.Fatal("Expected error when using old safe-jobs syntax, but got nil") + } + + // Error message should mention safe-jobs + if err != nil && !strings.Contains(err.Error(), "safe-jobs") { + t.Errorf("Expected error to mention 'safe-jobs', got: %v", err) + } +} + +// TestNewSafeOutputsJobsSyntaxAccepted verifies that the new safe-outputs.jobs syntax works +func TestNewSafeOutputsJobsSyntaxAccepted(t *testing.T) { + c := NewCompiler(false, "", "test") + + // Create a temporary workflow file with new safe-outputs.jobs syntax + tmpDir := t.TempDir() + workflowPath := filepath.Join(tmpDir, "test-new-syntax.md") + content := `--- +on: issues +permissions: + contents: read + actions: read +safe-outputs: + jobs: + deploy: + runs-on: ubuntu-latest + steps: + - name: Test + run: echo "test" +--- + +# Test workflow +Test new syntax +` + err := os.WriteFile(workflowPath, []byte(content), 0644) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + // Attempt to parse the workflow + data, err := c.ParseWorkflowFile(workflowPath) + + // Should succeed + if err != nil { + t.Fatalf("Expected no error with new safe-outputs.jobs syntax, got: %v", err) + } + + // Verify safe-jobs were parsed + if data.SafeOutputs == nil { + t.Fatal("Expected SafeOutputs to be populated") + } + + if len(data.SafeOutputs.Jobs) != 1 { + t.Fatalf("Expected 1 safe-job, got %d", len(data.SafeOutputs.Jobs)) + } + + if _, exists := data.SafeOutputs.Jobs["deploy"]; !exists { + t.Error("Expected 'deploy' job to exist in SafeOutputs.Jobs") + } +} diff --git a/pkg/workflow/safe_jobs_test.go b/pkg/workflow/safe_jobs_test.go index 7841c79785..3e1b09884a 100644 --- a/pkg/workflow/safe_jobs_test.go +++ b/pkg/workflow/safe_jobs_test.go @@ -8,7 +8,9 @@ import ( func TestParseSafeJobsConfig(t *testing.T) { c := NewCompiler(false, "", "test") - // Test basic safe-jobs configuration + // Test parseSafeJobsConfig internal function which expects a "safe-jobs" key. + // 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{