Skip to content

Harden Build workflow: drop eval, use $GITHUB_OUTPUT (PSIRT follow-up)#60

Merged
yogsototh merged 1 commit into
masterfrom
harden-actions-followup-eval-setoutput
Jun 16, 2026
Merged

Harden Build workflow: drop eval, use $GITHUB_OUTPUT (PSIRT follow-up)#60
yogsototh merged 1 commit into
masterfrom
harden-actions-followup-eval-setoutput

Conversation

@yogsototh

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #59. Addresses the two remaining findings from the PSIRT review of .github/workflows/build.yml that #59 left out to keep that PR's diff scoped to action-pinning and token permissions.

  • Replace deprecated ::set-output with $GITHUB_OUTPUT (setup job, set-matrix step). GitHub deprecated ::set-output citing output-injection concerns; the $GITHUB_OUTPUT environment file is the supported mechanism. Also fixes a stray trailing } in the emitted JSON (...["test"]}} -> ...["test"]}).
  • Remove eval from the test job's run step. eval 'lein do clean, compile :all, ${CMD}' re-parsed a matrix-derived variable through the shell -- an eval/command-injection anti-pattern. Passing it as a normal quoted argument (lein do clean, compile :all, "$CMD") removes the eval.

No behavior change

matrix.cmd is hardcoded to test, so both jobs run exactly as before:

  • setup still emits the same matrix, now via $GITHUB_OUTPUT; consumers read needs.setup.outputs.matrix unchanged.
  • test still runs lein do clean, compile :all, test.

Findings addressed

Deliberately not in this PR

  • Bumping action major versions (setup-java v2->v4, cache v3->v4): a runtime-behavior change that deserves its own CI run; better handled by Dependabot. The actions are already SHA-pinned (Harden GitHub Actions workflow (PSIRT-0328599349) #59), so the tag-mutation risk is already closed.
  • pull_request running PR code (bin/lint, project.clj): inherent to build-on-PR, not a workflow defect. Mitigated by the read-only token added in Harden GitHub Actions workflow (PSIRT-0328599349) #59; further hardening (require-approval-for-first-time-contributors) is a repo setting, not a workflow change.

🤖 Generated with Claude Code

Follow-up to #59, addressing the remaining findings from the PSIRT
review of build.yml.

- Replace the deprecated `::set-output` command with the `$GITHUB_OUTPUT`
  environment file in the setup job's set-matrix step. GitHub deprecated
  `::set-output` citing output-injection concerns; the environment file
  is the supported, safer mechanism. Also fixes a stray trailing `}` in
  the emitted JSON (`...["test"]}}` -> `...["test"]}`).
- Remove `eval` from the test job's run step. `eval 'lein ... ${CMD}'`
  re-parsed a matrix-derived variable through the shell; passing it as a
  normal quoted argument (`lein do clean, compile :all, "$CMD"`) removes
  the eval/command-injection anti-pattern.

No behavior change: `matrix.cmd` is hardcoded to `test`, so both jobs run
exactly as before (the matrix output is still consumed via
`needs.setup.outputs.matrix`).

Not in this PR: bumping action major versions (setup-java v2->v4,
cache v3->v4) is a runtime-behavior change better handled by Dependabot;
the actions are already SHA-pinned as of #59.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@msprunck msprunck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@yogsototh yogsototh merged commit 0bd6620 into master Jun 16, 2026
6 checks passed
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.

3 participants