Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/workflow/frontmatter_parsing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions pkg/workflow/frontmatter_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 9 additions & 4 deletions pkg/workflow/pull_request_target_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -120,9 +122,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
}

Expand Down
19 changes: 10 additions & 9 deletions pkg/workflow/pull_request_target_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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",
Expand Down Expand Up @@ -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:
Expand All @@ -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",
Expand Down Expand Up @@ -340,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:
Expand All @@ -358,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",
Expand Down
2 changes: 2 additions & 0 deletions pkg/workflow/workflow_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,11 @@ func (c *Compiler) buildInitialWorkflowData(
if toolsResult.parsedFrontmatter != nil {
workflowData.CheckoutConfigs = toolsResult.parsedFrontmatter.CheckoutConfigs
workflowData.CheckoutDisabled = toolsResult.parsedFrontmatter.CheckoutDisabled
workflowData.CheckoutExplicitlyDisabled = toolsResult.parsedFrontmatter.CheckoutExplicitlyDisabled
} 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
}
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/workflow_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading