From ea66de18f2c688114c51762ef66a5e29b1b4ace0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 10:22:19 +0000 Subject: [PATCH 1/3] Initial plan From f4cf597c4c40975d1633ca19a7fbc2e33f6b0429 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 10:33:57 +0000 Subject: [PATCH 2/3] Honor workflow strict opt-out in pull_request_target validation and warn for playwright MCP Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/playwright_validation.go | 10 +++------ pkg/workflow/playwright_validation_test.go | 18 +++++++-------- .../pull_request_target_validation.go | 14 ++++++++++-- .../pull_request_target_validation_test.go | 22 +++++++++++++++++++ 4 files changed, 45 insertions(+), 19 deletions(-) diff --git a/pkg/workflow/playwright_validation.go b/pkg/workflow/playwright_validation.go index 3df37684d9f..0989c09e4e2 100644 --- a/pkg/workflow/playwright_validation.go +++ b/pkg/workflow/playwright_validation.go @@ -37,9 +37,9 @@ import ( var playwrightValidationLog = logger.New("workflow:playwright_validation") -// validatePlaywrightMode warns (non-strict) or errors (strict) when the -// playwright tool is configured in MCP mode. MCP mode is deprecated; use -// mode: cli instead for token-efficient, container-free browser automation. +// validatePlaywrightMode warns when the playwright tool is configured in MCP +// mode. MCP mode is deprecated; use mode: cli instead for token-efficient, +// container-free browser automation. func (c *Compiler) validatePlaywrightMode(workflowData *WorkflowData) error { if workflowData == nil || workflowData.Tools == nil { return nil @@ -64,10 +64,6 @@ func (c *Compiler) validatePlaywrightMode(workflowData *WorkflowData) error { "Update your prompts to run `playwright-cli ` in bash instead of using MCP browser tools. " + "See: https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/playwright.md" - if c.strictMode { - return fmt.Errorf("strict mode: %s", warningMsg) - } - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warningMsg)) c.IncrementWarningCount() return nil diff --git a/pkg/workflow/playwright_validation_test.go b/pkg/workflow/playwright_validation_test.go index 7bd029c6b26..0a41219c1d8 100644 --- a/pkg/workflow/playwright_validation_test.go +++ b/pkg/workflow/playwright_validation_test.go @@ -55,18 +55,16 @@ func TestValidatePlaywrightMode(t *testing.T) { expectWarn: false, }, { - name: "playwright mcp mode in strict mode returns error", - tools: map[string]any{"playwright": map[string]any{"mode": "mcp"}}, - strictMode: true, - expectError: true, - errorSubstr: "strict mode", + name: "playwright mcp mode in strict mode warns only", + tools: map[string]any{"playwright": map[string]any{"mode": "mcp"}}, + strictMode: true, + expectWarn: true, }, { - name: "playwright nil (default MCP) in strict mode returns error", - tools: map[string]any{"playwright": nil}, - strictMode: true, - expectError: true, - errorSubstr: "strict mode", + name: "playwright nil (default MCP) in strict mode warns only", + tools: map[string]any{"playwright": nil}, + strictMode: true, + expectWarn: true, }, } diff --git a/pkg/workflow/pull_request_target_validation.go b/pkg/workflow/pull_request_target_validation.go index d152e51cacc..c2e9f74582d 100644 --- a/pkg/workflow/pull_request_target_validation.go +++ b/pkg/workflow/pull_request_target_validation.go @@ -12,6 +12,7 @@ // 1. In strict mode: always emit a warning that pull_request_target is a very // dangerous trigger, even when checkout: false is set, because the workflow // still runs with full write permissions and secret access. +// Workflows can opt out by setting strict: false in frontmatter. // // 2. When checkout is NOT explicitly disabled (checkout: false not set): // - In strict mode: return a hard error (extremely insecure). @@ -51,6 +52,7 @@ var pullRequestTargetLog = newValidationLogger("pull_request_target") // // 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. 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 @@ -87,10 +89,18 @@ func (c *Compiler) validatePullRequestTargetTrigger(workflowData *WorkflowData, return nil } + strictMode := c.strictMode + if strictVal, exists := workflowData.RawFrontmatter["strict"]; exists { + if strictBool, ok := strictVal.(bool); ok && !strictBool { + pullRequestTargetLog.Print("Frontmatter strict: false detected, disabling strict-mode error for pull_request_target validation") + strictMode = false + } + } + // 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. - if c.strictMode { + if strictMode { pullRequestTargetLog.Print("Emitting strict mode warning: pull_request_target is a very dangerous trigger") warningMsg := "pull_request_target is a very dangerous trigger.\n" + "This event runs with full write permissions and access to all repository secrets.\n" + @@ -122,7 +132,7 @@ func (c *Compiler) validatePullRequestTargetTrigger(workflowData *WorkflowData, "checkout: false\n\n" + "See: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/" - if c.strictMode { + if strictMode { return formatCompilerError(markdownPath, "error", message, nil) } diff --git a/pkg/workflow/pull_request_target_validation_test.go b/pkg/workflow/pull_request_target_validation_test.go index 0ffdb3b536d..a3c37d90315 100644 --- a/pkg/workflow/pull_request_target_validation_test.go +++ b/pkg/workflow/pull_request_target_validation_test.go @@ -159,6 +159,28 @@ Test workflow content.`, errorContains: "pull_request_target trigger with checkout enabled is extremely insecure", warningCount: 1, // dangerous-trigger warning }, + { + name: "pull_request_target with checkout enabled - strict CLI + frontmatter strict false - warning only", + frontmatter: `--- +strict: false +on: + pull_request_target: + types: [opened] +tools: + github: + toolsets: [pull_requests] +permissions: + pull-requests: read +--- + +# PR Target Strict Opt-Out +Test workflow content.`, + filename: "prt-checkout-enabled-strict-frontmatter-opt-out.md", + strictMode: true, + expectError: false, + expectWarning: true, + warningCount: 1, // insecure-checkout warning only + }, { name: "pull_request trigger (not target) - strict - no diagnostic", frontmatter: `--- From 6b4bac0e17e21f52dc290526d696c69fa399e690 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 10:36:18 +0000 Subject: [PATCH 3/3] Allow playwright MCP in strict mode and honor workflow strict opt-out for pull_request_target Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/pull_request_target_validation.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/workflow/pull_request_target_validation.go b/pkg/workflow/pull_request_target_validation.go index c2e9f74582d..6d8436f86aa 100644 --- a/pkg/workflow/pull_request_target_validation.go +++ b/pkg/workflow/pull_request_target_validation.go @@ -89,18 +89,18 @@ func (c *Compiler) validatePullRequestTargetTrigger(workflowData *WorkflowData, return nil } - strictMode := c.strictMode - if strictVal, exists := workflowData.RawFrontmatter["strict"]; exists { - if strictBool, ok := strictVal.(bool); ok && !strictBool { - pullRequestTargetLog.Print("Frontmatter strict: false detected, disabling strict-mode error for pull_request_target validation") - strictMode = false + effectiveStrictMode := c.strictMode + 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 } } // 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. - if strictMode { + if effectiveStrictMode { pullRequestTargetLog.Print("Emitting strict mode warning: pull_request_target is a very dangerous trigger") warningMsg := "pull_request_target is a very dangerous trigger.\n" + "This event runs with full write permissions and access to all repository secrets.\n" + @@ -132,7 +132,7 @@ func (c *Compiler) validatePullRequestTargetTrigger(workflowData *WorkflowData, "checkout: false\n\n" + "See: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/" - if strictMode { + if effectiveStrictMode { return formatCompilerError(markdownPath, "error", message, nil) }