ci: Add least-privilege permissions and fix output injection - #282
Conversation
…tput injection Co-authored-by: Anush008 <46051506+Anush008@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The updated $GITHUB_OUTPUT writes still permit newline-based output injection and should be switched to the delimiter-based output format to be safe.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR tightens GitHub Actions workflow security by explicitly scoping GITHUB_TOKEN permissions per job and by addressing an output-writing pattern that can allow $GITHUB_OUTPUT key injection.
Changes:
- Added explicit
permissions: contents: readto the CI jobs intest.ymlto enforce least-privilege repository access. - Added explicit
permissionsfor thereleasejob inrelease.ymlto scope token capabilities to what the release process needs. - Updated how author name/email are written to
$GITHUB_OUTPUT(but the current change still allows newline-based output injection and needs adjustment).
File summaries
| File | Description |
|---|---|
| .github/workflows/test.yml | Adds per-job least-privilege contents: read permissions for CI jobs. |
| .github/workflows/release.yml | Scopes release job permissions and modifies output-writing logic for author metadata. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| run: | | ||
| AUTHOR_NAME=$(git log -1 --pretty=format:%an ${{ github.sha }}) | ||
| AUTHOR_EMAIL=$(git log -1 --pretty=format:%ae ${{ github.sha }}) | ||
| echo "AUTHOR_NAME=$AUTHOR_NAME" >> $GITHUB_OUTPUT | ||
| echo "AUTHOR_EMAIL=$AUTHOR_EMAIL" >> $GITHUB_OUTPUT | ||
| printf 'AUTHOR_NAME=%s\n' "$AUTHOR_NAME" >> $GITHUB_OUTPUT | ||
| printf 'AUTHOR_EMAIL=%s\n' "$AUTHOR_EMAIL" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Fixed in acd5144 — the Get Author Name and Email step now uses the heredoc delimiter syntax (<<EOF) to safely write multiline values to $GITHUB_OUTPUT, preventing injection via embedded newlines.
|
@copilot Rename PR title as per semantic conventions. |
…jection Co-authored-by: Anush008 <46051506+Anush008@users.noreply.github.com>
Updated the PR title to follow semantic commit conventions: |
anush008
changed the title
There was a problem hiding this comment.
🟡 Changes recommended
The fixed output delimiter can collide with valid author metadata and fail the release job.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
| { | ||
| echo "AUTHOR_NAME<<EOF" | ||
| echo "$AUTHOR_NAME" | ||
| echo "EOF" | ||
| echo "AUTHOR_EMAIL<<EOF" | ||
| echo "$AUTHOR_EMAIL" | ||
| echo "EOF" | ||
| } >> "$GITHUB_OUTPUT" |
* Improve workflow security: add least-privilege permissions and fix output injection Co-authored-by: Anush008 <46051506+Anush008@users.noreply.github.com> * fix(ci): use heredoc delimiter syntax for GITHUB_OUTPUT to prevent injection Co-authored-by: Anush008 <46051506+Anush008@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Anush008 <46051506+Anush008@users.noreply.github.com>
Summary
Improves the security of the GitHub Actions workflows.
Changes
test.ymlpermissions: contents: readto all four jobs (build-onnx,test,lint,clippy-features) to enforce least-privilege access. Previously jobs inherited the default broad repository permissions.release.ymlpermissionsto thereleasejob — only the scopes thatsemantic-releaseactually needs (contents: write,issues: write,pull-requests: write).$GITHUB_OUTPUTinjection risk: the previousecho "AUTHOR_NAME=$AUTHOR_NAME"pattern can be exploited if a commit author name contains a newline followed by another key (e.g.foo\nSECRET_KEY=injected). Replaced withprintf 'AUTHOR_NAME=%s\n' "$AUTHOR_NAME"which safely treats the variable as data, not as a format string.