diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 3a8d6f0fa7..f0fe076e34 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -20,3 +20,7 @@ updates: directory: /.github/workflows schedule: interval: weekly +- package-ecosystem: gomod + directory: /.github/workflows + schedule: + interval: weekly diff --git a/.github/workflows/daily-multi-device-docs-tester.lock.yml b/.github/workflows/daily-multi-device-docs-tester.lock.yml index 3146dac49c..4d270fb16b 100644 --- a/.github/workflows/daily-multi-device-docs-tester.lock.yml +++ b/.github/workflows/daily-multi-device-docs-tester.lock.yml @@ -170,7 +170,7 @@ jobs: const determineAutomaticLockdown = require('/opt/gh-aw/actions/determine_automatic_lockdown.cjs'); await determineAutomaticLockdown(github, context, core); - name: Download container images - run: bash /opt/gh-aw/actions/download_docker_images.sh ghcr.io/github/gh-aw-mcpg:v0.0.96 ghcr.io/github/github-mcp-server:v0.30.2 mcr.microsoft.com/playwright/mcp node:lts-alpine + run: bash /opt/gh-aw/actions/download_docker_images.sh ghcr.io/github/gh-aw-mcpg:v0.0.94 ghcr.io/github/github-mcp-server:v0.30.2 mcr.microsoft.com/playwright/mcp node:lts-alpine - name: Write Safe Outputs Config run: | mkdir -p /opt/gh-aw/safeoutputs @@ -452,7 +452,7 @@ jobs: # Register API key as secret to mask it from logs echo "::add-mask::${MCP_GATEWAY_API_KEY}" export GH_AW_ENGINE="claude" - export MCP_GATEWAY_DOCKER_COMMAND='docker run -i --rm --network host -v /var/run/docker.sock:/var/run/docker.sock -e MCP_GATEWAY_PORT -e MCP_GATEWAY_DOMAIN -e MCP_GATEWAY_API_KEY -e DEBUG -e MCP_GATEWAY_LOG_DIR -e GH_AW_MCP_LOG_DIR -e GH_AW_SAFE_OUTPUTS -e GH_AW_SAFE_OUTPUTS_CONFIG_PATH -e GH_AW_SAFE_OUTPUTS_TOOLS_PATH -e GH_AW_ASSETS_BRANCH -e GH_AW_ASSETS_MAX_SIZE_KB -e GH_AW_ASSETS_ALLOWED_EXTS -e DEFAULT_BRANCH -e GITHUB_MCP_SERVER_TOKEN -e GITHUB_MCP_LOCKDOWN -e GITHUB_REPOSITORY -e GITHUB_SERVER_URL -e GITHUB_SHA -e GITHUB_WORKSPACE -e GITHUB_TOKEN -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RUN_ATTEMPT -e GITHUB_JOB -e GITHUB_ACTION -e GITHUB_EVENT_NAME -e GITHUB_EVENT_PATH -e GITHUB_ACTOR -e GITHUB_ACTOR_ID -e GITHUB_TRIGGERING_ACTOR -e GITHUB_WORKFLOW -e GITHUB_WORKFLOW_REF -e GITHUB_WORKFLOW_SHA -e GITHUB_REF -e GITHUB_REF_NAME -e GITHUB_REF_TYPE -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e GH_AW_SAFE_OUTPUTS_PORT -e GH_AW_SAFE_OUTPUTS_API_KEY -v /opt:/opt:ro -v /tmp:/tmp:rw -v '"${GITHUB_WORKSPACE}"':'"${GITHUB_WORKSPACE}"':rw ghcr.io/github/gh-aw-mcpg:v0.0.96' + export MCP_GATEWAY_DOCKER_COMMAND='docker run -i --rm --network host -v /var/run/docker.sock:/var/run/docker.sock -e MCP_GATEWAY_PORT -e MCP_GATEWAY_DOMAIN -e MCP_GATEWAY_API_KEY -e DEBUG -e MCP_GATEWAY_LOG_DIR -e GH_AW_MCP_LOG_DIR -e GH_AW_SAFE_OUTPUTS -e GH_AW_SAFE_OUTPUTS_CONFIG_PATH -e GH_AW_SAFE_OUTPUTS_TOOLS_PATH -e GH_AW_ASSETS_BRANCH -e GH_AW_ASSETS_MAX_SIZE_KB -e GH_AW_ASSETS_ALLOWED_EXTS -e DEFAULT_BRANCH -e GITHUB_MCP_SERVER_TOKEN -e GITHUB_MCP_LOCKDOWN -e GITHUB_REPOSITORY -e GITHUB_SERVER_URL -e GITHUB_SHA -e GITHUB_WORKSPACE -e GITHUB_TOKEN -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RUN_ATTEMPT -e GITHUB_JOB -e GITHUB_ACTION -e GITHUB_EVENT_NAME -e GITHUB_EVENT_PATH -e GITHUB_ACTOR -e GITHUB_ACTOR_ID -e GITHUB_TRIGGERING_ACTOR -e GITHUB_WORKFLOW -e GITHUB_WORKFLOW_REF -e GITHUB_WORKFLOW_SHA -e GITHUB_REF -e GITHUB_REF_NAME -e GITHUB_REF_TYPE -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e GH_AW_SAFE_OUTPUTS_PORT -e GH_AW_SAFE_OUTPUTS_API_KEY -v /opt:/opt:ro -v /tmp:/tmp:rw -v '"${GITHUB_WORKSPACE}"':'"${GITHUB_WORKSPACE}"':rw ghcr.io/github/gh-aw-mcpg:v0.0.94' cat << MCPCONFIG_EOF | bash /opt/gh-aw/actions/start_mcp_gateway.sh { diff --git a/.github/workflows/go.mod b/.github/workflows/go.mod index b322d484db..a537eb2f9b 100644 --- a/.github/workflows/go.mod +++ b/.github/workflows/go.mod @@ -1,3 +1,5 @@ module github.com/github/gh-aw-workflows-deps - go 1.21 + +require ( +) diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index 664bd89f41..1387af92f1 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/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 # ': ' or ' #' 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. diff --git a/pkg/cli/execution_context.go b/pkg/cli/execution_context.go new file mode 100644 index 0000000000..b9db1bbf9b --- /dev/null +++ b/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 +} diff --git a/pkg/cli/execution_context_test.go b/pkg/cli/execution_context_test.go new file mode 100644 index 0000000000..236dfb4309 --- /dev/null +++ b/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") +} diff --git a/pkg/cli/init.go b/pkg/cli/init.go index 6ae7b73ec4..65147899f4 100644 --- a/pkg/cli/init.go +++ b/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 { diff --git a/pkg/cli/secret_set_command.go b/pkg/cli/secret_set_command.go index 8463873d07..86ed8e292f 100644 --- a/pkg/cli/secret_set_command.go +++ b/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) diff --git a/pkg/cli/upgrade_command.go b/pkg/cli/upgrade_command.go index c9ea54b792..625963920c 100644 --- a/pkg/cli/upgrade_command.go +++ b/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)") diff --git a/pkg/cli/upgrade_command_secret_barrier_test.go b/pkg/cli/upgrade_command_secret_barrier_test.go new file mode 100644 index 0000000000..2f3a8548a2 --- /dev/null +++ b/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") +}