diff --git a/Get-DSAManifest.ps1 b/Get-DSAManifest.ps1 index 27d999a..f5d530e 100644 --- a/Get-DSAManifest.ps1 +++ b/Get-DSAManifest.ps1 @@ -53,9 +53,9 @@ $DsaExecutablePath = Join-Path -Path $Global:ToolsDirectory -ChildPath 'dsa-coll New-Item -Path $Global:ToolsDirectory -ItemType Directory -Force -ErrorAction Stop | Out-Null Write-Output "Downloading dsa-collect.exe to ${DsaExecutablePath}" -$DsaDownloadedFile = Download-File -URL $DsaDownloadUrl -File $DsaExecutablePath -ErrorAction Stop +$DsaDownloadedFile = Download-FileDirectly -URL $DsaDownloadUrl -File $DsaExecutablePath -ErrorAction Stop -# Download-File can return nothing on failure. Do not run a stale executable. +# Require a returned path and a nonempty download before starting the collector. If ( [string]::IsNullOrWhiteSpace([string]$DsaDownloadedFile) -or !(Test-Path -LiteralPath $DsaExecutablePath -PathType Leaf) ) { Throw "Failed to download dsa-collect.exe to ${DsaExecutablePath}" diff --git a/README.md b/README.md index 256fa9d..854b93a 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,8 @@ For DSA collection, paste [rmm/Get-DSAManifest.ps1](rmm/Get-DSAManifest.ps1) int For download failures, use the standalone [Test-FileDownload.ps1](Test-FileDownload.ps1) diagnostic. See [the RMM caller, report guide, and shared caller audit](docs/FileDownload-Diagnostics.md). It compares the existing helper, BITS, and direct GET without changing production download behavior or running downloaded files. +`Get-DSAManifest.ps1` uses `Download-FileDirectly`, which passes the original URL to BITS without preliminary `Get-AbsoluteURI` or `IsURLValid` requests. It accepts the same URL and optional `-File` arguments as `Download-File`, waits for completion, and throws on BITS errors. If `-File` is omitted, its temporary filename comes from the original URL; supply `-File` for extensionless URLs or URLs with query strings. Existing `Download-File` callers retain their previous behavior. Publish `Tools.ps1` together with `Get-DSAManifest.ps1` so the new helper is available. + ## Uploading files After dot-sourcing `Tools.ps1`, scripts can upload a completed file to the Emberkom FileDrop: diff --git a/Tools.ps1 b/Tools.ps1 index 1886407..21ccb35 100644 --- a/Tools.ps1 +++ b/Tools.ps1 @@ -209,6 +209,19 @@ Function Download-File { Return $File } +# Downloads from the original URL; BITS handles redirects without preliminary web requests. +# Without -File, the temporary filename comes from the original URL, not its redirect target. +Function Download-FileDirectly { + param( + [Parameter(Mandatory=$true,ValueFromPipeline=$true)][string]$URL, + [Parameter(Mandatory=$false,ValueFromPipeline=$false)][string]$File + ) + If ( [string]::IsNullOrEmpty($File) ) { $File = '{0}{1}' -f (Get-TempPath), (Split-Path $URL -Leaf) } + Try { Start-BitsTransfer -Source $URL -Destination $File -ErrorAction Stop } + Catch { Throw "BITS download failed from ${URL} to ${File}: $($_.Exception.Message)" } + Return $File +} + # Upload a file to a LiquidFiles Filedrop and submit its notification message. Function Upload-File { <# diff --git a/tests/Download-File.Tests.ps1 b/tests/Download-File.Tests.ps1 index 8ba9781..8ab95d3 100644 --- a/tests/Download-File.Tests.ps1 +++ b/tests/Download-File.Tests.ps1 @@ -6,16 +6,26 @@ $DownloadTokens = $null $DownloadParseErrors = $null $DownloadAst = [Management.Automation.Language.Parser]::ParseFile($DownloadToolsPath, [ref]$DownloadTokens, [ref]$DownloadParseErrors) If ($DownloadParseErrors.Count) { Throw ($DownloadParseErrors | Out-String) } -$DownloadDefinition = $DownloadAst.Find({ - param($Node) - $Node -is [Management.Automation.Language.FunctionDefinitionAst] -and $Node.Name -eq 'Download-File' -}, $true) -. ([scriptblock]::Create($DownloadDefinition.Extent.Text)) +Foreach ($DownloadFunctionName in @('Download-File','Download-FileDirectly')) { + $DownloadDefinition = $DownloadAst.Find({ + param($Node) + $Node -is [Management.Automation.Language.FunctionDefinitionAst] -and $Node.Name -eq $DownloadFunctionName + }, $true) + If (!$DownloadDefinition) { Throw "Missing helper: $DownloadFunctionName" } + . ([scriptblock]::Create($DownloadDefinition.Extent.Text)) +} -Function Get-AbsoluteURI { param($URL) Return $URL } +Function Get-AbsoluteURI { + param($URL) + If ($script:DownloadRejectPreflight) { Throw 'Direct download must not resolve the URL first.' } + Return $URL +} +Function IsURLValid { Throw 'Direct download must not make a preliminary validation request.' } +Function Get-TempPath { Return ($script:DownloadTestRoot + '\') } Function Start-BitsTransfer { [CmdletBinding()] param($Source, $Destination) + $script:DownloadRecordedSource = $Source If ($script:DownloadTestMode -eq 'success') { [IO.File]::WriteAllText($Destination, 'Synthetic download.') Return @@ -28,32 +38,53 @@ Function Start-BitsTransfer { $DownloadTestRoot = Join-Path ([IO.Path]::GetTempPath()) ('download-file-tests-' + [guid]::NewGuid().ToString('N')) $null = New-Item -Path $DownloadTestRoot -ItemType Directory Try { - $DownloadTestFile = Join-Path $DownloadTestRoot 'collector $test.exe' - $script:DownloadTestMode = 'success' - $DownloadResult = Download-File -URL 'https://example.invalid/collector.exe' -File $DownloadTestFile - If ($DownloadResult -cne $DownloadTestFile -or [IO.File]::ReadAllText($DownloadResult) -cne 'Synthetic download.') { - Throw 'Successful download did not return the completed file.' - } - Write-Host 'PASS: successful download returns its destination.' + Foreach ($DownloadFunctionName in @('Download-File','Download-FileDirectly')) { + $script:DownloadRejectPreflight = ($DownloadFunctionName -eq 'Download-FileDirectly') + $DownloadTestFile = Join-Path $DownloadTestRoot 'collector $test.exe' + [IO.File]::WriteAllBytes($DownloadTestFile, [byte[]]@()) + $script:DownloadTestMode = 'success' + $DownloadTestUrl = 'https://example.invalid/collector.exe?token=original' + $DownloadResult = @(& $DownloadFunctionName -URL $DownloadTestUrl -File $DownloadTestFile) + If ($DownloadResult.Count -ne 1 -or $DownloadResult[0] -cne $DownloadTestFile -or + [IO.File]::ReadAllText($DownloadResult[0]) -cne 'Synthetic download.' -or + $script:DownloadRecordedSource -cne $DownloadTestUrl) { + Throw 'Successful download must pass the source URL intact, replace an empty file, and return exactly one completed path.' + } + Write-Host "PASS: ${DownloadFunctionName}: URL forwarding, empty destination overwrite, and one completed path." - Foreach ($script:DownloadTestMode in @('nonterminating-error','terminating-error')) { - # Match an RMM that uses the normal Continue preference and does not pass -ErrorAction. - $ErrorActionPreference = 'Continue' - $DownloadCaught = $null - $DownloadReturnedPaths = @() - Try { - Download-File -URL 'https://example.invalid/collector.exe' -File $DownloadTestFile | - ForEach-Object { $DownloadReturnedPaths += $_ } + $DownloadAutomaticFile = Join-Path $DownloadTestRoot 'collector.msi' + Foreach ($DownloadBinding in @('positional','pipeline')) { + $DownloadResult = @(If ($DownloadBinding -eq 'positional') { + & $DownloadFunctionName 'https://example.invalid/collector.msi' + } Else { + 'https://example.invalid/collector.msi' | & $DownloadFunctionName + }) + If ($DownloadResult.Count -ne 1 -or $DownloadResult[0] -cne $DownloadAutomaticFile -or + [IO.File]::ReadAllText($DownloadAutomaticFile) -cne 'Synthetic download.') { + Throw "Automatic filename or $DownloadBinding URL binding failed." + } } - Catch { $DownloadCaught = $_ } - Finally { $ErrorActionPreference = 'Stop' } - If ($null -eq $DownloadCaught -or $DownloadCaught.Exception.Message -notlike '*Synthetic BITS*failure*') { - Throw 'Failed transfer must throw and preserve the BITS error.' + Write-Host "PASS: ${DownloadFunctionName}: positional/pipeline URL and automatic filename with extension." + + Foreach ($script:DownloadTestMode in @('nonterminating-error','terminating-error')) { + # Match an RMM that uses the normal Continue preference and does not pass -ErrorAction. + $ErrorActionPreference = 'Continue' + $DownloadCaught = $null + $DownloadReturnedPaths = @() + Try { + & $DownloadFunctionName -URL 'https://example.invalid/collector.exe' -File $DownloadTestFile | + ForEach-Object { $DownloadReturnedPaths += $_ } + } + Catch { $DownloadCaught = $_ } + Finally { $ErrorActionPreference = 'Stop' } + If ($null -eq $DownloadCaught -or $DownloadCaught.Exception.Message -notlike '*Synthetic BITS*failure*') { + Throw 'Failed transfer must throw and preserve the BITS error.' + } + If ($DownloadReturnedPaths.Count -ne 0) { Throw 'Failed transfer returned a success path.' } + Write-Host "PASS: ${DownloadFunctionName}: $script:DownloadTestMode stops the caller and preserves the transfer error." } - If ($DownloadReturnedPaths.Count -ne 0) { Throw 'Failed transfer returned a success path.' } - Write-Host "PASS: $script:DownloadTestMode stops the caller and preserves the transfer error." } - Write-Host "All Download-File tests passed on PowerShell $($PSVersionTable.PSVersion), $([IntPtr]::Size * 8)-bit." + Write-Host "All download helper tests passed on PowerShell $($PSVersionTable.PSVersion), $([IntPtr]::Size * 8)-bit." } Finally { $DownloadResolvedRoot = (Resolve-Path -LiteralPath $DownloadTestRoot).Path diff --git a/tests/Get-DSAManifest.Tests.ps1 b/tests/Get-DSAManifest.Tests.ps1 index 891aa2d..b379580 100644 --- a/tests/Get-DSAManifest.Tests.ps1 +++ b/tests/Get-DSAManifest.Tests.ps1 @@ -9,7 +9,8 @@ $null = [Management.Automation.Language.Parser]::ParseFile($DsaScriptUnderTest, If ($DsaErrors.Count) { Throw ($DsaErrors | Out-String) } Function Assert-Dsa { param([bool]$Condition, [string]$Description) If (!$Condition) { Throw $Description } } -Function Download-File { +Function Download-File { Throw 'DSA must use Download-FileDirectly.' } +Function Download-FileDirectly { [CmdletBinding()] param($URL, $File) $script:DsaDownloadCalls++ @@ -80,6 +81,7 @@ Function Invoke-DsaCase { Assert-Dsa ($script:DsaDownloadCalls -eq 0) 'Invalid flag must fail before downloading.' Return } + Assert-Dsa ($script:DsaDownloadCalls -eq 1) 'Expected one direct download.' If ($Mode -in @('empty-download','download-failure')) { Assert-Dsa ($null -ne $DsaCaught) 'Download failure must stop collection.' Assert-Dsa ($script:DsaProcessCalls -eq 0 -and $script:DsaRecordedUploads.Count -eq 0) 'Download failure must prevent execution and upload.'