Add runtime barrier to prevent secret modifications during upgrade

.github/dependabot.yml

@@ -20,3 +20,7 @@ updates:
   directory: /.github/workflows
   schedule:
     interval: weekly
+- package-ecosystem: gomod
+  directory: /.github/workflows
+  schedule:
+    interval: weekly

.github/workflows/go.mod

@@ -1,3 +1,5 @@
 module github.com/github/gh-aw-workflows-deps
-
 go 1.21
+
+require (
+)

docs/src/content/docs/reference/frontmatter-full.md

@@ -1975,12 +1975,11 @@ safe-outputs:
         # (optional)
         description: "Description of the workflow"
 
-    # Optional array of project custom fields to create up-front. Useful for project
-    # setups that require a fixed set of fields.
+    # Optional array of project custom fields to create up-front.
     # (optional)
     field-definitions: []
       # Array items:
-        # The field name to create (e.g., 'status', 'tracker_id')
+        # The field name to create (e.g., 'status', 'priority')
         name: "My Workflow"
 
         # The GitHub Projects v2 custom field type
@@ -2024,8 +2023,8 @@ safe-outputs:
     # (optional)
     target-owner: "example-value"
 
-    # Optional prefix for auto-generated project titles (default: 'Project'). When
-    # the agent doesn't provide a title, the project title is auto-generated as
+    # Optional prefix for auto-generated project titles (default: 'Project'). When the
+    # agent doesn't provide a title, the project title is auto-generated as
     # '<title-prefix>: <issue-title>' or '<title-prefix> #<issue-number>' based on the
     # issue context.
     # (optional)
@@ -2058,11 +2057,11 @@ safe-outputs:
         description: "Description of the workflow"
 
     # Optional array of project custom fields to create automatically after project
-    # creation. Useful for projects that require a fixed set of fields.
+    # creation.
     # (optional)
     field-definitions: []
       # Array items:
-        # The field name to create (e.g., 'Tracker Id', 'Priority')
+        # The field name to create (e.g., 'Priority', 'Classification')
         name: "My Workflow"
 
         # The GitHub Projects v2 custom field type
@@ -2088,8 +2087,8 @@ safe-outputs:
   # progress. Requires a Personal Access Token (PAT) or GitHub App token with
   # Projects: Read+Write permission. The GITHUB_TOKEN cannot be used for Projects
   # v2. Status updates are created on the specified project board and appear in the
-  # Updates tab. Often used by scheduled workflows and orchestrator workflows to
-  # post run summaries with progress, findings, and next steps.
+  # Updates tab. Typically used by orchestrators to post run summaries with
+  # progress, findings, and next steps.
   create-project-status-update:
     # Maximum number of status updates to create (default: 1). Typically 1 per
     # orchestrator run.

pkg/cli/execution_context.go

@@ -0,0 +1,61 @@
+package cli
+
+import (
+	"fmt"
+	"sync"
+)
+
+// executionContext tracks the current command execution context
+// This is used to enforce security boundaries, such as preventing
+// secret modifications during the upgrade command.
+type executionContext struct {
+	mu              sync.RWMutex
+	currentCommand  string
+	allowSecretMods bool
+}
+
+var globalExecutionContext = &executionContext{
+	allowSecretMods: true, // Default: allow secret modifications
+}
+
+// SetUpgradeContext marks that we're running the upgrade command
+// This disables secret modifications as a security measure
+func SetUpgradeContext() func() {
+	globalExecutionContext.mu.Lock()
+	defer globalExecutionContext.mu.Unlock()
+
+	// Save previous state
+	prevCommand := globalExecutionContext.currentCommand
+	prevAllow := globalExecutionContext.allowSecretMods
+
+	// Set upgrade context
+	globalExecutionContext.currentCommand = "upgrade"
+	globalExecutionContext.allowSecretMods = false
+
+	// Return cleanup function to restore previous state
+	return func() {
+		globalExecutionContext.mu.Lock()
+		defer globalExecutionContext.mu.Unlock()
+		globalExecutionContext.currentCommand = prevCommand
+		globalExecutionContext.allowSecretMods = prevAllow
+	}
+}
+
+// CheckSecretModificationAllowed checks if secret modifications are allowed
+// in the current execution context. Returns an error if they are not allowed.
+func CheckSecretModificationAllowed() error {
+	globalExecutionContext.mu.RLock()
+	defer globalExecutionContext.mu.RUnlock()
+
+	if !globalExecutionContext.allowSecretMods {
+		return fmt.Errorf("secret modifications are not allowed during %s command execution", globalExecutionContext.currentCommand)
+	}
+	return nil
+}
+
+// GetCurrentCommand returns the currently executing command name
+func GetCurrentCommand() string {
+	globalExecutionContext.mu.RLock()
+	defer globalExecutionContext.mu.RUnlock()
+	return globalExecutionContext.currentCommand
+}

