Skip to content

Commit 27e0d8d

Browse files
authored
Resolve context propagation and environment-mutation lint findings in CLI/workflow paths (#39007)
1 parent 4163393 commit 27e0d8d

7 files changed

Lines changed: 72 additions & 17 deletions

File tree

pkg/cli/add_interactive_orchestrator.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/github/gh-aw/pkg/constants"
1313
"github.com/github/gh-aw/pkg/logger"
1414
"github.com/github/gh-aw/pkg/styles"
15+
"github.com/github/gh-aw/pkg/workflow"
1516
)
1617

1718
var addInteractiveLog = logger.New("cli:add_interactive")
@@ -76,7 +77,7 @@ func RunAddInteractive(ctx context.Context, config *AddInteractiveConfig) error
7677
detectedHost := getHostFromOriginRemote()
7778
if detectedHost != "github.com" {
7879
addInteractiveLog.Printf("Auto-detected GHES host from git remote: %s", detectedHost)
79-
os.Setenv("GH_HOST", detectedHost)
80+
workflow.SetDefaultGHHost(detectedHost)
8081
if config.Verbose {
8182
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Auto-detected GitHub Enterprise host: "+detectedHost))
8283
}

pkg/cli/engine_secrets.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -325,8 +325,6 @@ func promptForCopilotPATUnified(req SecretRequirement, config EngineSecretConfig
325325
return fmt.Errorf("failed to get Copilot token: %w", err)
326326
}
327327

328-
// Store in environment for later use
329-
_ = os.Setenv(req.Name, token)
330328
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Valid fine-grained Copilot token received"))
331329

332330
// Upload to repository if we have a repo slug
@@ -373,8 +371,6 @@ func promptForSystemTokenUnified(req SecretRequirement, config EngineSecretConfi
373371
return fmt.Errorf("failed to get %s token: %w", req.Name, err)
374372
}
375373

376-
// Store in environment for later use
377-
_ = os.Setenv(req.Name, token)
378374
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(req.Name+" token received"))
379375

380376
// Upload to repository if we have a repo slug
@@ -426,8 +422,6 @@ func promptForGenericAPIKeyUnified(req SecretRequirement, config EngineSecretCon
426422
return fmt.Errorf("failed to get %s API key: %w", label, err)
427423
}
428424

429-
// Store in environment for later use
430-
_ = os.Setenv(req.Name, apiKey)
431425
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(label+" API key received"))
432426

433427
// Upload to repository if we have a repo slug

