Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

### Features

- Updater - Add `post-update-script` input parameter to run custom scripts after dependency updates ([#130](https://github.com/getsentry/github-workflows/pull/130))
- Updater - Add `post-update-script` input parameter to run custom scripts after dependency updates ([#130](https://github.com/getsentry/github-workflows/pull/130), [#133](https://github.com/getsentry/github-workflows/pull/133))
- Scripts receive original and new version as arguments
- Support both bash (`.sh`) and PowerShell (`.ps1`) scripts
- Enables workflows like updating lock files, running code generators, or modifying configuration files
Expand Down
2 changes: 1 addition & 1 deletion updater/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ runs:
DEPENDENCY_PATH: ${{ inputs.path }}
POST_UPDATE_SCRIPT: ${{ inputs.post-update-script }}
GH_TOKEN: ${{ inputs.api-token }}
run: ${{ github.action_path }}/scripts/update-dependency.ps1 -Path $env:DEPENDENCY_PATH -Tag '${{ steps.target.outputs.latestTag }}' -PostUpdateScript $env:POST_UPDATE_SCRIPT
run: ${{ github.action_path }}/scripts/update-dependency.ps1 -Path $env:DEPENDENCY_PATH -Tag '${{ steps.target.outputs.latestTag }}' -OriginalTag '${{ steps.target.outputs.originalTag }}' -PostUpdateScript $env:POST_UPDATE_SCRIPT

- name: Update Changelog
if: ${{ inputs.changelog-entry == 'true' && ( steps.target.outputs.latestTag != steps.target.outputs.originalTag ) && ( steps.root.outputs.changed == 'false') }}
Expand Down
9 changes: 6 additions & 3 deletions updater/scripts/update-dependency.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ param(
[string] $GhTitlePattern = '',
# Specific version - if passed, no discovery is performed and the version is set directly
[string] $Tag = '',
# Version that the dependency was on before the update - should be only passed if $Tag is set. Necessary for PostUpdateScript.
[string] $OriginalTag = '',
# Optional post-update script to run after successful dependency update
# The script receives the original and new version as arguments
[string] $PostUpdateScript = ''
Comment on lines 16 to 23
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential bug: The script uses lowercase variables $tag and $originalTag, but the parameters are defined as $Tag and $OriginalTag, leading to incorrect behavior.
  • Description: In the execution path where Tag and OriginalTag are explicitly provided, the script attempts to use lowercase variables $tag and $originalTag. However, these variables are not defined in this scope; the correct, case-sensitive parameters are $Tag and $OriginalTag. Because PowerShell treats undefined variables as empty strings, the call to DependencyConfig 'set-version' $tag will incorrectly update the dependency file with an empty version. Additionally, the post-update script will be called with empty parameters, causing the dependency update process to fail or produce incorrect results. This bug occurs specifically in the new scenario introduced by the code change.

  • Suggested fix: Correct the variable names used in the core functionality to match the case of the defined parameters. Change $tag to $Tag and $originalTag to $OriginalTag where they are used to call DependencyConfig and the post-update script.
    severity: 0.85, confidence: 0.98

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down Expand Up @@ -134,6 +136,8 @@ if (-not $isSubmodule) {
}

if ("$Tag" -eq '') {
$OriginalTag | Should -Be ''

if ($isSubmodule) {
git submodule update --init --no-fetch --single-branch $Path
Push-Location $Path
Expand Down Expand Up @@ -250,11 +254,10 @@ if ("$Tag" -eq '') {
}

$Tag = $latestTag
} else {
$OriginalTag | Should -Not -Be ''
}

$originalTagForPostUpdate = if ($originalTag) { $originalTag } else { '' }
$newTagForPostUpdate = $Tag

if ($isSubmodule) {
Write-Host "Updating submodule $Path to $Tag"
Push-Location $Path
Expand Down
57 changes: 57 additions & 0 deletions updater/tests/update-dependency.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,63 @@ param([string] $originalVersion, [string] $newVersion)
Remove-Item $postUpdateScript -ErrorAction SilentlyContinue
}

It 'runs PowerShell post-update script when Tag and OriginalTag are explicitly provided' {
$testFile = "$testDir/test.properties"
$repo = 'https://github.com/getsentry/sentry-cli'
@("repo=$repo", 'version=0.27.0') | Out-File $testFile

$postUpdateScript = "$testDir/post-update-explicit.ps1"
$markerFile = "$testDir/post-update-marker-explicit.txt"
@'
param([string] $originalVersion, [string] $newVersion)
"$originalVersion|$newVersion" | Out-File
'@ + " '$markerFile'" | Out-File $postUpdateScript

# Simulate the second run where we explicitly set Tag and OriginalTag
$params = @{
Path = $testFile
Tag = '0.28.0'
OriginalTag = '0.27.0'
PostUpdateScript = $postUpdateScript
}
$result = & "$PSScriptRoot/../scripts/update-dependency.ps1" @params
if (-not $?) {
throw $result
}

# Verify post-update script was executed with correct versions
Test-Path $markerFile | Should -Be $true
$markerContent = Get-Content $markerFile
$markerContent | Should -Match '^0\.27\.0\|0\.28\.0$'

# Clean up
Remove-Item $markerFile -ErrorAction SilentlyContinue
Remove-Item $postUpdateScript -ErrorAction SilentlyContinue
}

It 'fails when Tag is provided without OriginalTag' {
$testFile = "$testDir/test.properties"
$repo = 'https://github.com/getsentry/sentry-cli'
@("repo=$repo", 'version=0.27.0') | Out-File $testFile

$postUpdateScript = "$testDir/post-update-fail.ps1"
@'
param([string] $originalVersion, [string] $newVersion)
"$originalVersion|$newVersion" | Out-File marker.txt
'@ | Out-File $postUpdateScript

# This should fail because Tag requires OriginalTag
$params = @{
Path = $testFile
Tag = '0.28.0'
PostUpdateScript = $postUpdateScript
}
{ & "$PSScriptRoot/../scripts/update-dependency.ps1" @params } | Should -Throw '*Expected*to be different*'

# Clean up
Remove-Item $postUpdateScript -ErrorAction SilentlyContinue
}

It 'runs bash post-update script with version arguments' -Skip:$IsWindows {
$testFile = "$testDir/test.properties"
$repo = 'https://github.com/getsentry/sentry-cli'
Expand Down