Skip to content

Preserve SHA-pinned actions in ChangeAction/ChangeActionVersion via opt-in oldSha#199

Open
MBoegers wants to merge 1 commit into
mainfrom
MBoegers/early-return-recipe-audit
Open

Preserve SHA-pinned actions in ChangeAction/ChangeActionVersion via opt-in oldSha#199
MBoegers wants to merge 1 commit into
mainfrom
MBoegers/early-return-recipe-audit

Conversation

@MBoegers

@MBoegers MBoegers commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

What

ChangeActionVersion and ChangeAction rewrite a uses: reference. Today they also strip a deliberate commit-SHA pin (e.g. actions/checkout@8f4b7f84…actions/checkout@v4), silently downgrading a workflow's supply-chain posture and potentially leaving a stale # vX comment.

This adds an opt-in, per-uses: oldSha option mirroring the Docker ChangeFrom oldDigest sentinel:

oldSha behavior
unset (default) change regardless of ref — unchanged behavior
"" change only refs that are not a 40-char SHA → preserves deliberate pins
a commit SHA change only refs pinned to exactly that SHA

The filter is per-entry, so a workflow mixing SHA-pinned and tag-pinned steps upgrades only the tag-pinned ones.

How

  • New UsesRefs — parses the ref from a uses: value and implements the oldSha sentinel (matchesOldSha).
  • New ChangeUsesVisitor — shared match → filter → rewrite visitor; the two recipes differ only by their value-transform lambda (ChangeActionVersion preserves each entry's action path, ChangeAction substitutes the new action).
  • Selection uses a live JsonPathMatcher (the same mechanism the prior ChangeValue delegation used), so matching — including the actions/nested.* wildcard and job-level uses: — is unchanged.
  • recipes.csv updated for the new option.

Tests

Both recipes get the full matrix: skip SHA pin, skip SHA pin with # vX comment preserved, still-upgrade tag, per-entry mixed file, specific-SHA match, and a null-default regression guard. Existing tests unchanged.

ChangeActionVersion and ChangeAction stripped deliberate commit-SHA pins when rewriting a `uses:` reference. Add an opt-in `oldSha` sentinel mirroring Docker ChangeFrom's `oldDigest`:

- unset -> change regardless of ref (default, unchanged behavior)
- ""    -> change only non-SHA refs, preserving SHA pins (per-entry)
- <sha> -> change only refs pinned to exactly that SHA

The per-uses match/filter/rewrite logic is shared via a new ChangeUsesVisitor; SHA parsing and the sentinel live in UsesRefs. recipes.csv updated for the new option (two rows only).

Refs: moderneinc/customer-requests#2594, openrewrite/rewrite#8110
Comment on lines 26 to +51
@Option(displayName = "Action",
description = "Name of the action to match.",
example = "gradle/wrapper-validation-action")
String oldAction;

@Option(displayName = "Action",
description = "Name of the action to use instead.",
example = "gradle/actions/wrapper-validation")
String newAction;

@Option(displayName = "Version",
description = "New version to use.",
example = "v3")
String newVersion;

@Option(displayName = "Old commit SHA",
description = "Restricts the change by the existing `uses:` ref. When omitted, the " +
"action is changed regardless of how it is pinned (the default; commit SHA pins " +
"are rewritten). When set to an empty string, only references that are **not** " +
"pinned to a 40-character commit SHA are changed, leaving deliberate SHA pins on " +
"the original action untouched. When set to a specific commit SHA, only " +
"references pinned to exactly that SHA are changed.",
required = false,
example = "8f4b7f84864484a7bf31766abe9204da3cbe65b3")
@Nullable
String oldSha;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry a bit about the order of the fields here becoming confusing, especially since they are all Stirngs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted the new option to be last, this allows the recipe deal with it.
Agree the order could be better but if we rearrange, thanks to all stings, strange things can happen if you just append null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants