Skip to content

Chore(ci): pin every GitHub Actions uses: to a full commit SHA - #505

Open
AmaadMartin wants to merge 2 commits into
mainfrom
fix/pin-github-actions-to-commit-shas
Open

Chore(ci): pin every GitHub Actions uses: to a full commit SHA#505
AmaadMartin wants to merge 2 commits into
mainfrom
fix/pin-github-actions-to-commit-shas

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):

No existing issue — description below.

  1. Or, if no issue exists, describe the change:

Problem: Every uses: reference in .github/workflows/ resolved a mutable Git tag (@v3, @v4, @v5, @v6, @v7). A tag is a movable pointer: whoever controls an upstream action repository — or anyone who compromises it — can force-move v6 to an arbitrary commit, and every adk-js workflow run after that moment executes the attacker's code with whatever token scopes the workflow holds.

Three of the twelve references sit in workflows holding write-scoped credentials, which is where the exposure stops being theoretical:

Workflow Token scopes Secrets in scope
csat.yml contents: read, issues: write, pull-requests: write
auto-assignment.yml contents: read, issues: write, pull-requests: write
release-please.yml contents: write, issues: write, pull-requests: write secrets.MY_RELEASE_PLEASE_TOKEN

release-please.yml is the worst case: it runs on every push to main with contents: write and a dedicated release token, so a moved v4 tag would be executing inside a job that can push commits, create tags, and cut releases.

That these tags really do move is directly demonstrable: the sibling repo adk-python pins actions/checkout # v6 at df4cb1c0…, but resolving the v6 tag today gives d23441a4… (release v6.1.0). The same @v6 reference points at two different commits depending on when you look.

Solution: Pin all twelve references to immutable 40-character commit SHAs, using the form GitHub documents for third-party action hardening and that adk-python already uses — <sha> # v<major>. This is repo-to-repo consistency, not a new policy.

The change is two commits so the version decision is reviewable separately from the mechanical pinning:

  1. chore(ci): converge csat and auto-assignment on the repo's current action majors — bumps the stale majors before any SHA is written, so pinning does not freeze the staleness permanently:
    • csat.yml: actions/checkout@v3@v6, actions/github-script@v6@v7
    • auto-assignment.yml: actions/checkout@v4@v6
  2. chore(ci): pin every GitHub Actions uses: to a full commit SHA — the twelve pins.

Resulting pins (every SHA re-resolved from the upstream repository at authoring time, not copied from adk-python, whose pins are older and internally inconsistent):

Action Tag Pinned commit Release
actions/checkout v6 d23441a48e516b6c34aea4fa41551a30e30af803 v6.1.0
actions/github-script v7 f28e40c7f34bde8b3046d885e986cb6290c5673b v7.1.0
actions/setup-node v6 249970729cb0ef3589644e2896645e5dc5ba9c38 v6.5.0
actions/setup-go v5 40f1582b2485089dde7abd97c1529aa768e1baff v5.6.0
actions/setup-python v5 a26af69be951a213d495a4c3e4e4022e16d87065 v5.6.0
googleapis/release-please-action v4 5c625bfb5d1ff62eadeeb3772007f7f66fdcf071 v4.4.1

Nine of the twelve pins are behavioural no-ops — each SHA is exactly what its tag resolves to right now. Only the three major bumps in commit 1 change what code runs.

googleapis/release-please-action@v4 is an annotated tag — the trap in this change. refs/tags/v4 points at a tag object whose own SHA is 8b8fd2cc23b2e18957157a9d923d75aa0c6f6ad5. That is not a commit, and GitHub Actions resolves uses: against commits, so writing it would have broken release-please.yml on the next push to main — in the one workflow holding contents: write and the release token. Confirmed empirically:

$ gh api repos/googleapis/release-please-action/commits/8b8fd2cc23b2e18957157a9d923d75aa0c6f6ad5
{"message":"No commit found for SHA: 8b8fd2cc...","status":"422"}

The dereferenced commit 5c625bfb… (refs/tags/v4^{}) is pinned instead, and is identical to the commit behind refs/tags/v4.4.1. The other five tags are lightweight and point directly at commits.

Why the three major bumps are safe.

  • actions/checkout v3→v6 (csat.yml), v4→v6 (auto-assignment.yml). Across v4/v5/v6 the changes are the Node runtime (16→20→24), a minimum runner requirement of v2.327.1 for v5+, and credentials persisting to a separate file in v6. No input contract changed, and neither bumped call site passes any with: inputs — both are bare uses: actions/checkout@vN. Runner compatibility is already proven in this repo: validation.yaml runs checkout@v6 on ubuntu/windows/macOS today, and both bumped workflows run on ubuntu-latest.
  • actions/github-script v6→v7 (csat.yml). v7.0.0 adds a Node 20 runtime, an optional base-url input, and exported JSDoc types. Nothing was removed. .github/scripts/csat.cjs uses only require() (still injected by v7), context.payload.issue, context.issue.number, context.repo and github.rest.issues.createComment — verified by reading the script, all unchanged in v7. Already proven in-repo: auto-assignment.yml runs github-script@v7 against the same github.rest.* shape.
  • Every other pin has no version movement at all.