pkg/cli/execution_context_test.go

@@ -0,0 +1,93 @@
+//go:build !integration
+
+package cli
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestExecutionContext_Default(t *testing.T) {
+	// Default state should allow secret modifications
+	err := CheckSecretModificationAllowed()
+	require.NoError(t, err, "Should allow secret modifications by default")
+
+	cmd := GetCurrentCommand()
+	assert.Empty(t, cmd, "Should have no current command by default")
+}
+
+func TestExecutionContext_SetUpgradeContext(t *testing.T) {
+	// Ensure we start in a clean state
+	globalExecutionContext.currentCommand = ""
+	globalExecutionContext.allowSecretMods = true
+
+	// Set upgrade context
+	cleanup := SetUpgradeContext()
+	defer cleanup()
+
+	// Check that secret modifications are blocked
+	err := CheckSecretModificationAllowed()
+	require.Error(t, err, "Should not allow secret modifications in upgrade context")
+	assert.Contains(t, err.Error(), "upgrade", "Error should mention upgrade command")
+
+	cmd := GetCurrentCommand()
+	assert.Equal(t, "upgrade", cmd, "Should be in upgrade command context")
+}
+
+func TestExecutionContext_RestoreAfterCleanup(t *testing.T) {
+	// Ensure we start in a clean state
+	globalExecutionContext.currentCommand = ""
+	globalExecutionContext.allowSecretMods = true
+
+	// Set upgrade context and immediately clean up
+	cleanup := SetUpgradeContext()
+
+	// Verify upgrade context is set
+	err := CheckSecretModificationAllowed()
+	require.Error(t, err, "Should block secrets in upgrade context")
+
+	// Clean up
+	cleanup()
+
+	// Verify we're back to default state
+	err = CheckSecretModificationAllowed()
+	require.NoError(t, err, "Should allow secret modifications after cleanup")
+
+	cmd := GetCurrentCommand()
+	assert.Empty(t, cmd, "Should have no current command after cleanup")
+}
+
+func TestExecutionContext_NestedContexts(t *testing.T) {
+	// Ensure we start in a clean state
+	globalExecutionContext.currentCommand = ""
+	globalExecutionContext.allowSecretMods = true
+
+	// First context
+	cleanup1 := SetUpgradeContext()
+	defer cleanup1()
+
+	err := CheckSecretModificationAllowed()
+	require.Error(t, err, "Should block secrets in upgrade context")
+
+	// Second context (nested)
+	cleanup2 := SetUpgradeContext()
+
+	err = CheckSecretModificationAllowed()
+	require.Error(t, err, "Should still block secrets in nested context")
+
+	// Clean up second context
+	cleanup2()
+
+	// Should still be in first context
+	err = CheckSecretModificationAllowed()
+	require.Error(t, err, "Should still block secrets after cleaning up nested context")
+
+	// Clean up first context
+	cleanup1()
+
+	// Now should be back to default
+	err = CheckSecretModificationAllowed()
+	assert.NoError(t, err, "Should allow secret modifications after all cleanups")
+}

pkg/cli/init.go

