ci: harden metadata release and triage workflow permissions - #458
Merged
Conversation
finalize_metadata_release.yml interpolated
${{ github.event.pull_request.head.ref }} and
${{ github.event.pull_request.merge_commit_sha }} directly into the run
block's bash command line. The expression engine substitutes those before
the shell sees the script, so a branch name containing shell
metacharacters - backticks, $(...), a semicolon - would be executed as
code by the runner, with the job's contents: write and actions: write
token in the environment. The existing `pull_request: closed` trigger and
the user.login == 'libphonenumber-csharp-bot' / metadata-update/* branch
gate make that hard to reach, but they do not remove the injection, so
both values are now passed to the step via env: and referenced as quoted
"$HEAD_REF" / "$MERGE_SHA". The script still gets the same two positional
arguments in the same order.
The job keeps contents: write and actions: write, now with a comment
naming what needs each: createRelease POSTs /repos/.../releases, which
creates the vX.Y.Z tag as well as the release entry (contents), and
dispatchPublish POSTs the publish_nuget.yml workflow_dispatch (actions).
github suppresses push events raised by GITHUB_TOKEN, so the tag alone
cannot start the publish run and the dispatch cannot be dropped.
triage_metadata_issues.yml granted contents/issues/pull-requests/
copilot-requests write at the top level, so every job in the file - and
any job added later - inherited them. The top level is now contents: read
and the triage job requests those four explicitly, each with a comment
naming the step that needs it.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #458 +/- ##
=======================================
Coverage 87.51% 87.51%
=======================================
Files 43 43
Lines 3886 3886
Branches 991 991
=======================================
Hits 3401 3401
Misses 280 280
Partials 205 205 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Sep 10, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1. Script injection in
finalize_metadata_release.yml(Scorecard: Dangerous-Workflow, critical)The release step interpolated event data straight into a
run:body:head.refis attacker-supplied text, and the expression engine substitutes it before bash parses the line — so a branch name containing shell metacharacters executes as code, in a job holdingcontents: writeandactions: write.Both values now reach the step through
env:and are referenced as quoted shell variables. The script receives the same two positional arguments in the same order.The existing
if:gates (merged PR, basemain,metadata-update/*prefix, authorlibphonenumber-csharp-bot) made this hard to reach but did not remove it. They are unchanged.2. Over-broad token permissions in
triage_metadata_issues.yml(Scorecard: Token-Permissions, high)The top-level block granted
contents/issues/pull-requests/copilot-requests: writeto every job in the file. Top level is nowcontents: read, with the four writes moved onto the singletriagejob that needs them, each commented with the step that requires it. No future job added to this file silently inherits write access.Deliberately not changed: the two
finalize_metadata_release.ymljob permissionsScorecard also flags
contents: write(line 32) andactions: write(line 33) on that job. Both are the minimum for the flow, verified againstlib/finalize-metadata-release.sh,lib/github-release-helpers.shandpublish_nuget.yml:contents: write—createReleasePOSTs/repos/.../releaseswithtarget_commitish, which creates thevX.Y.Ztag as well as the release. There is no finer-grained release scope, andcontents: readcannot create a ref.actions: write—dispatchPublishPOSTs to/actions/workflows/publish_nuget.yml/dispatches, which requires it. The dispatch cannot be dropped: GitHub suppressespushevents raised byGITHUB_TOKEN, so the tag created above will not firepublish_nuget.yml'spush: tags: ['v*']trigger on its own — which is what the# github suppresses push events from GITHUB_TOKENcomment in the helper records.Both permissions now carry a comment naming the exact API call that needs them. Suggest dismissing those two alerts as used-as-designed rather than weakening the release path.
Verification
run:body in both files: the only remaining${{ }}is${{ runner.temp }}, which is runner-provided, not event-controlled. (actions/github-scriptscript:blocks still use${{ toJSON(...) }}, the correct JS-context-safe pattern — not a shell body.)contents: read, jobcontents: write, actions: write; triage: topcontents: read, jobcontents/issues/pull-requests/copilot-requests: write.