-
-
Notifications
You must be signed in to change notification settings - Fork 8
fix(updater): Prevent script injection vulnerabilities #98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
2fa8e16
3841d24
71bc194
4526fc8
15af118
fff8b7b
9af03f8
7bb1cd7
b67288f
2feccc3
97fbb71
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,187 +72,217 @@ | |
| with: | ||
| access_token: ${{ github.token }} | ||
|
|
||
| validate-inputs: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Validate dependency name | ||
| shell: pwsh | ||
| run: | | ||
| # Validate that inputs.name contains only safe characters | ||
| if ('${{ inputs.name }}' -notmatch '^[a-zA-Z0-9_\./@\s-]+$') { | ||
| Write-Output "::error::Invalid dependency name: '${{ inputs.name }}'. Only alphanumeric characters, spaces, and _-./@ are allowed." | ||
| exit 1 | ||
| } | ||
| Write-Output "✓ Dependency name '${{ inputs.name }}' is valid" | ||
|
|
||
| - name: Validate dependency path | ||
| shell: pwsh | ||
| run: | | ||
| # Validate that inputs.path contains only safe characters | ||
| if ('${{ inputs.path }}' -notmatch '^[a-zA-Z0-9_\./-]+$') { | ||
| Write-Output "::error::Invalid dependency path: '${{ inputs.path }}'. Only alphanumeric characters and _-./ are allowed." | ||
| exit 1 | ||
| } | ||
| Write-Output "✓ Dependency path '${{ inputs.path }}' is valid" | ||
|
|
||
| # What we need to accomplish: | ||
| # * update to the latest tag | ||
| # * create a PR | ||
| # * update changelog (including the link to the just created PR) | ||
| # | ||
| # What we actually do is based on whether a PR exists already: | ||
| # * YES it does: | ||
| # * make the update | ||
| # * update changelog (with the ID of an existing PR) | ||
| # * push to the PR | ||
| # * NO it doesn't: | ||
| # * make the update | ||
| # * push to a new PR | ||
| # * update changelog (with the ID of the just created PR) | ||
| # * push to the PR | ||
| # We do different approach on subsequent runs because otherwise we would spam users' mailboxes | ||
| # with notifications about pushes to existing PRs. This way there is actually no push if not needed. | ||
| update: | ||
Check warningCode scanning / CodeQL Workflow does not contain permissions Medium
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {}
|
||
| needs: validate-inputs | ||
| runs-on: ${{ inputs.runs-on }} | ||
| # Map the job outputs to step outputs | ||
| outputs: | ||
| prUrl: ${{ steps.pr.outputs.url }} | ||
| baseBranch: ${{ steps.root.outputs.baseBranch }} | ||
| prBranch: ${{ steps.root.outputs.prBranch }} | ||
| originalTag: ${{ steps.target.outputs.originalTag }} | ||
| latestTag: ${{ steps.target.outputs.latestTag }} | ||
| timeout-minutes: 30 | ||
| defaults: | ||
| run: | ||
| shell: pwsh | ||
| env: | ||
| DEPENDENCY_NAME: ${{ inputs.name }} | ||
| DEPENDENCY_PATH: ${{ inputs.path }} | ||
| DEPENDENCY_PATTERN: ${{ inputs.pattern }} | ||
| CHANGELOG_SECTION: ${{ inputs.changelog-section }} | ||
| PR_STRATEGY: ${{ inputs.pr-strategy }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| ssh-key: ${{ secrets.api-token }} | ||
|
|
||
| # In order to run scripts from this repo, we need to check it out manually, doesn't seem available locally. | ||
| - name: Check out workflow scripts | ||
| # Note: cannot use `actions/checkout` at the moment because you can't clone outside of the repo root. | ||
| # Follow https://github.com/actions/checkout/issues/197 | ||
| run: | | ||
| mkdir -p ${{ runner.temp }}/ghwf | ||
| cd ${{ runner.temp }}/ghwf | ||
| git init | ||
| git remote add origin https://github.com/getsentry/github-workflows.git | ||
| git fetch --depth 1 origin ${{ inputs._workflow_version }} | ||
| git checkout FETCH_HEAD | ||
|
|
||
| - name: Update to the latest version | ||
| id: target | ||
| run: ${{ runner.temp }}/ghwf/updater/scripts/update-dependency.ps1 -Path '${{ inputs.path }}' -Pattern '${{ inputs.pattern }}' | ||
| run: ${{ runner.temp }}/ghwf/updater/scripts/update-dependency.ps1 -Path "$env:DEPENDENCY_PATH" -Pattern "$env:DEPENDENCY_PATTERN" | ||
vaind marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| - name: Get the base repo info | ||
| if: steps.target.outputs.latestTag != steps.target.outputs.originalTag | ||
| id: root | ||
| run: | | ||
| $mainBranch = $(git remote show origin | Select-String "HEAD branch: (.*)").Matches[0].Groups[1].Value | ||
| $prBranch = switch ('${{ inputs.pr-strategy }}') | ||
| $prBranch = switch ($env:PR_STRATEGY) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this also have double-quotes around?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no because we can just switch directly on the variable value instead of converting it to string (which it is anyway), and actually I don't think we should be quoting in the step above, let me check |
||
| { | ||
| 'create' { 'deps/${{ inputs.path }}/${{ steps.target.outputs.latestTag }}' } | ||
| 'update' { 'deps/${{ inputs.path }}' } | ||
| default { throw "Unkown PR strategy '${{ inputs.pr-strategy }}'." } | ||
| 'create' { "deps/$env:DEPENDENCY_PATH/${{ steps.target.outputs.latestTag }}" } | ||
| 'update' { "deps/$env:DEPENDENCY_PATH" } | ||
| default { throw "Unkown PR strategy '$env:PR_STRATEGY'." } | ||
| } | ||
| "baseBranch=$mainBranch" | Tee-Object $env:GITHUB_OUTPUT -Append | ||
| "prBranch=$prBranch" | Tee-Object $env:GITHUB_OUTPUT -Append | ||
| $nonBotCommits = ${{ runner.temp }}/ghwf/updater/scripts/nonbot-commits.ps1 ` | ||
| -RepoUrl "$(git config --get remote.origin.url)" -PrBranch $prBranch -MainBranch $mainBranch | ||
| $changed = $nonBotCommits.Length -gt 0 ? 'true' : 'false' | ||
| "changed=$changed" | Tee-Object $env:GITHUB_OUTPUT -Append | ||
| if ("$changed" -eq "true") | ||
| { | ||
| Write-Output "::warning::Target branch '$prBranch' has been changed manually - skipping updater to avoid overwriting these changes." | ||
| } | ||
|
|
||
| - name: Parse the existing PR URL | ||
| if: ${{ ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.root.outputs.changed == 'false') }} | ||
| id: existing-pr | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| $urls = @(gh api 'repos/${{ github.repository }}/pulls?base=${{ steps.root.outputs.baseBranch }}&head=${{ github.repository_owner }}:${{ steps.root.outputs.prBranch }}' --jq '.[].html_url') | ||
| if ($urls.Length -eq 0) | ||
| { | ||
| "url=" | Tee-Object $env:GITHUB_OUTPUT -Append | ||
| } | ||
| elseif ($urls.Length -eq 1) | ||
| { | ||
| "url=$($urls[0])" | Tee-Object $env:GITHUB_OUTPUT -Append | ||
| } | ||
| else | ||
| { | ||
| throw "Unexpected number of PRs matched ($($urls.Length)): $urls" | ||
| } | ||
|
|
||
| - run: git --no-pager diff | ||
| if: ${{ ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.existing-pr.outputs.url == '') && ( steps.root.outputs.changed == 'false') }} | ||
|
|
||
| - name: Get target changelog | ||
| if: ${{ ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.root.outputs.changed == 'false') }} | ||
| run: | | ||
| $changelog = ${{ runner.temp }}/ghwf/updater/scripts/get-changelog.ps1 ` | ||
| -RepoUrl '${{ steps.target.outputs.url }}' ` | ||
| -OldTag '${{ steps.target.outputs.originalTag }}' ` | ||
| -NewTag '${{ steps.target.outputs.latestTag }}' | ||
| ${{ runner.temp }}/ghwf/updater/scripts/set-github-env.ps1 TARGET_CHANGELOG $changelog | ||
|
|
||
| # First we create a PR only if it doesn't exist. We will later overwrite the content with the same action. | ||
| - name: Create a PR | ||
| if: ${{ ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.existing-pr.outputs.url == '') && ( steps.root.outputs.changed == 'false') }} | ||
| uses: peter-evans/create-pull-request@a4f52f8033a6168103c2538976c07b467e8163bc # pin#v6.0.1 | ||
| id: create-pr | ||
| with: | ||
| base: ${{ steps.root.outputs.baseBranch }} | ||
| branch: ${{ steps.root.outputs.prBranch }} | ||
| commit-message: 'chore: update ${{ inputs.path }} to ${{ steps.target.outputs.latestTag }}' | ||
| commit-message: 'chore: update ${{ env.DEPENDENCY_PATH }} to ${{ steps.target.outputs.latestTag }}' | ||
| author: 'GitHub <[email protected]>' | ||
| title: 'chore(deps): update ${{ inputs.name }} to ${{ steps.target.outputs.latestTagNice }}' | ||
| title: 'chore(deps): update ${{ env.DEPENDENCY_NAME }} to ${{ steps.target.outputs.latestTagNice }}' | ||
| body: | | ||
| Bumps ${{ inputs.path }} from ${{ steps.target.outputs.originalTag }} to ${{ steps.target.outputs.latestTag }}. | ||
| Bumps ${{ env.DEPENDENCY_PATH }} from ${{ steps.target.outputs.originalTag }} to ${{ steps.target.outputs.latestTag }}. | ||
|
|
||
| Auto-generated by a [dependency updater](https://github.com/getsentry/github-workflows/blob/main/.github/workflows/updater.yml). | ||
| ${{ env.TARGET_CHANGELOG }} | ||
| labels: dependencies | ||
| # draft: true | ||
|
|
||
| - name: Verify we have a PR | ||
| if: ${{ ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.root.outputs.changed == 'false') }} | ||
| id: pr | ||
| run: | | ||
| if ('${{ steps.create-pr.outputs.pull-request-url }}' -ne '') | ||
| { | ||
| "url=${{ steps.create-pr.outputs.pull-request-url }}" | Tee-Object $env:GITHUB_OUTPUT -Append | ||
| } | ||
| elseif ('${{ steps.existing-pr.outputs.url }}' -ne '') | ||
| { | ||
| "url=${{ steps.existing-pr.outputs.url }}" | Tee-Object $env:GITHUB_OUTPUT -Append | ||
| } | ||
| else | ||
| { | ||
| throw "PR hasn't been created" | ||
| } | ||
|
|
||
| # If we had to create a new PR, we must do a clean checkout & update the submodule again. | ||
| # If we didn't do this, the new PR would only have a changelog... | ||
| - name: 'After new PR: restore repo' | ||
| if: ${{ ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.existing-pr.outputs.url == '') && ( steps.root.outputs.changed == 'false') }} | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ssh-key: ${{ secrets.api-token }} | ||
|
|
||
| - name: 'After new PR: redo the update' | ||
| if: ${{ ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.existing-pr.outputs.url == '') && ( steps.root.outputs.changed == 'false') }} | ||
| run: ${{ runner.temp }}/ghwf/updater/scripts/update-dependency.ps1 -Path '${{ inputs.path }}' -Tag '${{ steps.target.outputs.latestTag }}' | ||
| run: ${{ runner.temp }}/ghwf/updater/scripts/update-dependency.ps1 -Path "$env:DEPENDENCY_PATH" -Tag '${{ steps.target.outputs.latestTag }}' | ||
vaind marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| - name: Update Changelog | ||
| if: ${{ inputs.changelog-entry && ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.root.outputs.changed == 'false') }} | ||
| run: | | ||
| ${{ runner.temp }}/ghwf/updater/scripts/update-changelog.ps1 ` | ||
| -Name '${{ inputs.name }}' ` | ||
| -Name "$env:DEPENDENCY_NAME" ` | ||
vaind marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| -PR '${{ steps.pr.outputs.url }}' ` | ||
| -RepoUrl '${{ steps.target.outputs.url }}' ` | ||
| -MainBranch '${{ steps.target.outputs.mainBranch }}' ` | ||
| -OldTag '${{ steps.target.outputs.originalTag }}' ` | ||
| -NewTag '${{ steps.target.outputs.latestTag }}' ` | ||
| -Section '${{ inputs.changelog-section }}' | ||
| -Section "$env:CHANGELOG_SECTION" | ||
vaind marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| - run: git --no-pager diff | ||
| if: ${{ ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.root.outputs.changed == 'false') }} | ||
|
|
||
| # Now make the PR in its final state. This way we only have one commit and no updates if there are no changes between runs. | ||
| - name: Update the PR | ||
| if: ${{ ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.root.outputs.changed == 'false') }} | ||
| uses: peter-evans/create-pull-request@a4f52f8033a6168103c2538976c07b467e8163bc # pin#v6.0.1 | ||
| with: | ||
| base: ${{ steps.root.outputs.baseBranch }} | ||
| branch: ${{ steps.root.outputs.prBranch }} | ||
| commit-message: 'chore: update ${{ inputs.path }} to ${{ steps.target.outputs.latestTag }}' | ||
| commit-message: 'chore: update ${{ env.DEPENDENCY_PATH }} to ${{ steps.target.outputs.latestTag }}' | ||
| author: 'GitHub <[email protected]>' | ||
| title: 'chore(deps): update ${{ inputs.name }} to ${{ steps.target.outputs.latestTagNice }}' | ||
| title: 'chore(deps): update ${{ env.DEPENDENCY_NAME }} to ${{ steps.target.outputs.latestTagNice }}' | ||
| body: | | ||
| Bumps ${{ inputs.path }} from ${{ steps.target.outputs.originalTag }} to ${{ steps.target.outputs.latestTag }}. | ||
| Bumps ${{ env.DEPENDENCY_PATH }} from ${{ steps.target.outputs.originalTag }} to ${{ steps.target.outputs.latestTag }}. | ||
|
|
||
| Auto-generated by a [dependency updater](https://github.com/getsentry/github-workflows/blob/main/.github/workflows/updater.yml). | ||
| ${{ env.TARGET_CHANGELOG }} | ||
| labels: dependencies | ||
Check warningCode scanning / CodeQL Workflow does not contain permissions Medium
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}
|
||
Uh oh!
There was an error while loading. Please reload this page.