@@ -388,6 +388,11 @@ func setupEngineSecrets(engine string, verbose bool) error {
 func attemptSetSecret(secretName, repoSlug string, verbose bool) error {
 	initLog.Printf("Attempting to set secret: %s for repo: %s", secretName, repoSlug)
 
+	// Check if secret modifications are allowed in current execution context
+	if err := CheckSecretModificationAllowed(); err != nil {
+		return err
+	}
+
 	// Check if secret already exists
 	exists, err := checkSecretExistsInRepo(secretName, repoSlug)
 	if err != nil {

pkg/cli/secret_set_command.go

@@ -170,6 +170,11 @@ func resolveSecretValueForSet(fromEnv, fromFlag string) (string, error) {
 }
 
 func setRepoSecret(client *api.RESTClient, owner, repo, name, value string) error {
+	// Check if secret modifications are allowed in current execution context
+	if err := CheckSecretModificationAllowed(); err != nil {
+		return err
+	}
+
 	pubKey, err := getRepoPublicKey(client, owner, repo)
 	if err != nil {
 		return fmt.Errorf("get repo public key: %w", err)

pkg/cli/upgrade_command.go

@@ -85,6 +85,10 @@ func runUpgradeCommand(verbose bool, workflowDir string, noFix bool, noCompile b
 	upgradeLog.Printf("Running upgrade command: verbose=%v, workflowDir=%s, noFix=%v, noCompile=%v, push=%v, noActions=%v",
 		verbose, workflowDir, noFix, noCompile, push, noActions)
 
+	// Set execution context to prevent secret modifications during upgrade
+	cleanup := SetUpgradeContext()
+	defer cleanup()
+
 	// Step 0a: If --push is enabled, ensure git status is clean before starting
 	if push {
 		upgradeLog.Print("Checking for clean working directory (--push enabled)")

pkg/cli/upgrade_command_secret_barrier_test.go

@@ -0,0 +1,94 @@
+//go:build !integration
+
+package cli
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestUpgradeCommand_BlocksSecretModification(t *testing.T) {
+	// Ensure we start in a clean state
+	globalExecutionContext.currentCommand = ""
+	globalExecutionContext.allowSecretMods = true
+
+	// Verify that secrets can be modified before upgrade context
+	err := CheckSecretModificationAllowed()
+	require.NoError(t, err, "Should allow secret modifications before upgrade")
+
+	// Set upgrade context
+	cleanup := SetUpgradeContext()
+	defer cleanup()
+
+	// Verify that secret modifications are now blocked
+	err = CheckSecretModificationAllowed()
+	require.Error(t, err, "Should block secret modifications during upgrade")
+	assert.Contains(t, err.Error(), "upgrade", "Error message should mention upgrade")
+	assert.Contains(t, err.Error(), "not allowed", "Error message should indicate operation is not allowed")
+}
+
+func TestSetRepoSecret_BlockedDuringUpgrade(t *testing.T) {
+	// This test verifies that setRepoSecret checks the execution context
+	// We can't easily test the full function without mocking the GitHub API,
+	// but we can verify the guard is in place by setting upgrade context
+	// and checking that the error propagates
+
+	// Ensure we start in a clean state
+	globalExecutionContext.currentCommand = ""
+	globalExecutionContext.allowSecretMods = true
+
+	// Set upgrade context
+	cleanup := SetUpgradeContext()
+	defer cleanup()
+
+	// Attempt to call setRepoSecret - it should fail immediately with context check
+	// Note: We pass nil for client which would normally cause an error later,
+	// but the context check should fail first
+	err := setRepoSecret(nil, "owner", "repo", "TEST_SECRET", "value")
+
+	require.Error(t, err, "Should fail when trying to set secret during upgrade")
+	assert.Contains(t, err.Error(), "upgrade", "Error should mention upgrade command")
+}
+
+func TestAttemptSetSecret_BlockedDuringUpgrade(t *testing.T) {
+	// This test verifies that attemptSetSecret checks the execution context
+
+	// Ensure we start in a clean state
+	globalExecutionContext.currentCommand = ""
+	globalExecutionContext.allowSecretMods = true
+
+	// Set upgrade context
+	cleanup := SetUpgradeContext()
+	defer cleanup()
+
+	// Attempt to call attemptSetSecret - it should fail immediately with context check
+	err := attemptSetSecret("TEST_SECRET", "owner/repo", false)
+
+	require.Error(t, err, "Should fail when trying to set secret during upgrade")
+	assert.Contains(t, err.Error(), "upgrade", "Error should mention upgrade command")
+}
+
+func TestUpgradeCommand_ContextCleanupOnError(t *testing.T) {
+	// Ensure context is cleaned up even if upgrade fails
+	// We'll simulate this by setting the context and cleaning up
+
+	// Ensure we start in a clean state
+	globalExecutionContext.currentCommand = ""
+	globalExecutionContext.allowSecretMods = true
+
+	// Set upgrade context
+	cleanup := SetUpgradeContext()
+
+	// Verify upgrade context is active
+	err := CheckSecretModificationAllowed()
+	require.Error(t, err, "Should block secrets during upgrade")
+
+	// Simulate cleanup (what defer does in runUpgradeCommand)
+	cleanup()
+
+	// Verify context is cleaned up
+	err = CheckSecretModificationAllowed()
+	assert.NoError(t, err, "Should allow secrets after cleanup")
+}