diff --git a/actions/setup/js/update_issue.cjs b/actions/setup/js/update_issue.cjs index 02bbfcd2ae..3c7af9d8fa 100644 --- a/actions/setup/js/update_issue.cjs +++ b/actions/setup/js/update_issue.cjs @@ -115,11 +115,16 @@ function buildIssueUpdateData(item, config) { // Sanitize title for Unicode security (no prefix handling needed for updates) updateData.title = sanitizeTitle(item.title); } - if (item.body !== undefined) { + // Check if body updates are allowed (defaults to true if not specified) + const canUpdateBody = config.allow_body !== false; + if (item.body !== undefined && canUpdateBody) { // Store operation information for consistent footer/append behavior. // Default to "append" so we preserve the original issue text. updateData._operation = item.operation || "append"; updateData._rawBody = item.body; + } else if (item.body !== undefined && !canUpdateBody) { + // Body update attempted but not allowed by configuration + core.warning("Body update not allowed by safe-outputs configuration"); } // The safe-outputs schema uses "status" (open/closed), while the GitHub API uses "state". // Accept both for compatibility. diff --git a/actions/setup/js/update_issue.test.cjs b/actions/setup/js/update_issue.test.cjs index 83bc8fd8a2..3eb1bbf892 100644 --- a/actions/setup/js/update_issue.test.cjs +++ b/actions/setup/js/update_issue.test.cjs @@ -486,3 +486,127 @@ describe("update_issue.cjs - footer support", () => { }); }); }); + +describe("update_issue.cjs - allow_body configuration", () => { + beforeEach(async () => { + // Reset all mocks before each test + vi.clearAllMocks(); + vi.resetModules(); + }); + + it("should allow body updates when allow_body is true", async () => { + const { buildIssueUpdateData } = await import("./update_issue.cjs"); + + const item = { + body: "New body content", + }; + + const config = { + allow_body: true, + }; + + const result = buildIssueUpdateData(item, config); + + expect(result.success).toBe(true); + expect(result.data._rawBody).toBe("New body content"); + expect(result.data._operation).toBe("append"); + }); + + it("should allow body updates when allow_body is not specified (defaults to true)", async () => { + const { buildIssueUpdateData } = await import("./update_issue.cjs"); + + const item = { + body: "New body content", + }; + + const config = {}; + + const result = buildIssueUpdateData(item, config); + + expect(result.success).toBe(true); + expect(result.data._rawBody).toBe("New body content"); + expect(result.data._operation).toBe("append"); + }); + + it("should block body updates when allow_body is false", async () => { + const { buildIssueUpdateData } = await import("./update_issue.cjs"); + + const item = { + body: "New body content", + }; + + const config = { + allow_body: false, + }; + + const result = buildIssueUpdateData(item, config); + + expect(result.success).toBe(true); + expect(result.data._rawBody).toBeUndefined(); + expect(result.data._operation).toBeUndefined(); + expect(mockCore.warning).toHaveBeenCalledWith("Body update not allowed by safe-outputs configuration"); + }); + + it("should still allow other fields when body is blocked", async () => { + const { buildIssueUpdateData } = await import("./update_issue.cjs"); + + const item = { + title: "New Title", + body: "New body content", + status: "closed", + labels: ["bug", "enhancement"], + }; + + const config = { + allow_body: false, + }; + + const result = buildIssueUpdateData(item, config); + + expect(result.success).toBe(true); + expect(result.data.title).toBe("New Title"); + expect(result.data.state).toBe("closed"); + expect(result.data.labels).toEqual(["bug", "enhancement"]); + expect(result.data._rawBody).toBeUndefined(); + expect(mockCore.warning).toHaveBeenCalledWith("Body update not allowed by safe-outputs configuration"); + }); + + it("should respect allow_body true with explicit operation", async () => { + const { buildIssueUpdateData } = await import("./update_issue.cjs"); + + const item = { + body: "New body content", + operation: "prepend", + }; + + const config = { + allow_body: true, + }; + + const result = buildIssueUpdateData(item, config); + + expect(result.success).toBe(true); + expect(result.data._rawBody).toBe("New body content"); + expect(result.data._operation).toBe("prepend"); + }); + + it("should block body update even with explicit operation when allow_body is false", async () => { + const { buildIssueUpdateData } = await import("./update_issue.cjs"); + + const item = { + body: "New body content", + operation: "replace", + }; + + const config = { + allow_body: false, + }; + + const result = buildIssueUpdateData(item, config); + + expect(result.success).toBe(true); + expect(result.data._rawBody).toBeUndefined(); + expect(result.data._operation).toBeUndefined(); + expect(mockCore.warning).toHaveBeenCalledWith("Body update not allowed by safe-outputs configuration"); + }); +}); diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index ea47b467eb..f310d9176e 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -5311,8 +5311,9 @@ "description": "Allow updating issue title - presence of key indicates field can be updated" }, "body": { - "type": "null", - "description": "Allow updating issue body - presence of key indicates field can be updated" + "type": ["boolean", "null"], + "description": "Allow updating issue body. Set to true to enable body updates, false to disable. For backward compatibility, null (body:) also enables body updates.", + "default": true }, "footer": { "type": "boolean", diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index 9d6b42a580..0786e33a39 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -234,9 +234,8 @@ var handlerRegistry = map[string]handlerBuilder{ if c.Title != nil { builder.AddDefault("allow_title", true) } - if c.Body != nil { - builder.AddDefault("allow_body", true) - } + // Body uses boolean value mode - add the actual boolean value + builder.AddBoolPtrOrDefault("allow_body", c.Body, true) return builder. AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). diff --git a/pkg/workflow/update_issue.go b/pkg/workflow/update_issue.go index 752a2f0938..72a0f50661 100644 --- a/pkg/workflow/update_issue.go +++ b/pkg/workflow/update_issue.go @@ -11,7 +11,7 @@ type UpdateIssuesConfig struct { UpdateEntityConfig `yaml:",inline"` Status *bool `yaml:"status,omitempty"` // Allow updating issue status (open/closed) - presence indicates field can be updated Title *bool `yaml:"title,omitempty"` // Allow updating issue title - presence indicates field can be updated - Body *bool `yaml:"body,omitempty"` // Allow updating issue body - presence indicates field can be updated + Body *bool `yaml:"body,omitempty"` // Allow updating issue body - boolean value controls permission (defaults to true) Footer *bool `yaml:"footer,omitempty"` // Controls whether AI-generated footer is added. When false, visible footer is omitted but XML markers are kept. } @@ -23,7 +23,7 @@ func (c *Compiler) parseUpdateIssuesConfig(outputMap map[string]any) *UpdateIssu return []UpdateEntityFieldSpec{ {Name: "status", Mode: FieldParsingKeyExistence, Dest: &cfg.Status}, {Name: "title", Mode: FieldParsingKeyExistence, Dest: &cfg.Title}, - {Name: "body", Mode: FieldParsingKeyExistence, Dest: &cfg.Body}, + {Name: "body", Mode: FieldParsingBoolValue, Dest: &cfg.Body}, {Name: "footer", Mode: FieldParsingBoolValue, Dest: &cfg.Footer}, } }, nil) diff --git a/pkg/workflow/update_issue_test.go b/pkg/workflow/update_issue_test.go index 3515b56fe1..ff2e9984f6 100644 --- a/pkg/workflow/update_issue_test.go +++ b/pkg/workflow/update_issue_test.go @@ -103,7 +103,7 @@ safe-outputs: target: "*" status: title: - body: + body: true --- # Test Update Issue Full Configuration @@ -153,6 +153,11 @@ This workflow tests the update-issue configuration with all options. if workflowData.SafeOutputs.UpdateIssues.Body == nil { t.Fatal("Expected body to be non-nil (updatable)") } + + // Verify body is set to true + if !*workflowData.SafeOutputs.UpdateIssues.Body { + t.Fatal("Expected body to be true") + } } func TestUpdateIssueConfigTargetParsing(t *testing.T) { @@ -213,3 +218,152 @@ This workflow tests the update-issue target configuration parsing. t.Fatal("Expected title to be non-nil (updatable)") } } + +func TestUpdateIssueBodyBooleanTrue(t *testing.T) { + // Test that body: true explicitly enables body updates + tmpDir := testutil.TempDir(t, "output-update-issue-body-true-test") + + testContent := `--- +on: + issues: + types: [opened] +permissions: + contents: read + issues: write + pull-requests: read +engine: claude +features: + dangerous-permissions-write: true +strict: false +safe-outputs: + update-issue: + body: true +--- + +# Test Update Issue Body True + +This workflow tests body: true configuration. +` + + testFile := filepath.Join(tmpDir, "test-update-issue-body-true.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + workflowData, err := compiler.ParseWorkflowFile(testFile) + if err != nil { + t.Fatalf("Unexpected error parsing workflow: %v", err) + } + + if workflowData.SafeOutputs.UpdateIssues == nil { + t.Fatal("Expected update-issue configuration to be parsed") + } + + if workflowData.SafeOutputs.UpdateIssues.Body == nil { + t.Fatal("Expected body to be non-nil") + } + + if !*workflowData.SafeOutputs.UpdateIssues.Body { + t.Fatal("Expected body to be true") + } +} + +func TestUpdateIssueBodyBooleanFalse(t *testing.T) { + // Test that body: false explicitly disables body updates + tmpDir := testutil.TempDir(t, "output-update-issue-body-false-test") + + testContent := `--- +on: + issues: + types: [opened] +permissions: + contents: read + issues: write + pull-requests: read +engine: claude +features: + dangerous-permissions-write: true +strict: false +safe-outputs: + update-issue: + body: false +--- + +# Test Update Issue Body False + +This workflow tests body: false configuration. +` + + testFile := filepath.Join(tmpDir, "test-update-issue-body-false.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + workflowData, err := compiler.ParseWorkflowFile(testFile) + if err != nil { + t.Fatalf("Unexpected error parsing workflow: %v", err) + } + + if workflowData.SafeOutputs.UpdateIssues == nil { + t.Fatal("Expected update-issue configuration to be parsed") + } + + if workflowData.SafeOutputs.UpdateIssues.Body == nil { + t.Fatal("Expected body to be non-nil") + } + + if *workflowData.SafeOutputs.UpdateIssues.Body { + t.Fatal("Expected body to be false") + } +} + +func TestUpdateIssueBodyNullBackwardCompatibility(t *testing.T) { + // Test that body: (null) maintains backward compatibility and defaults to true + tmpDir := testutil.TempDir(t, "output-update-issue-body-null-test") + + testContent := `--- +on: + issues: + types: [opened] +permissions: + contents: read + issues: write + pull-requests: read +engine: claude +features: + dangerous-permissions-write: true +strict: false +safe-outputs: + update-issue: + body: +--- + +# Test Update Issue Body Null + +This workflow tests body: (null) for backward compatibility. +` + + testFile := filepath.Join(tmpDir, "test-update-issue-body-null.md") + if err := os.WriteFile(testFile, []byte(testContent), 0644); err != nil { + t.Fatal(err) + } + + compiler := NewCompiler() + workflowData, err := compiler.ParseWorkflowFile(testFile) + if err != nil { + t.Fatalf("Unexpected error parsing workflow: %v", err) + } + + if workflowData.SafeOutputs.UpdateIssues == nil { + t.Fatal("Expected update-issue configuration to be parsed") + } + + // With FieldParsingBoolValue mode, null values result in nil pointer + // The handler will default to true via AddBoolPtrOrDefault + // This maintains backward compatibility where body: enables body updates + if workflowData.SafeOutputs.UpdateIssues.Body != nil { + t.Fatal("Expected body to be nil when set to null (will default to true in handler)") + } +}