Add action SHA validation to compile --validate command#3631
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
compile --validate command
…e-validate-command
There was a problem hiding this comment.
Pull Request Overview
This PR adds action SHA validation to the compile --validate command. The feature checks if pinned GitHub Actions in compiled .lock.yml files are using the latest available SHAs for their version tags and emits non-blocking warnings when updates are available.
Key changes:
- Implements SHA validation logic to extract, check, and report outdated action SHAs
- Integrates validation into the compile command when
--validateflag is used - Provides detailed warnings with short SHAs (7 chars) and full SHAs in verbose mode
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/action_sha_checker.go |
Core implementation for extracting actions, checking SHA updates, and validating lock files |
pkg/workflow/action_sha_checker_test.go |
Unit tests for action extraction, deduplication, and SHA update checking |
pkg/workflow/action_sha_checker_integration_test.go |
Integration tests for validation flow including up-to-date and outdated scenarios |
pkg/cli/compile_command.go |
Integration of SHA validation into compile functions, enabled by validateActionSHAs parameter |
pkg/cli/add_command.go |
Updated function calls to pass false for the new validateActionSHAs parameter |
cmd/gh-aw/main.go |
Updated help text to document action SHA validation feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Parse YAML to extract actions from "uses" fields | ||
| var workflowData map[string]any | ||
| if err := yaml.Unmarshal(content, &workflowData); err != nil { | ||
| return nil, fmt.Errorf("failed to parse lock file YAML: %w", err) | ||
| } |
There was a problem hiding this comment.
The YAML parsing on lines 39-43 appears to be unnecessary. The code parses the YAML into a workflowData variable but never uses it. Instead, the code converts the content to a string on line 52 and uses regex matching. The YAML parsing serves only as a validation step to ensure the file is valid YAML, but this is redundant since the file was already validated in the compile command. Consider removing lines 39-43 and the yaml import, or add a comment explaining why the YAML validation is needed here.
|
|
||
| // Skip if we don't have a version to check against | ||
| if action.Version == "" { | ||
| actionSHACheckerLog.Printf("Skipping %s: no version tag available", action.Repo) |
There was a problem hiding this comment.
When an action has no version tag (line 104-107), the function skips it by using continue, but it never appends any result to the results slice. This means the returned slice will have fewer elements than the input actions slice, which could be confusing to callers. Consider either: (1) appending a check result with NeedsUpdate: false and an informative message, or (2) documenting that the returned slice may be shorter than the input.
| actionSHACheckerLog.Printf("Skipping %s: no version tag available", action.Repo) | |
| actionSHACheckerLog.Printf("Skipping %s: no version tag available", action.Repo) | |
| check.Message = "No version tag available; cannot check for updates" | |
| results = append(results, check) |
| if err := workflow.ValidateActionSHAsInLockFile(lockFile, actionCache, verbose); err != nil { | ||
| // Action SHA validation warnings are non-fatal | ||
| compileLog.Printf("Action SHA validation completed with warnings: %v", err) | ||
| } |
There was a problem hiding this comment.
The comment on line 61 says 'Action SHA validation warnings are non-fatal,' but looking at ValidateActionSHAsInLockFile in action_sha_checker.go (line 193), the function always returns nil and never returns an error. This means the error handling code on lines 60-63 can never execute. Either remove the error handling since it's unreachable, or update ValidateActionSHAsInLockFile to return an error in appropriate cases.
| if err := workflow.ValidateActionSHAsInLockFile(lockFile, actionCache, verbose); err != nil { | |
| // Action SHA validation warnings are non-fatal | |
| compileLog.Printf("Action SHA validation completed with warnings: %v", err) | |
| } | |
| workflow.ValidateActionSHAsInLockFile(lockFile, actionCache, verbose) |
Plan: Add Action SHA Validation to
compile --validateCommandObjective ✅ COMPLETE
Update the
compile --validatecommand to validate all pinned action SHAs and emit warnings if SHAs need to be upgraded.Implementation Status
Recent Changes
✅ Merged main branch - Successfully merged origin/main into the feature branch to incorporate latest changes. All tests pass after merge.
Summary
Successfully implemented action SHA validation for the
compile --validatecommand. The feature:✅ Extracts all pinned actions from compiled
.lock.ymlfiles using regex matching✅ Validates SHA currency by comparing current SHAs with latest available SHAs for the same version tag
✅ Emits warnings when outdated actions are detected (non-blocking)
✅ Uses GitHub CLI (via existing ActionResolver) for SHA resolution
✅ Leverages action cache to minimize API calls
✅ Provides clear output with short SHAs and full SHAs in verbose mode
✅ Includes comprehensive tests - unit tests, integration tests, and edge case coverage
✅ Up to date with main - Latest changes from main branch merged successfully
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.