Scope notes for the reviewer.

  • auto-assignment.yml is trivially separable. The task text spells out only csat.yml as stale, but auto-assignment.yml was the last remaining checkout@v4 holdout; leaving it would freeze a third distinct checkout major into the repo. If you would rather it moved separately, that is a one-line revert.
  • These are the repo's current majors, not the latest upstream majors — deliberately. actions/checkout has since published a v7, and actions/github-script a v8 and v9. This PR converges on the majors already in use elsewhere in this repo (checkout@v6, github-script@v7) because the goal here is one version per action plus immutability; adopting a brand-new major is a separate version-upgrade decision with its own compatibility review, and folding it into a pinning PR would hide it. Once Dependabot is configured (the queued follow-up) it will raise those bumps as reviewable PRs, which is the right venue.
  • Related observation from the CI run on this PR: github-script@v7 targets Node 20 and the runner now warns it is being forced to run on Node.js 24. This is pre-existing on mainauto-assignment.yml was already on @v7 — and is not introduced here; extending v7 to csat.yml is still a strict improvement over v6 (Node 16). It does add weight to doing the major-upgrade follow-up reasonably soon.
  • No Dependabot/Renovate config is added here. The repo has neither today, so nothing will auto-advance these pins — a real maintenance gap, deliberately out of scope for this PR and queued separately. This PR is the fix; keeping pins fresh is the follow-up.
  • No license headers were added to the workflow files. scripts/check_license.sh only scans *.js/*.ts (check_license.sh:7-8), none of the six files has one today, and adding them is out of scope.
  • Collision check (required before implementation): gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 returned no PR that pins actions to SHAs — every open PR touching .github/workflows/ was checked with gh pr diff <n> and none adds a line matching uses: .*@[0-9a-f]{40}. The nearest neighbour is Chore(ci): add job-level timeout-minutes to the remaining five workflows #403 (feat/workflow-job-timeout-minutes), which touches five of the same six files but adds only timeout-minutes: lines at job level and no uses: line, so it does not conflict. Branched from main rather than stacking.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[ ] I have added or updated unit tests for my change. — not applicable; this change adds zero lines of executable code (see below)
[ ] All unit tests pass locally. — no test file is affected by this change

No unit tests were added, deliberately. This change contains zero lines of executable application code — six YAML workflow files, twelve changed lines, nothing for vitest to import and nothing for @vitest/coverage-v8 to instrument. The coverage bar is satisfied vacuously because the denominator is zero. Please read the absence of tests as a scope decision, not an oversight:

  • A TypeScript test that parses .github/workflows/*.yml and asserts a pinning regex would test the YAML fixture, not the product, and would need updating on every legitimate pin bump.
  • A scripts/check_pinning.sh plus a CI job is a new enforcement mechanism, not this fix, and the sustainable answer to pin freshness is the separately queued Dependabot work.

Verification is therefore static analysis plus real workflow runs.

Static assertions (all run from the repo root, all passing):

  1. No unpinned reference remains — prints nothing:
    grep -rn "uses:" .github/workflows/ | grep -v '@[0-9a-f]\{40\} # v'
  2. Twelve references exist (guards against an accidental deletion): count = 12.
  3. Every line matches the invariant ^\s*(- )?uses: [\w.-]+/[\w.-]+@[0-9a-f]{40} # v\d+$12/12.
  4. Every SHA is a real commit in the repo it is attributed to — six gh api repos/<owner>/<repo>/commits/<sha> calls, each echoing back the same SHA.
  5. One SHA per action: checkout×5, github-script×2, setup-node×2, setup-go×1, setup-python×1, release-please-action×1 = 12, each action carrying an identical SHA at every occurrence.
  6. All six files still parse as YAML (yaml.safe_load), with job and step counts unchanged.
  7. Secret scanning clean: npx secretlint "**/*" (the exact command validation.yaml runs) exits 0 — a 40-hex SHA trips neither the recommended preset nor this repo's custom AIza… pattern.
  8. Diff hygiene: git diff main --stat = 6 files, 12 insertions, 12 deletions; and a -U0 diff filtered to non-uses: lines prints nothing, i.e. no with:, permissions:, step name or trigger was touched.
  9. .editorconfig compliance: no CRLF, no trailing whitespace, final newline intact in all six files. Indentation preserved exactly — note csat.yml and release-please.yml legitimately sit at a different indent from the other four because their steps are unnamed.
  10. actionlint was not run — not installed, and the plan explicitly rules out installing a new toolchain for it.

Proof the assertions can actually fail. Coverage/pass-rate is not evidence, so each assertion was run against a deliberately mutated tree. The two interesting results show the assertion set is non-redundant rather than three spellings of the same grep:

