ci: pass changed files through env in release-notes reST check#11856
Open
camgrimsec wants to merge 1 commit into
Open
ci: pass changed files through env in release-notes reST check#11856camgrimsec wants to merge 1 commit into
camgrimsec wants to merge 1 commit into
Conversation
The Check reStructuredText code formatting step in release_notes.yml uses
shell: python and builds its file list by interpolating the
tj-actions/changed-files output directly into a Python source literal:
files = "${{ steps.changed-files.outputs.all_changed_files }}".split()
At runtime the workflow expression is expanded by the Actions runner
before Python starts, so the contents of that output become part of the
Python source of the step. Because git allows filenames to contain a
double-quote character as well as newlines and backslashes, a
pull_request contributor can add a release-note file whose path breaks
out of the string literal. GitHub renders the workflow using the head
ref's copy of the file, so an attacker does not need write access to
main to reach this code path.
The trigger is pull_request (not pull_request_target), so GITHUB_TOKEN
is read-only for this repo and repository secrets are not attached to
the job. The direct blast radius is limited to code execution in the
release-notes job itself, but that job still runs on the deepset-owned
runner pool and shares the ephemeral filesystem with the checked-out
source tree, so injecting Python here is undesirable regardless.
The fix follows GitHub's documented secure workflows pattern: expose the
changed-files output as an environment variable and read it from Python
via os.environ, so the untrusted value never becomes part of the source
string that the interpreter parses.
Same class of hardening as deepset-ai#11733 (persist-credentials: false) and
deepset-ai#11777 (further dependency pinning + CodeQL), extended one step further
into the workflow body.
|
@camgrimsec is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
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
1 participant
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.
The Check reStructuredText code formatting step in release_notes.yml uses shell: python and builds its file list by interpolating the tj-actions/changed-files output directly into a Python source literal:
files = "${{ steps.changed-files.outputs.all_changed_files }}".split()
At runtime the workflow expression is expanded by the Actions runner before Python starts, so the contents of that output become part of the Python source of the step. Because git allows filenames to contain a double-quote character as well as newlines and backslashes, a pull_request contributor can add a release-note file whose path breaks out of the string literal. GitHub renders the workflow using the head ref's copy of the file, so an attacker does not need write access to main to reach this code path.
The trigger is pull_request (not pull_request_target), so GITHUB_TOKEN is read-only for this repo and repository secrets are not attached to the job. The direct blast radius is limited to code execution in the release-notes job itself, but that job still runs on the deepset-owned runner pool and shares the ephemeral filesystem with the checked-out source tree, so injecting Python here is undesirable regardless.
The fix follows GitHub's documented secure workflows pattern: expose the changed-files output as an environment variable and read it from Python via os.environ, so the untrusted value never becomes part of the source string that the interpreter parses.
Same class of hardening as #11733 (persist-credentials: false) and #11777 (further dependency pinning + CodeQL), extended one step further into the workflow body.
Related Issues
Proposed Changes:
How did you test it?
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.