From 5d22cf04f52f5297a00dd7561fdcf415127da96d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 9 Nov 2025 05:49:46 +0000 Subject: [PATCH 1/3] Initial plan From 865d6d30a69b5667eabb25e9abccd1a36e4f83d0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 9 Nov 2025 06:01:29 +0000 Subject: [PATCH 2/3] Remove old safe-jobs syntax support and update documentation - Update misleading comment in compiler.go that claimed old syntax was supported - Clarify that parseSafeJobsConfig() is an internal helper function - Update extractSafeJobsFromFrontmatter() comment to explicitly state old syntax is NOT supported - Add clarifying comments in test to explain it's testing internal parsing logic - Old syntax (top-level safe-jobs:) is already rejected by JSON schema validation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler.go | 2 +- pkg/workflow/safe_jobs.go | 8 +++++--- pkg/workflow/safe_jobs_test.go | 4 +++- 3 files changed, 9 insertions(+), 5 deletions(-) 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_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{ From 4d388b8b3e1c81fcf33ce71416a979e93d0c2f2d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 9 Nov 2025 06:04:14 +0000 Subject: [PATCH 3/3] Add tests for safe-jobs syntax validation - TestOldSafeJobsSyntaxRejected: Verifies old safe-jobs syntax is rejected by schema - TestNewSafeOutputsJobsSyntaxAccepted: Verifies new safe-outputs.jobs syntax works - Tests confirm JSON schema properly enforces the correct syntax Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/safe_jobs_syntax_test.go | 100 ++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 pkg/workflow/safe_jobs_syntax_test.go 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") + } +}