Skip to content

Reduce EnforceSafeUpdate parameter count with options struct #52614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Copilot wants to merge 3 commits into
base: main
Choose a base branch
from
+38 −16
Open

Reduce EnforceSafeUpdate parameter count with options struct #52614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d93b24b
Initial plan
Copilot Aug 14, 2026
6527b49
Reduce EnforceSafeUpdate parameter count via options struct
Copilot Aug 14, 2026
61d76fd
Merge branch 'main' into copilot/lint-monster-parser-helper-function-…
github-actions[bot] Aug 15, 2026
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
8 changes: 7 additions & 1 deletion pkg/workflow/compiler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down
22 changes: 11 additions & 11 deletions pkg/workflow/compiler_threat_detection_formal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,67 +9,67 @@ 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")
}

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")
Expand Down
14 changes: 12 additions & 2 deletions pkg/workflow/safe_update_enforcement.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand All @@ -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.
Expand All @@ -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)",
Expand Down
10 changes: 8 additions & 2 deletions pkg/workflow/safe_update_enforcement_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)")
Expand Down
Loading