🔐 Validate PAT tokens and improve RunOptions

[dsyme]

Summary

• Added validation for fine-grained Personal Access Tokens (PATs) for GitHub Copilot
• Removed --use-local-secrets flag and updated CLI workflow execution
• Simplified workflow execution options with a new RunOptions struct

Changes

• Introduced strict PAT validation for COPILOT_GITHUB_TOKEN
• Removed pushSecrets flag from workflow execution methods
• Created a unified RunOptions struct to simplify workflow execution
• Added new PAT validation utility functions in stringutil package
• Updated command-line interface to use new RunOptions

Rationale

• Improve security by enforcing fine-grained PAT requirements
• Simplify CLI workflow execution by consolidating options
• Provide clearer error messages for incorrect token types

[Copilot]

Pull request overview

This PR tightens GitHub Copilot authentication by enforcing fine-grained PATs, removes the --use-local-secrets workflow execution mode, and refactors workflow execution APIs to use a consolidated RunOptions struct across the CLI and tests.

Changes:

• Added fine-grained PAT detection/validation utilities (and tests) and integrated validation into interactive Copilot token collection and setup secret validation.
• Removed --use-local-secrets / secret-pushing plumbing from workflow execution and trial flows, shifting toward ensuring repository secrets via a new secret-collection path.
• Introduced RunOptions to simplify workflow execution function signatures and updated CLI entrypoints + tests accordingly.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/stringutil/pat_validation.go	Adds PAT classification + Copilot fine-grained PAT validation helpers.
pkg/stringutil/pat_validation_test.go	Adds unit tests for PAT classification/validation behavior.
pkg/cli/secret_collection.go	New interactive secret collection/ensuring logic for engine secrets (Copilot/Claude/Codex).
pkg/cli/trial_command.go	Removes push-secrets flow and ensures engine secrets before trial installation/runs.
pkg/cli/trial_repository.go	Updates trial install path to rely on TrialOptions and removes secret-determination/pushing from install.
pkg/cli/trial_command_lipgloss_test.go	Updates confirmation rendering tests after removing push-secrets option.
pkg/cli/run_workflow_execution.go	Replaces large parameter list with RunOptions; removes secret-pushing logic; keeps run/enable/push/wait behavior.
pkg/cli/run_workflow_execution_test.go	Updates workflow execution tests to use RunOptions.
pkg/cli/run_interactive.go	Updates interactive run entrypoints to use RunOptions and removes --use-local-secrets display.
pkg/cli/run_interactive_test.go	Updates command-string tests to remove --use-local-secrets.
pkg/cli/run_command_test.go	Updates compile-time signature checks to reflect RunOptions.
pkg/cli/local_workflow_trial_test.go	Updates trial local install test for updated install function signature.
pkg/cli/context_cancellation_test.go	Updates cancellation tests to call execution APIs via RunOptions.
pkg/cli/commands_test.go	Updates CLI behavior tests to use RunOptions signatures.
pkg/cli/add_interactive_engine.go	Validates existing/provided Copilot token as fine-grained PAT during interactive engine setup.
pkg/cli/add_interactive_workflow.go	Updates call to RunSpecificWorkflowInteractively after signature change.
cmd/gh-aw/main.go	Removes --use-local-secrets flag and routes execution via RunOptions.
actions/setup/sh/validate_multi_secret.sh	Adds COPILOT token prefix validation to enforce fine-grained PATs in setup validation step.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (3)

pkg/cli/secret_collection.go:119

