Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions actions/setup/md/stale_lock_file_failed.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ The workflow run logs contain a verbose debug pass that shows exactly what was h

This makes it easy to spot accidental whitespace changes, encoding differences, or import path drift.

This mismatch can also happen on a **fresh install** (even without any manual edits) if `gh aw add <path>@<ref>` could not resolve the ref to an exact commit SHA during installation. In that case, rerun the add command with an exact SHA (or retry when API/rate-limit conditions recover), then recompile:

```bash
gh aw add <path>@<exact-sha>
gh aw compile
```

</details>

<details>
Expand Down
3 changes: 3 additions & 0 deletions cmd/gh-aw/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ Examples:
forceOverwrite, _ := cmd.Flags().GetBool("force")
refreshStopTime, _ := cmd.Flags().GetBool("refresh-stop-time")
forceRefreshActionPins, _ := cmd.Flags().GetBool("force-refresh-action-pins")
allowActionRefs, _ := cmd.Flags().GetBool("allow-action-refs")
zizmor, _ := cmd.Flags().GetBool("zizmor")
poutine, _ := cmd.Flags().GetBool("poutine")
actionlint, _ := cmd.Flags().GetBool("actionlint")
Expand Down Expand Up @@ -335,6 +336,7 @@ Examples:
ForceOverwrite: forceOverwrite,
RefreshStopTime: refreshStopTime,
ForceRefreshActionPins: forceRefreshActionPins,
AllowActionRefs: allowActionRefs,
Zizmor: zizmor,
Poutine: poutine,
Actionlint: actionlint,
Expand Down Expand Up @@ -684,6 +686,7 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all
compileCmd.Flags().Bool("force", false, "Force overwrite of existing dependency files (e.g., dependabot.yml)")
compileCmd.Flags().Bool("refresh-stop-time", false, "Force regeneration of stop-after times instead of preserving existing values from lock files")
compileCmd.Flags().Bool("force-refresh-action-pins", false, "Force refresh of action pins by clearing the cache and resolving all action SHAs from GitHub API")
compileCmd.Flags().Bool("allow-action-refs", false, "Allow unresolved action refs and emit warnings instead of failing compilation")
compileCmd.Flags().Bool("zizmor", false, "Run zizmor security scanner on generated .lock.yml files")
compileCmd.Flags().Bool("poutine", false, "Run poutine security scanner on generated .lock.yml files")
compileCmd.Flags().Bool("actionlint", false, "Run actionlint linter on generated .lock.yml files")
Expand Down
47 changes: 47 additions & 0 deletions pkg/actionpins/actionpins.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,23 @@ type SHAResolver interface {
ResolveSHA(repo, version string) (string, error)
}

// ResolutionErrorType classifies unresolved action-ref pinning outcomes for auditing.
type ResolutionErrorType string

const (
// ResolutionErrorTypeDynamicResolutionFailed indicates dynamic tag/ref -> SHA resolution failed.
ResolutionErrorTypeDynamicResolutionFailed ResolutionErrorType = "dynamic_resolution_failed"
// ResolutionErrorTypePinNotFound indicates no usable hardcoded pin was found for the ref.
ResolutionErrorTypePinNotFound ResolutionErrorType = "pin_not_found"
)

// ResolutionFailure captures an unresolved action-ref pinning event.
type ResolutionFailure struct {
Repo string
Ref string
ErrorType ResolutionErrorType
}

// PinContext provides the runtime context needed for action pin resolution.
// Callers construct one from their own state (e.g. WorkflowData fields).
// The Warnings map is mutated in place to deduplicate warning output.
Expand All @@ -57,9 +74,16 @@ type PinContext struct {
Resolver SHAResolver
// StrictMode controls how resolution failures are handled.
StrictMode bool
// EnforcePinned requires unresolved refs to fail unless AllowActionRefs is true.
EnforcePinned bool
// AllowActionRefs lowers unresolved pinning failures to warnings.
// When false, unresolved action refs return an error.
AllowActionRefs bool
// Warnings is a shared map for deduplicating warning messages.
// Keys are cache keys in the form "repo@version".
Warnings map[string]bool
// RecordResolutionFailure receives unresolved pinning failures for auditing.
RecordResolutionFailure func(f ResolutionFailure)
}

