From b71c9e82ab7cca0cde0568a762f92553bbe881b6 Mon Sep 17 00:00:00 2001 From: Cody Date: Thu, 27 Aug 2026 16:01:10 -0500 Subject: [PATCH 1/5] fix(install): qualify Windows archive extraction Use the Microsoft.PowerShell.Archive cmdlet explicitly so modules such as Pscx cannot shadow the installer extraction command. Add a focused Windows regression test for both installer variants.\n\nFixes #651. --- .github/workflows/tests.yml | 13 ++++++ scripts/install-windows-dev.ps1 | 2 +- scripts/install-windows-test.ps1 | 70 ++++++++++++++++++++++++++++++++ scripts/install-windows.ps1 | 2 +- 4 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 scripts/install-windows-test.ps1 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 716cc079..d8b6565d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -34,6 +34,19 @@ jobs: files: ./coverage.out fail_ci_if_error: false + windows-install: + name: Windows Installer + runs-on: windows-latest + permissions: + contents: read + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Test Windows installer + shell: pwsh + run: ./scripts/install-windows-test.ps1 + # Monitor code coverage and TODO/FIXME-type comments health-score: name: Health Score diff --git a/scripts/install-windows-dev.ps1 b/scripts/install-windows-dev.ps1 index e24cfd22..b389c23a 100644 --- a/scripts/install-windows-dev.ps1 +++ b/scripts/install-windows-dev.ps1 @@ -163,7 +163,7 @@ function install_slack_cli { $slack_cli_new_binary_path = "$($slack_cli_dir)\bin\${confirmed_alias}.exe" delay 0.3 "Extracting the executable to:`n $slack_cli_new_binary_path" - Expand-Archive "$($slack_cli_dir)\slack_cli.zip" -DestinationPath "$($slack_cli_dir)" -Force + Microsoft.PowerShell.Archive\Expand-Archive "$($slack_cli_dir)\slack_cli.zip" -DestinationPath "$($slack_cli_dir)" -Force Move-Item -Path $slack_cli_binary_path -Destination $slack_cli_new_binary_path -Force $User = [System.EnvironmentVariableTarget]::User diff --git a/scripts/install-windows-test.ps1 b/scripts/install-windows-test.ps1 new file mode 100644 index 00000000..537cf410 --- /dev/null +++ b/scripts/install-windows-test.ps1 @@ -0,0 +1,70 @@ +# Copyright 2022-2026 Salesforce, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +$ErrorActionPreference = "Stop" +$qualifiedCommand = "Microsoft.PowerShell.Archive\Expand-Archive" +$installerPaths = @( + (Join-Path $PSScriptRoot "install-windows.ps1"), + (Join-Path $PSScriptRoot "install-windows-dev.ps1") +) + +foreach ($installerPath in $installerPaths) { + $tokens = $null + $parseErrors = $null + $ast = [System.Management.Automation.Language.Parser]::ParseFile( + $installerPath, + [ref]$tokens, + [ref]$parseErrors + ) + + if ($parseErrors.Count -gt 0) { + throw "PowerShell parser errors in $installerPath" + } + + $archiveCommands = $ast.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.CommandAst] -and + $node.GetCommandName() -like "*Expand-Archive" + }, $true) + + if ($archiveCommands.Count -ne 1 -or $archiveCommands[0].GetCommandName() -ne $qualifiedCommand) { + throw "$installerPath must call $qualifiedCommand exactly once" + } +} + +$testRoot = Join-Path ([System.IO.Path]::GetTempPath()) "slack-cli-install-$([guid]::NewGuid())" +$sourcePath = Join-Path $testRoot "source" +$destinationPath = Join-Path $testRoot "destination" +$archivePath = Join-Path $testRoot "slack_cli.zip" + +try { + New-Item -ItemType Directory -Path (Join-Path $sourcePath "bin") -Force | Out-Null + Set-Content -LiteralPath (Join-Path $sourcePath "bin\slack.exe") -Value "test" + Microsoft.PowerShell.Archive\Compress-Archive -Path (Join-Path $sourcePath "*") -DestinationPath $archivePath + + function Expand-Archive { + throw "The shadowing command should not be called" + } + + Microsoft.PowerShell.Archive\Expand-Archive -LiteralPath $archivePath -DestinationPath $destinationPath -Force + + if (!(Test-Path -LiteralPath (Join-Path $destinationPath "bin\slack.exe"))) { + throw "The qualified archive command did not extract bin\slack.exe" + } +} +finally { + if (Test-Path -LiteralPath $testRoot) { + Remove-Item -LiteralPath $testRoot -Recurse -Force + } +} diff --git a/scripts/install-windows.ps1 b/scripts/install-windows.ps1 index 63f6d70a..aa648390 100644 --- a/scripts/install-windows.ps1 +++ b/scripts/install-windows.ps1 @@ -154,7 +154,7 @@ function install_slack_cli { $slack_cli_new_binary_path = "$($slack_cli_dir)\bin\${confirmed_alias}.exe" delay 0.3 "Extracting the executable to:`n $slack_cli_new_binary_path" - Expand-Archive "$($slack_cli_dir)\slack_cli.zip" -DestinationPath "$($slack_cli_dir)" -Force + Microsoft.PowerShell.Archive\Expand-Archive "$($slack_cli_dir)\slack_cli.zip" -DestinationPath "$($slack_cli_dir)" -Force Move-Item -Path $slack_cli_binary_path -Destination $slack_cli_new_binary_path -Force $User = [System.EnvironmentVariableTarget]::User From a2a94d5bbc859600aec56da91efbb39c097a89bb Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Fri, 28 Aug 2026 12:33:25 -0700 Subject: [PATCH 2/5] test(install): run the real Windows installers end-to-end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior Windows test only lint-checked the AST for the qualified Expand-Archive call, then exercised a re-implemented extraction snippet that shadowed a *function* named Expand-Archive — it never ran the installer, so it could not catch the module-command shadow that #651 actually hit (Pscx's Expand-Archive winning unqualified resolution). Rework it to mirror the Unix install-test.sh pattern: for each Windows installer (release + dev), keep the AST guard, then actually invoke the installer and assert the aliased binary lands and reports a version. Move install tests out of the macOS-only lint-test job into a dedicated install-tests job with an OS matrix (macos/ubuntu/windows) so all three platforms' installers are exercised on every PR. Unix runs make test-install; Windows runs the reworked script. Co-Authored-By: Claude --- .github/workflows/tests.yml | 21 +++++++---- scripts/install-windows-test.ps1 | 60 ++++++++++++++++++-------------- 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d8b6565d..377cdaef 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -25,8 +25,6 @@ jobs: run: go tool golangci-lint run --timeout=5m - name: Unit Tests run: make test - - name: Install Tests - run: make test-install - name: Upload coverage to Codecov uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: @@ -34,16 +32,27 @@ jobs: files: ./coverage.out fail_ci_if_error: false - windows-install: - name: Windows Installer - runs-on: windows-latest + install-tests: + name: Install Tests + strategy: + fail-fast: false + matrix: + os: + - macos-latest + - ubuntu-latest + - windows-latest + runs-on: ${{ matrix.os }} permissions: contents: read steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - - name: Test Windows installer + - name: Install Tests (Unix) + if: runner.os != 'Windows' + run: make test-install + - name: Install Tests (Windows) + if: runner.os == 'Windows' shell: pwsh run: ./scripts/install-windows-test.ps1 diff --git a/scripts/install-windows-test.ps1 b/scripts/install-windows-test.ps1 index 537cf410..d5b6e11f 100644 --- a/scripts/install-windows-test.ps1 +++ b/scripts/install-windows-test.ps1 @@ -13,23 +13,35 @@ # limitations under the License. $ErrorActionPreference = "Stop" -$qualifiedCommand = "Microsoft.PowerShell.Archive\Expand-Archive" -$installerPaths = @( - (Join-Path $PSScriptRoot "install-windows.ps1"), - (Join-Path $PSScriptRoot "install-windows-dev.ps1") + +$qualifiedCommand = "Microsoft.PowerShell.Archive\Expand-Archive" # https://github.com/slackapi/slack-cli/issues/651 +$installDir = Join-Path $env:LOCALAPPDATA "slack-cli" + +$cases = @( + @{ Installer = "install-windows.ps1"; Alias = "slack-test"; Version = "4.6.0"; ExpectVersion = $true }, + @{ Installer = "install-windows-dev.ps1"; Alias = "slack-dev-test"; Version = "dev"; ExpectVersion = $false } ) -foreach ($installerPath in $installerPaths) { +function Remove-Install { + if (Test-Path -LiteralPath $installDir) { + Remove-Item -LiteralPath $installDir -Recurse -Force + } +} + +foreach ($case in $cases) { + $installer = Join-Path $PSScriptRoot $case.Installer + $aliasBinary = Join-Path $installDir "bin\$($case.Alias).exe" + $tokens = $null $parseErrors = $null $ast = [System.Management.Automation.Language.Parser]::ParseFile( - $installerPath, + $installer, [ref]$tokens, [ref]$parseErrors ) if ($parseErrors.Count -gt 0) { - throw "PowerShell parser errors in $installerPath" + throw "PowerShell parser errors in $installer" } $archiveCommands = $ast.FindAll({ @@ -39,32 +51,26 @@ foreach ($installerPath in $installerPaths) { }, $true) if ($archiveCommands.Count -ne 1 -or $archiveCommands[0].GetCommandName() -ne $qualifiedCommand) { - throw "$installerPath must call $qualifiedCommand exactly once" + throw "$installer must call $qualifiedCommand exactly once" } -} -$testRoot = Join-Path ([System.IO.Path]::GetTempPath()) "slack-cli-install-$([guid]::NewGuid())" -$sourcePath = Join-Path $testRoot "source" -$destinationPath = Join-Path $testRoot "destination" -$archivePath = Join-Path $testRoot "slack_cli.zip" + try { + Remove-Install -try { - New-Item -ItemType Directory -Path (Join-Path $sourcePath "bin") -Force | Out-Null - Set-Content -LiteralPath (Join-Path $sourcePath "bin\slack.exe") -Value "test" - Microsoft.PowerShell.Archive\Compress-Archive -Path (Join-Path $sourcePath "*") -DestinationPath $archivePath + & $installer -Alias $case.Alias -Version $case.Version -SkipGit $true - function Expand-Archive { - throw "The shadowing command should not be called" - } + if (!(Test-Path -LiteralPath $aliasBinary)) { + throw "$($case.Installer) did not place $($case.Alias).exe at $aliasBinary (extraction failed?)" + } - Microsoft.PowerShell.Archive\Expand-Archive -LiteralPath $archivePath -DestinationPath $destinationPath -Force + $versionOutput = & $aliasBinary --version + if ($case.ExpectVersion -and $versionOutput -notmatch [regex]::Escape($case.Version)) { + throw "Version mismatch: expected '$($case.Version)' in output, got '$versionOutput'" + } - if (!(Test-Path -LiteralPath (Join-Path $destinationPath "bin\slack.exe"))) { - throw "The qualified archive command did not extract bin\slack.exe" + Write-Host "$($case.Installer) E2E passed: $($case.Alias).exe installed, version '$versionOutput'" } -} -finally { - if (Test-Path -LiteralPath $testRoot) { - Remove-Item -LiteralPath $testRoot -Recurse -Force + finally { + Remove-Install } } From 63424883d7917f7d0f0932a976cbd8b2e306ba3a Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Fri, 28 Aug 2026 13:41:32 -0700 Subject: [PATCH 3/5] test(install): guard #651 fix statically, not via live install The Windows install test's real end-to-end run hangs in CI: after extraction, install-windows.ps1's post-install courtesy check runs `& slack _fingerprint | Tee-Object -Variable | Out-Null`, which blocks under pwsh 7 on current Windows runner images (a native-command pipeline with no console never returns). Confirmed against two CI runs -- a cold install hung for 12 min then cancelled, while an older-image run of the same installer completed -- so the hang tracks the runner image/pwsh build, not this branch's change, and is independent of the install alias. Revert the Windows leg to a static AST guard: parse each installer and assert it calls Microsoft.PowerShell.Archive\Expand-Archive exactly once (the #651 fix). This verifies the regression is present without running the installer, so the check is fast and can't hang. The installer's _fingerprint hang is a real pre-existing bug tracked separately. Co-Authored-By: Claude --- scripts/install-windows-test.ps1 | 50 ++++++++++---------------------- 1 file changed, 15 insertions(+), 35 deletions(-) diff --git a/scripts/install-windows-test.ps1 b/scripts/install-windows-test.ps1 index d5b6e11f..3a4f27a4 100644 --- a/scripts/install-windows-test.ps1 +++ b/scripts/install-windows-test.ps1 @@ -14,23 +14,21 @@ $ErrorActionPreference = "Stop" -$qualifiedCommand = "Microsoft.PowerShell.Archive\Expand-Archive" # https://github.com/slackapi/slack-cli/issues/651 -$installDir = Join-Path $env:LOCALAPPDATA "slack-cli" - -$cases = @( - @{ Installer = "install-windows.ps1"; Alias = "slack-test"; Version = "4.6.0"; ExpectVersion = $true }, - @{ Installer = "install-windows-dev.ps1"; Alias = "slack-dev-test"; Version = "dev"; ExpectVersion = $false } +# Guard against the Pscx-shadowing regression from https://github.com/slackapi/slack-cli/issues/651: +# Pscx 3.3.2 ships its own Expand-Archive that shadows the built-in, so the extraction call must be +# qualified as Microsoft.PowerShell.Archive\Expand-Archive. We assert this statically (parse the AST, +# not run the installer) because the installers' post-install courtesy check hangs under pwsh 7 on +# current Windows runner images -- `& slack _fingerprint | Tee-Object -Variable | Out-Null` blocks +# with no console -- so a real end-to-end install is not yet runnable in CI. Tracked separately. +$qualifiedCommand = "Microsoft.PowerShell.Archive\Expand-Archive" + +$installers = @( + "install-windows.ps1", + "install-windows-dev.ps1" ) -function Remove-Install { - if (Test-Path -LiteralPath $installDir) { - Remove-Item -LiteralPath $installDir -Recurse -Force - } -} - -foreach ($case in $cases) { - $installer = Join-Path $PSScriptRoot $case.Installer - $aliasBinary = Join-Path $installDir "bin\$($case.Alias).exe" +foreach ($name in $installers) { + $installer = Join-Path $PSScriptRoot $name $tokens = $null $parseErrors = $null @@ -51,26 +49,8 @@ foreach ($case in $cases) { }, $true) if ($archiveCommands.Count -ne 1 -or $archiveCommands[0].GetCommandName() -ne $qualifiedCommand) { - throw "$installer must call $qualifiedCommand exactly once" + throw "$installer must call $qualifiedCommand exactly once (issue #651)" } - try { - Remove-Install - - & $installer -Alias $case.Alias -Version $case.Version -SkipGit $true - - if (!(Test-Path -LiteralPath $aliasBinary)) { - throw "$($case.Installer) did not place $($case.Alias).exe at $aliasBinary (extraction failed?)" - } - - $versionOutput = & $aliasBinary --version - if ($case.ExpectVersion -and $versionOutput -notmatch [regex]::Escape($case.Version)) { - throw "Version mismatch: expected '$($case.Version)' in output, got '$versionOutput'" - } - - Write-Host "$($case.Installer) E2E passed: $($case.Alias).exe installed, version '$versionOutput'" - } - finally { - Remove-Install - } + Write-Host "$name calls $qualifiedCommand exactly once (issue #651 guard passed)" } From 6d01e4fcb243b3087f4102b9b7f2eff894177194 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Fri, 28 Aug 2026 13:44:15 -0700 Subject: [PATCH 4/5] test(install): assert the qualification invariant, not a call count Per review on #652: the guard checked `Count -ne 1`, which breaks the moment an installer legitimately extracts more than once and points its error at the count rather than the qualification. Invert it to the actual invariant -- every *Expand-Archive command must be the qualified Microsoft.PowerShell.Archive\Expand-Archive form, and at least one must exist so a removed extraction can't pass vacuously. Co-Authored-By: Claude --- scripts/install-windows-test.ps1 | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/scripts/install-windows-test.ps1 b/scripts/install-windows-test.ps1 index 3a4f27a4..c51d9e65 100644 --- a/scripts/install-windows-test.ps1 +++ b/scripts/install-windows-test.ps1 @@ -48,9 +48,17 @@ foreach ($name in $installers) { $node.GetCommandName() -like "*Expand-Archive" }, $true) - if ($archiveCommands.Count -ne 1 -or $archiveCommands[0].GetCommandName() -ne $qualifiedCommand) { - throw "$installer must call $qualifiedCommand exactly once (issue #651)" + # The invariant is "no unqualified extraction call," not a fixed count: every + # *Expand-Archive invocation must be the qualified form, and there must be at + # least one (so a removed/renamed extraction can't pass vacuously). This won't + # break if an installer legitimately extracts more than once. + if ($archiveCommands.Count -lt 1) { + throw "$installer calls no Expand-Archive; expected $qualifiedCommand (issue #651)" + } + $unqualified = $archiveCommands | Where-Object { $_.GetCommandName() -ne $qualifiedCommand } + if ($unqualified) { + throw "$installer must call $qualifiedCommand (found unqualified Expand-Archive; issue #651)" } - Write-Host "$name calls $qualifiedCommand exactly once (issue #651 guard passed)" + Write-Host "$name calls $qualifiedCommand ($($archiveCommands.Count)x), all qualified (issue #651 guard passed)" } From 65b47ca8206eae8449ad0b5cd681943782d686d9 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Fri, 28 Aug 2026 14:06:49 -0700 Subject: [PATCH 5/5] chore: trim install-windows-test comments to inline issue ref Drop the two explanatory comment blocks; keep the issue reference as a trailing comment on the $qualifiedCommand line. Logic unchanged. Co-Authored-By: Claude --- scripts/install-windows-test.ps1 | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/scripts/install-windows-test.ps1 b/scripts/install-windows-test.ps1 index c51d9e65..d51e7419 100644 --- a/scripts/install-windows-test.ps1 +++ b/scripts/install-windows-test.ps1 @@ -14,13 +14,7 @@ $ErrorActionPreference = "Stop" -# Guard against the Pscx-shadowing regression from https://github.com/slackapi/slack-cli/issues/651: -# Pscx 3.3.2 ships its own Expand-Archive that shadows the built-in, so the extraction call must be -# qualified as Microsoft.PowerShell.Archive\Expand-Archive. We assert this statically (parse the AST, -# not run the installer) because the installers' post-install courtesy check hangs under pwsh 7 on -# current Windows runner images -- `& slack _fingerprint | Tee-Object -Variable | Out-Null` blocks -# with no console -- so a real end-to-end install is not yet runnable in CI. Tracked separately. -$qualifiedCommand = "Microsoft.PowerShell.Archive\Expand-Archive" +$qualifiedCommand = "Microsoft.PowerShell.Archive\Expand-Archive" # https://github.com/slackapi/slack-cli/issues/651 $installers = @( "install-windows.ps1", @@ -48,10 +42,6 @@ foreach ($name in $installers) { $node.GetCommandName() -like "*Expand-Archive" }, $true) - # The invariant is "no unqualified extraction call," not a fixed count: every - # *Expand-Archive invocation must be the qualified form, and there must be at - # least one (so a removed/renamed extraction can't pass vacuously). This won't - # break if an installer legitimately extracts more than once. if ($archiveCommands.Count -lt 1) { throw "$installer calls no Expand-Archive; expected $qualifiedCommand (issue #651)" }