fix(ci): replace the disabled ::set-output command with $GITHUB_OUTPUT - #3094
fix(ci): replace the disabled ::set-output command with $GITHUB_OUTPUT#3094boleklebovski wants to merge 1 commit into
::set-output command with $GITHUB_OUTPUT#3094Conversation
GitHub disabled ::set-output in runner 2.298.2. Both outputs are consumed: the bundle-analysis comment body, and the PR number that becomes issue_number for the CI-run comment, which resolves to 0 when empty. Also drops the percent-encoding that ::set-output required. Signed-off-by: boleklebovski <160799963+boleklebovski@users.noreply.github.com>
Mjolnir Security Review
14 PRs reviewed · 1 finding |
|
@boleklebovski is attempting to deploy a commit to the Aave Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Mjolnir Security Review
REVIEW · 0 findings · 1 observation · ed18686 · 57s
This PR improves the security posture by migrating two GitHub Actions workflow files from the deprecated ::set-output command (which was vulnerable to log-injection attacks via stdout) to the recommended $GITHUB_OUTPUT environment file approach. No new security vulnerabilities are introduced. The PR number extraction in test-deploy-fork.yml correctly uses an allowlist-based filter (tr -dc '[:digit:]'), and the heredoc delimiter in the bundle analysis comment action is appropriately specific for its context.
1 observation — see Odin for details
- INFO PR improves security by migrating from deprecated ::set-output to $GITHUB_OUTPUT —
.github/actions/analyze-comment/action.yml
Mjolnir by Borg · View all suggestions →
There was a problem hiding this comment.
Automated review
Commit ed186862. This review replaces any earlier review on this pull request.
Verdict: minor issues
1 finding(s) are commented on the lines below.
Full detail, other severities, and the accept/reject controls are on the scan page.
Findings you reject there stop being reported for this repository.
| body="${body//$'\n'/'%0A'}" | ||
| body="${body//$'\r'/'%0D'}" | ||
| echo ::set-output name=body::$body | ||
| { |
There was a problem hiding this comment.
Fixed heredoc delimiter allows output truncation or injection
medium · security
The step writes the bundle analysis body with the fixed delimiter BUNDLE_ANALYSIS_EOF. The GitHub output parser closes the value at the first line that equals the delimiter. Two failure modes exist. First, if .next/analyze/__bundle_analysis_comment.txt contains a line equal to BUNDLE_ANALYSIS_EOF, the parser truncates the body. Later file lines then become new output records for this step. A pull request that controls the analysis content can therefore replace the bot comment body. Second, if the file does not end with a newline, the last content line and the delimiter join into one line. The parser never finds the delimiter, and the output is empty or corrupt. Both cases break the output that this change repairs.
Why this severity: The analysis file is machine generated, so the trigger is not common. But a pull request can influence the content, and the result is a broken or forged step output.
How it is reached: An attacker opens a pull request. The attacker makes the bundle analysis text include a line with the value BUNDLE_ANALYSIS_EOF. The following lines become step output records and change the posted comment.
| { | |
| DELIMITER="BUNDLE_ANALYSIS_EOF_$(openssl rand -hex 16)" | |
| { | |
| echo "body<<${DELIMITER}" | |
| cat .next/analyze/__bundle_analysis_comment.txt | |
| echo | |
| echo "${DELIMITER}" | |
| } >> "$GITHUB_OUTPUT" |
Review harness · reply here to accept or reject this finding
Problem
Two workflow steps still use the
::set-outputworkflow command, which GitHub disabled in Actions runner 2.298.2 (October 2022). Steps that use it produce no output at all, and every consumer silently receives an empty string.Both outputs here are consumed:
1.
.github/actions/analyze-comment/action.yml:80—steps.get-comment-body.outputs.bodyfeeds thebody:input of bothCreate Comment(line 95) andUpdate Comment(line 102), so the bundle-analysis comment is posted with an empty body. This composite action is used bybuild-test-deploy.yml,build-test-deploy-dev.ymlandtest-deploy-fork.yml.2.
.github/workflows/test-deploy-fork.yml:43—steps.get_pr_number.outputs.pr_numberis passed asPR_NUMBERintoLink this CI run to PR, where it becomesissue_number: Number(process.env.PR_NUMBER). With an empty string that evaluates to0, so the "Tests and deployment are running now!" comment is addressed to issue0instead of the pull request.Change
Both sites move to the supported
$GITHUB_OUTPUTfile.For the multiline bundle-analysis text I used the documented heredoc delimiter form, which also lets the three percent-encoding lines go away — that encoding was a workaround for
::set-outputnot accepting newlines, and it is no longer needed:Dropping the encoding is a small bonus fix: a
%in the analysis output used to reach the comment as%25.That step declares
shell: sh, and the redirect form used here is POSIX, so it stays portable.Verification
$GITHUB_OUTPUT:100%intact;shform turns aNUMfile containing12345abcintopr_number=12345.::set-outputoccurrences — no action versions, permissions or triggers were touched.