pkg/cli/mcp_inspect_mcp.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,12 +147,12 @@ func connectStdioMCPServer(ctx context.Context, config parser.RegistryMCPServerC
147147
if config.Container != "" {
148148
// Docker container mode
149149
args := append([]string{"run", "--rm", "-i"}, config.Args...)
150-
cmd = exec.Command("docker", args...)
150+
cmd = exec.CommandContext(ctx, "docker", args...)
151151
} else {
152152
// Direct command mode
153153
// #nosec G204 -- config.Command is validated via exec.LookPath above (line 138);
154154
// exec.Command with separate args (not shell execution) prevents shell injection.
155-
cmd = exec.Command(config.Command, config.Args...)
155+
cmd = exec.CommandContext(ctx, config.Command, config.Args...)
156156
}
157157

158158
// Set environment variables

pkg/cli/run_push.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ func collectImports(workflowPath string, files map[string]bool, visited map[stri
372372
// pushWorkflowFiles commits and pushes the workflow files to the repository
373373
func pushWorkflowFiles(ctx context.Context, workflowName string, files []string, refOverride string, verbose bool) error {
374374
if ctx == nil {
375-
ctx = context.Background()
375+
return errors.New("context is required")
376376
}
377377
runPushLog.Printf("Pushing %d files for workflow: %s", len(files), workflowName)
378378
runPushLog.Printf("Files to push: %v", files)

pkg/cli/run_workflow_execution.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,14 @@ func RunWorkflowsOnGitHub(ctx context.Context, workflowNames []string, opts RunO
571571

572572
// Add a small delay between workflows to avoid overwhelming GitHub API
573573
if i < len(workflowNames)-1 {
574-
time.Sleep(betweenWorkflowsDelay)
574+
timer := time.NewTimer(betweenWorkflowsDelay)
575+
select {
576+
case <-ctx.Done():
577+
timer.Stop()
578+
fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Operation cancelled"))
579+
return ctx.Err()
580+
case <-timer.C:
581+
}
575582
}
576583
}
577584

pkg/workflow/github_cli.go

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,28 +9,45 @@ import (
99
"os"
1010
"os/exec"
1111
"strings"
12+
"sync"
1213

1314
"github.com/github/gh-aw/pkg/console"
1415
"github.com/github/gh-aw/pkg/logger"
1516
"github.com/github/gh-aw/pkg/tty"
1617
)
1718

1819
var githubCLILog = logger.New("workflow:github_cli")
20+
var defaultGHHost struct {
21+
mu sync.RWMutex
22+
host string
23+
}
24+
25+
// SetDefaultGHHost sets the default host used by gh CLI helper commands when GH_HOST
26+
// is not set in the process environment.
27+
func SetDefaultGHHost(host string) {
28+
defaultGHHost.mu.Lock()
29+
defer defaultGHHost.mu.Unlock()
30+
defaultGHHost.host = host
31+
}
32+
33+
func getDefaultGHHost() string {
34+
defaultGHHost.mu.RLock()
35+
defer defaultGHHost.mu.RUnlock()
36+
return defaultGHHost.host
37+
}
1938

2039
// setupGHCommand creates an exec.Cmd for gh CLI with proper token configuration.
2140
// This is the core implementation shared by ExecGH and ExecGHContext.
22-
// When ctx is nil, it uses exec.Command; when ctx is provided, it uses exec.CommandContext.
41+
// When ctx is nil, it falls back to context.TODO().
2342
func setupGHCommand(ctx context.Context, args ...string) *exec.Cmd {
2443
// Check if GH_TOKEN or GITHUB_TOKEN is available
2544
ghToken := os.Getenv("GH_TOKEN")
2645
githubToken := os.Getenv("GITHUB_TOKEN")
2746

28-
var cmd *exec.Cmd
29-
if ctx != nil {
30-
cmd = exec.CommandContext(ctx, "gh", args...)
31-
} else {
32-
cmd = exec.Command("gh", args...)
47+
if ctx == nil {
48+
ctx = context.TODO()
3349
}
50+
cmd := exec.CommandContext(ctx, "gh", args...)
3451

3552
if ghToken != "" || githubToken != "" {
3653
githubCLILog.Printf("Token detected, using gh CLI for command: gh %v", args)
@@ -44,6 +61,9 @@ func setupGHCommand(ctx context.Context, args ...string) *exec.Cmd {
4461
githubCLILog.Printf("GH_TOKEN not set, using GITHUB_TOKEN for gh CLI")
4562
cmd.Env = append(os.Environ(), "GH_TOKEN="+githubToken)
4663
}
64+
if os.Getenv("GH_HOST") == "" {
65+
SetGHHostEnv(cmd, getDefaultGHHost())
66+
}
4767

4868
return cmd
4969
}

pkg/workflow/github_cli_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,3 +473,36 @@ func TestSetGHHostEnv(t *testing.T) {
473473
})
474474
}
475475
}
476+
477+
func TestSetupGHCommandUsesDefaultGHHost(t *testing.T) {
478+
originalDefaultHost := getDefaultGHHost()
479+
defer SetDefaultGHHost(originalDefaultHost)
480+
481+
t.Run("applies default host when GH_HOST is not set", func(t *testing.T) {
482+
originalHost, hadOriginalHost := os.LookupEnv("GH_HOST")
483+
require.NoError(t, os.Unsetenv("GH_HOST"))
484+
t.Cleanup(func() {
485+
if hadOriginalHost {
486+
require.NoError(t, os.Setenv("GH_HOST", originalHost))
487+
return
488+
}
489+
require.NoError(t, os.Unsetenv("GH_HOST"))
490+
})
491+
SetDefaultGHHost("myorg.ghe.com")
492+
493+
cmd := setupGHCommand(context.Background(), "auth", "status")
494+
require.NotNil(t, cmd.Env)
495+
assert.Contains(t, cmd.Env, "GH_HOST=myorg.ghe.com")
496+
})
497+
498+
t.Run("does not apply default host when GH_HOST is already set", func(t *testing.T) {
499+
t.Setenv("GH_HOST", "explicit.ghe.com")
500+
SetDefaultGHHost("myorg.ghe.com")
501+
502+
cmd := setupGHCommand(context.Background(), "auth", "status")
503+
if cmd.Env == nil {
504+
return
505+
}
506+
assert.NotContains(t, cmd.Env, "GH_HOST=myorg.ghe.com")
507+
})
508+
}

0 commit comments

Comments
 (0)