From d52bf7f45872853a86ecf0213bbd1064af06fb8b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Aug 2026 02:50:30 +0000 Subject: [PATCH 1/3] Initial plan From 58294fc630d388d8271ab38f851fa035e5d83102 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Aug 2026 02:59:58 +0000 Subject: [PATCH 2/3] Refactor parameter-heavy signatures for lint-monster findings Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/add_package_manifest.go | 22 +++++++++++++++---- pkg/workflow/compiler.go | 9 +++++++- .../compiler_threat_detection_formal_test.go | 22 +++++++++---------- pkg/workflow/safe_update_enforcement.go | 17 ++++++++++---- pkg/workflow/safe_update_enforcement_test.go | 12 ++++++++-- 5 files changed, 60 insertions(+), 22 deletions(-) diff --git a/pkg/cli/add_package_manifest.go b/pkg/cli/add_package_manifest.go index 719a136b669..7951a50fa47 100644 --- a/pkg/cli/add_package_manifest.go +++ b/pkg/cli/add_package_manifest.go @@ -104,7 +104,13 @@ func resolveRepositoryPackage(ctx context.Context, repoSpec *RepoSpec, host stri return nil, err } - extensionFiles, err := resolveRepositoryPackageExtensionFiles(ctx, owner, repo, packagePath, ref, host, manifest, includeSkillDirs, includeAgentFiles) + extensionFiles, err := resolveRepositoryPackageExtensionFiles(ctx, repositoryPackageLocation{ + owner: owner, + repo: repo, + packagePath: packagePath, + ref: ref, + host: host, + }, manifest, includeSkillDirs, includeAgentFiles) if err != nil { return nil, err } @@ -171,11 +177,19 @@ type repositoryPackageExtensionFiles struct { warnings []string } -func resolveRepositoryPackageExtensionFiles(ctx context.Context, owner, repo, packagePath, ref, host string, manifest *repositoryPackageManifest, includeSkillDirs, includeAgentFiles []string) (*repositoryPackageExtensionFiles, error) { +type repositoryPackageLocation struct { + owner string + repo string + packagePath string + ref string + host string +} + +func resolveRepositoryPackageExtensionFiles(ctx context.Context, packageLocation repositoryPackageLocation, manifest *repositoryPackageManifest, includeSkillDirs, includeAgentFiles []string) (*repositoryPackageExtensionFiles, error) { // Resolve skill files: explicit from manifest or auto-scanned. explicitSkillDirs := append([]string{}, manifest.Skills...) explicitSkillDirs = append(explicitSkillDirs, includeSkillDirs...) - skillFiles, skillWarnings, err := resolvePackageSkillFiles(ctx, owner, repo, packagePath, ref, host, explicitSkillDirs) + skillFiles, skillWarnings, err := resolvePackageSkillFiles(ctx, packageLocation.owner, packageLocation.repo, packageLocation.packagePath, packageLocation.ref, packageLocation.host, explicitSkillDirs) if err != nil { return nil, err } @@ -183,7 +197,7 @@ func resolveRepositoryPackageExtensionFiles(ctx context.Context, owner, repo, pa // Resolve agent files: explicit from manifest or auto-scanned. explicitAgentFiles := append([]string{}, manifest.Agents...) explicitAgentFiles = append(explicitAgentFiles, includeAgentFiles...) - agentFiles, agentWarnings, err := resolvePackageAgentFiles(ctx, owner, repo, packagePath, ref, host, explicitAgentFiles) + agentFiles, agentWarnings, err := resolvePackageAgentFiles(ctx, packageLocation.owner, packageLocation.repo, packageLocation.packagePath, packageLocation.ref, packageLocation.host, explicitAgentFiles) if err != nil { return nil, err } diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 05a8154adf5..18aa0541923 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -560,7 +560,14 @@ 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 { + if enforceErr := EnforceSafeUpdate(oldManifest, bodySecrets, bodyActions, SafeUpdateEnforcementOptions{ + CurrentRedirect: workflowData.Redirect, + OldHasPullRequest: oldHasPR, + OldHasPullRequestTarget: oldHasPRTarget, + CurrentHasPullRequest: currentHasPR, + CurrentHasPullRequestTarget: currentHasPRTarget, + CurrentMemoryValidationScripts: 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..255188896e0 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"}, SafeUpdateEnforcementOptions{}) 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, SafeUpdateEnforcementOptions{}) 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, SafeUpdateEnforcementOptions{}) 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, SafeUpdateEnforcementOptions{}) 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, SafeUpdateEnforcementOptions{}) 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, SafeUpdateEnforcementOptions{}) 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"}, SafeUpdateEnforcementOptions{}) 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{}, SafeUpdateEnforcementOptions{}) 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"}, SafeUpdateEnforcementOptions{}) 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, SafeUpdateEnforcementOptions{CurrentRedirect: " owner/repo/workflows/new.md@main "}) 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, SafeUpdateEnforcementOptions{CurrentRedirect: "owner/repo/workflows/new.md@main"}) 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..3c59a0c57c9 100644 --- a/pkg/workflow/safe_update_enforcement.go +++ b/pkg/workflow/safe_update_enforcement.go @@ -50,7 +50,16 @@ 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 { +type SafeUpdateEnforcementOptions struct { + CurrentRedirect string + OldHasPullRequest bool + OldHasPullRequestTarget bool + CurrentHasPullRequest bool + CurrentHasPullRequestTarget bool + CurrentMemoryValidationScripts []GHAWManifestMemoryValidationScript +} + +func EnforceSafeUpdate(manifest *GHAWManifest, secretNames []string, actionRefs []string, opts SafeUpdateEnforcementOptions) 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. @@ -60,9 +69,9 @@ func EnforceSafeUpdate(manifest *GHAWManifest, secretNames []string, actionRefs secretViolations := collectSecretViolations(manifest, secretNames) addedActions, removedActions := collectActionViolations(manifest, actionRefs) - addedRedirect, removedRedirect := collectRedirectViolations(manifest, currentRedirect) - memoryValidationScriptChanges := collectMemoryValidationScriptChanges(manifest, currentMemoryValidationScripts) - pullRequestTargetEscalation := hasPullRequestTargetEscalation(oldHasPullRequest, oldHasPullRequestTarget, currentHasPullRequest, currentHasPullRequestTarget) + addedRedirect, removedRedirect := collectRedirectViolations(manifest, opts.CurrentRedirect) + memoryValidationScriptChanges := collectMemoryValidationScriptChanges(manifest, opts.CurrentMemoryValidationScripts) + pullRequestTargetEscalation := hasPullRequestTargetEscalation(opts.OldHasPullRequest, opts.OldHasPullRequestTarget, opts.CurrentHasPullRequest, opts.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..945b4a52c56 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) + err := EnforceSafeUpdate(tt.manifest, tt.secretNames, tt.actionRefs, SafeUpdateEnforcementOptions{ + CurrentRedirect: tt.redirect, + OldHasPullRequest: tt.oldHasPR, + OldHasPullRequestTarget: tt.oldHasPRTarget, + CurrentHasPullRequest: tt.currentHasPR, + CurrentHasPullRequestTarget: tt.currentHasPRTarget, + }) if tt.wantErr { require.Error(t, err, "expected safe update enforcement error") for _, msg := range tt.wantErrMsgs { @@ -470,7 +476,9 @@ 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, SafeUpdateEnforcementOptions{ + CurrentMemoryValidationScripts: current, + }) require.Error(t, err) require.ErrorContains(t, err, "Memory validation script changes") require.ErrorContains(t, err, "cache-memory:added (added)") From 027d64d3ba70ccd0d1bfcbd76f2caacc68020da6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 15 Aug 2026 06:06:37 +0000 Subject: [PATCH 3/3] docs: restore EnforceSafeUpdate doc attachment and options godoc Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/safe_update_enforcement.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/pkg/workflow/safe_update_enforcement.go b/pkg/workflow/safe_update_enforcement.go index 3c59a0c57c9..1eb6b5bf9cb 100644 --- a/pkg/workflow/safe_update_enforcement.go +++ b/pkg/workflow/safe_update_enforcement.go @@ -29,6 +29,17 @@ var ghAwInternalSecrets = map[string]bool{ "COPILOT_GITHUB_TOKEN": true, } +// SafeUpdateEnforcementOptions carries current-compilation context used by +// EnforceSafeUpdate to detect unsafe changes against the existing manifest. +type SafeUpdateEnforcementOptions struct { + CurrentRedirect string + OldHasPullRequest bool + OldHasPullRequestTarget bool + CurrentHasPullRequest bool + CurrentHasPullRequestTarget bool + CurrentMemoryValidationScripts []GHAWManifestMemoryValidationScript +} + // EnforceSafeUpdate validates that no new restricted secrets or unapproved action // changes have been introduced compared to those recorded in the existing manifest. // @@ -50,15 +61,6 @@ var ghAwInternalSecrets = map[string]bool{ // e.g. "actions/checkout@abc1234 # v4". // // Returns a structured, actionable error when violations are found. -type SafeUpdateEnforcementOptions struct { - CurrentRedirect string - OldHasPullRequest bool - OldHasPullRequestTarget bool - CurrentHasPullRequest bool - CurrentHasPullRequestTarget bool - CurrentMemoryValidationScripts []GHAWManifestMemoryValidationScript -} - func EnforceSafeUpdate(manifest *GHAWManifest, secretNames []string, actionRefs []string, opts SafeUpdateEnforcementOptions) error { if manifest == nil { // Lock file exists but predates the safe-updates feature (no gh-aw-manifest