Guard against artifact poisoning in commenter#10149
Merged
Merged
Conversation
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Contributor
|
No changes needing a change description found. |
Collaborator
|
You can try these changes here
|
markcowl
approved these changes
Mar 25, 2026
msyyc
pushed a commit
that referenced
this pull request
Apr 1, 2026
Potential fix for [https://github.com/microsoft/typespec/security/code-scanning/107](https://github.com/microsoft/typespec/security/code-scanning/107) In general, to prevent artifact poisoning you should never let downloaded artifacts overwrite files in the repository workspace, and you should treat their contents as untrusted: extract them into a dedicated temporary directory and only read specific expected files from there, validating them if necessary. You should also avoid running build tools or package managers (`npm install`, `pnpm install`, etc.) in a workspace that may have been modified by untrusted artifact contents. For this specific workflow, the best minimal fix is: 1. Create a temporary directory under `${{ runner.temp }}` to hold the downloaded artifact. 2. Change the `actions/download-artifact@v4` step to use `path: ${{ runner.temp }}/comment-artifact` (or similar) so that artifact contents cannot overwrite the repository’s code or config. 3. Adjust the later use of `comment.json` to point at that safe location (e.g., `--comment-file ${{ runner.temp }}/comment-artifact/comment.json`). 4. Leave `pnpm install` as-is, since once the artifact is isolated from the workspace, it can no longer affect what `pnpm install` reads or executes. All changes occur in `.github/workflows/commenter.yml`: - Add a shell step before `actions/download-artifact@v4` to `mkdir -p ${{ runner.temp }}/comment-artifact`. - Modify the `download-artifact` step to include `path: ${{ runner.temp }}/comment-artifact`. - Modify the `pnpm chronus-github-pr-commenter` step to read the comment file from that temp directory. No extra actions or external libraries are required. _Suggested fixes powered by Copilot Autofix. Review carefully before merging._ Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
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.
Potential fix for https://github.com/microsoft/typespec/security/code-scanning/107
In general, to prevent artifact poisoning you should never let downloaded artifacts overwrite files in the repository workspace, and you should treat their contents as untrusted: extract them into a dedicated temporary directory and only read specific expected files from there, validating them if necessary. You should also avoid running build tools or package managers (
npm install,pnpm install, etc.) in a workspace that may have been modified by untrusted artifact contents.For this specific workflow, the best minimal fix is:
${{ runner.temp }}to hold the downloaded artifact.actions/download-artifact@v4step to usepath: ${{ runner.temp }}/comment-artifact(or similar) so that artifact contents cannot overwrite the repository’s code or config.comment.jsonto point at that safe location (e.g.,--comment-file ${{ runner.temp }}/comment-artifact/comment.json).pnpm installas-is, since once the artifact is isolated from the workspace, it can no longer affect whatpnpm installreads or executes.All changes occur in
.github/workflows/commenter.yml:actions/download-artifact@v4tomkdir -p ${{ runner.temp }}/comment-artifact.download-artifactstep to includepath: ${{ runner.temp }}/comment-artifact.pnpm chronus-github-pr-commenterstep to read the comment file from that temp directory.No extra actions or external libraries are required.
Suggested fixes powered by Copilot Autofix. Review carefully before merging.