From d93b24b365238c37022fd905ec59e81c4dd195bb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 03:21:44 +0000 Subject: [PATCH 1/2] Initial plan From 6527b49594c59110abe4418aa94f45cf85d4ac3e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 03:29:25 +0000 Subject: [PATCH 2/2] Reduce EnforceSafeUpdate parameter count via options struct Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler.go | 8 ++++++- .../compiler_threat_detection_formal_test.go | 22 +++++++++---------- pkg/workflow/safe_update_enforcement.go | 14 ++++++++++-- pkg/workflow/safe_update_enforcement_test.go | 10 +++++++-- 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 05a8154adf5..c9edd4cab9d 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -560,7 +560,13 @@ func (c *Compiler) CompileWorkflowData(workflowData *WorkflowData, markdownPath // file is written and the agent receives the actionable guidance embedded in the warning. if safeUpdateEnabled { currentHasPR, currentHasPRTarget := extractPullRequestEventPresenceFromOnField(workflowData.RawFrontmatter["on"]) - if enforceErr := EnforceSafeUpdate(oldManifest, bodySecrets, bodyActions, workflowData.Redirect, oldHasPR, oldHasPRTarget, currentHasPR, currentHasPRTarget, collectMemoryValidationScripts(workflowData)); enforceErr != nil { + prTransition := PullRequestEventTransition{ + OldHasPullRequest: oldHasPR, + OldHasPullRequestTarget: oldHasPRTarget, + CurrentHasPullRequest: currentHasPR, + CurrentHasPullRequestTarget: currentHasPRTarget, + } + if enforceErr := EnforceSafeUpdate(oldManifest, bodySecrets, bodyActions, workflowData.Redirect, prTransition, collectMemoryValidationScripts(workflowData)); enforceErr != nil { warningMsg := buildSafeUpdateWarningPrompt(enforceErr.Error()) c.AddSafeUpdateWarning(warningMsg) fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", enforceErr.Error())) diff --git a/pkg/workflow/compiler_threat_detection_formal_test.go b/pkg/workflow/compiler_threat_detection_formal_test.go index 560991561a8..9215978bbff 100644 --- a/pkg/workflow/compiler_threat_detection_formal_test.go +++ b/pkg/workflow/compiler_threat_detection_formal_test.go @@ -9,47 +9,47 @@ import ( ) func TestFormal_CTR016_NilManifestSkipsEnforcement(t *testing.T) { - err := EnforceSafeUpdate(nil, []string{"MY_SECRET"}, []string{"evil-org/action@deadbeef # v1"}, "", false, false, false, false, nil) + err := EnforceSafeUpdate(nil, []string{"MY_SECRET"}, []string{"evil-org/action@deadbeef # v1"}, "", PullRequestEventTransition{}, nil) require.NoError(t, err) } func TestFormal_CTR016_EmptyManifestRejectsNewSecret(t *testing.T) { - err := EnforceSafeUpdate(&GHAWManifest{Version: currentGHAWManifestVersion}, []string{"MY_SECRET"}, nil, "", false, false, false, false, nil) + err := EnforceSafeUpdate(&GHAWManifest{Version: currentGHAWManifestVersion}, []string{"MY_SECRET"}, nil, "", PullRequestEventTransition{}, nil) require.Error(t, err) require.ErrorContains(t, err, "MY_SECRET") } func TestFormal_CTR016_GitHubTokenExempt_BareForm(t *testing.T) { - err := EnforceSafeUpdate(&GHAWManifest{Version: currentGHAWManifestVersion}, []string{"GITHUB_TOKEN"}, nil, "", false, false, false, false, nil) + err := EnforceSafeUpdate(&GHAWManifest{Version: currentGHAWManifestVersion}, []string{"GITHUB_TOKEN"}, nil, "", PullRequestEventTransition{}, nil) require.NoError(t, err) } func TestFormal_CTR016_GitHubTokenExempt_PrefixedForm(t *testing.T) { - err := EnforceSafeUpdate(&GHAWManifest{Version: currentGHAWManifestVersion}, []string{"secrets.GITHUB_TOKEN"}, nil, "", false, false, false, false, nil) + err := EnforceSafeUpdate(&GHAWManifest{Version: currentGHAWManifestVersion}, []string{"secrets.GITHUB_TOKEN"}, nil, "", PullRequestEventTransition{}, nil) require.NoError(t, err) } func TestFormal_CTR016_GhAwInternalSecretExempt(t *testing.T) { - err := EnforceSafeUpdate(&GHAWManifest{Version: currentGHAWManifestVersion}, []string{"GH_AW_GITHUB_TOKEN"}, nil, "", false, false, false, false, nil) + err := EnforceSafeUpdate(&GHAWManifest{Version: currentGHAWManifestVersion}, []string{"GH_AW_GITHUB_TOKEN"}, nil, "", PullRequestEventTransition{}, nil) require.NoError(t, err) } func TestFormal_CTR016_SecretPrefixNormalization(t *testing.T) { manifest := &GHAWManifest{Version: currentGHAWManifestVersion, Secrets: []string{"MY_SECRET"}} - err := EnforceSafeUpdate(manifest, []string{"secrets.MY_SECRET"}, nil, "", false, false, false, false, nil) + err := EnforceSafeUpdate(manifest, []string{"secrets.MY_SECRET"}, nil, "", PullRequestEventTransition{}, nil) require.NoError(t, err) } func TestFormal_CTR016_NewActionDriftRejected(t *testing.T) { manifest := &GHAWManifest{Version: currentGHAWManifestVersion, Actions: []GHAWManifestAction{{Repo: "actions/checkout", SHA: "abc1234", Version: "v4"}}} - err := EnforceSafeUpdate(manifest, nil, []string{"actions/checkout@abc1234 # v4", "evil-org/steal@deadbeef # v1"}, "", false, false, false, false, nil) + err := EnforceSafeUpdate(manifest, nil, []string{"actions/checkout@abc1234 # v4", "evil-org/steal@deadbeef # v1"}, "", PullRequestEventTransition{}, nil) require.Error(t, err) require.ErrorContains(t, err, "evil-org/steal") } func TestFormal_CTR016_RemovedActionDriftRejected(t *testing.T) { manifest := &GHAWManifest{Version: currentGHAWManifestVersion, Actions: []GHAWManifestAction{{Repo: "my-org/approved-action", SHA: "abc1234", Version: "v1"}}} - err := EnforceSafeUpdate(manifest, nil, []string{}, "", false, false, false, false, nil) + err := EnforceSafeUpdate(manifest, nil, []string{}, "", PullRequestEventTransition{}, nil) require.Error(t, err) require.ErrorContains(t, err, "Previously-approved action") require.ErrorContains(t, err, "my-org/approved-action") @@ -57,19 +57,19 @@ func TestFormal_CTR016_RemovedActionDriftRejected(t *testing.T) { func TestFormal_CTR016_KnownActionPinUpdateAllowed(t *testing.T) { manifest := &GHAWManifest{Version: currentGHAWManifestVersion, Actions: []GHAWManifestAction{{Repo: "my-org/action", SHA: "abc1234", Version: "v1"}}} - err := EnforceSafeUpdate(manifest, nil, []string{"my-org/action@def5678 # v2"}, "", false, false, false, false, nil) + err := EnforceSafeUpdate(manifest, nil, []string{"my-org/action@def5678 # v2"}, "", PullRequestEventTransition{}, nil) require.NoError(t, err) } func TestFormal_CTR016_RedirectWhitespaceNormalization(t *testing.T) { manifest := &GHAWManifest{Version: currentGHAWManifestVersion, Redirect: "owner/repo/workflows/new.md@main"} - err := EnforceSafeUpdate(manifest, nil, nil, " owner/repo/workflows/new.md@main ", false, false, false, false, nil) + err := EnforceSafeUpdate(manifest, nil, nil, " owner/repo/workflows/new.md@main ", PullRequestEventTransition{}, nil) require.NoError(t, err) } func TestFormal_CTR016_RedirectChangeRejected(t *testing.T) { manifest := &GHAWManifest{Version: currentGHAWManifestVersion, Redirect: "owner/repo/workflows/old.md@main"} - err := EnforceSafeUpdate(manifest, nil, nil, "owner/repo/workflows/new.md@main", false, false, false, false, nil) + err := EnforceSafeUpdate(manifest, nil, nil, "owner/repo/workflows/new.md@main", PullRequestEventTransition{}, nil) require.Error(t, err) require.ErrorContains(t, err, "New redirect configured") require.ErrorContains(t, err, "Previously-approved redirect removed") diff --git a/pkg/workflow/safe_update_enforcement.go b/pkg/workflow/safe_update_enforcement.go index 2bd70f032dd..234491fb219 100644 --- a/pkg/workflow/safe_update_enforcement.go +++ b/pkg/workflow/safe_update_enforcement.go @@ -29,6 +29,16 @@ var ghAwInternalSecrets = map[string]bool{ "COPILOT_GITHUB_TOKEN": true, } +// PullRequestEventTransition captures the pull_request / pull_request_target trigger +// presence before and after a workflow update, used to detect privilege escalation +// where a workflow is converted from pull_request to pull_request_target. +type PullRequestEventTransition struct { + OldHasPullRequest bool + OldHasPullRequestTarget bool + CurrentHasPullRequest bool + CurrentHasPullRequestTarget bool +} + // EnforceSafeUpdate validates that no new restricted secrets or unapproved action // changes have been introduced compared to those recorded in the existing manifest. // @@ -50,7 +60,7 @@ var ghAwInternalSecrets = map[string]bool{ // e.g. "actions/checkout@abc1234 # v4". // // Returns a structured, actionable error when violations are found. -func EnforceSafeUpdate(manifest *GHAWManifest, secretNames []string, actionRefs []string, currentRedirect string, oldHasPullRequest bool, oldHasPullRequestTarget bool, currentHasPullRequest bool, currentHasPullRequestTarget bool, currentMemoryValidationScripts []GHAWManifestMemoryValidationScript) error { +func EnforceSafeUpdate(manifest *GHAWManifest, secretNames []string, actionRefs []string, currentRedirect string, prTransition PullRequestEventTransition, currentMemoryValidationScripts []GHAWManifestMemoryValidationScript) error { if manifest == nil { // Lock file exists but predates the safe-updates feature (no gh-aw-manifest // section). Skip enforcement so legacy lock files are not flagged on upgrade. @@ -62,7 +72,7 @@ func EnforceSafeUpdate(manifest *GHAWManifest, secretNames []string, actionRefs addedActions, removedActions := collectActionViolations(manifest, actionRefs) addedRedirect, removedRedirect := collectRedirectViolations(manifest, currentRedirect) memoryValidationScriptChanges := collectMemoryValidationScriptChanges(manifest, currentMemoryValidationScripts) - pullRequestTargetEscalation := hasPullRequestTargetEscalation(oldHasPullRequest, oldHasPullRequestTarget, currentHasPullRequest, currentHasPullRequestTarget) + pullRequestTargetEscalation := hasPullRequestTargetEscalation(prTransition.OldHasPullRequest, prTransition.OldHasPullRequestTarget, prTransition.CurrentHasPullRequest, prTransition.CurrentHasPullRequestTarget) if len(secretViolations) == 0 && len(addedActions) == 0 && len(removedActions) == 0 && addedRedirect == "" && removedRedirect == "" && len(memoryValidationScriptChanges) == 0 && !pullRequestTargetEscalation { safeUpdateLog.Printf("Safe update check passed (%d secret(s), %d action(s) verified)", diff --git a/pkg/workflow/safe_update_enforcement_test.go b/pkg/workflow/safe_update_enforcement_test.go index 65394897c50..1710c4b050c 100644 --- a/pkg/workflow/safe_update_enforcement_test.go +++ b/pkg/workflow/safe_update_enforcement_test.go @@ -363,7 +363,13 @@ func TestEnforceSafeUpdate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := EnforceSafeUpdate(tt.manifest, tt.secretNames, tt.actionRefs, tt.redirect, tt.oldHasPR, tt.oldHasPRTarget, tt.currentHasPR, tt.currentHasPRTarget, nil) + prTransition := PullRequestEventTransition{ + OldHasPullRequest: tt.oldHasPR, + OldHasPullRequestTarget: tt.oldHasPRTarget, + CurrentHasPullRequest: tt.currentHasPR, + CurrentHasPullRequestTarget: tt.currentHasPRTarget, + } + err := EnforceSafeUpdate(tt.manifest, tt.secretNames, tt.actionRefs, tt.redirect, prTransition, nil) if tt.wantErr { require.Error(t, err, "expected safe update enforcement error") for _, msg := range tt.wantErrMsgs { @@ -470,7 +476,7 @@ func TestMemoryValidationScriptChangesRequireSafeUpdateReview(t *testing.T) { "repo-memory:removed (removed)", }, changes) - err := EnforceSafeUpdate(manifest, nil, nil, "", false, false, false, false, current) + err := EnforceSafeUpdate(manifest, nil, nil, "", PullRequestEventTransition{}, current) require.Error(t, err) require.ErrorContains(t, err, "Memory validation script changes") require.ErrorContains(t, err, "cache-memory:added (added)")