Mutation assert 1/3 (regex) Caught by
A: revert release-please to @v4 FAIL 11/12, reports the line regex
B: truncate a checkout SHA to 39 chars FAIL 11/12, reports the line regex
C: pin release-please to the tag object 8b8fd2cc… PASS — mutation invisible only assert 4: No commit found for SHA: 8b8fd2cc… (HTTP 422)
D: give one checkout occurrence a different real SHA PASS — mutation invisible only assert 5: reports duplicate actions/checkout

Mutations C and D are exactly the two mistakes this change could plausibly ship, and both sail past the regex checks — C is the annotated-tag trap and D is a copy-paste slip. The tree was restored clean (git status --porcelain = 0) after each.

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

Automatic on this PR — ran and passed, 9 of the 12 pins exercised for real. All CI checks are green on this PR:

Job Workflow Result
run-tests (ubuntu-latest) validation.yaml pass, 5m34s
run-tests (windows-latest) validation.yaml pass, 9m23s
run-tests (macos-latest) validation.yaml pass, 4m52s
run-tests cross-language-integration.yml pass, 1m17s
check-license license-check.yml pass, 6s
auto-assign auto-assignment.yml pass, 7s

auto-assignment.yml also fires on pull_request: types: [opened], so it ran here too — that is two more pins exercised than expected, including github-script.

The runner logs confirm the pins resolved as SHAs rather than tags, which is the property under test:

Download action repository 'actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803'      (SHA:d23441a4...)
Download action repository 'actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b' (SHA:f28e40c7...)
Download action repository 'actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38'    (SHA:24997072...)
Download action repository 'actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff'      (SHA:40f1582b...)
Download action repository 'actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065'  (SHA:a26af69b...)

That is five of the six distinct actions verified at runtime across Linux, Windows and macOS. The three unexercised lines are csat.yml's two (manual, below) and release-please.yml's one (inspection-only, below).

Manual, for the two on: issues workflows. Gotcha: GitHub only runs issues-triggered workflows from the repository's default branch, so opening an issue while this sits on a feature branch runs the old main version and proves nothing. To exercise them, first land the change on (or temporarily repoint) the default branch, then:

  1. Open a throwaway issue → auto-assignment.yml should run and the job should be green. Its addAssignees call may legitimately fail where the hard-coded assignee is not a collaborator; the script catches that and logs Failed to assign, so a green job is still the correct signal — the point is that checkout@<sha> and github-script@<sha> resolved and executed.
  2. Add a CSAT label (e.g. bug) to that issue and close it → csat.yml should run green and post the survey comment. This is the single most valuable manual check, because csat.yml is the only file where the code being run actually changes (checkout v3→v6, github-script v6→v7).
  3. Delete the throwaway issue and restore the default branch.

release-please.yml is inspection-verified only, by design. It is on: push: branches: [main] and consumes secrets.MY_RELEASE_PLEASE_TOKEN; firing a real release run is not an acceptable test. Verified instead by: (a) the file parses as YAML; (b) 5c625bfb… confirmed via gh api repos/googleapis/release-please-action/commits/… to be a commit in the correct repository; (c) confirmed to be the dereference of refs/tags/v4 and identical to refs/tags/v4.4.1, so the pin is behaviourally a no-op. Flagging this explicitly so it is clear one line is inspection-verified rather than run-verified.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas. (the # v<major> trailing comments)
[ ] I have added tests that prove my fix is effective or that my feature works. — replaced by the static assertions and the mutation table above
[x] New and existing unit tests pass locally with my changes. — verified in CI on this PR (validation.yaml runs npm run test:coverage on ubuntu/windows/macOS, all three green); not re-run locally, per the targeted-tests-only policy.

Amaad Martin added 2 commits August 2, 2026 00:27
…tion majors

csat.yml was stranded on actions/checkout@v3 and actions/github-script@v6,
and auto-assignment.yml on actions/checkout@v4, while every other workflow
already uses checkout@v6 and github-script@v7. Pinning those stale majors to
a commit SHA would freeze the staleness permanently, so bump them first as a
separate, independently reviewable step.

Neither bumped checkout call site passes any `with:` inputs, and no
github-script input contract changed in v7; the CSAT script uses only
require(), context.payload.issue, context.repo and
github.rest.issues.createComment, all of which v7 still provides.
Every `uses:` reference in .github/workflows/ resolved a mutable Git tag. A
tag is a movable pointer: whoever controls (or compromises) an upstream action
repository can force-move `v6` to arbitrary code, which then executes with
whatever token scopes the workflow holds. release-please.yml is the sharp
edge - it runs on every push to main with `contents: write` and a dedicated
release token.

Pin all twelve references to immutable 40-character commit SHAs, following the
convention GitHub documents and adk-python already uses: `<sha> # v<major>`.
Each SHA is what its tag resolves to right now, so the ten already-current
references are behavioural no-ops.

googleapis/release-please-action@v4 is an *annotated* tag: refs/tags/v4 points
at a tag object (8b8fd2cc...), which is not a commit and would not resolve as a
`uses:` ref. The dereferenced commit 5c625bfb... is pinned instead, confirmed
identical to refs/tags/v4.4.1.

Nothing outside a `uses:` line changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant