fix: ensure all actions/github-script steps resolve to a single SHA#26444
fix: ensure all actions/github-script steps resolve to a single SHA#26444
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/77596465-a2e1-46e0-b6e6-3921e2047999 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ctionPin Adds a new GetCachedActionPin helper that checks the user's actions-lock.json cache first (via WorkflowData.ActionResolver) before falling back to the embedded action_pins.json. Updates all compilation code paths that generate actions/github-script@<sha> steps to use this helper, ensuring they all produce the same SHA within a single compiled lock workflow. Previously, generateGitHubMCPLockdownDetectionStep used GetActionPinWithData (cache-aware) while all other steps used GetActionPin (embedded pins only). If the cache and embedded pins had different SHAs for the same v9 tag, the compiled workflow would contain two different SHAs for actions/github-script — causing security review noise and reproducibility concerns. Fixes: gh-aw compile pins actions/github-script@v9 to two different SHAs Agent-Logs-Url: https://github.com/github/gh-aw/sessions/77596465-a2e1-46e0-b6e6-3921e2047999 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot to minimize confusing, reduce the list of public function to access action pins so that only the cached version is used outside of the action pin package. |
There was a problem hiding this comment.
Pull request overview
This PR updates the workflow compiler to consistently pin actions/github-script@v9 using a cache-first lookup to avoid “split SHA” compilation output when embedded pins and actions-lock.json diverge.
Changes:
- Added
GetCachedActionPin(repo, data)that prefers cached action pins viaWorkflowData.ActionResolver, falling back to embedded pins. - Threaded
data *WorkflowDatathrough multiple YAML-generation helpers and updated call sites to useGetCachedActionPin. - Recompiled locked workflow YAMLs to ensure a single consistent SHA is used for
actions/github-script.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/action_pins.go | Adds cache-first action pin helper used to unify action SHAs across code paths. |
| pkg/workflow/compiler_github_actions_steps.go | Updates helper function signatures to accept data and use cached pin resolution. |
| pkg/workflow/compiler_yaml_ai_execution.go | Threads data into log parsing/token summary generators and uses cached pin resolution. |
| pkg/workflow/compiler_yaml.go | Uses the updated placeholder substitution helper and cached pin resolution in YAML generation. |
| pkg/workflow/compiler_yaml_main_job.go | Updates helper invocations to pass data so pinned action refs are consistent. |
| pkg/workflow/* (various .go) | Replaces GetActionPin("actions/github-script") with GetCachedActionPin(..., data) in several step generators. |
| pkg/workflow/*_test.go | Updates tests to pass data or nil to updated helper signatures. |
| .github/workflows/*.lock.yml | Regenerates lock workflows so actions/github-script is pinned consistently to a single SHA. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/workflow/compiler_github_actions_steps.go:61
generatePlaceholderSubstitutionStepnow acceptsdata *WorkflowData, but the doc comment doesn't mention this new argument. Please update the comment to describe whydatais needed (cached action pin resolution) and clarify expected behavior whendatais nil.
// generatePlaceholderSubstitutionStep generates a JavaScript-based step that performs
// safe placeholder substitution using the substitute_placeholders script.
// This replaces the multiple sed commands with a single JavaScript step.
func generatePlaceholderSubstitutionStep(yaml *strings.Builder, expressionMappings []*ExpressionMapping, indent string, data *WorkflowData) {
if len(expressionMappings) == 0 {
- Files reviewed: 44/44 changed files
- Comments generated: 1
| // - condition: Optional if condition (e.g., "always()"). Empty string means no condition. | ||
| // | ||
| // Returns a string containing the complete YAML for the github-script step. | ||
| func generateInlineGitHubScriptStep(stepName, script, condition string) string { | ||
| func generateInlineGitHubScriptStep(stepName, script, condition string, data *WorkflowData) string { | ||
| compilerGitHubActionsStepsLog.Printf("Generating inline GitHub script step: name=%q, condition=%q", stepName, condition) |
There was a problem hiding this comment.
The function signature now includes a data *WorkflowData parameter, but the doc comment still lists only stepName, script, and condition. Please update the comment to document the new parameter and how it affects action pinning (via GetCachedActionPin).
This issue also appears on line 57 of the same file.
…n as public API Per code review: reduce public API surface for action pin access so that only GetCachedActionPin is visible to callers outside pkg/workflow. Renames GetActionPin → getActionPin (unexported), keeping GetCachedActionPin, GetActionPinWithData, and GetActionPinByRepo as the public interface. All internal callers (maintenance_workflow.go, compiler_activation_job.go, checkout_step_generator.go, cache.go, and all other compilation files) now use getActionPin directly since they are in the same package, which is equivalent behavior. Test files in package workflow can still call getActionPin directly. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/94dcd50a-bf45-41be-8144-06206dc70d3d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 81d98c4. |
|
@copilot move action pins manager and code into own Go package |
Done — commit |
|
@copilot keep working |
|
@copilot merge main and recompile |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/75a504f3-da87-4d13-98f0-59f0bcc357f9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done — commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/53d6568a-981c-4989-8a94-521e90648d00 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done — commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot merge main and recompile |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/406ffa57-83df-472b-b3ff-3096ed9f847d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done — commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot review all uses of GetActionPin and GetCacheActionPin to ensure only the publicly exported functions are used outside the action-pin package. |
…nce workflows Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4ee89652-08af-43b7-a468-655498473b08 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot make sure function capitalization is consistent with public usage. Review usage of get action functions outside the package |
…usage scope) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/393fdc45-72fe-4108-82b0-0124de2109ee Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot merge main and recompile |
…pile-sha-pinning # Conflicts: # pkg/workflow/compiler_activation_job.go # pkg/workflow/maintenance_workflow_yaml.go # pkg/workflow/side_repo_maintenance.go # pkg/workflow/workflow_builder.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
When
gh aw compilegenerates a lock workflow,actions/github-script@v9could resolve to two different SHAs within the same file — one from the user'sactions-lock.jsoncache (used by the MCP lockdown detection step viagetActionPinWithData) and a different one from the embeddedaction_pins.json(used by every other step viagetActionPin). This inconsistency surfaced whenever the embedded pins and the cache diverged across a version bump.Changes
New
pkg/actionpinspackage: all action pin types (ActionPin,ActionPinsData,ActionYAMLInput), loading/caching, and resolution logic have been moved to a dedicatedpkg/actionpinspackage. ASHAResolverinterface andPinContextstruct replace the direct*WorkflowDatadependency, keeping the new package free of anypkg/workflowimports.pkg/workflow/action_pins.gois now a thin adapter: provides package-private wrappers used throughoutpkg/workflowand retainsapplyActionPinToTypedStep/applyActionPinsToTypedSteps(which depend onWorkflowStep).WorkflowData.PinContext()bridgesWorkflowDatatoactionpins.PinContext.All action pin functions in
pkg/workfloware package-private:getActionPin,getCachedActionPin,getActionPinWithData,getActionPinByRepo,applyActionPinToTypedStep, andapplyActionPinsToTypedStepsare all unexported since they are only called withinpkg/workflow. Thepkg/actionpinspackage retains an exported public API for use by any package.getCachedActionPinis the sole resolution API for latest-version pins:getActionPinis only used for non-github-scriptactions (checkout, upload-artifact, etc.) that don't require cache-first resolution. There are zero remaining uses ofgetActionPin("actions/github-script")anywhere in the codebase.Replaced all
getActionPin("actions/github-script")call sites in compilation code with cache-aware resolution across all files — includingmaintenance_workflow_yaml.goandside_repo_maintenance.gowhich now use agetCachedActionPinFromResolver(repo, resolver)helper that builds aPinContextfrom theActionSHAResolveralready available in those functions.Propagated
data *WorkflowDatainto helper functions that lacked it (generateInlineGitHubScriptStep,generatePlaceholderSubstitutionStep,generateLogParsing,generateMCPScriptsLogParsing,generateMCPGatewayLogParsing,generateTokenUsageSummary,generateResolveHostRepoStep) so each can callgetCachedActionPin.Makefile updated:
sync-action-pinsnow writesactions-lock.jsontopkg/actionpins/data/action_pins.json(the embedded data location for the new package).