From 1ed2acba1a951b276dabfd283277afc6e8310866 Mon Sep 17 00:00:00 2001 From: Jan Friedrich Date: Sun, 27 Sep 2026 21:16:37 +0200 Subject: [PATCH] bind a release to the commit it was built from #330 build-release.ps1 built the binaries from the working tree but archived the local master ref, so the signed source zip need not match the signed binaries. * Refuse a dirty tree, archive HEAD, and ship a signed .manifest recording the commit and the artifact set. No origin/master check: that needs the network and forbids cutting a release from a tag or a release branch. * Both verifiers check the commit against the zip archive comment and the listed set against the files present. The hash and signature loops only see the files that are there, so an artifact deleted with its .sha512 and .asc passed before. * Everything the release writes gets LF, asserted in the tests. Set-Content writes CRLF on Windows, where a trailing CR breaks sha512sum on macOS and makes every manifest name compare unequal. Includes the .sha512 fix from #328; #328 gets rebased onto this. * sign-log4net-libraries.sh gains set -euo pipefail and shopt -s nullglob. An empty directory iterated the glob patterns and still exited 0. audit da18b6fd-f025 --- CLAUDE.md | 12 ++ scripts/FakeCommands.TestHelper.ps1 | 23 ++- scripts/build-preview.ps1 | 3 +- scripts/build-release.Tests.ps1 | 65 ++++++- scripts/build-release.ps1 | 146 ++++++++++++--- scripts/sign-log4net-libraries.sh | 7 +- scripts/verify-release.Tests.ps1 | 90 ++++++++- scripts/verify-release.ps1 | 172 ++++++++++++++---- scripts/verify-release.sh | 58 ++++++ .../3.5.0/330-release-from-one-commit.xml | 14 ++ 10 files changed, 515 insertions(+), 75 deletions(-) create mode 100644 src/changelog/3.5.0/330-release-from-one-commit.xml diff --git a/CLAUDE.md b/CLAUDE.md index 8792b2c35..397f09ea4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -247,6 +247,18 @@ The manual lives in `src/site/antora/modules/ROOT/pages/`. A new appender page n not one: the page itself, an `xref` line in `nav.adoc` (kept alphabetical), and the appender table in `manual/configuration/appenders.adoc`. +## Release scripts + +- **Every file `build-release.ps1` writes gets LF on every platform.** `Set-Content` writes + `[Environment]::NewLine`; use `-NoNewline` with the lines joined by `` `n ``. A CR is invisible in + an editor, and `sha512sum` on macOS reads it as part of the file name. +- **Each release ships the verifier built with it**, so never add backward compatibility to + `verify-release.ps1` or `.sh`: a compatibility skip path is a hole, not a courtesy. +- The scripts are tested with Pester through `scripts/FakeCommands.TestHelper.ps1`, which shadows + `dotnet`, `git`, `gpg`, `zip` and `mvnw` with shims on a prepended `PATH`, because Pester `Mock` + intercepts only functions and cmdlets. CI runs the suite on macOS, Ubuntu and Windows, so keep + the helper's `$IsWindows` branch working and put byte-level assertions where all three see them. + ## Security findings **[AGENTS.md](AGENTS.md) decides whether something is in scope and whether it is a vulnerability.** diff --git a/scripts/FakeCommands.TestHelper.ps1 b/scripts/FakeCommands.TestHelper.ps1 index 280ea4cd6..da9ff21e8 100644 --- a/scripts/FakeCommands.TestHelper.ps1 +++ b/scripts/FakeCommands.TestHelper.ps1 @@ -9,12 +9,24 @@ commands ran without building, signing, tagging or pushing anything. #> +# The commit the fake "git rev-parse" reports, so a test can assert what was archived and recorded. +$script:FakeCommitHash = 'a1b2c3d4e5f6071829a3b4c5d6e7f80912345678' + # Logs its call, fails when named in FAKE_FAIL, and otherwise writes the files the real command -# would, where it is told to, so a wrong output path fails as it would for real. +# would, where it is told to, so a wrong output path fails as it would for real. "git status" and +# "git rev-parse" answer on stdout instead, FAKE_DIRTY faking an unclean tree. $script:FakeCommand = @' param([string]$Name) Add-Content -Path $env:FAKE_LOG -Value "$Name $($args -join ' ')" if ($env:FAKE_FAIL -eq $Name) { exit 1 } +if ($Name -eq 'git') +{ + switch ($args[0]) + { + 'status' { if ($env:FAKE_DIRTY) { ' M src/log4net/Core/LogImpl.cs' } } + 'rev-parse' { $env:FAKE_SHA } + } +} $outputs = switch ($Name) { 'dotnet' @@ -61,7 +73,8 @@ function New-ScratchTree function Invoke-InScratchTree { - param ([Parameter(Mandatory)][string]$Root, [Parameter(Mandatory)][string]$Script, [string]$Fail) + param ([Parameter(Mandatory)][string]$Root, [Parameter(Mandatory)][string]$Script, [string]$Fail, + [switch]$Dirty) $log = Join-Path $Root 'calls.log' New-Item -ItemType File -Path $log -Force | Out-Null @@ -71,6 +84,8 @@ function Invoke-InScratchTree $env:PATH = (Join-Path $Root 'fakebin') + [System.IO.Path]::PathSeparator + $path $env:FAKE_LOG = $log $env:FAKE_FAIL = $Fail + $env:FAKE_SHA = $script:FakeCommitHash + $env:FAKE_DIRTY = $Dirty ? '1' : '' # NonInteractive makes the confirmation pause throw, standing in for a release manager who says no. $output = pwsh -NoProfile -NonInteractive -File (Join-Path $Root 'scripts' $Script) 2>&1 $exitCode = $LASTEXITCODE @@ -78,11 +93,13 @@ function Invoke-InScratchTree finally { $env:PATH = $path - Remove-Item Env:FAKE_LOG, Env:FAKE_FAIL -ErrorAction SilentlyContinue + Remove-Item Env:FAKE_LOG, Env:FAKE_FAIL, Env:FAKE_SHA, Env:FAKE_DIRTY -ErrorAction SilentlyContinue } return [pscustomobject]@{ ExitCode = $exitCode Calls = @(Get-Content $log | ForEach-Object { ($_ -split ' ')[0..1] -join ' ' }) + # The whole logged line, for an assertion about an argument rather than about the command. + Lines = @(Get-Content $log) Output = $output -join [Environment]::NewLine } } diff --git a/scripts/build-preview.ps1 b/scripts/build-preview.ps1 index 4b535934d..193f27861 100644 --- a/scripts/build-preview.ps1 +++ b/scripts/build-preview.ps1 @@ -2,7 +2,8 @@ param( $Version = '3.5.0', - $Preview = '1' + [ValidateRange('Positive')] + [int]$Preview = 1 ) Set-StrictMode -Version Latest diff --git a/scripts/build-release.Tests.ps1 b/scripts/build-release.Tests.ps1 index 125634712..dc6ac8508 100644 --- a/scripts/build-release.Tests.ps1 +++ b/scripts/build-release.Tests.ps1 @@ -28,14 +28,15 @@ Describe 'build-release.ps1' { $result = Invoke-InScratchTree -Root $script:Root -Script 'build-release.ps1' -Fail 'dotnet' $result.ExitCode | Should -Not -Be 0 - $result.Calls | Should -Be @('dotnet test') + $result.Calls | Should -Be @('git status', 'git rev-parse', 'dotnet test') } It 'builds no site when signing fails' { $result = Invoke-InScratchTree -Root $script:Root -Script 'build-release.ps1' -Fail 'gpg' $result.ExitCode | Should -Not -Be 0 - $result.Calls | Should -Be @('dotnet test', 'git archive', 'zip -r', 'gpg --armor') + $result.Calls | Should -Be @('git status', 'git rev-parse', 'dotnet test', 'git archive', 'zip -r', + 'gpg --armor') } It 'asks for no tag when the site build fails' { @@ -46,20 +47,72 @@ Describe 'build-release.ps1' { $result.Calls[-1] | Should -Be 'mvnw site' } - It 'signs all six artifacts and tags nothing without confirmation' { + It 'signs all seven artifacts and tags nothing without confirmation' { $result = Invoke-InScratchTree -Root $script:Root -Script 'build-release.ps1' $result.ExitCode | Should -Not -Be 0 $result.Output | Should -BeLike '*NonInteractive*' - $result.Calls | Should -Be @('dotnet test', 'git archive', 'zip -r', - 'gpg --armor', 'gpg --armor', 'gpg --armor', 'gpg --armor', 'gpg --armor', 'gpg --armor', 'mvnw site') + $result.Calls | Should -Be @('git status', 'git rev-parse', 'dotnet test', 'git archive', 'zip -r', + 'gpg --armor', 'gpg --armor', 'gpg --armor', 'gpg --armor', 'gpg --armor', 'gpg --armor', + 'gpg --armor', 'mvnw site') } It 'ships no artifact without a hash' { Invoke-InScratchTree -Root $script:Root -Script 'build-release.ps1' | Out-Null $artifacts = Get-ChildItem (Join-Path $script:Root 'build' 'artifacts') -Exclude '*.sha512', '*.asc' - $artifacts.Name | Should -HaveCount 6 + $artifacts.Name | Should -HaveCount 7 $artifacts | Where-Object { !(Test-Path "$($_.FullName).sha512") } | Should -BeNullOrEmpty } + + It 'writes every hash as one LF-terminated line' { + Invoke-InScratchTree -Root $script:Root -Script 'build-release.ps1' | Out-Null + + $hashes = Get-ChildItem (Join-Path $script:Root 'build' 'artifacts') -Filter '*.sha512' + $hashes | Should -HaveCount 7 + foreach ($hash in $hashes) + { + [System.IO.File]::ReadAllText($hash.FullName) | Should -MatchExactly '^[0-9a-f]{128} \*\./\S+\n\z' + } + } + + It 'removes the artifacts of an earlier run' { + $stale = Join-Path $script:Root 'build' 'artifacts' 'apache-log4net-0.0.0.nupkg' + New-Item -ItemType File -Force -Path $stale | Out-Null + + Invoke-InScratchTree -Root $script:Root -Script 'build-release.ps1' | Out-Null + + Test-Path $stale | Should -BeFalse + } + + It 'builds nothing when the working tree is dirty' { + $result = Invoke-InScratchTree -Root $script:Root -Script 'build-release.ps1' -Dirty + + $result.ExitCode | Should -Not -Be 0 + $result.Calls | Should -Be @('git status') + } + + It 'archives the commit it built' { + $result = Invoke-InScratchTree -Root $script:Root -Script 'build-release.ps1' + + $archive = @($result.Lines | Where-Object { $_ -like 'git archive*' }) + $archive | Should -HaveCount 1 + $archive[0] | Should -BeLike "* $script:FakeCommitHash" + } + + It 'records the commit and the artifact set in the manifest' { + Invoke-InScratchTree -Root $script:Root -Script 'build-release.ps1' | Out-Null + + $artifacts = Join-Path $script:Root 'build' 'artifacts' + $manifest = Get-ChildItem $artifacts -Filter '*.manifest' + $manifest | Should -HaveCount 1 + $lines = Get-Content $manifest.FullName + $lines | Should -Contain "commit=$script:FakeCommitHash" + $listed = @($lines | Where-Object { $_ -like 'artifact=*' }) + $listed | Should -HaveCount 7 + $present = @(Get-ChildItem $artifacts -Exclude '*.sha512', '*.asc' | ForEach-Object { "artifact=$($_.Name)" }) + $listed | Sort-Object | Should -Be ($present | Sort-Object) + # LF on every platform, or verify-release.sh compares names with a trailing CR. + [System.IO.File]::ReadAllText($manifest.FullName) | Should -Not -BeLike "*`r*" + } } diff --git a/scripts/build-release.ps1 b/scripts/build-release.ps1 index 82e4cef01..61b25ab08 100644 --- a/scripts/build-release.ps1 +++ b/scripts/build-release.ps1 @@ -1,7 +1,9 @@ #Requires -Version 7.4 param( - $Version = '3.5.0' + $Version = '3.5.0', + [ValidateRange('Positive')] + [int]$Rc = 1 ) Set-StrictMode -Version Latest @@ -11,6 +13,89 @@ $ErrorActionPreference = 'Stop' # Only honored from PowerShell 7.4, hence the #Requires above. $PSNativeCommandUseErrorActionPreference = $true +$Root = "$PSScriptRoot/.." +$ArtifactDirectory = "$Root/build/artifacts" +$ManifestName = "apache-log4net-$Version.manifest" +$ArtifactNames = @( + "apache-log4net.$Version.nupkg", + "apache-log4net.Ext.Mail.$Version.nupkg", + "apache-log4net-source-$Version.zip", + "apache-log4net-binaries-$Version.zip", + 'verify-release.ps1', + 'verify-release.sh', + $ManifestName) + +# Paired, so a throw inside cannot leave the caller in the wrong directory. +function Invoke-InDirectory +{ + param + ( + [Parameter(Mandatory=$true, HelpMessage='The directory to run in.')] + [string]$Directory, + [Parameter(Mandatory=$true, HelpMessage='What to run there.')] + [scriptblock]$Action + ) + + Push-Location $Directory + try + { + & $Action + } + finally + { + Pop-Location + } +} + +# The binaries come from the working tree, the source archive from a git ref, and both are signed +# as one release, so they must come from one commit. +function Get-ReleaseCommit +{ + Invoke-InDirectory $Root { + $GitStatus = git status --porcelain + if ($GitStatus) + { + throw "the working tree is not clean, so the binaries and the source archive would not match:$([Environment]::NewLine)$($GitStatus -join [Environment]::NewLine)" + } + + git rev-parse --verify HEAD + } +} + +# Records the commit and the artifact set, and is signed with them. Without the set a missing +# artifact goes unnoticed, since the verifiers can only check the files that are there. +function Write-Manifest +{ + param + ( + [Parameter(Mandatory=$true, HelpMessage='The commit the release was built from.')] + [string]$Commit, + [Parameter(Mandatory=$true, HelpMessage='The artifact names, the manifest included.')] + [string[]]$Names + ) + + # LF on every platform: Set-Content would write CRLF on Windows and the artifact names would then + # carry a trailing CR into the comparison in verify-release.sh. + $Lines = @("commit=$Commit") + ($Names | ForEach-Object { "artifact=$_" }) + Set-Content -Path $ArtifactDirectory/$ManifestName -NoNewline -Value (($Lines -join "`n") + "`n") +} + +# Tested rather than silenced: -ErrorAction SilentlyContinue would also swallow a delete that +# failed, leaving a stale artifact behind for whoever copies the directory to dist. +function Remove-Directory +{ + param + ( + [Parameter(Mandatory=$true, HelpMessage='The directory to remove if it exists.')] + [string]$Directory + ) + + if (Test-Path $Directory) + { + Remove-Item $Directory -Force -Recurse + } +} + function Write-HashAndSignature { param @@ -21,42 +106,49 @@ function Write-HashAndSignature $File.FullName $ComputedHash = (Get-FileHash -Algorithm 'SHA512' $File).Hash.ToLowerInvariant() $ComputedHash - Set-Content -Path "$($File.FullName).sha512" -Value "$ComputedHash *./$($File.Name)" + # LF on every platform: the macOS sha512sum reads a CR from a Windows build as part of the file name. + Set-Content -NoNewline -Path "$($File.FullName).sha512" -Value "$ComputedHash *./$($File.Name)`n" gpg --armor --output "$($File.FullName).asc" --detach-sig $File.FullName } -"cleaning $PSScriptRoot/../build/ ..." -Remove-Item $PSScriptRoot/../build/ -Force -Recurse -ErrorAction SilentlyContinue +"cleaning $Root/build/ ..." +Remove-Directory $Root/build/ + +'verifying release tree ...' +$CommitHash = Get-ReleaseCommit + 'building ...' -dotnet test -c Release "-p:GeneratePackages=true;PackageVersion=$Version" $PSScriptRoot/../src/log4net.sln +dotnet test -c Release "-p:GeneratePackages=true;PackageVersion=$Version" $Root/src/log4net.sln + 'compressing source ...' -pushd $PSScriptRoot/.. -git archive --format=zip --output $PSScriptRoot/../build/artifacts/apache-log4net-source-$Version.zip master -popd +Invoke-InDirectory $Root { + git archive --format=zip --output $ArtifactDirectory/apache-log4net-source-$Version.zip $CommitHash +} + 'compressing binaries ...' -Copy-Item $PSScriptRoot/verify-release.ps1, $PSScriptRoot/verify-release.sh $PSScriptRoot/../build/artifacts/ -Copy-Item $PSScriptRoot/../LICENSE $PSScriptRoot/../build/Release/ -Copy-Item $PSScriptRoot/../NOTICE $PSScriptRoot/../build/Release/ -pushd $PSScriptRoot/../build/Release -zip -r $PSScriptRoot/../build/artifacts/apache-log4net-binaries-$Version.zip . -popd +Copy-Item $PSScriptRoot/verify-release.ps1, $PSScriptRoot/verify-release.sh $ArtifactDirectory/ +Copy-Item $Root/LICENSE, $Root/NOTICE $Root/build/Release/ +Invoke-InDirectory $Root/build/Release { + zip -r $ArtifactDirectory/apache-log4net-binaries-$Version.zip . +} + 'signing ...' -Move-Item $PSScriptRoot/../build/artifacts/log4net.$Version.nupkg $PSScriptRoot/../build/artifacts/apache-log4net.$Version.nupkg -Write-HashAndSignature $PSScriptRoot/../build/artifacts/apache-log4net.$Version.nupkg -Move-Item $PSScriptRoot/../build/artifacts/log4net.Ext.Mail.$Version.nupkg $PSScriptRoot/../build/artifacts/apache-log4net.Ext.Mail.$Version.nupkg -Write-HashAndSignature $PSScriptRoot/../build/artifacts/apache-log4net.Ext.Mail.$Version.nupkg -Write-HashAndSignature $PSScriptRoot/../build/artifacts/apache-log4net-source-$Version.zip -Write-HashAndSignature $PSScriptRoot/../build/artifacts/apache-log4net-binaries-$Version.zip -Write-HashAndSignature $PSScriptRoot/../build/artifacts/verify-release.ps1 -Write-HashAndSignature $PSScriptRoot/../build/artifacts/verify-release.sh +Move-Item $ArtifactDirectory/log4net.$Version.nupkg $ArtifactDirectory/apache-log4net.$Version.nupkg +Move-Item $ArtifactDirectory/log4net.Ext.Mail.$Version.nupkg $ArtifactDirectory/apache-log4net.Ext.Mail.$Version.nupkg +Write-Manifest -Commit $CommitHash -Names $ArtifactNames +foreach ($ArtifactName in $ArtifactNames) +{ + Write-HashAndSignature $ArtifactDirectory/$ArtifactName +} + 'cleaning site ...' -Remove-Item $PSScriptRoot/../target/ -Force -Recurse -ErrorAction SilentlyContinue +Remove-Directory $Root/target/ + 'building site ...' -pushd $PSScriptRoot/.. -./mvnw site -popd +Invoke-InDirectory $Root { ./mvnw site } + 'creating tag ...' pause -git tag "rc/$Version-rc1" +git tag "rc/$Version-rc$Rc" 'pushing tag ...' git push --tags diff --git a/scripts/sign-log4net-libraries.sh b/scripts/sign-log4net-libraries.sh index 59702f74d..be34edd21 100644 --- a/scripts/sign-log4net-libraries.sh +++ b/scripts/sign-log4net-libraries.sh @@ -1,10 +1,15 @@ #!/bin/bash # see https://infra.apache.org/release-signing#openpgp-ascii-detach-sig +set -euo pipefail + +# Without nullglob an empty directory iterates the patterns, so the guard below never fires. +shopt -s nullglob + DID_SOMETHING=0 for f in *log4net*.nupkg *log4net*.zip; do DID_SOMETHING=1 echo "signing: $f" - gpg --armor --output $f.asc --detach-sig $f + gpg --armor --output "$f.asc" --detach-sig "$f" done if test "$DID_SOMETHING" = "0"; then diff --git a/scripts/verify-release.Tests.ps1 b/scripts/verify-release.Tests.ps1 index 1af5f6fa4..940abbe69 100644 --- a/scripts/verify-release.Tests.ps1 +++ b/scripts/verify-release.Tests.ps1 @@ -8,7 +8,9 @@ .DESCRIPTION Only the stages before the KEYS download are covered, because everything after it needs the network and a gpg installation. Those stages are the ones that used to pass silently: a release - with no artifacts, an artifact with no hash file, and an artifact whose hash does not match. + with no artifacts, an artifact with no hash file, an artifact whose hash does not match, an + artifact set that disagrees with the manifest, and a source archive that is not the commit the + manifest records. Run with: Invoke-Pester ./scripts/verify-release.Tests.ps1 #> @@ -24,10 +26,12 @@ BeforeAll { function Add-Artifact { - param ([string]$Directory, [string]$Name = 'apache-log4net-binaries-9.9.9.zip', [switch]$WithHash, [string]$Hash) + param ([string]$Directory, [string]$Name = 'apache-log4net-binaries-9.9.9.zip', [switch]$WithHash, + [string]$Hash, [string]$Content = 'artifact contents') $path = Join-Path $Directory $Name - 'artifact contents' | Out-File -FilePath $path -Encoding ascii + # NoNewline, so an empty Content really is a zero byte file. + $Content | Out-File -FilePath $path -Encoding ascii -NoNewline if ($WithHash) { if (!$Hash) @@ -38,6 +42,45 @@ BeforeAll { } return $path } + + # A zip with a commit id in its archive comment, as git archive produces. + function Add-SourceArchive + { + param ([string]$Directory, [string]$Commit, [string]$Name = 'apache-log4net-source-9.9.9.zip') + + $path = Join-Path $Directory $Name + $zip = [System.IO.Compression.ZipFile]::Open($path, 'Create') + try + { + $zip.Comment = $Commit + $writer = New-Object System.IO.StreamWriter $zip.CreateEntry('README.md').Open() + try { $writer.Write('sources') } finally { $writer.Dispose() } + } + finally + { + $zip.Dispose() + } + "$((Get-FileHash -Algorithm SHA512 $path).Hash) *$Name" | Out-File -FilePath "$path.sha512" -Encoding ascii + return $path + } + + # The manifest lists itself, as the one build-release.ps1 writes does. + function Add-Manifest + { + param ([string]$Directory, [string]$Commit, [string[]]$Listed, + [string]$Name = 'apache-log4net-9.9.9.manifest') + + $path = Join-Path $Directory $Name + $lines = @() + if ($Commit) + { + $lines += "commit=$Commit" + } + $lines += @(@($Listed) + $Name | ForEach-Object { "artifact=$_" }) + $lines | Out-File -FilePath $path -Encoding ascii + "$((Get-FileHash -Algorithm SHA512 $path).Hash) *$Name" | Out-File -FilePath "$path.sha512" -Encoding ascii + return $path + } } Describe 'verify-release.ps1' { @@ -76,6 +119,47 @@ Describe 'verify-release.ps1' { Should -Throw -ExpectedMessage 'No artifacts to verify*' } + It 'refuses a release that has no manifest' { + Add-SourceArchive -Directory $script:Directory -Commit ('a' * 40) | Out-Null + + { & $script:VerifyRelease -Directory $script:Directory } | + Should -Throw -ExpectedMessage 'expected one .manifest file*found 0*' + } + + It 'refuses an artifact the manifest does not list' { + $source = Add-SourceArchive -Directory $script:Directory -Commit ('a' * 40) + Add-Artifact -Directory $script:Directory -WithHash | Out-Null + Add-Manifest -Directory $script:Directory -Commit ('a' * 40) -Listed (Split-Path $source -Leaf) | Out-Null + + { & $script:VerifyRelease -Directory $script:Directory } | + Should -Throw -ExpectedMessage '*in the release but not listed, apache-log4net-binaries-9.9.9.zip*' + } + + It 'refuses a release missing an artifact the manifest lists' { + $source = Add-SourceArchive -Directory $script:Directory -Commit ('a' * 40) + Add-Manifest -Directory $script:Directory -Commit ('a' * 40) ` + -Listed @((Split-Path $source -Leaf), 'apache-log4net-9.9.9.nupkg') | Out-Null + + { & $script:VerifyRelease -Directory $script:Directory } | + Should -Throw -ExpectedMessage '*listed but not in the release, apache-log4net-9.9.9.nupkg*' + } + + It 'refuses a manifest that records no commit' { + $source = Add-SourceArchive -Directory $script:Directory -Commit ('a' * 40) + Add-Manifest -Directory $script:Directory -Listed (Split-Path $source -Leaf) | Out-Null + + { & $script:VerifyRelease -Directory $script:Directory } | + Should -Throw -ExpectedMessage '*expected one commit line, found 0*' + } + + It 'refuses a source archive that is not the commit the manifest records' { + $source = Add-SourceArchive -Directory $script:Directory -Commit ('a' * 40) + Add-Manifest -Directory $script:Directory -Commit ('b' * 40) -Listed (Split-Path $source -Leaf) | Out-Null + + { & $script:VerifyRelease -Directory $script:Directory } | + Should -Throw -ExpectedMessage '*built from commit aaa*but the release records bbb*' + } + It 'leaves GNUPGHOME alone when it fails before reaching gpg' { Add-Artifact -Directory $script:Directory | Out-Null diff --git a/scripts/verify-release.ps1 b/scripts/verify-release.ps1 index 5ee3bca89..b7c8300be 100644 --- a/scripts/verify-release.ps1 +++ b/scripts/verify-release.ps1 @@ -43,55 +43,159 @@ function Assert-Hash "$($File.Name): hash ok" } -# Driven from the artifacts, not from the .sha512 and .asc files present, so a missing one fails -# instead of being one loop iteration fewer. -$Artifacts = @(Get-ChildItem $Directory -File | - Where-Object { $_.Extension -notin '.asc', '.sha512' -and $_.Name -ne 'KEYS' }) - -if ($Artifacts.Count -eq 0) +# One file records the commit and the artifact set, and is signed with them. +function Get-Manifest { - throw "No artifacts to verify in $Directory" + param + ( + [Parameter(Mandatory=$true, HelpMessage='The artifacts of the release.')] + [System.IO.FileInfo[]]$Artifacts + ) + + $Manifest = @($Artifacts | Where-Object { $_.Extension -eq '.manifest' }) + if ($Manifest.Count -ne 1) + { + throw "expected one .manifest file describing the release, found $($Manifest.Count)" + } + + return $Manifest[0] } -foreach ($Artifact in $Artifacts) +# The hash and signature loops only see the files that are there, so without the listed set an +# artifact removed together with its .sha512 and .asc would pass. +function Assert-ArtifactSet { - Assert-Hash $Artifact + param + ( + [Parameter(Mandatory=$true, HelpMessage='The artifacts of the release.')] + [System.IO.FileInfo[]]$Artifacts, + [Parameter(Mandatory=$true, HelpMessage='The manifest listing them.')] + [System.IO.FileInfo]$Manifest + ) + + $Listed = @(Get-Content $Manifest.FullName | Where-Object { $_ -like 'artifact=*' } | + ForEach-Object { $_.Substring('artifact='.Length) }) + if ($Listed.Count -eq 0) + { + throw "$($Manifest.Name): lists no artifact" + } + + $Present = @($Artifacts | ForEach-Object { $_.Name }) + $Missing = @($Listed | Where-Object { $_ -notin $Present }) + if ($Missing.Count -gt 0) + { + throw "$($Manifest.Name): listed but not in the release, $($Missing -join ', ')" + } + + $Extra = @($Present | Where-Object { $_ -notin $Listed }) + if ($Extra.Count -gt 0) + { + throw "$($Manifest.Name): in the release but not listed, $($Extra -join ', ')" + } + + "$($Manifest.Name): all $($Listed.Count) artifacts present" } -# A home of its own, so only the downloaded KEYS can verify. Not --keyring: gpg ignores that where -# common.conf sets use-keyboxd. -$GnupgHome = New-Item -ItemType Directory -Path (Join-Path ([System.IO.Path]::GetTempPath()) ([guid]::NewGuid())) -$PreviousGnupgHome = $env:GNUPGHOME -$env:GNUPGHOME = $GnupgHome -try +# The manifest and the comment git archive writes into the zip are two independent records of the +# same commit. +function Assert-SourceCommit { - # Never the KEYS next to the artifacts: nothing above verifies it, so importing it would let - # anyone who can write there supply a release key. - $Keys = Join-Path $GnupgHome 'KEYS' - Invoke-WebRequest https://downloads.apache.org/logging/KEYS -OutFile $Keys + param + ( + [Parameter(Mandatory=$true, HelpMessage='The artifacts of the release.')] + [System.IO.FileInfo[]]$Artifacts, + [Parameter(Mandatory=$true, HelpMessage='The manifest recording the commit.')] + [System.IO.FileInfo]$Manifest + ) + + $Recorded = @(Get-Content $Manifest.FullName | Where-Object { $_ -like 'commit=*' } | + ForEach-Object { $_.Substring('commit='.Length) }) + if ($Recorded.Count -ne 1) + { + throw "$($Manifest.Name): expected one commit line, found $($Recorded.Count)" + } - gpg --batch --quiet --import $Keys + $SourceArchive = @($Artifacts | Where-Object { $_.Name -like '*source*.zip' }) + if ($SourceArchive.Count -ne 1) + { + throw "expected one source archive, found $($SourceArchive.Count)" + } - foreach ($Artifact in $Artifacts) + $Zip = [System.IO.Compression.ZipFile]::OpenRead($SourceArchive[0].FullName) + $ArchivedCommit = $Zip.Comment + $Zip.Dispose() + if ($Recorded[0].Trim() -ne $ArchivedCommit) { - $Signature = "$($Artifact.FullName).asc" - if (!(Test-Path $Signature)) + throw "$($SourceArchive[0].Name): built from commit $ArchivedCommit but the release records $($Recorded[0])" + } + + "$($SourceArchive[0].Name): commit $ArchivedCommit ok" +} + +function Assert-Signature +{ + param + ( + [Parameter(Mandatory=$true, HelpMessage='The artifacts of the release.')] + [System.IO.FileInfo[]]$Artifacts + ) + + # A home of its own, so only the downloaded KEYS can verify. Not --keyring: gpg ignores that where + # common.conf sets use-keyboxd. + $GnupgHome = New-Item -ItemType Directory -Path (Join-Path ([System.IO.Path]::GetTempPath()) ([guid]::NewGuid())) + $PreviousGnupgHome = $env:GNUPGHOME + $env:GNUPGHOME = $GnupgHome + try + { + # Never the KEYS next to the artifacts: nothing above verifies it, so importing it would let + # anyone who can write there supply a release key. + $Keys = Join-Path $GnupgHome 'KEYS' + Invoke-WebRequest https://downloads.apache.org/logging/KEYS -OutFile $Keys + + gpg --batch --quiet --import $Keys + + foreach ($Artifact in $Artifacts) { - throw "$($Artifact.Name): no $($Artifact.Name).asc to verify it with" - } + $Signature = "$($Artifact.FullName).asc" + if (!(Test-Path $Signature)) + { + throw "$($Artifact.Name): no $($Artifact.Name).asc to verify it with" + } - gpg --batch --verify $Signature $Artifact.FullName - "$($Artifact.Name): signature ok" + gpg --batch --verify $Signature $Artifact.FullName + "$($Artifact.Name): signature ok" + } + } + finally + { + # The daemons hold the directory open until told to stop. Wrapped, or a non-zero exit throws under + # $PSNativeCommandUseErrorActionPreference and abandons the rest of the finally. + try { gpgconf --kill all 2>&1 | Out-Null } catch { } + $env:GNUPGHOME = $PreviousGnupgHome + Remove-Item $GnupgHome -Recurse -Force -ErrorAction SilentlyContinue } } -finally + +# Driven from the artifacts, not from the .sha512 and .asc files present, so a missing one fails +# instead of being one loop iteration fewer. +$Artifacts = @(Get-ChildItem $Directory -File | + Where-Object { $_.Extension -notin '.asc', '.sha512' -and $_.Name -ne 'KEYS' }) + +if ($Artifacts.Count -eq 0) { - # The daemons hold the directory open until told to stop. Wrapped, or a non-zero exit throws under - # $PSNativeCommandUseErrorActionPreference and abandons the rest of the finally. - try { gpgconf --kill all 2>&1 | Out-Null } catch { } - $env:GNUPGHOME = $PreviousGnupgHome - Remove-Item $GnupgHome -Recurse -Force -ErrorAction SilentlyContinue + throw "No artifacts to verify in $Directory" +} + +foreach ($Artifact in $Artifacts) +{ + Assert-Hash $Artifact } +$Manifest = Get-Manifest $Artifacts +Assert-ArtifactSet $Artifacts $Manifest +Assert-SourceCommit $Artifacts $Manifest + +Assert-Signature $Artifacts + Expand-Archive $Directory/*source*.zip -DestinationPath $Directory/src -pushd "$Directory/src/" +Push-Location "$Directory/src/" diff --git a/scripts/verify-release.sh b/scripts/verify-release.sh index 2b82fc7e6..55ef7263d 100644 --- a/scripts/verify-release.sh +++ b/scripts/verify-release.sh @@ -37,6 +37,64 @@ for file in "${artifacts[@]}"; do sha512sum --check "$file.sha512" done +# One file records the commit and the artifact set, and is signed with them. +manifests=(*.manifest) +if test ${#manifests[@]} -ne 1; then + echo "expected one .manifest file describing the release, found ${#manifests[@]}" >&2 + exit 1 +fi +manifest="${manifests[0]}" + +# The hash and signature loops only see the files that are there, so without the listed set an +# artifact removed together with its .sha512 and .asc would pass. +listed=() +while IFS= read -r name; do listed+=("$name"); done < <(sed -n 's/^artifact=//p' "$manifest") +if test ${#listed[@]} -eq 0; then + echo "$manifest: lists no artifact" >&2 + exit 1 +fi + +for name in "${listed[@]}"; do + if test ! -f "$name"; then + echo "$manifest: listed but not in the release, $name" >&2 + exit 1 + fi +done + +for file in "${artifacts[@]}"; do + found=0 + for name in "${listed[@]}"; do + if test "$file" = "$name"; then found=1; break; fi + done + if test "$found" -eq 0; then + echo "$manifest: in the release but not listed, $file" >&2 + exit 1 + fi +done +echo "$manifest: all ${#listed[@]} artifacts present" + +# The manifest and the comment git archive writes into the zip are two independent records of the +# same commit. +recorded="$(sed -n 's/^commit=//p' "$manifest" | tr -d '[:space:]')" +if test -z "$recorded"; then + echo "$manifest: no commit line" >&2 + exit 1 +fi + +sources=(*source*.zip) +if test ${#sources[@]} -ne 1; then + echo "expected one source archive, found ${#sources[@]}" >&2 + exit 1 +fi + +# -qq, or the "Archive:" banner lands in the comparison. +archived="$(unzip -z -qq "${sources[0]}" | tr -d '[:space:]')" +if test "$recorded" != "$archived"; then + echo "${sources[0]}: built from commit $archived but the release records $recorded" >&2 + exit 1 +fi +echo "${sources[0]}: commit $archived ok" + # A home of its own, so only the downloaded KEYS can verify. Not --keyring: gpg ignores that where # common.conf sets use-keyboxd. Assigned before exporting, or a failed mktemp would go unnoticed # and an empty GNUPGHOME means the reviewer's own home. diff --git a/src/changelog/3.5.0/330-release-from-one-commit.xml b/src/changelog/3.5.0/330-release-from-one-commit.xml new file mode 100644 index 000000000..14361333c --- /dev/null +++ b/src/changelog/3.5.0/330-release-from-one-commit.xml @@ -0,0 +1,14 @@ + + + + + bind a release to a single commit. `build-release.ps1` built the binaries from the working tree + but archived the local `master` ref, so the signed source zip need not match the signed binaries. + It now refuses an unclean working tree, archives `HEAD`, and ships a signed `.manifest` recording + that commit and the artifact set, which the verification scripts check against the zip archive + comment and against the files present (audit da18b6fd-f025, fixed by @FreeAndNil) + +