var (
Expand Down Expand Up @@ -210,6 +234,17 @@ func findCompatiblePin(pins []ActionPin, version string) (ActionPin, bool) {
return ActionPin{}, false
}

func notifyResolutionFailure(ctx *PinContext, actionRepo, version string, errorType ResolutionErrorType) {
if ctx == nil || ctx.RecordResolutionFailure == nil {
return
}
ctx.RecordResolutionFailure(ResolutionFailure{
Repo: actionRepo,
Ref: version,
ErrorType: errorType,
})
}

// ResolveActionPin returns the pinned action reference for a given action@version.
// It consults ctx.Resolver first, then falls back to embedded pins.
// If ctx is nil, only embedded pins are consulted.
Expand Down Expand Up @@ -301,6 +336,18 @@ func ResolveActionPin(actionRepo, version string, ctx *PinContext) (string, erro
ctx.Warnings = make(map[string]bool)
}
cacheKey := FormatCacheKey(actionRepo, version)
errorType := ResolutionErrorTypePinNotFound
if ctx.Resolver != nil {
errorType = ResolutionErrorTypeDynamicResolutionFailed
}
notifyResolutionFailure(ctx, actionRepo, version, errorType)
if ctx.EnforcePinned && !ctx.AllowActionRefs {
if ctx.Resolver != nil {
return "", fmt.Errorf("unable to pin action %s@%s: resolution failed", actionRepo, version)
}
return "", fmt.Errorf("unable to pin action %s@%s", actionRepo, version)
}

if !ctx.Warnings[cacheKey] {
warningMsg := fmt.Sprintf("Unable to pin action %s@%s", actionRepo, version)
if ctx.Resolver != nil {
Expand Down
14 changes: 10 additions & 4 deletions pkg/cli/add_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ Note: For guided interactive setup, use the 'add-wizard' command instead.`,
StopAfter: stopAfter,
DisableSecurityScanner: disableSecurityScanner,
}
_, err := AddWorkflows(workflows, opts)
_, err := AddWorkflows(cmd.Context(), workflows, opts)
return err
},
}
Expand Down Expand Up @@ -172,9 +172,9 @@ Note: For guided interactive setup, use the 'add-wizard' command instead.`,
// AddWorkflows adds one or more workflows from components to .github/workflows
// with optional repository installation and PR creation.
// Returns AddWorkflowsResult containing PR number (if created) and other metadata.
func AddWorkflows(workflows []string, opts AddOptions) (*AddWorkflowsResult, error) {
func AddWorkflows(ctx context.Context, workflows []string, opts AddOptions) (*AddWorkflowsResult, error) {
// Resolve workflows first - fetches content directly from GitHub
resolved, err := ResolveWorkflows(workflows, opts.Verbose)
resolved, err := ResolveWorkflows(ctx, workflows, opts.Verbose)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -494,12 +494,18 @@ func addWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTracker, o
if err := os.WriteFile(destFile, []byte(content), 0600); err != nil {
return fmt.Errorf("failed to write destination file '%s': %w", destFile, err)
}
// Read back the just-written file to ensure downstream processing (including
// frontmatter hash computation) uses the exact bytes on disk and avoids parity drift.
writtenContent, err := os.ReadFile(destFile)
if err != nil {
return fmt.Errorf("failed to read back destination file '%s': %w", destFile, err)
}

// Show output
if !opts.Quiet {
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Added workflow: "+destFile))

if description := ExtractWorkflowDescription(content); description != "" {
if description := ExtractWorkflowDescription(string(writtenContent)); description != "" {
fmt.Fprintln(os.Stderr, "")
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(description))
fmt.Fprintln(os.Stderr, "")
Expand Down
3 changes: 2 additions & 1 deletion pkg/cli/add_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cli

import (
"context"
"testing"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -99,7 +100,7 @@ func TestAddWorkflows(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
opts := AddOptions{}
_, err := AddWorkflows(tt.workflows, opts)
_, err := AddWorkflows(context.Background(), tt.workflows, opts)

if tt.expectError {
require.Error(t, err, "Expected error for test case: %s", tt.name)
Expand Down
7 changes: 4 additions & 3 deletions pkg/cli/add_current_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package cli

import (
"context"
"os"
"os/exec"
"strings"
Expand Down Expand Up @@ -69,7 +70,7 @@ func TestAddWorkflowsFromCurrentRepository(t *testing.T) {
ClearCurrentRepoSlugCache()

opts := AddOptions{}
_, err := AddWorkflows(tt.workflowSpecs, opts)
_, err := AddWorkflows(context.Background(), tt.workflowSpecs, opts)

if tt.expectError {
if err == nil {
Expand Down Expand Up @@ -153,7 +154,7 @@ func TestAddWorkflowsFromCurrentRepositoryMultiple(t *testing.T) {
ClearCurrentRepoSlugCache()

opts := AddOptions{}
_, err := AddWorkflows(tt.workflowSpecs, opts)
_, err := AddWorkflows(context.Background(), tt.workflowSpecs, opts)

if tt.expectError {
if err == nil {
Expand Down Expand Up @@ -195,7 +196,7 @@ func TestAddWorkflowsFromCurrentRepositoryNotInGitRepo(t *testing.T) {
// When not in a git repo, the check should be skipped (can't determine current repo)
// The function should proceed and fail for other reasons (e.g., workflow not found)
opts := AddOptions{}
_, err = AddWorkflows([]string{"some-owner/some-repo/workflow"}, opts)
_, err = AddWorkflows(context.Background(), []string{"some-owner/some-repo/workflow"}, opts)

// Should NOT get the "cannot add workflows from the current repository" error
if err != nil && strings.Contains(err.Error(), "cannot add workflows from the current repository") {
Expand Down
43 changes: 43 additions & 0 deletions pkg/cli/add_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"testing"

"github.com/github/gh-aw/pkg/fileutil"
"github.com/github/gh-aw/pkg/parser"
"github.com/github/gh-aw/pkg/workflow"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -356,6 +358,47 @@ Please analyze the repository and provide a summary.
assert.Contains(t, lockContentStr, "name: \"Test Local Workflow\"", "lock file should have workflow name")
assert.Contains(t, lockContentStr, "workflow_dispatch", "lock file should have trigger")
assert.Contains(t, lockContentStr, "jobs:", "lock file should have jobs section")

// Verify frontmatter hash parity between source markdown and lock metadata.
computedHash, hashErr := parser.ComputeFrontmatterHashFromFile(destWorkflowFile, parser.NewImportCache(setup.tempDir))
require.NoError(t, hashErr, "should compute frontmatter hash from added markdown file")
metadata, _, metadataErr := workflow.ExtractMetadataFromLockFile(lockContentStr)
require.NoError(t, metadataErr, "should extract lock metadata from compiled lock file")
require.NotNil(t, metadata, "lock metadata should be present")
assert.Equal(t, computedHash, metadata.FrontmatterHash,
"lock file frontmatter hash should match the hash recomputed from markdown file bytes")
}

// TestAddRemoteWorkflowFailsWhenSHAResolutionFails tests that add fails loudly when ref-to-SHA
// resolution fails and does not write partial workflow artifacts.
func TestAddRemoteWorkflowFailsWhenSHAResolutionFails(t *testing.T) {
setup := setupAddIntegrationTest(t)
defer setup.cleanup()

nonExistentWorkflowSpec := "github/gh-aw-does-not-exist/.github/workflows/not-real.md@main"

cmd := exec.Command(setup.binaryPath, "add", nonExistentWorkflowSpec)
cmd.Dir = setup.tempDir
output, err := cmd.CombinedOutput()
outputStr := string(output)

require.Error(t, err, "add command should fail when SHA resolution fails")
assert.Contains(t, outputStr, "failed to resolve 'main' to commit SHA",
"error output should clearly explain SHA resolution failure")
assert.Contains(t, outputStr, "Expected the GitHub API to return a commit SHA for the ref",
"error output should explain expected behavior")
assert.Contains(t, outputStr, "gh aw add github/gh-aw-does-not-exist/.github/workflows/not-real.md@<40-char-sha>",
"error output should provide a concrete retry example")

workflowsDir := filepath.Join(setup.tempDir, ".github", "workflows")
workflowFile := filepath.Join(workflowsDir, "not-real.md")
lockFile := filepath.Join(workflowsDir, "not-real.lock.yml")

_, workflowErr := os.Stat(workflowFile)
assert.True(t, os.IsNotExist(workflowErr), "workflow markdown file should not be written on SHA resolution failure")

_, lockErr := os.Stat(lockFile)
assert.True(t, os.IsNotExist(lockErr), "lock file should not be written on SHA resolution failure")
}

// TestAddWorkflowWithCustomName tests adding a workflow with a custom name
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/add_interactive_orchestrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func RunAddInteractive(ctx context.Context, workflowSpecs []string, verbose bool
func (c *AddInteractiveConfig) resolveWorkflows() error {
addInteractiveLog.Print("Resolving workflows early for description display")

resolved, err := ResolveWorkflows(c.WorkflowSpecs, c.Verbose)
resolved, err := ResolveWorkflows(c.Ctx, c.WorkflowSpecs, c.Verbose)
if err != nil {
return fmt.Errorf("failed to resolve workflows: %w", err)
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/cli/add_workflow_resolution.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"context"
"errors"
"fmt"
"os"
Expand Down Expand Up @@ -46,7 +47,7 @@ type ResolvedWorkflows struct {
// ResolveWorkflows resolves workflow specifications by parsing specs and fetching workflow content.
// For remote workflows, content is fetched directly from GitHub without cloning.
// Wildcards are only supported for local workflows (not remote repositories).
func ResolveWorkflows(workflows []string, verbose bool) (*ResolvedWorkflows, error) {
func ResolveWorkflows(ctx context.Context, workflows []string, verbose bool) (*ResolvedWorkflows, error) {
resolutionLog.Printf("Resolving workflows: count=%d", len(workflows))

if len(workflows) == 0 {
Expand Down Expand Up @@ -117,7 +118,7 @@ func ResolveWorkflows(workflows []string, verbose bool) (*ResolvedWorkflows, err

for _, spec := range parsedSpecs {
// Fetch workflow content - FetchWorkflowFromSource handles both local and remote
fetched, err := FetchWorkflowFromSource(spec, verbose)
fetched, err := FetchWorkflowFromSourceWithContext(ctx, spec, verbose)
if err != nil {
return nil, fmt.Errorf("workflow '%s' not found: %w", spec.String(), err)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/compile_compiler_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ func configureCompilerFlags(compiler *workflow.Compiler, config CompileConfig) {

// Set strict mode if specified
compiler.SetStrictMode(config.Strict)
compiler.SetAllowActionRefs(config.AllowActionRefs)

// Set trial mode if specified
if config.TrialMode {
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/compile_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type CompileConfig struct {
ForceOverwrite bool // Force overwrite of existing files (dependabot.yml)
RefreshStopTime bool // Force regeneration of stop-after times instead of preserving existing ones
ForceRefreshActionPins bool // Force refresh of action pins by clearing cache and resolving from GitHub API
AllowActionRefs bool // Allow unresolved action refs as warnings instead of errors
Zizmor bool // Run zizmor security scanner on generated .lock.yml files
Poutine bool // Run poutine security scanner on generated .lock.yml files
Actionlint bool // Run actionlint linter on generated .lock.yml files
Expand Down
Loading
Loading