• When a valid COPILOT_GITHUB_TOKEN is present in the environment, this code uploads it to the repository automatically if RepoSlug is set. Given that trial host repos are preserved by default, this can persist a PAT without an explicit “upload this secret” confirmation. Consider prompting before uploading (or only uploading when the host repo will be deleted, or when the user explicitly opts in), and make the persistence behavior very explicit in the CLI output.

	// Check if COPILOT_GITHUB_TOKEN is already in environment
	existingToken := os.Getenv(secretName)
	if existingToken != "" {
		// Validate the existing token is a fine-grained PAT
		if err := stringutil.ValidateCopilotPAT(existingToken); err != nil {
			fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("COPILOT_GITHUB_TOKEN in environment is not a fine-grained PAT: %s", stringutil.GetPATTypeDescription(existingToken))))
			fmt.Fprintln(os.Stderr, console.FormatErrorMessage(err.Error()))
			// Continue to prompt for a new token
		} else {
			fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Found valid fine-grained COPILOT_GITHUB_TOKEN in environment"))
			// Upload to repository if we have a repo slug
			if config.RepoSlug != "" {
				return uploadSecretIfNeeded(secretName, existingToken, config.RepoSlug, config.Verbose)
			}

pkg/cli/secret_collection.go:92

• This helper parses gh secret list output and is critical to avoid unnecessary secret prompts/uploads, but it currently has no unit tests. Consider adding table-driven tests for stringContainsSecret to cover exact-match-at-line-start behavior, tabs/spaces, and false positives (e.g., SECRET vs SECRET_SUFFIX).

// stringContainsSecret checks if the output contains a secret name
func stringContainsSecret(output, secretName string) bool {
	// The secret list output format is typically "SECRET_NAME\tUpdated ...\n"
	// We need to check for the exact secret name at the start of a line
	lines := strings.Split(output, "\n")
	for _, line := range lines {
		if len(line) >= len(secretName) {
			// Check if line starts with secret name followed by tab or space
			if line[:len(secretName)] == secretName && (len(line) == len(secretName) || line[len(secretName)] == '\t' || line[len(secretName)] == ' ') {
				return true
			}
		}
	}
	return false

pkg/cli/trial_command.go:665

• The confirmation output only mentions secret handling when an engine override is provided. In the no-override case, users won’t be warned that required engine secrets must already exist in the host repo, and in the override case it doesn’t clarify that the secret may remain stored when the host repo is preserved. Consider always displaying a secrets note (e.g., “requires pre-configured secrets” vs “will prompt and store”), and explicitly warning about persistence when --delete-host-repo-after is not set.

	// Display secret usage information (only when engine override is specified)
	if engineOverride != "" {
		configInfo.WriteString("\n")
		fmt.Fprintf(&configInfo, "Secrets:   Will prompt for %s API key if needed (stored as repository secret)", engineOverride)
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The troubleshooting guidance says trials automatically prompt for missing secrets and upload them, but in code this only happens when an explicit engine override is provided. Update the doc to match behavior, or ensure secrets for the engine configured in workflow frontmatter/default even when --engine is not used.

[Copilot]

EnsureEngineSecret’s docstring says it checks whether the secret exists in the repository, but the implementation relies on config.ExistingSecrets being pre-populated by the caller. If ExistingSecrets is empty/nil (or CheckExistingSecrets fails), this can unnecessarily prompt the user and attempt uploads even when the secret already exists. Consider having EnsureEngineSecret call CheckExistingSecrets internally when RepoSlug is set (or update the docstring/API to make the precondition explicit).

This issue also appears in the following locations of the same file:

• line 106
• line 79

Suggested change

	// Ensure ExistingSecrets is populated when we have a RepoSlug but no pre-populated map.
	if config.RepoSlug != "" && config.ExistingSecrets == nil {
	if existing, err := CheckExistingSecrets(config.RepoSlug); err != nil {
	// Log the error but continue with an empty ExistingSecrets map to preserve previous behavior.
	secretCollectionLog.Printf("Failed to check existing secrets for %s: %v", config.RepoSlug, err)
	} else {
	config.ExistingSecrets = existing
	}
	}

[Copilot]

The secret-handling behavior here is gated on EngineOverride, but the comment says secrets are handled during compilation when no override is specified. Compilation only generates workflows; it doesn’t ensure repository secrets exist, so trials may fail later with missing secrets when EngineOverride is empty. Consider ensuring secrets for the engine(s) actually used by the compiled workflow(s) (frontmatter/default) or updating the comment and UX to clearly state secrets must already be configured unless --engine is used.

This issue also appears on line 661 of the same file.

Suggested change

	// Step 2.5: Ensure engine secrets are configured when an explicit engine override is provided
	// When no override is specified, the workflow will use its frontmatter engine and handle secrets during compilation
	// Step 2.5: Ensure engine secrets are configured when an explicit engine override is provided.
	// When no override is specified, this command does not configure secrets; required secrets for the
	// workflow's default/frontmatter engine must already be present in the repository.

[Copilot]

This documentation claims trial will automatically prompt for missing secrets and upload them, but the current implementation only calls secret prompting/upload when --engine (engine override) is provided. Either update the docs to reflect the actual behavior, or expand the trial flow to detect the workflow’s configured engine(s) and ensure their secrets even without an override.