Add post-update SHA integrity validation for actions-lock entries

docs/adr/47907-post-update-sha-integrity-validation-actions-lock.md

@@ -0,0 +1,44 @@
+# ADR-47907: Post-Update SHA Integrity Validation for actions-lock.json
+
+**Date**: 2026-07-25
+**Status**: Draft
+**Deciders**: pelikhan, copilot-swe-agent
+
+---
+
+### Context
+
+The `gh aw update` command resolves and pins GitHub Action commit SHAs and container image digests into `.github/aw/actions-lock.json`. Prior to this change, the file was written without any final structural check, meaning malformed entries (e.g., a truncated SHA, a mismatched map key, or an incorrectly formatted container digest) could be silently persisted and later used in production workflows. The only time a bad entry would be caught was at workflow execution time — far downstream from where the corruption was introduced.
+
+### Decision
+
+We will add a terminal validation phase at the end of `RunUpdateWorkflows` that re-reads and structurally verifies all entries in `actions-lock.json` after the update, recompile, and pin-refresh steps complete. Validation failures surface as a hard error (`update validation failed: ...`) rather than being silently ignored. This validation only runs when no earlier error occurred (`firstErr == nil`), so it does not mask upstream failures.
+
+### Alternatives Considered
+
+#### Alternative 1: Inline validation during update (validate-as-you-write)
+
+Each action or container entry could be validated at the point it is written into the cache, as part of the existing resolve-and-pin logic. This would catch errors closer to their origin and avoid re-reading the file. It was not chosen because it would require threading validation logic into multiple existing write paths (action resolution, container pinning, recompile step), significantly increasing the coupling and risk of regressions. A single terminal pass is simpler and keeps validation isolated from write logic.
+
+#### Alternative 2: No post-update validation (status quo)
+
+The existing approach relied on correctness guarantees from the upstream resolution APIs and the write path. Malformed entries would only surface at workflow-run time. This was rejected because supply-chain integrity requires catching bad pins before they are persisted; silent failures at the lock-write layer are a security risk in an action-pinning tool.
+
+### Consequences
+
+#### Positive
+- Malformed or inconsistent action commit SHAs and container digests are caught immediately at update time, before being committed to the repository.
+- The validator produces aggregated, human-readable diagnostics covering all invalid entries at once, rather than stopping at the first error.
+- Validation logic is isolated in a dedicated file (`update_validation.go`) with injected resolver functions, making it straightforward to unit-test without real network calls.
+
+#### Negative
+- The terminal validation phase makes additional GitHub API calls (one per action entry, one per container entry) to re-resolve SHAs. For repositories with many pinned actions this adds latency to every `gh aw update` invocation.
+- Transient API failures during validation will cause the update command to return an error even when the lock file is structurally correct, potentially creating spurious failures in CI.
+
+#### Neutral
+- The validation step is gated on `firstErr == nil`, so it does not run when the main update phase already encountered an error. This is consistent with the existing error-propagation strategy but means validation is skipped on partial updates.
+- Verbose mode logs a confirmation message to stderr when validation succeeds, matching the existing log pattern used elsewhere in the update flow.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/cli/compile_orchestrator.go

@@ -46,6 +46,18 @@ func CompileWorkflows(ctx context.Context, config CompileConfig) ([]*workflow.Wo
 		return nil, err
 	}
 
+	// When --validate is set, run a pre-gate integrity check on actions-lock.json
+	// before compiling. This catches malformed or inconsistent pins early so the
+	// user sees the problem before any lock files are rewritten.
+	// Structural-only validation is used (no live API calls) to keep compile fast
+	// and avoid false positives from floating tags.
+	if config.Validate {
+		compileOrchestratorLog.Print("Running actions-lock.json SHA integrity pre-gate check")
+		if err := validateUpdateSHAEntriesStructural(ctx, "."); err != nil {
+			return nil, fmt.Errorf("actions-lock.json integrity check failed (pre-compile): %w", err)
+		}
+	}
+
 	// Initialize actionlint statistics if actionlint is enabled
 	if config.Actionlint && !config.NoEmit {
 		initActionlintStats()

pkg/cli/update_command.go

@@ -249,6 +249,16 @@ func RunUpdateWorkflows(ctx context.Context, opts UpdateWorkflowsOptions) error
 		}
 	}
 
