diff --git a/.github/workflows/installer-smoke.yml b/.github/workflows/installer-smoke.yml new file mode 100644 index 0000000..b767074 --- /dev/null +++ b/.github/workflows/installer-smoke.yml @@ -0,0 +1,61 @@ +name: Installer smoke + +# Compiles packaging/keel.iss on a real Windows runner against a PLACEHOLDER bundle. +# +# WHY THIS EXISTS: the release workflow is manual-only and its desktop job runs AFTER the +# release is published, so the first real Inno Setup compile of keel.iss would otherwise +# be the release dispatch itself -- a script typo failing a release with the tag already +# pushed. ISCC does not run what it packages, so a one-line placeholder keel.exe exercises +# the whole SCRIPT (directives, relative paths, defines, output naming) for the price of +# one cheap compile, with no freeze and no secrets. +# +# TRIGGERS. `workflow_dispatch` alone would have been the obvious choice, but a workflow +# that does not exist on the default branch cannot be dispatched onto a branch at all +# (GitHub registers workflow_dispatch targets from the default branch), so the first +# compile of a NEW script would still be whatever release dispatched first -- the exact +# failure this workflow exists to prevent. So it also runs on pull requests that touch +# the script or this file, which is where a compile break is introduced in the first +# place. The `paths` filter is what keeps that honest: the Windows runner is spent only +# when the thing that can break it changed, not on every PR. +on: + workflow_dispatch: + pull_request: + paths: + - packaging/keel.iss + - .github/workflows/installer-smoke.yml + +permissions: + contents: read + +jobs: + compile: + name: Compile the Inno Setup script + runs-on: windows-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v7 + + # Full path first, PATH second, never the other way round: the hosted image ships + # Inno Setup 6 at this path, and preferring it means a PATH change on the image + # cannot silently hand the build a different compiler version. + - name: Compile keel.iss against a placeholder bundle + shell: pwsh + run: | + New-Item -ItemType Directory -Force -Path dist\keel, out | Out-Null + Set-Content -Path dist\keel\keel.exe -Value "placeholder" + $iscc = "C:\Program Files (x86)\Inno Setup 6\ISCC.exe" + if (-not (Test-Path $iscc)) { + $iscc = (Get-Command iscc -ErrorAction SilentlyContinue).Source + } + if (-not $iscc) { + "::error::ISCC.exe not found -- the Windows runner image no longer ships Inno Setup 6" + exit 1 + } + & $iscc "/DKeelVersion=0.0.0-smoke" "/DKeelArch=x86_64" "packaging\keel.iss" + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + $setup = Get-ChildItem out -Filter "*-setup.exe" + if (-not $setup) { + "::error::ISCC reported success but out/ holds no *-setup.exe -- the script's output naming drifted" + exit 1 + } + $setup | ForEach-Object { "compiled $($_.Name) ($($_.Length) bytes)" } diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 04c4077..460bd99 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -112,9 +112,12 @@ jobs: run: | set -euo pipefail # Must be the PINNED interpreter, not the runner's default `python`. The wheel - # carries `Requires-Python: >=3.14.4`, and pip enforces that even for an explicit - # local wheel path -- a default-python venv fails here with "requires a different - # Python". --seed provides the pip that the install below uses. + # accepts `Requires-Python: >=3.11` (every pyproject.toml in the workspace says + # so, pinned by tests/test_python_floor.py), so a default-python venv would + # INSTALL -- and verify the artifact on an interpreter nobody tested or shipped: + # the release is built and typed against .python-version, and the verify step + # must exercise that same one. --seed provides the pip that the install below + # uses. uv venv --seed --python "$(cat .python-version)" /tmp/verify # Install by explicit PATH, never by name. The name `keel` belongs to an unrelated # project on PyPI, so a name-based install can silently fetch a stranger's package -- @@ -297,9 +300,20 @@ jobs: # # keel is open source on a small budget and has chosen not to pay either. # + # #438 REVISITED THE "IF WE EVER PAY" HALF WITHOUT PAYING: the signing steps are now + # IMPLEMENTED BELOW, per OS, gated on secrets that do not exist -- the #402 pattern the + # code-quality scans use (a missing prerequisite skips with a notice that names it, it + # never reddens the run). The macOS leg signs (codesign --options runtime), notarises + # (notarytool submit --wait) and staples (stapler) the .app and the .dmg when the Apple + # credentials appear on the `signing` environment; the Windows leg signs the setup.exe + # with signtool when the certificate secret does. Until then the artifacts ship exactly + # as before, with the same honest skips naming every secret to add and what it costs. + # The OPERATOR CHECKLIST (which product to buy, which secrets to create where) lives in + # docs/desktop-install.md -- it is a purchase decision, not a code one. + # # What that costs the user is real and is documented rather than hidden: a `.dmg` downloaded - # from the internet carries a quarantine flag, so macOS refuses the first open until they go to - # System Settings -> Privacy & Security -> Open Anyway. Windows SmartScreen warns similarly. + # from the internet carries a quarantine flag, so macOS refuses the first open until they go + # to System Settings -> Privacy & Security -> Open Anyway. Windows SmartScreen warns similarly. # # What replaces OS-level trust here is PROVENANCE, which is free and is arguably the more # honest answer for an auditable project anyway: every artifact carries a GitHub build @@ -316,6 +330,13 @@ jobs: desktop: needs: release if: ${{ inputs.desktop != 'skip' }} + # #438: the signing secrets live on the `signing` ENVIRONMENT, not the repo, so they are + # visible only to this job and only after the environment's reviewers (once any are + # configured) approve the run. A repo-level secret would leak into every same-repo PR + # build; an environment secret stops at the gate. Until the environment exists this line + # is inert -- GitHub treats a missing environment as unprotected -- and the signing steps + # below skip themselves honestly rather than fail the release. + environment: signing timeout-minutes: 45 permissions: contents: write # attach the artifacts to the release @@ -419,14 +440,213 @@ jobs: if: runner.os == 'Windows' shell: bash run: | - # Inno Setup is the intended installer (per-user, no admin prompt, and the install-path - # and version-decision UX #438 specifies). Until that script is written and tested on a - # Windows runner, a zip is shipped rather than an untested installer -- an installer - # nobody has run is a worse artifact than an archive everyone understands. + # The zip STAYS beside the installer, deliberately: it is the no-install route + # (extract anywhere, run keel.exe) that docs/desktop-install.md documents -- + # including the Unblock-before-extract step that keeps SmartScreen from + # re-prompting. An installer nobody has run was a worse artifact than an archive + # everyone understands; now the installer has been compiled first (see + # .github/workflows/installer-smoke.yml, which runs ISCC on the same script on + # every PR that touches it), both ship. mkdir -p out 7z a -tzip "out/keel-${{ inputs.version }}-windows-${{ matrix.arch }}.zip" ./dist/keel/* >/dev/null ls -l out + # The installer #438 specifies: ONE setup.exe wrapping the frozen --onedir tree, + # per-user to %LOCALAPPDATA%\Programs\keel, no admin prompt. The requirements live + # in packaging/keel.iss (privileges, paths, what is never touched); this step only + # passes the release's version and arch and points ISCC at it. + # Full path first, PATH second, never the other way round: the hosted image ships + # Inno Setup 6 at this path, and preferring it means a PATH change on the image + # cannot silently hand the build a different compiler version. pwsh because ISCC's + # /D arguments look like Unix paths to MSYS bash, which rewrites them. + - name: Build the installer (Windows) + if: runner.os == 'Windows' + shell: pwsh + run: | + $iscc = "C:\Program Files (x86)\Inno Setup 6\ISCC.exe" + if (-not (Test-Path $iscc)) { + $iscc = (Get-Command iscc -ErrorAction SilentlyContinue).Source + } + if (-not $iscc) { + "::error::ISCC.exe not found -- the Windows runner image no longer ships Inno Setup 6" + exit 1 + } + New-Item -ItemType Directory -Force -Path out | Out-Null + & $iscc "/DKeelVersion=${{ inputs.version }}" "/DKeelArch=${{ matrix.arch }}" "packaging\keel.iss" + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + Get-ChildItem out -Filter "*-setup.exe" | ForEach-Object { + "built $($_.Name) ($($_.Length) bytes)" + } + + # ── SIGNING (#438): implemented, gated, honest about skipping ──────────────────────────── + # + # These four steps are the "#402 pattern" applied to certificates: the signing work is + # REAL and complete, and each leg runs it only when that leg's credentials exist as + # secrets on the `signing` environment. When they do not, the paired notice step fires + # INSTEAD and says exactly which secrets are missing, what they cost, and where the + # checklist is -- a skip that announces itself, never a red X on a release. + # + # ORDER MATTERS: packaging first (the steps sign out/keel.app and out/*-setup.exe), + # Checksums AFTER (the sums must cover the signed bytes -- a checksum of an unsigned + # binary that is then signed proves nothing about what the user downloads). + + # macOS: Developer ID Application certificate (p12) + App Store Connect API key (the + # notarytool credential). Sign with the hardened runtime notarisation requires, + # notarise the app and the dmg (--wait, so a Rejected submission fails the step), + # staple both, and re-cut the dmg from the signed app -- the unsigned dmg + # macos_app.sh produced carries a READ ME that explains Gatekeeper refusals, and a + # signed build must not ship a note about a warning it no longer triggers. + - name: Sign, notarise and staple (macOS) + if: >- + runner.os == 'macOS' && secrets.MACOS_CERT_P12_BASE64 != '' && + secrets.MACOS_CERT_PASSWORD != '' && secrets.APP_STORE_CONNECT_KEY_ID != '' && + secrets.APP_STORE_CONNECT_ISSUER_ID != '' && secrets.APP_STORE_CONNECT_KEY_CONTENT != '' + env: + MACOS_CERT_P12_BASE64: ${{ secrets.MACOS_CERT_P12_BASE64 }} + MACOS_CERT_PASSWORD: ${{ secrets.MACOS_CERT_PASSWORD }} + APP_STORE_CONNECT_KEY_ID: ${{ secrets.APP_STORE_CONNECT_KEY_ID }} + APP_STORE_CONNECT_ISSUER_ID: ${{ secrets.APP_STORE_CONNECT_ISSUER_ID }} + APP_STORE_CONNECT_KEY_CONTENT: ${{ secrets.APP_STORE_CONNECT_KEY_CONTENT }} + run: | + set -euo pipefail + VERSION="${{ inputs.version }}" + DMG="out/keel-$VERSION-$(uname -m).dmg" + + # The certificate lives in an EPHEMERAL keychain that dies with the step (trap, so + # a failed notarisation cannot leave it behind either). The runner is wiped after + # the job regardless; the habit is the point. + KEYCHAIN="keel-signing.keychain-db" + KEYCHAIN_PW="$(openssl rand -hex 16)" + CERT_P12="$(mktemp -t keel-cert)" + ASC_KEY="$(mktemp -t keel-asc)" + trap 'security delete-keychain "$KEYCHAIN" >/dev/null 2>&1 || true; rm -f "$CERT_P12" "$ASC_KEY"' EXIT + + printf '%s' "$MACOS_CERT_P12_BASE64" | base64 --decode > "$CERT_P12" + security create-keychain -p "$KEYCHAIN_PW" "$KEYCHAIN" + security unlock-keychain -p "$KEYCHAIN_PW" "$KEYCHAIN" + security import "$CERT_P12" -k "$KEYCHAIN" -P "$MACOS_CERT_PASSWORD" -T /usr/bin/codesign + # Without the partition list codesign cannot use the private key headlessly. + security set-key-partition-list -S apple-tool:,apple: -k "$KEYCHAIN_PW" "$KEYCHAIN" + # First in the search list, KEEPING the login keychain -- dropping it breaks the + # notarytool profile store below. + security list-keychains -d user -s "$KEYCHAIN" \ + $(security list-keychains -d user | tr -d '"') + + IDENTITY="$(security find-identity -v -p codesigning "$KEYCHAIN" \ + | awk -F'"' '/Developer ID Application/ {print $2; exit}')" + [ -n "$IDENTITY" ] || { + echo "::error::no 'Developer ID Application' identity in the imported certificate. MACOS_CERT_P12_BASE64 must hold a Developer ID APPLICATION cert -- an Installer cert productsign uses will not sign an .app." + exit 1 + } + + # 1. Sign the app. `--options runtime` (hardened runtime) is REQUIRED for + # notarisation; `--timestamp` is the secure counterpart of Windows RFC 3161. + codesign --deep --force --options runtime --timestamp --sign "$IDENTITY" out/keel.app + codesign --verify --deep --strict --verbose=2 out/keel.app + + # 2. Notarise the APP (as a zip, per Apple's guidance for bundles) and staple it. + # `store-credentials` once; the profile is a generic password in the login + # keychain and dies with the runner. + ditto -c -k --keepParent out/keel.app keel-notary.zip + printf '%s' "$APP_STORE_CONNECT_KEY_CONTENT" > "$ASC_KEY" + xcrun notarytool store-credentials keel-notary \ + --key "$ASC_KEY" --key-id "$APP_STORE_CONNECT_KEY_ID" \ + --issuer "$APP_STORE_CONNECT_ISSUER_ID" --force + xcrun notarytool submit keel-notary.zip --keychain-profile keel-notary --wait + xcrun stapler staple out/keel.app + spctl -a -t exec -vv out/keel.app + + # 3. Re-cut the DMG from the signed, stapled app, with the note a SIGNED build + # deserves: the unsigned README's "macOS will refuse this" instructions would + # be false in exactly the dangerous direction. + STAGE="$(mktemp -d)" + cp -R out/keel.app "$STAGE/keel.app" + cat > "$STAGE/READ ME FIRST.txt" < --repo CodeGateSoftware/keel + + Drag keel.app to your Applications folder, eject this image, and open keel. + + Your config, database and credentials live in your user Application Support + folder and are never touched by an update. + + keel is a personal tool. It is not financial advice and not religious (Shariah) + advice. + NOTE + rm -f "$DMG" + hdiutil create -quiet -srcfolder "$STAGE" -volname "keel $VERSION" -format UDZO "$DMG" + rm -rf "$STAGE" + + # 4. Notarise and staple the DMG itself: it is what the user downloads, so its own + # ticket is what Gatekeeper assesses first. + xcrun notarytool submit "$DMG" --keychain-profile keel-notary --wait + xcrun stapler staple "$DMG" + echo "signed, notarised and stapled: $DMG" + + # The honest skip: this fires exactly when the sign step above does not, and it names + # every missing secret and the product that unlocks them -- the #402 discipline, so a + # future reader of a green run learns signing was SKIPPED, not forgotten. + - name: "Notice: macOS signing skipped" + if: >- + runner.os == 'macOS' && (secrets.MACOS_CERT_P12_BASE64 == '' || + secrets.MACOS_CERT_PASSWORD == '' || secrets.APP_STORE_CONNECT_KEY_ID == '' || + secrets.APP_STORE_CONNECT_ISSUER_ID == '' || secrets.APP_STORE_CONNECT_KEY_CONTENT == '') + run: | + echo "::notice::macOS build shipped UNSIGNED -- signing, notarisation and stapling are implemented in this workflow but skipped: the Apple credentials are not configured. Notarisation requires the \$99/yr Apple Developer Program (a Developer ID Application certificate, plus an App Store Connect API key for notarytool). To activate: Settings > Environments > 'signing' > add repository secrets MACOS_CERT_P12_BASE64, MACOS_CERT_PASSWORD, APP_STORE_CONNECT_KEY_ID, APP_STORE_CONNECT_ISSUER_ID, APP_STORE_CONNECT_KEY_CONTENT -- the full checklist is in docs/desktop-install.md. Until then the artifacts carry build attestations and SHA256SUMS instead." + + # Windows: sign the setup.exe ONLY (the zip cannot carry a signature). RFC 3161 + # timestamping is not optional: an untimestamped signature dies with the certificate. + - name: Sign the installer (Windows) + if: >- + runner.os == 'Windows' && secrets.WINDOWS_CERT_PFX_BASE64 != '' && + secrets.WINDOWS_CERT_PASSWORD != '' + env: + WINDOWS_CERT_PFX_BASE64: ${{ secrets.WINDOWS_CERT_PFX_BASE64 }} + WINDOWS_CERT_PASSWORD: ${{ secrets.WINDOWS_CERT_PASSWORD }} + shell: pwsh + run: | + $ErrorActionPreference = "Stop" + $pfx = New-TemporaryFile + [IO.File]::WriteAllBytes($pfx, [Convert]::FromBase64String($env:WINDOWS_CERT_PFX_BASE64)) + # Full SDK path first, PATH second: preferring the SDK copy means a PATH change on + # the image cannot silently hand the build a different signtool. + $signtool = (Get-Command signtool -ErrorAction SilentlyContinue).Source + if (-not $signtool) { + $signtool = (Get-ChildItem "C:\Program Files (x86)\Windows Kits\10\bin" -Recurse -Filter signtool.exe | + Where-Object { $_.FullName -match "\\x64\\" } | + Sort-Object FullName | Select-Object -Last 1).FullName + } + if (-not $signtool) { + "::error::signtool not found -- the Windows runner image no longer ships the SDK" + exit 1 + } + $setup = Get-ChildItem out -Filter "*-setup.exe" | Select-Object -First 1 + if (-not $setup) { + "::error::no *-setup.exe under out/ -- the installer must be built before it is signed" + exit 1 + } + & $signtool sign /fd SHA256 /td SHA256 /tr http://timestamp.digicert.com /f $pfx /p $env:WINDOWS_CERT_PASSWORD $setup.FullName + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + & $signtool verify /pa /v $setup.FullName + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + Remove-Item $pfx -Force + "signed $($setup.Name)" + + - name: "Notice: Windows signing skipped" + if: >- + runner.os == 'Windows' && (secrets.WINDOWS_CERT_PFX_BASE64 == '' || + secrets.WINDOWS_CERT_PASSWORD == '') + run: | + echo "::notice::setup.exe shipped UNSIGNED -- signing is implemented in this workflow but skipped: the Windows certificate is not configured. An OV code-signing certificate costs ~\$70-500/yr from a CA (SSL.com, Certum, Sectigo) or Azure Trusted Signing is \$9.99/mo (~\$120/yr); since 2024 an EV certificate no longer buys an instant SmartScreen pass, so EV is not worth paying extra for. To activate: Settings > Environments > 'signing' > add repository secrets WINDOWS_CERT_PFX_BASE64 and WINDOWS_CERT_PASSWORD -- the full checklist is in docs/desktop-install.md. Until then the artifacts carry build attestations and SHA256SUMS instead." + # Checksums travel WITH the artifacts, in the same release, so a user who was handed a # download link somewhere else has something local to compare against. FILES ONLY: # macOS packaging leaves the unzipped `keel.app/` and the `dmg-stage/` build directory @@ -463,7 +683,10 @@ jobs: if: runner.os == 'Windows' uses: actions/attest-build-provenance@v4 with: - subject-path: out/*.zip + # Both Windows deliverables: the zip AND the setup.exe. Multiple globs in one + # subject-path are space-separated (attestations grew multi-subject support in + # Dec 2024); a second attestation step would work too but duplicates the job. + subject-path: out/*.zip out/*-setup.exe - name: Upload the artifact uses: actions/upload-artifact@v7 diff --git a/docs/desktop-install.md b/docs/desktop-install.md index a645b05..f15f191 100644 --- a/docs/desktop-install.md +++ b/docs/desktop-install.md @@ -113,3 +113,83 @@ If keel ever has the budget, signing is a small change on our side — the relea already built to accept it, on both platforms — and this page will be replaced by a sentence saying the builds are signed. Until then, we would rather tell you the truth about what you are downloading than say nothing and let your computer deliver the news. + +That "already built" is now literal. The release workflow carries the signing steps for both +platforms, each gated on its own credentials: when they are absent the build ships exactly as +described above and the workflow log carries a notice saying which secrets would turn signing +on. Paying for the certificates is the whole activation — there is no code left to write. The +checklist for whoever makes that purchase is below. + +## For the maintainer: turning signing on (a purchase, then ten minutes of GitHub settings) + +Nothing in this section changes any code. The desktop job already references a `signing` +environment and already runs the macOS and Windows signing steps when — and only when — that +environment holds the credentials. A release dispatched before the secrets exist ships +unsigned, with a `::notice` in the run naming exactly what was skipped and why; a release +dispatched after ships signed. That is the entire switch. + +### What to buy + +**macOS — Apple Developer Program, $99/yr.** Notarisation requires a *Developer ID +Application* certificate, and there is no cheaper tier that Gatekeeper honours (the table at +the top of this page is the whole market). You need two things from that membership: + +- the **Developer ID Application certificate**, exported from Keychain Access together with + its private key as a `.p12`; +- an **App Store Connect API key** (appstoreconnect.apple.com → Users and Access → + Integrating/Keys, Account Holder or Admin), which is what `notarytool` authenticates with + from CI — it gives you a Key ID, an Issuer ID, and a one-time-download `.p8` file. + +**Windows — one of:** + +- an **OV code-signing certificate** as a `.pfx`, ~$70–500/yr depending on the CA (SSL.com, + Certum, Sectigo and friends), or +- **Azure Trusted Signing**, $9.99/mo (~$120/yr). It may be restricted to US/Canada signing + identities — verify your identity qualifies before budgeting for it. + +Do **not** pay extra for EV. Since 2024 an EV certificate no longer buys an instant +SmartScreen pass — reputation accrues from download volume over time regardless of certificate +class, so EV costs more for the same warning. + +### The ten minutes of GitHub settings + +1. **Create the environment.** Repository *Settings → Environments → New environment*, name + it exactly `signing` — the workflow references it by name, and a typo means the job runs + against an unprotected no-op environment (harmless while there are no secrets, silent + once there are). +2. **Add required reviewers** (yourself is fine) on that environment. This is the protection + that makes the whole design safe: a dispatch pauses for approval before any leg can see + the certificates, and the secrets are invisible to every other workflow — pull requests + included — because environment secrets are only handed to jobs that declare the + environment. +3. **Add the environment secrets.** macOS needs all five (the leg skips with its notice + until then — a signed-but-un-notarised app is the worst state on macOS, so the gate is + all-or-nothing): + + | secret | what it holds | + |---|---| + | `MACOS_CERT_P12_BASE64` | the Developer ID Application `.p12`, base64-encoded (`base64 -i cert.p12 \| pbcopy`) | + | `MACOS_CERT_PASSWORD` | that `.p12`'s export password | + | `APP_STORE_CONNECT_KEY_ID` | the Key ID of the App Store Connect API key | + | `APP_STORE_CONNECT_ISSUER_ID` | the Issuer ID from the same page | + | `APP_STORE_CONNECT_KEY_CONTENT` | the contents of the `.p8` private key | + + Windows needs both: + + | secret | what it holds | + |---|---| + | `WINDOWS_CERT_PFX_BASE64` | the OV certificate `.pfx`, base64-encoded | + | `WINDOWS_CERT_PASSWORD` | that `.pfx`'s password | + +4. **Dispatch the next release normally.** The signing steps run; the skip notices are gone. + The ephemeral-keychain import, `codesign --options runtime`, `notarytool submit --wait`, + `stapler`, the signed re-cut of the DMG, and `signtool` with an RFC 3161 timestamp are + already in `.github/workflows/release.yml`, between packaging and the checksum step (so + the sums cover the signed bytes). + +One manual step remains, deliberately: **the release-notes wording and this page still say +"not code-signed" and must be updated in the same change.** The notes are composed in the +release job, which cannot see the `signing` environment's secrets — least privilege is why it +cannot know the desktop legs will sign — so there is nothing for it to auto-detect. Forgetting +this step errs in the safe direction: the notes warn about a warning that no longer appears. +Fix it anyway, and this page becomes the one sentence it always promised to be. diff --git a/packaging/keel.iss b/packaging/keel.iss new file mode 100644 index 0000000..d6a9ace --- /dev/null +++ b/packaging/keel.iss @@ -0,0 +1,78 @@ +; Inno Setup script for the keel Windows installer (#438). +; +; Compiled by the release workflow's desktop job (and by the dispatch-only +; .github/workflows/installer-smoke.yml) with the version and arch passed as defines: +; +; ISCC.exe /DKeelVersion=0.12.0 /DKeelArch=x86_64 packaging\keel.iss +; +; The #ifndef defaults exist so the script still compiles when opened directly in Inno's +; compiler GUI; a real release always passes the real values, and the smoke workflow +; passes a placeholder -- so a compile failure is caught before the release dispatch, +; never during one. +#ifndef KeelVersion + #define KeelVersion "0.0.0" +#endif +#ifndef KeelArch + #define KeelArch "x86_64" +#endif + +; THE PROGRAM, NOT THE DEPLOYMENT. #438 separates two locations and the installer must +; keep them separate: +; +; program this directory ({localappdata}\Programs\keel): keel.exe and the bundled +; runtime. Replaced wholesale on every install. +; deployment {localappdata}\keel: config.yaml, keel*.db, .env, logs/. Created by the +; app, NEVER by this installer -- "config.yaml is never overwritten by an +; installer" and "no database is ever replaced, moved, or migrated by the +; installer" are #438's hard rules. This is also why there is no +; [UninstallDelete] section: uninstalling must leave the deployment intact. +; +; NOT HERE YET, DELIBERATELY: the install-over-existing-keel UX #438 specifies (read the +; installed version from on-disk metadata -- never execute it; update on newer; confirm +; on same version; confirm with a migrations-do-not-reverse warning on downgrade, because +; keel/data/db.py has no down-migrations). That needs [Code] against the on-disk build +; info and was deferred with the rest of the installer UX; this script is the packaging +; and signing vehicle that #438's workflow work needed first. + +[Setup] +; A fixed AppId is how Inno recognizes a previous install of the SAME app for upgrades +; and uninstall entries. It must never change across versions. +AppId={{6B1C9D2E-8E4A-4F0B-9A17-2D5C0E4F8A31} +AppName=keel +AppVersion={#KeelVersion} +AppPublisher=CodeGateSoftware +AppPublisherURL=https://github.com/CodeGateSoftware/keel +AppUpdatesURL=https://github.com/CodeGateSoftware/keel/releases +; PER-USER, NO ADMIN PROMPT (#438): `lowest` installs without an elevation dialog, and +; {localappdata}\Programs is the per-user location Windows itself uses for per-user +; application installs. An admin prompt on a tool that then asks for exchange API keys +; is a security habit this project refuses to teach. +PrivilegesRequired=lowest +DefaultDirName={localappdata}\Programs\keel +; [Files] Source paths and OutputDir are relative to SourceDir, i.e. the repository root +; (this script lives in packaging/). Both workflows invoke ISCC from the root. +SourceDir=.. +OutputDir=out +OutputBaseFilename=keel-{#KeelVersion}-windows-{#KeelArch}-setup +; A signed log under %TEMP% for every install: support asks "what did the installer do" +; exactly once per user, and this is the answer that needs no reproducing. +SetupLogging=yes +Compression=lzma2/max +SolidCompression=yes +WizardStyle=modern +DisableProgramGroupPage=yes +UninstallDisplayName=keel + +[Files] +; The whole PyInstaller --onedir tree: keel.exe plus the bundled interpreter and its +; native extensions. #438 chose --onedir over --onefile for faster start, simpler +; per-binary signing for notarisation-class tooling, and a lower AV/SmartScreen +; false-positive rate. `ignoreversion` because the whole program directory is replaced +; on every install -- the deployment is not in here (see the header). +Source: "dist\keel\*"; DestDir: "{app}"; Flags: recursesubdirs createallsubdirs ignoreversion + +[Icons] +; Per-user Start Menu shortcut only ({userprograms} writes HKCU, no elevation). Windows +; allocates a console on launch, which is the designed Windows experience (#438): no +; wrapper is needed there, unlike macOS where the .app must launch `keel serve`. +Name: "{userprograms}\keel"; Filename: "{app}\keel.exe" diff --git a/tests/test_desktop_packaging.py b/tests/test_desktop_packaging.py index bc82d5c..6be9529 100644 --- a/tests/test_desktop_packaging.py +++ b/tests/test_desktop_packaging.py @@ -10,6 +10,7 @@ from __future__ import annotations import stat +import tomllib from pathlib import Path import pytest @@ -19,6 +20,8 @@ _ROOT = Path(__file__).resolve().parents[1] _WORKFLOW = _ROOT / ".github" / "workflows" / "release.yml" _MACOS_SCRIPT = _ROOT / "packaging" / "macos_app.sh" +_INNO_SCRIPT = _ROOT / "packaging" / "keel.iss" +_SMOKE_WORKFLOW = _ROOT / ".github" / "workflows" / "installer-smoke.yml" @pytest.fixture(scope="module") @@ -87,6 +90,33 @@ def test_the_lockfile_is_checked_before_anything_can_mutate_it( ) +# -- the release must not lie about what the wheel requires ------------------------------------- + + +def test_the_verify_step_comment_states_the_real_python_floor() -> None: + """#438: the verify step's comment claimed the wheel carries `Requires-Python: >=3.14.4` + while every pyproject.toml declares `>=3.11` (pinned by tests/test_python_floor.py). + + Harmless to the run -- the pinned interpreter is used either way -- but the comment was + the only place a reader could learn what the wheel demands, and the next person to build + packaging on top of it (the desktop job, the installer) would have built on a floor that + does not exist. The floor stated in the workflow is now DERIVED from the manifest, so it + cannot drift again.""" + text = _WORKFLOW.read_text(encoding="utf-8") + floor = tomllib.loads((_ROOT / "pyproject.toml").read_text(encoding="utf-8"))["project"][ + "requires-python" + ] + assert f"Requires-Python: {floor}" in text, ( + f"the verify step's comment must state the wheel's real floor ({floor!r}, from " + "pyproject.toml) -- a stale floor is how the next packaging job gets built on a " + "requirement that does not exist" + ) + assert "3.14.4'" not in text.replace(".python-version", ""), ( + "the old false claim (`Requires-Python: >=3.14.4`) must be gone -- 3.14.4 is the " + "interpreter .python-version pins for the BUILD, not a floor the wheel enforces" + ) + + # -- the thing that must not happen ------------------------------------------------------------ @@ -128,10 +158,13 @@ def test_every_artifact_carries_provenance_and_checksums(desktop_job: dict) -> N # Subjects are per-OS FILES-ONLY globs: `out/*` would hand the action the unzipped # keel.app/ and dmg-stage/ directories macOS packaging leaves beside the .dmg, and a # shared `*.dmg *.zip` pattern would hand each leg one glob matching nothing. + # Windows carries BOTH its deliverables (the zip and the setup.exe) in one + # space-separated subject-path -- multi-subject attestation, supported since Dec 2024 -- + # because both are attached to the release, so both must be verifiable. subjects = {s["with"]["subject-path"] for s in attest} - assert subjects == {"out/*.dmg", "out/*.zip"} + assert subjects == {"out/*.dmg", "out/*.zip out/*-setup.exe"} macos = next(s for s in attest if s["with"]["subject-path"] == "out/*.dmg") - windows = next(s for s in attest if s["with"]["subject-path"] == "out/*.zip") + windows = next(s for s in attest if s["with"]["subject-path"] == "out/*.zip out/*-setup.exe") assert str(macos.get("if", "")).strip() == "runner.os == 'macOS'" assert str(windows.get("if", "")).strip() == "runner.os == 'Windows'" assert "SHA256SUMS-" in _steps_text(desktop_job) @@ -262,6 +295,226 @@ def test_the_script_runs_no_signing_command() -> None: assert not offenders, f"{command} is invoked: {offenders}" +# -- the Windows installer --------------------------------------------------------------------- + + +def test_the_inno_script_installs_per_user_without_an_admin_prompt() -> None: + """#438's Windows deliverable: one setup.exe installing per-user to + %LOCALAPPDATA%\\Programs\\keel, so the first thing a non-technical user is asked for is + not an administrator password. `PrivilegesRequired=lowest` is the directive that keeps + the UAC dialog away; `{localappdata}` is the per-user location Windows itself uses for + per-user application installs.""" + text = _INNO_SCRIPT.read_text(encoding="utf-8") + assert "PrivilegesRequired=lowest" in text + assert "DefaultDirName={localappdata}\\Programs\\keel" in text + + +def test_the_installer_touches_the_program_and_never_the_deployment() -> None: + """#438's two locations, kept separate by the installer: the PROGRAM (the frozen binary + and its bundled runtime) is replaced wholesale; the DEPLOYMENT (config.yaml, keel*.db, + .env, logs/) is never written, moved, or uninstalled. "config.yaml is never overwritten + by an installer" and "no database is ever replaced, moved, or migrated by the installer" + are the issue's hard rules -- an operator's allowlist and caps are hand-edited and + irreplaceable, and keel has no down-migrations.""" + # Comment lines are excluded, as with the macOS script: the .iss EXPLAINS in prose + # why there is no [UninstallDelete] section, and a test that forbade the words would + # forbid documenting them. + code = [ + line + for line in _INNO_SCRIPT.read_text(encoding="utf-8").splitlines() + if line.strip() and not line.lstrip().startswith(";") + ] + assert not any("[UninstallDelete]" in line for line in code), ( + "an [UninstallDelete] section is how an installer starts deleting things it was " + "never pointed at -- the deployment must survive an uninstall" + ) + for forbidden in ("config.yaml", "keel.db", ".env", "logs"): + assert not any(forbidden in line for line in code), ( + f"the installer must never name the deployment's {forbidden} -- the program " + "directory is all it owns" + ) + # The only directory it creates is the program directory. + assert 'DestDir: "{app}"' in "\n".join(code) + + +def test_the_workflow_builds_the_setup_exe_beside_the_zip(desktop_job: dict) -> None: + """The zip is the no-install route and stays; the setup.exe is the installer #438 + specified. BOTH are attached to the release, so both must be built on the Windows leg + and covered by the same files-only checksum/upload globs (which they are by + construction -- those steps glob every file in out/).""" + text = _steps_text(desktop_job) + assert "keel.iss" in text, "the Windows leg must compile packaging/keel.iss" + assert "ISCC" in text, "the Windows leg must invoke the Inno Setup compiler" + assert "7z a -tzip" in text, "the zip must stay -- it is the no-install route" + assert "-setup.exe" in "\n".join( + str(s.get("with", {}).get("subject-path", "")) for s in desktop_job["steps"] + ), "the setup.exe must be an attestation subject -- it is attached to the release" + + +def test_the_inno_script_is_smoke_compiled_before_any_release_needs_it() -> None: + """The release workflow is manual-only and its desktop job runs AFTER the release is + published, so without this the first real ISCC compile of keel.iss would be a release + dispatch -- a script typo failing a release with the tag already pushed. The smoke + workflow compiles the same script against a placeholder bundle on a Windows runner. + + It must trigger on PRs touching the script (that is where a compile break is + introduced, and a workflow not yet on the default branch cannot be dispatched onto a + branch at all), but ONLY those -- the `paths` filter is what keeps a Windows runner + from being spent on every PR.""" + assert _SMOKE_WORKFLOW.is_file() + smoke = strict_load(_SMOKE_WORKFLOW.read_text(encoding="utf-8"), source="installer-smoke.yml") + triggers = smoke[True] # PyYAML parses the bare `on` key as boolean True + assert "workflow_dispatch" in triggers, "a human must always be able to ask for a compile" + assert set(triggers["pull_request"]["paths"]) == { + "packaging/keel.iss", + ".github/workflows/installer-smoke.yml", + }, ( + "the PR trigger must fire ONLY when the compile can have broken -- the .iss or " + "the workflow itself -- or every PR pays for a Windows runner" + ) + run = "\n".join( + str(step.get("run", "")) for job in smoke["jobs"].values() for step in job["steps"] + ) + assert "keel.iss" in run and "ISCC" in run, "the smoke must compile the real script" + assert "placeholder" in run.lower(), ( + "the smoke must not need a freeze -- ISCC packages files, it does not run them, " + "so a placeholder keel.exe exercises the whole script for one cheap compile" + ) + + +# -- signing: implemented, gated on the certificates, honest about skipping --------------------- +# +# #438's delta on the "ship unsigned" decision: the signing work is REAL -- codesign with +# the hardened runtime, notarytool --wait, stapler, signtool with an RFC 3161 timestamp -- +# but each leg runs only when that leg's credentials exist on the `signing` environment. +# Missing credentials must SKIP WITH A NOTICE that names every secret and what it costs +# (#402's discipline: a missing prerequisite is announced, never a red release), and the +# checks below exist so the gate can never be detached from the step it guards. + +_MACOS_SIGNING_SECRETS = ( + "MACOS_CERT_P12_BASE64", + "MACOS_CERT_PASSWORD", + "APP_STORE_CONNECT_KEY_ID", + "APP_STORE_CONNECT_ISSUER_ID", + "APP_STORE_CONNECT_KEY_CONTENT", +) +_WINDOWS_SIGNING_SECRETS = ("WINDOWS_CERT_PFX_BASE64", "WINDOWS_CERT_PASSWORD") + + +def _step(job: dict, name: str) -> dict: + step = next((s for s in job["steps"] if str(s.get("name", "")) == name), None) + assert step is not None, f"release.yml's desktop job must keep a step named {name!r}" + return step + + +def test_the_signing_secrets_live_on_a_protected_environment(desktop_job: dict) -> None: + """#438: the certificates must be ENVIRONMENT secrets, not repository ones -- a + repository secret is handed to every same-repo PR build, while an environment secret + stops at the environment's reviewers. Until the environment exists the reference is + inert (GitHub treats a missing environment as unprotected) and the signing steps skip + honestly, which is why this can be declared unconditionally.""" + assert desktop_job["environment"] == "signing", ( + "the desktop job must reference the `signing` environment -- that is the only " + "thing that stands between a certificate secret and every PR build" + ) + + +def test_macos_signing_runs_only_when_every_apple_credential_exists(desktop_job: dict) -> None: + """All FIVE or none: a signed-but-un-notarised app is the worst state on macOS (it + still trips Gatekeeper, and now looks like it tried not to), so the gate is the whole + Apple set -- the Developer ID Application .p12 to sign, and the App Store Connect API + key trio notarytool needs. #402's lesson is measured per component, not by one token.""" + condition = str(_step(desktop_job, "Sign, notarise and staple (macOS)").get("if", "")) + assert "runner.os == 'macOS'" in condition + for secret in _MACOS_SIGNING_SECRETS: + assert f"secrets.{secret} != ''" in condition, ( + f"the macOS sign step must require {secret} -- a partial credential set must " + "skip, not half-sign" + ) + + +def test_windows_signing_runs_only_when_the_certificate_exists(desktop_job: dict) -> None: + condition = str(_step(desktop_job, "Sign the installer (Windows)").get("if", "")) + assert "runner.os == 'Windows'" in condition + for secret in _WINDOWS_SIGNING_SECRETS: + assert f"secrets.{secret} != ''" in condition + + +def test_each_skip_notice_names_every_missing_secret_and_the_price_of_fixing_it( + desktop_job: dict, +) -> None: + """The honest skip: each notice fires on the exact COMPLEMENT of its sign step's gate, + and says what to buy, which secrets to create, and where the checklist is -- so a + reader of a green run learns signing was SKIPPED, never believes it happened, and + knows the purchase that would turn it on (#438's signing table, restated as text).""" + cases = [ + ("Notice: macOS signing skipped", _MACOS_SIGNING_SECRETS, "$99"), + ("Notice: Windows signing skipped", _WINDOWS_SIGNING_SECRETS, "SmartScreen"), + ] + for name, secrets, product in cases: + notice = _step(desktop_job, name) + condition = str(notice.get("if", "")) + run = str(notice.get("run", "")) + assert "::notice" in run, f"{name} must be a ::notice, not a log line" + assert "docs/desktop-install.md" in run + assert product in run.replace("\\", ""), ( + f"{name} must name the paid product that unlocks signing -- the reader is " + "being asked to accept an unsigned binary, and the price is the context" + ) + for secret in secrets: + assert f"secrets.{secret} == ''" in condition, ( + f"{name} must fire when {secret} is missing -- every gap in the gate " + "needs its explanation" + ) + assert secret in run, ( + f"{name} must NAME {secret} -- a notice that says only 'not configured' " + "has already been failed by code-quality.yml's preflight prose (#402)" + ) + + +def test_signing_happens_between_packaging_and_the_checksums(desktop_job: dict) -> None: + """The sums must cover the SIGNED bytes: checksumming first and signing after would + publish hashes that prove nothing about what the user downloads. Signing must also + come after packaging, because the steps sign out/keel.app and out/*-setup.exe.""" + names = [str(s.get("name", "")) for s in desktop_job["steps"]] + package_at = names.index("Package (macOS)") + installer_at = names.index("Build the installer (Windows)") + sign_at = names.index("Sign, notarise and staple (macOS)") + win_sign_at = names.index("Sign the installer (Windows)") + checksums_at = names.index("Checksums") + assert package_at < sign_at and installer_at < win_sign_at < checksums_at + assert sign_at < checksums_at + + +def test_the_macos_leg_uses_the_hardened_runtime_and_waits_for_notarisation( + desktop_job: dict, +) -> None: + """`--options runtime` is REQUIRED for notarisation (an app signed without the + hardened runtime is rejected server-side, after the upload); `--wait` is what makes a + Rejected submission FAIL the step instead of returning an id; stapling is what lets a + machine that never queries Apple verify the ticket. And the certificate must live in + an EPHEMERAL keychain that is deleted after -- imported into the login keychain it + would outlive the step until the job ends.""" + run = str(_step(desktop_job, "Sign, notarise and staple (macOS)").get("run", "")) + assert "--options runtime" in run + assert "notarytool" in run and "--wait" in run + assert "stapler staple" in run + assert "security create-keychain" in run + assert "security delete-keychain" in run + + +def test_the_windows_leg_timestamps_its_signature_and_verifies_it( + desktop_job: dict, +) -> None: + """An untimestamped signature dies with the certificate -- /tr (RFC 3161) is what + makes it outlive the cert's expiry -- and an unverified signature is a hope, so the + step must signtool-verify /pa against the machine's default policy afterwards.""" + run = str(_step(desktop_job, "Sign the installer (Windows)").get("run", "")) + assert "signtool" in run + assert "/tr http" in run and "/td SHA256" in run and "/fd SHA256" in run + assert "verify" in run + + # -- what the person downloading it is told ---------------------------------------------------- _INSTALL_DOC = _ROOT / "docs" / "desktop-install.md" @@ -354,3 +607,30 @@ def test_the_release_notes_point_at_the_full_explanation() -> None: text = _WORKFLOW.read_text(encoding="utf-8") assert "docs/desktop-install.md" in text assert "cannot currently afford" in text + + +def test_the_install_note_carries_the_operator_activation_checklist() -> None: + """#438 made activation a PURCHASE, not a code change -- and the skip notices in the + workflow point at this page. So the page must hold the complete shopping list: every + secret name the gates check, the product that sells it, the price from #438's signing + table, the `signing` environment by name, and the one manual step (release-notes + wording) whose forgetting errs safe. A checklist missing a name would send the + operator to GitHub with an incomplete list and a second dispatch they did not expect.""" + text = _INSTALL_DOC.read_text(encoding="utf-8") + for secret in (*_MACOS_SIGNING_SECRETS, *_WINDOWS_SIGNING_SECRETS): + assert secret in text, f"the activation checklist must name {secret}" + assert "Environments" in text and "`signing`" in text, ( + "the checklist must say WHERE the secrets go -- an environment secret in the wrong " + "place is a repository secret, visible to every same-repo PR build" + ) + # The prices from #438's signing table, restated where the decision is made. + assert "$99" in text and "$9.99" in text and "SmartScreen" in text + assert "EV" in text, ( + "the checklist must warn EV is not worth extra -- no instant SmartScreen pass since 2024" + ) + # The honest asymmetry: the notes wording cannot auto-detect signing, and the failure + # mode of forgetting it must be stated (and must be the safe direction). + assert "safe direction" in text + assert "notarised" in text or "un-notarised" in text, ( + "the checklist must explain WHY the macOS gate is all five secrets or none" + ) diff --git a/tests/test_security_scans.py b/tests/test_security_scans.py index 3d52da8..15d3dfe 100644 --- a/tests/test_security_scans.py +++ b/tests/test_security_scans.py @@ -189,22 +189,54 @@ def _code_lines(text: str) -> str: return "\n".join(re.split(r"(^|\s)#", line)[0] for line in text.split("\n")) +#: Workflows allowed to reference secrets beyond GITHUB_TOKEN, and EXACTLY which ones +#: (#438). `release.yml` is manual-only (workflow_dispatch) and its desktop job's signing +#: steps are the #402 pattern: fully implemented, gated per secret on the protected +#: `signing` environment, and paired with notice steps that fire INSTEAD when a secret is +#: absent -- a skip that names what to add, never a red run. The gates themselves are +#: pinned in tests/test_desktop_packaging.py; this table only bounds which names may ever +#: appear, so a new secret reference here is a decision, not an accident. +_GATED_SIGNING_SECRETS: dict[str, frozenset[str]] = { + "release.yml": frozenset( + { + "MACOS_CERT_P12_BASE64", + "MACOS_CERT_PASSWORD", + "APP_STORE_CONNECT_KEY_ID", + "APP_STORE_CONNECT_ISSUER_ID", + "APP_STORE_CONNECT_KEY_CONTENT", + "WINDOWS_CERT_PFX_BASE64", + "WINDOWS_CERT_PASSWORD", + } + ), +} + + def test_the_baseline_scans_reference_no_secrets_at_all(): """The always-on workflows run on GITHUB_TOKEN alone -- no wishlist tokens, none. The baseline is what makes the scans real: `security.yml` and `ci.yml` must reference no secret whatsoever, or they would be scans that run only for a hypothetical maintainer with hypothetical tokens -- the gap #291 exists to close, restated as YAML. + + The ONE exception is the gated signing tier (#438, table above): secrets that exist + only on the `signing` environment, referenced by steps that skip with a notice when + they are missing. An always-on workflow must still reference NOTHING beyond + GITHUB_TOKEN -- a per-PR job that needed a certificate would fail for every + contributor without it. """ for path in sorted(_WORKFLOWS.glob("*.yml")): if path.name == _OPTIONAL_TIER: continue executable = _code_lines(path.read_text()) + allowed = _GATED_SIGNING_SECRETS.get(path.name, frozenset()) | {"GITHUB_TOKEN"} referenced = sorted(set(re.findall(r"secrets\.([A-Za-z_][A-Za-z0-9_]*)", executable))) - assert not referenced or referenced == ["GITHUB_TOKEN"], ( + assert not referenced or set(referenced) <= allowed, ( f"{path.name} references secrets {referenced} -- a workflow that always runs " "must run on GITHUB_TOKEN alone, or it fails for every contributor without " - "the missing tokens" + "the missing tokens. (Beyond GITHUB_TOKEN, only the gated signing tier in " + f"{sorted(_GATED_SIGNING_SECRETS)} may reference secrets, and only those it " + "declares -- extend the table deliberately, with the gate that skips without " + "them.)" )