Add boolean value support for safe-outputs.body configuration#15097
Add boolean value support for safe-outputs.body configuration#15097
Conversation
🔍 PR Triage ResultsCategory: feature | Risk: medium | Priority: 32/100 Scores Breakdown
📋 Recommended Action: deferExplanation: This WIP PR needs completion before review. The feature adds body update support to safe-outputs which is valuable, but should be revisited when the [WIP] tag is removed and CI validation passes. Next Steps:
Triaged by PR Triage Agent on 2026-02-12
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds explicit boolean control for the safe-outputs.update-issue.body setting, allowing workflows to enable (true) or disable (false) issue body updates while keeping body: (null) backward-compatible.
Changes:
- Extend the workflow JSON schema and Go parser to accept
body: true|false|nullforsafe-outputs.update-issue. - Emit
allow_bodyinto the handler config using a default-true boolean pointer helper. - Update the JS
update_issuehandler to block body updates whenallow_bodyisfalse, and add JS/Go tests for the new behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/update_issue_test.go | Updates existing config test and adds new parsing tests for body: true, body: false, and body: (null). |
| pkg/workflow/update_issue.go | Switches body parsing from key-presence to boolean-value mode. |
| pkg/workflow/compiler_safe_outputs_config.go | Always emits allow_body using AddBoolPtrOrDefault(..., default=true). |
| pkg/parser/schemas/main_workflow_schema.json | Allows body to be `boolean |
| actions/setup/js/update_issue.cjs | Adds allow_body gating + warning when body updates are blocked. |
| actions/setup/js/update_issue.test.cjs | Adds tests validating allow_body behavior (true/default/false). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (item.title !== undefined) { | ||
| // 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) |
There was a problem hiding this comment.
buildIssueUpdateData now honors config.allow_body, but config.allow_title/config.allow_status (which the Go compiler can emit as allow_title/allow_status in handler config) are still not consulted. This makes safe-outputs field-permission settings for title/status ineffective and can lead to unexpected updates even when the workflow didn’t enable those fields; consider gating title/status updates similarly or removing those config keys from generation/schema if they’re not meant to be enforced here.
| 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. |
There was a problem hiding this comment.
This change makes allow_body always present (defaulting to true when c.Body is nil) and also enables body: false to flow through as allow_body: false. There isn’t currently a Go unit test asserting the emitted handler-config JSON contains allow_body: false when configured, and contains the expected default when body is omitted/null; adding an assertion in compiler_safe_outputs_config_test.go would help prevent regressions.
| {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}, |
There was a problem hiding this comment.
Switching body from FieldParsingKeyExistence to FieldParsingBoolValue changes the meaning of body:/"body": nil in frontmatter maps: it will now parse as a nil pointer instead of a non-nil *bool. There are existing tests/fixtures that set "body": nil and assert UpdateIssues.Body is non-nil (e.g., pkg/workflow/safe_outputs_max_test.go); those will fail unless updated to use body: true or to expect nil + defaulting behavior.
| {Name: "body", Mode: FieldParsingBoolValue, Dest: &cfg.Body}, | |
| {Name: "body", Mode: FieldParsingKeyExistence, Dest: &cfg.Body}, |
The
update-issuesafe-output only supported key-presence semantics (body:enables updates). This adds explicit boolean control:body: trueenables,body: falsedisables.Changes
Schema & Parser
type: ["boolean", "null"]for body fieldFieldParsingKeyExistencetoFieldParsingBoolValuemodeAddBoolPtrOrDefaultwithtruedefaultHandler Validation
config.allow_bodybefore processing body updatesBehavior
Backward Compatibility
Existing workflows unchanged:
body:(null) →allow_body: true(maintained key-existence semantics)allow_body: true(safe default)Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.