+	if firstErr == nil {
+		updateLog.Print("Validating action and container SHAs in actions-lock.json")
+		if err := validateUpdateSHAEntries(ctx, "."); err != nil {
+			return fmt.Errorf("update validation failed: %w", err)
+		}
+		if opts.Verbose {
+			fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Validated action and container SHAs in actions-lock.json"))
+		}
+	}
+
 	updateLog.Printf("Update process complete: had_error=%v", firstErr != nil)
 	return firstErr
 }

pkg/cli/update_validation.go

@@ -0,0 +1,180 @@
+package cli
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"regexp"
+	"sort"
+	"strings"
+
+	"github.com/github/gh-aw/pkg/gitutil"
+	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/parser"
+	"github.com/github/gh-aw/pkg/workflow"
+)
+
+var updateValidationLog = logger.New("cli:update_validation")
+
+var sha256DigestPattern = regexp.MustCompile(`^sha256:[a-f0-9]{64}$`)
+
+// validationResolvers holds injectable resolver functions for testability.
+// Using a struct instead of package-level vars allows callers to supply their own
+// resolvers inline without touching globals, enabling t.Parallel() in tests.
+type validationResolvers struct {
+	// verifyActionCommitExists checks that a commit SHA actually exists in the
+	// action repository. It must return parser.ErrVerificationSkipped (wrapped)
+	// for auth/network failures so callers can treat those as non-fatal.
+	verifyActionCommitExists func(ctx context.Context, repo, sha string) error
+	// resolveActionVersionToSHA resolves a version tag/ref to its commit SHA.
+	// Used to verify that the stored SHA matches the pinned version.
+	resolveActionVersionToSHA func(ctx context.Context, repo, ref string) (string, error)
+}
+
+func defaultValidationResolvers() validationResolvers {
+	return validationResolvers{
+		verifyActionCommitExists: func(ctx context.Context, repo, sha string) error {
+			baseRepo := gitutil.ExtractBaseRepo(repo)
+			owner, name, ok := strings.Cut(baseRepo, "/")
+			if !ok || owner == "" || name == "" {
+				return fmt.Errorf("invalid action repository %q", repo)
+			}
+			return parser.VerifyCommitExists(ctx, owner, name, sha, "")
+		},
+		resolveActionVersionToSHA: func(ctx context.Context, repo, ref string) (string, error) {
+			baseRepo := gitutil.ExtractBaseRepo(repo)
+			owner, name, ok := strings.Cut(baseRepo, "/")
+			if !ok || owner == "" || name == "" {
+				return "", fmt.Errorf("invalid action repository %q", repo)
+			}
+			return parser.ResolveRefToSHAForHost(ctx, owner, name, ref, "")
+		},
+	}
+}
+
+// validateUpdateSHAEntries validates the structural and liveness integrity of
+// .github/aw/actions-lock.json after a workflow update. Container pins are
+// validated structurally only (format, key/image/pinned_image consistency);
+// live digest re-resolution is not performed because refreshing a mutable tag
+// is an explicit update operation.
+func validateUpdateSHAEntries(ctx context.Context, repoRoot string) error {
+	return validateUpdateSHAEntriesWithResolvers(ctx, repoRoot, defaultValidationResolvers())
+}
+
+// validateUpdateSHAEntriesStructural validates only the structural integrity of
+// .github/aw/actions-lock.json (format, non-empty fields, key consistency) without
+// performing any live network calls to resolve or verify SHAs. This is suitable for
+// a fast pre-gate check during compilation.
+func validateUpdateSHAEntriesStructural(ctx context.Context, repoRoot string) error {
+	return validateUpdateSHAEntriesWithResolvers(ctx, repoRoot, structuralOnlyResolvers())
+}
+
+// structuralOnlyResolvers returns resolvers that skip all live network checks.
+// Used for structural-only validation (format, key consistency) without API calls.
+func structuralOnlyResolvers() validationResolvers {
+	return validationResolvers{
+		verifyActionCommitExists: func(_ context.Context, _, _ string) error {
+			return nil
+		},
+		resolveActionVersionToSHA: func(_ context.Context, _, _ string) (string, error) {
+			// Return ErrVerificationSkipped so the caller treats this as non-fatal and
+			// skips the version→SHA round-trip check entirely. Structural-only mode
+			// validates format and key consistency without live API calls.
+			return "", fmt.Errorf("%w: structural-only mode", parser.ErrVerificationSkipped)
+		},
+	}
+}
+
+func validateUpdateSHAEntriesWithResolvers(ctx context.Context, repoRoot string, r validationResolvers) error {
+	actionsLockPath := filepath.Join(repoRoot, ".github", "aw", "actions-lock.json")
+	if _, err := os.Stat(actionsLockPath); err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return fmt.Errorf("failed to read actions-lock.json metadata: %w", err)
+	}
+
+	cache := workflow.NewActionCache(repoRoot)
+	if err := cache.Load(); err != nil {
+		return fmt.Errorf("failed to load actions-lock.json: %w", err)
+	}
+
+	var issues []string
+
+	entryKeys := make([]string, 0, len(cache.Entries))
+	for key := range cache.Entries {
+		entryKeys = append(entryKeys, key)
+	}
+	sort.Strings(entryKeys)
+	for _, key := range entryKeys {
+		entry := cache.Entries[key]
+		if entry.Repo == "" {
+			issues = append(issues, fmt.Sprintf("action entry %q has empty repo", key))
+		}
+		if entry.Version == "" {
+			issues = append(issues, fmt.Sprintf("action entry %q has empty version", key))
+		}
+		validSHA := false
+		if entry.SHA == "" {
+			issues = append(issues, fmt.Sprintf("action entry %q has empty SHA", key))
+		} else if !IsCommitSHA(entry.SHA) {
+			issues = append(issues, fmt.Sprintf("action entry %q has invalid SHA %q (expected 40-character commit SHA)", key, entry.SHA))
+		} else {
+			validSHA = true
+			// Verify the commit SHA actually exists in the repository via the
+			// GitHub commits API. Auth/network failures are non-fatal and logged;
+			// only a definitive not-found (e.g. HTTP 422/404) is an error.
+			if entry.Repo != "" {
+				if err := r.verifyActionCommitExists(ctx, entry.Repo, entry.SHA); err != nil {
+					if errors.Is(err, parser.ErrVerificationSkipped) {
+						updateValidationLog.Printf("action entry %q: skipping commit existence check (auth/network error): %v", key, err)
+					} else {
+						issues = append(issues, fmt.Sprintf("action entry %q: commit SHA %q not found in %q: %v", key, entry.SHA, entry.Repo, err))
+					}
+				}
+			}
+		}
+		if entry.Repo != "" && entry.Version != "" {
+			expectedKey := entry.Repo + "@" + entry.Version
+			if key != expectedKey {
+				issues = append(issues, fmt.Sprintf("action entry key/version mismatch: key %q should be %q", key, expectedKey))
+			}
+			if validSHA {
+				// Verify the stored version tag resolves to the stored SHA.
+				// Auth/network failures are non-fatal; only a confirmed mismatch is an error.
+				resolvedVersionSHA, err := r.resolveActionVersionToSHA(ctx, entry.Repo, entry.Version)
+				if err != nil {
+					updateValidationLog.Printf("action entry %q: skipping version/SHA check (resolution failed): %v", key, err)
+				} else if !strings.EqualFold(resolvedVersionSHA, entry.SHA) {
+					issues = append(issues, fmt.Sprintf("action entry %q SHA/version mismatch: version %q resolves to %q but stored SHA is %q", key, entry.Version, resolvedVersionSHA, entry.SHA))
+				}
+			}
+		}
+	}
+
+	containerKeys := make([]string, 0, len(cache.ContainerPins))
+	for image := range cache.ContainerPins {
+		containerKeys = append(containerKeys, image)
+	}
+	sort.Strings(containerKeys)
+	for _, image := range containerKeys {
+		pin := cache.ContainerPins[image]
+		if pin.Image != image {
+			issues = append(issues, fmt.Sprintf("container pin key/image mismatch: key %q has image %q", image, pin.Image))
+		}
+		if !sha256DigestPattern.MatchString(pin.Digest) {
+			issues = append(issues, fmt.Sprintf("container pin %q has invalid digest %q (expected sha256:<64 lowercase hex chars>)", image, pin.Digest))
+		}
+		expectedPinnedImage := image + "@" + pin.Digest
+		if pin.PinnedImage != expectedPinnedImage {
+			issues = append(issues, fmt.Sprintf("container pin %q has inconsistent pinned_image %q (expected %q)", image, pin.PinnedImage, expectedPinnedImage))
+		}
+	}
+
+	if len(issues) > 0 {
+		return fmt.Errorf("actions-lock.json validation failed:\n  - %s", strings.Join(issues, "\n  - "))
+	}
+	return nil
+}