Harden the release scripts and connection string redaction - #313
Merged
Conversation
FreeAndNil
force-pushed
the
Feature/312-more-hardening
branch
from
September 1, 2026 18:32
711e9b9 to
ddf09c1
Compare
FreeAndNil
force-pushed
the
Feature/312-more-hardening
branch
2 times, most recently
from
September 1, 2026 19:40
f48a1d1 to
937df6b
Compare
"wget URL" refuses to overwrite, so a KEYS file planted next to the artifacts stayed put, the real download landed in KEYS.1, and the planted keys were imported into the verification key ring. Artifacts signed by whoever placed it then verified and the script exited 0. Both scripts now download into the temporary key ring directory and never read KEYS from the directory being verified. Not a regression: the same fixture bypasses the pre-rewrite script, which carried the bare wget line from 3.2.0 on.
CLAUDE.md told AI coding agents to execute a "graphify" binary resolved from PATH that is not vendored, pinned or checksummed anywhere in the tree. The generated graphify-out/ (26 MB, two tracked files) goes with it, along with its .gitignore block.
All native command error handling in these scripts rests on $PSNativeCommandUseErrorActionPreference, which exists only from 7.4. Under Windows PowerShell 5.1 the assignment is a silent no-op, so a failing gpg --verify was ignored and verify-release.ps1 reported success and exited 0, undoing the fail-closed work in 3.4.0. The comments claimed 7.3, but there it was only an experimental feature. release-review.adoc said "Windows (PowerShell)" and never installed PowerShell 7, so it now installs it and runs the script with pwsh.
Hiding password-bearing keywords missed Extended Properties, which nests a whole connection string the parser returns as one opaque value, and keywords such as AccessToken. Only keywords naming the server and account are kept. The old test only covered a flat Password=, which is why this survived.
FreeAndNil
force-pushed
the
Feature/312-more-hardening
branch
from
September 2, 2026 04:31
937df6b to
77af21a
Compare
fluffynuts
approved these changes
Sep 2, 2026
swebb2066
reviewed
Sep 2, 2026
swebb2066
left a comment
swebb2066
left a comment
There was a problem hiding this comment.
Temporarily disabling keyboxd in the configuration file seems to be the only way to avoid this, which is rather ugly
gpg ignores --keyring where common.conf sets use-keyboxd, so the import landed in the reviewer's own key database and the verification ran against it. Measured: a decoy signed by a key they already held verified with exit 0; with a throwaway GNUPGHOME it is rejected with exit 2. Two defects in the change itself, both found in review: "export X=$(mktemp -d)" masks a failed mktemp under set -e and an empty GNUPGHOME is the default home, and a failing gpgconf aborts the rest of the cleanup in both scripts. Pester tests for the stages needing neither network nor gpg, run in CI. Pester 5.9 ships on all three runner images. Reported by @swebb2066.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups from the second security scan, on top of 3.4.0.
verified, so a planted KEYS file was the one imported and its keys were
accepted as release keys. Both scripts now download into a temporary
keyring of their own. Not a regression, the bare wget line dates from 3.2.0.
which only exists from PowerShell 7.4. On 5.1 the assignment was a silent
no-op and a failing gpg --verify was ignored. Added #Requires -Version 7.4,
and release-review.adoc now installs PowerShell 7 and uses pwsh.
which missed Extended Properties and AccessToken. Replaced with an
allowlist of diagnostic keywords.