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/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,156 @@
+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())
+}
+
+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
+}

pkg/cli/update_validation_test.go

@@ -0,0 +1,161 @@
+//go:build !integration
+
+package cli
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/github/gh-aw/pkg/parser"
+	"github.com/github/gh-aw/pkg/testutil"
+	"github.com/github/gh-aw/pkg/workflow"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// noopResolvers returns resolvers that perform no network calls.
+// Tests that want specific resolver behaviour should override individual fields.
+func noopResolvers() validationResolvers {
+	return validationResolvers{
+		verifyActionCommitExists: func(_ context.Context, _, _ string) error {
+			return nil
+		},
+		resolveActionVersionToSHA: func(_ context.Context, _, ref string) (string, error) {
+			return ref, nil
+		},
+	}
+}
+
+func TestValidateUpdateSHAEntries_NoActionsLock(t *testing.T) {
+	t.Parallel()
+	tmpDir := testutil.TempDir(t, "validate-update-sha-*")
+	require.NoError(t, validateUpdateSHAEntriesWithResolvers(context.Background(), tmpDir, noopResolvers()))
+}
+
+func TestValidateUpdateSHAEntries_ValidEntries(t *testing.T) {
+	t.Parallel()
+	tmpDir := testutil.TempDir(t, "validate-update-sha-*")
+	cache := workflow.NewActionCache(tmpDir)
+	cache.Set("actions/checkout", "v5", "93cb6efe18208431cddfb8368fd83d5badbf9bfd")
+	digest := "sha256:" + strings.Repeat("a", 64)
+	image := "ghcr.io/github/gh-aw-firewall/agent:0.27.9"
+	cache.SetContainerPin(image, digest, image+"@"+digest)
+	require.NoError(t, cache.Save())
+
+	r := validationResolvers{
+		verifyActionCommitExists: func(_ context.Context, _, _ string) error {
+			return nil
+		},
+		resolveActionVersionToSHA: func(_ context.Context, _, ref string) (string, error) {
+			if ref == "v5" {
+				return "93cb6efe18208431cddfb8368fd83d5badbf9bfd", nil
+			}
+			return ref, nil
+		},
+	}
+
+	require.NoError(t, validateUpdateSHAEntriesWithResolvers(context.Background(), tmpDir, r))
+}
+
+func TestValidateUpdateSHAEntries_InvalidEntries(t *testing.T) {
+	t.Parallel()
+	tmpDir := testutil.TempDir(t, "validate-update-sha-*")
+	awDir := filepath.Join(tmpDir, ".github", "aw")
+	require.NoError(t, os.MkdirAll(awDir, 0o755))
+
+	const invalidActionsLock = `{
+  "entries": {
+    "actions/checkout@v5": {
+      "repo": "actions/checkout",
+      "version": "v5",
+      "sha": "short"
+    },
+    "actions/setup-node@v6": {
+      "repo": "actions/setup-node",
+      "version": "v7",
+      "sha": "395ad3262231945c25e8478fd5baf05154b1d79f"
+    }
+  },
+  "containers": {
+    "ghcr.io/test/image:v1": {
+      "image": "ghcr.io/test/other:v1",
+      "digest": "sha256:XYZ",
+      "pinned_image": "ghcr.io/test/image:v1@sha256:bad"
+    }
+  }
+}
+`
+	require.NoError(t, os.WriteFile(filepath.Join(awDir, "actions-lock.json"), []byte(invalidActionsLock), 0o644))
+
+	r := validationResolvers{
+		// Commit existence check: setup-node SHA is not found; checkout SHA passes.
+		verifyActionCommitExists: func(_ context.Context, repo, sha string) error {
+			if repo == "actions/setup-node" && sha == "395ad3262231945c25e8478fd5baf05154b1d79f" {
+				return errors.New("commit not found")
+			}
+			return nil
+		},
+		// Version resolution: setup-node v7 resolves to a different SHA (mismatch).
+		resolveActionVersionToSHA: func(_ context.Context, repo, ref string) (string, error) {
+			if repo == "actions/setup-node" && ref == "v7" {
+				return "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", nil
+			}
+			return ref, nil
+		},
+	}
+
+	err := validateUpdateSHAEntriesWithResolvers(context.Background(), tmpDir, r)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), `action entry "actions/checkout@v5" has invalid SHA`)
+	assert.Contains(t, err.Error(), `action entry key/version mismatch: key "actions/setup-node@v6" should be "actions/setup-node@v7"`)
+	assert.Contains(t, err.Error(), `action entry "actions/setup-node@v6": commit SHA`)
+	assert.Contains(t, err.Error(), `action entry "actions/setup-node@v6" SHA/version mismatch`)
+	assert.Contains(t, err.Error(), `container pin key/image mismatch`)
+	assert.Contains(t, err.Error(), `container pin "ghcr.io/test/image:v1" has invalid digest`)
+	assert.Contains(t, err.Error(), `container pin "ghcr.io/test/image:v1" has inconsistent pinned_image`)
+}
+
+func TestValidateUpdateSHAEntries_NonFatalErrors(t *testing.T) {
+	t.Parallel()
+	tmpDir := testutil.TempDir(t, "validate-update-sha-*")
+	cache := workflow.NewActionCache(tmpDir)
+	cache.Set("actions/checkout", "v5", "93cb6efe18208431cddfb8368fd83d5badbf9bfd")
+	digest := "sha256:" + strings.Repeat("a", 64)
+	image := "ghcr.io/github/gh-aw-firewall/agent:0.27.9"
+	cache.SetContainerPin(image, digest, image+"@"+digest)
+	require.NoError(t, cache.Save())
+
+	r := validationResolvers{
+		// Auth error on commit existence check — should be skipped (non-fatal).
+		verifyActionCommitExists: func(_ context.Context, _, _ string) error {
+			return fmt.Errorf("%w: auth error", parser.ErrVerificationSkipped)
+		},
+		// Network error on version resolution — should be skipped (non-fatal).
+		resolveActionVersionToSHA: func(_ context.Context, _, _ string) (string, error) {
+			return "", errors.New("network timeout")
+		},
+	}
+
+	// All network failures should be non-fatal; validation should still pass.
+	require.NoError(t, validateUpdateSHAEntriesWithResolvers(context.Background(), tmpDir, r))
+}
+
+func TestValidateUpdateSHAEntries_ContainerStructuralOnly(t *testing.T) {
+	t.Parallel()
+	tmpDir := testutil.TempDir(t, "validate-update-sha-*")
+	digest := "sha256:" + strings.Repeat("a", 64)
+	image := "ghcr.io/github/gh-aw-firewall/agent:latest"
+	cache := workflow.NewActionCache(tmpDir)
+	cache.SetContainerPin(image, digest, image+"@"+digest)
+	require.NoError(t, cache.Save())
+
+	// No container resolver in the struct — container pins are structural-only.
+	// Even if the tag has moved to a different digest, the stored pin is still valid
+	// and validation passes without performing any live lookup.
+	require.NoError(t, validateUpdateSHAEntriesWithResolvers(context.Background(), tmpDir, noopResolvers()))
+}