From eb545521161b5191eaf50515a0a9ed99e1f24a74 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Jul 2026 14:58:07 +0000 Subject: [PATCH 1/4] Initial plan From ed3d1160ba0ee7713dcc69b50574a4acebe5f752 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:05:43 +0000 Subject: [PATCH 2/4] Initial progress report - plan Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 1fe0f0b02c9..125be9efc41 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/securego/gosec/v2 v2.28.0 github.com/sourcegraph/conc v0.3.0 github.com/spf13/cobra v1.10.2 + github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 golang.org/x/crypto v0.54.0 golang.org/x/mod v0.38.0 @@ -89,7 +90,6 @@ require ( github.com/robfig/cron/v3 v3.0.1 // indirect github.com/segmentio/asm v1.1.3 // indirect github.com/segmentio/encoding v0.5.4 // indirect - github.com/spf13/pflag v1.0.10 // indirect github.com/standard-webhooks/standard-webhooks/libraries v0.0.1 // indirect github.com/thlib/go-timezone-local v0.0.7 // indirect github.com/tidwall/gjson v1.19.0 // indirect From 0242579343bb90a606ad3830aef9da257d466ceb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:23:07 +0000 Subject: [PATCH 3/4] Fix pull_request_target insecure-checkout warning silenced by auto-disable When a pull_request_target workflow has no explicit 'checkout:' key in its frontmatter, the compiler previously auto-disabled checkout (setting CheckoutDisabled=true for runtime reasons), which inadvertently caused the security validation to skip the 'extremely insecure' warning/error. The fix introduces a new CheckoutExplicitlyDisabled field that is only set when the user explicitly writes 'checkout: false' in their frontmatter. The security validation now checks this explicit flag instead of the general CheckoutDisabled flag, ensuring that auto-disabled workflows still receive the appropriate security diagnostic. Additionally, when 'strict: false' is present in frontmatter, ALL pull_request_target security warnings are now suppressed (including non-strict mode warnings), since the author has acknowledged the risks. Also updates the unit test for the 'no checkout key, strict mode' case to reflect the new correct behavior: a strict-mode compile of a workflow without explicit checkout:false now returns a hard error. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../pull_request_target_validation.go | 19 ++++++++++++++++--- .../pull_request_target_validation_test.go | 9 +++++---- pkg/workflow/workflow_builder.go | 4 ++++ pkg/workflow/workflow_data.go | 1 + 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/pkg/workflow/pull_request_target_validation.go b/pkg/workflow/pull_request_target_validation.go index b087c96ed96..6c72d3d6a3c 100644 --- a/pkg/workflow/pull_request_target_validation.go +++ b/pkg/workflow/pull_request_target_validation.go @@ -97,13 +97,23 @@ func (c *Compiler) validatePullRequestTargetTrigger(workflowData *WorkflowData, } effectiveStrictMode := c.strictMode + frontmatterOptOut := false if workflowData.RawFrontmatter != nil { if strictBool, ok := workflowData.RawFrontmatter["strict"].(bool); ok && !strictBool { pullRequestTargetLog.Print("Frontmatter strict: false detected, disabling strict mode error for pull_request_target validation") effectiveStrictMode = false + frontmatterOptOut = true } } + // If the workflow frontmatter explicitly opts out of strict mode (strict: false), skip all + // pull_request_target security warnings. The author has acknowledged the risks and chosen to + // suppress these diagnostics for their specific use case. + if frontmatterOptOut { + pullRequestTargetLog.Print("Frontmatter strict: false opt-out detected, skipping all pull_request_target security warnings") + return nil + } + // In strict mode, always emit a warning that pull_request_target is a very dangerous trigger, // regardless of whether checkout is disabled. The workflow still runs with full write // permissions and has access to all repository secrets. @@ -120,9 +130,12 @@ func (c *Compiler) validatePullRequestTargetTrigger(workflowData *WorkflowData, c.IncrementWarningCount() } - // If checkout is disabled, the workflow will not execute PR code — no further action needed. - if workflowData.CheckoutDisabled { - pullRequestTargetLog.Print("checkout: false is set, skipping insecure-checkout error") + // If checkout was explicitly disabled by the user (checkout: false in frontmatter), + // the workflow will not execute PR code — no further action needed. + // Auto-disabled checkout (when no checkout key is present) does not count as explicit + // acknowledgement of the security risk, so the warning/error is still emitted in that case. + if workflowData.CheckoutExplicitlyDisabled { + pullRequestTargetLog.Print("checkout: false is explicitly set by user, skipping insecure-checkout error") return nil } diff --git a/pkg/workflow/pull_request_target_validation_test.go b/pkg/workflow/pull_request_target_validation_test.go index ef25b0a85b1..779393d9dfe 100644 --- a/pkg/workflow/pull_request_target_validation_test.go +++ b/pkg/workflow/pull_request_target_validation_test.go @@ -170,7 +170,7 @@ Test workflow content.`, warningCount: 1, // dangerous-trigger warning }, { - name: "pull_request_target with no checkout key - strict - checkout auto-disabled, dangerous-trigger warning only", + name: "pull_request_target with no checkout key - strict - insecure checkout error", frontmatter: `--- on: pull_request_target: @@ -186,9 +186,10 @@ permissions: Test workflow content.`, filename: "prt-checkout-enabled-strict.md", strictMode: true, - expectError: false, - expectWarning: true, // dangerous-trigger warning only; checkout auto-disabled so no insecure-checkout error - warningCount: 1, // dangerous-trigger warning + expectError: true, + expectWarning: true, + errorContains: "pull_request_target trigger with checkout enabled is extremely insecure", + warningCount: 1, // dangerous-trigger warning; insecure-checkout is returned as an error }, { name: "pull_request_target with explicit checkout pinned to base sha - strict - warning only", diff --git a/pkg/workflow/workflow_builder.go b/pkg/workflow/workflow_builder.go index 53615ad5dc3..9135630d41c 100644 --- a/pkg/workflow/workflow_builder.go +++ b/pkg/workflow/workflow_builder.go @@ -97,9 +97,13 @@ func (c *Compiler) buildInitialWorkflowData( if toolsResult.parsedFrontmatter != nil { workflowData.CheckoutConfigs = toolsResult.parsedFrontmatter.CheckoutConfigs workflowData.CheckoutDisabled = toolsResult.parsedFrontmatter.CheckoutDisabled + if toolsResult.parsedFrontmatter.CheckoutDisabled { + workflowData.CheckoutExplicitlyDisabled = true + } } else if rawCheckout, ok := result.Frontmatter["checkout"]; ok { if checkoutValue, ok := rawCheckout.(bool); ok && !checkoutValue { workflowData.CheckoutDisabled = true + workflowData.CheckoutExplicitlyDisabled = true } else if configs, err := ParseCheckoutConfigs(rawCheckout); err == nil { workflowData.CheckoutConfigs = configs } diff --git a/pkg/workflow/workflow_data.go b/pkg/workflow/workflow_data.go index 939b59e7ab1..2198c926e76 100644 --- a/pkg/workflow/workflow_data.go +++ b/pkg/workflow/workflow_data.go @@ -163,6 +163,7 @@ type WorkflowData struct { InlinedImports bool // if true, inline all imports at compile time (from inlined-imports frontmatter field) CheckoutConfigs []*CheckoutConfig // user-configured checkout settings from frontmatter CheckoutDisabled bool // true when checkout: false is set in frontmatter, or auto-disabled for pull_request_target + CheckoutExplicitlyDisabled bool // true only when checkout: false is explicitly set in frontmatter (not auto-disabled) IsPullRequestTarget bool // true when the workflow's on: triggers contain pull_request_target (but NOT pull_request) HasDispatchItemNumber bool // true when workflow_dispatch has item_number input (generated by label trigger shorthand) ConcurrencyJobDiscriminator string // optional discriminator expression appended to job-level concurrency groups (from concurrency.job-discriminator) From 8f1bcc5dcfda2cdaba39537b5a411b6a452011e8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Jul 2026 16:32:45 +0000 Subject: [PATCH 4/4] Fix: remove frontmatterOptOut early return; add CheckoutExplicitlyDisabled to FrontmatterConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove the `frontmatterOptOut` early return block from validatePullRequestTargetTrigger. strict: false now lowers to non-strict mode (effectiveStrictMode=false) without suppressing the insecure-checkout warning. A pull_request_target workflow with strict: false and no explicit checkout: false still emits the non-strict insecure-checkout warning. - Add CheckoutExplicitlyDisabled field to FrontmatterConfig (frontmatter_types.go) and set it at parse time in frontmatter_parsing.go, removing the fragile inference from parsedFrontmatter.CheckoutDisabled in workflow_builder.go. - Update two tests to match the new behavior: - 'no checkout key - non-strict': warningCount 1→2 (sandbox + insecure-checkout warning) - 'strict CLI + frontmatter strict false': warningCount 0→1, expectWarning false→true Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/frontmatter_parsing.go | 1 + pkg/workflow/frontmatter_types.go | 7 ++++--- pkg/workflow/pull_request_target_validation.go | 14 +++----------- .../pull_request_target_validation_test.go | 10 +++++----- pkg/workflow/workflow_builder.go | 4 +--- 5 files changed, 14 insertions(+), 22 deletions(-) diff --git a/pkg/workflow/frontmatter_parsing.go b/pkg/workflow/frontmatter_parsing.go index 253d32e8d91..7ca1fbc8d98 100644 --- a/pkg/workflow/frontmatter_parsing.go +++ b/pkg/workflow/frontmatter_parsing.go @@ -67,6 +67,7 @@ func ParseFrontmatterConfig(frontmatter map[string]any) (*FrontmatterConfig, err if config.Checkout != nil { if checkoutValue, ok := config.Checkout.(bool); ok && !checkoutValue { config.CheckoutDisabled = true + config.CheckoutExplicitlyDisabled = true frontmatterTypesLog.Print("Checkout disabled via checkout: false") } else { checkoutConfigs, err := ParseCheckoutConfigs(config.Checkout) diff --git a/pkg/workflow/frontmatter_types.go b/pkg/workflow/frontmatter_types.go index f4e09bb204d..04e7f13340a 100644 --- a/pkg/workflow/frontmatter_types.go +++ b/pkg/workflow/frontmatter_types.go @@ -410,9 +410,10 @@ type FrontmatterConfig struct { // Controls how actions/checkout is invoked. // Can be a single CheckoutConfig object or an array of CheckoutConfig objects. // Set to false to disable the default checkout step entirely. - Checkout any `json:"checkout,omitempty"` // Raw value (object, array, or false) - CheckoutConfigs []*CheckoutConfig `json:"-"` // Parsed checkout configs (not in JSON) - CheckoutDisabled bool `json:"-"` // true when checkout: false is set in frontmatter + Checkout any `json:"checkout,omitempty"` // Raw value (object, array, or false) + CheckoutConfigs []*CheckoutConfig `json:"-"` // Parsed checkout configs (not in JSON) + CheckoutDisabled bool `json:"-"` // true when checkout: false is set in frontmatter + CheckoutExplicitlyDisabled bool `json:"-"` // true only when checkout: false is explicitly written by the user in frontmatter // Model is the top-level LLM model override. When set, it takes precedence over // engine.model. Use this field instead of engine.model. diff --git a/pkg/workflow/pull_request_target_validation.go b/pkg/workflow/pull_request_target_validation.go index 6c72d3d6a3c..d31a4209e37 100644 --- a/pkg/workflow/pull_request_target_validation.go +++ b/pkg/workflow/pull_request_target_validation.go @@ -59,7 +59,9 @@ var pullRequestTargetGitHubExpressionPattern = regexp.MustCompile(`^\$\{\{\s*([^ // // In strict mode, a warning is always emitted that pull_request_target is inherently dangerous // even with checkout disabled, since the workflow still runs with elevated permissions. -// This strict behavior is disabled when the workflow frontmatter explicitly sets strict: false. +// When the workflow frontmatter sets strict: false, effectiveStrictMode is lowered so the +// dangerous-trigger strict-only warning is skipped; the insecure-checkout check still runs +// and emits a non-strict warning when checkout is not explicitly disabled. func (c *Compiler) validatePullRequestTargetTrigger(workflowData *WorkflowData, markdownPath string) error { // Fast path: skip expensive YAML parsing when the On field cannot possibly contain // a pull_request_target trigger. This avoids yaml.Unmarshal on every @@ -97,23 +99,13 @@ func (c *Compiler) validatePullRequestTargetTrigger(workflowData *WorkflowData, } effectiveStrictMode := c.strictMode - frontmatterOptOut := false if workflowData.RawFrontmatter != nil { if strictBool, ok := workflowData.RawFrontmatter["strict"].(bool); ok && !strictBool { pullRequestTargetLog.Print("Frontmatter strict: false detected, disabling strict mode error for pull_request_target validation") effectiveStrictMode = false - frontmatterOptOut = true } } - // If the workflow frontmatter explicitly opts out of strict mode (strict: false), skip all - // pull_request_target security warnings. The author has acknowledged the risks and chosen to - // suppress these diagnostics for their specific use case. - if frontmatterOptOut { - pullRequestTargetLog.Print("Frontmatter strict: false opt-out detected, skipping all pull_request_target security warnings") - return nil - } - // In strict mode, always emit a warning that pull_request_target is a very dangerous trigger, // regardless of whether checkout is disabled. The workflow still runs with full write // permissions and has access to all repository secrets. diff --git a/pkg/workflow/pull_request_target_validation_test.go b/pkg/workflow/pull_request_target_validation_test.go index 779393d9dfe..cf9260f314b 100644 --- a/pkg/workflow/pull_request_target_validation_test.go +++ b/pkg/workflow/pull_request_target_validation_test.go @@ -52,7 +52,7 @@ Test workflow content.`, warningCount: 1, // sandbox.agent: false }, { - name: "pull_request_target with no checkout key - non-strict - checkout auto-disabled", + name: "pull_request_target with no checkout key - non-strict - insecure checkout warning", frontmatter: `--- strict: false on: @@ -72,7 +72,7 @@ Test workflow content.`, strictMode: false, expectError: false, expectWarning: true, - warningCount: 1, // checkout is auto-disabled for pull_request_target; only sandbox.agent: false warning + warningCount: 2, // sandbox.agent: false warning + insecure-checkout warning (non-strict mode) }, { name: "pull_request_target with trusted checkout - non-strict - no warnings no error", @@ -341,7 +341,7 @@ Test workflow content.`, warningCount: 1, // dangerous-trigger warning }, { - name: "pull_request_target with no checkout key - strict CLI + frontmatter strict false - no warnings", + name: "pull_request_target with no checkout key - strict CLI + frontmatter strict false - insecure checkout warning", frontmatter: `--- strict: false on: @@ -359,8 +359,8 @@ Test workflow content.`, filename: "prt-checkout-enabled-strict-frontmatter-opt-out.md", strictMode: true, expectError: false, - expectWarning: false, - warningCount: 0, // checkout auto-disabled; strict:false suppresses dangerous-trigger warning; no insecure-checkout warning + expectWarning: true, + warningCount: 1, // strict: false lowers to non-strict mode (skips dangerous-trigger warning), but insecure-checkout warning still emits }, { name: "pull_request trigger (not target) - strict - no diagnostic", diff --git a/pkg/workflow/workflow_builder.go b/pkg/workflow/workflow_builder.go index 9135630d41c..280efeb03f4 100644 --- a/pkg/workflow/workflow_builder.go +++ b/pkg/workflow/workflow_builder.go @@ -97,9 +97,7 @@ func (c *Compiler) buildInitialWorkflowData( if toolsResult.parsedFrontmatter != nil { workflowData.CheckoutConfigs = toolsResult.parsedFrontmatter.CheckoutConfigs workflowData.CheckoutDisabled = toolsResult.parsedFrontmatter.CheckoutDisabled - if toolsResult.parsedFrontmatter.CheckoutDisabled { - workflowData.CheckoutExplicitlyDisabled = true - } + workflowData.CheckoutExplicitlyDisabled = toolsResult.parsedFrontmatter.CheckoutExplicitlyDisabled } else if rawCheckout, ok := result.Frontmatter["checkout"]; ok { if checkoutValue, ok := rawCheckout.(bool); ok && !checkoutValue { workflowData.CheckoutDisabled = true