Windows 5/5: release integration and full-system proof - #1010
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
9de88ae to
ecd30ad
Compare
ca3f537 to
093ba1c
Compare
|
@CodeRabbit review. @greptile-apps review. |
|
✅ Action performedReview finished.
|
ecd30ad to
d4d6c68
Compare
093ba1c to
ee93abd
Compare
d4d6c68 to
763067f
Compare
ee93abd to
10c36ef
Compare
763067f to
cfa91b8
Compare
10c36ef to
e2915e0
Compare
cfa91b8 to
76cc8c3
Compare
e2915e0 to
eb4385e
Compare
76cc8c3 to
6940838
Compare
eb4385e to
d46319b
Compare
6940838 to
37cb390
Compare
d46319b to
5db8b0e
Compare
37cb390 to
dcb7fc5
Compare
5db8b0e to
c95b678
Compare
dcb7fc5 to
a8c5c3f
Compare
c95b678 to
a7881be
Compare
a8c5c3f to
0637a09
Compare
Bind release tags to the approved target, replace draft assets with the exact validated inventory, and reject unauthorized runtime or checksum entries. Co-authored-by: David Whatley <nsxdavid@gmail.com> Based-on: nsxdavid/ADE#999
PR5 renamed prepare-release.yml to "Prepare signed Windows proof" and made its resolve job hard-fail unless ADE_WINDOWS_SIGNED_BUILD_ENABLED=1 and ADE_WINDOWS_PUBLIC_RELEASE_ENABLED != 1. That destroyed the pre-existing platform-neutral non-publishing validation run for macOS and the standalone runtime: in today's flag state, with no Windows variables set, the workflow cannot be dispatched at all. It was also circular, because enabling public Windows releases made the proof workflow unrunnable until the same gate was turned back off. Restore the workflow as the platform-neutral dry run it has always been and move the Windows proof-mode preconditions behind an explicit windows_proof dispatch input, so both uses coexist. Proof mode still refuses to run while publication is enabled, which keeps the documented "collect proof with the public gate off" rule, but that requirement no longer applies to ordinary validation runs. Based-on: nsxdavid/ADE#999 (cherry picked from commit d7040859f5b27fb84388efc9c0082e03fd91b9f3)
promote-approved-win-proof downloads ade-win-release-<tag> from an arbitrarily old run named by ADE_WINDOWS_APPROVED_PROOF_RUN_ID, but the proof upload set no retention-days. Once the approved run's artifacts aged out, or once a lowered repository or organization default shortened the window, a tagged release failed with an opaque download error. Pin retention-days: 90 on the proof upload and on the promotion re-upload, and check that the named artifact still exists and has not expired before downloading it. The check distinguishes missing, expired and unreadable and names the recovery path in each case: re-run the proof for the approved commit and rebind ADE_WINDOWS_APPROVED_PROOF_RUN_ID and ADE_WINDOWS_APPROVED_BUILD_MANIFEST_SHA256. Based-on: nsxdavid/ADE#999 (cherry picked from commit a4308f34b57c25a52aac4bc0ed507b3a96560d7a)
When the Windows gate was on, publish-release rebuilt the upload list as base_files + windows_files, which dropped release-assets/runtime entirely. windows_files supplied install.sh, SHA256SUMS and all ten ade-darwin-*, ade-linux-* and ade-win32-x64 runtime binaries from release-assets/win, that is from the earlier approved proof run, while the macOS DMG and zip came from the current run. Darwin runtime binaries are notarized per run, so the published standalone ade-darwin-* were not byte-identical to the ones inside the DMG shipped beside them, and the published SHA256SUMS described the proof run's bytes rather than the uploaded ones. The job also generated and validated release-assets/runtime/SHA256SUMS and then never uploaded it. Publish the cross-platform standalone set from the current run and take only the genuinely Windows-specific files from the approved proof: the installer, its blockmap, latest.yml, install.ps1, and the signed ade-win32-x64 executable and native archive. Generate one SHA256SUMS over that exact merged set, and verify it names precisely the published standalone assets. The published asset names are unchanged in every flag state, so install-runtime.sh and install-runtime.ps1 keep resolving the same entries. The proof bundle's own inventory and checksum allowlist are untouched and still validate the full ten-file cross-platform set plus install.sh, install.ps1 and its SHA256SUMS; those superseded members are now asserted present as proof-internal files rather than published. Based-on: nsxdavid/ADE#999 (cherry picked from commit a3b0205afd74bfd00c8dc410f2d7789f74de790c)
ADE_WINDOWS_INSTALLED_UPDATE_PROOF_APPROVED is required at six sites in release-core.yml but appeared nowhere in the Windows release playbook. A maintainer following that playbook exactly would set the signed-build flag, the three APPROVED_* bindings and the public flag, tag a release, and have the run die in verify with "requires approved two-version installed-update proof." Document it in "Enable Windows releases" and in the pre-tag checklist: what it attests, what must be true before setting it, and where the supporting evidence lives. Add a "Required GitHub Actions settings" reference covering every repository variable and secret release-core.yml actually reads, audited by grep against the workflow, including the pre-existing macOS signing and notarization secrets the playbook had never named. Also document the signed Windows proof mode input, the 90-day approved-proof artifact retention window and its recovery path, and the deliberate two-run provenance of the published asset set. Based-on: nsxdavid/ADE#999 (cherry picked from commit a56345e0733dcebdccba42dff536af65fb0853f0)
Windows was confined to non-publishing runs and a tagged release could only ship Windows by promoting an artifact from an earlier approved proof run. The repo owner has decided Windows should build fresh on the tag exactly like macOS, accepting that a release may publish Windows bytes that never went through a manual clean-host proof sweep, so that every desktop and CLI release publishes Windows automatically. Rename build-win-proof to build-win-release and drop its inputs.publish == false restriction, so a v* tag builds, signs, validates and publishes Windows in-run with the same shape as build-mac-release. publish-release now needs build-win-release, and with the Windows gate on a failed or skipped Windows build blocks the draft exactly as a failed macOS build does. Every machine-checkable gate is retained on the publish path: the job-level signing-secret requirement, --require-signing in run-electron-builder.mjs, --require-signed Authenticode verification in validate-win-artifacts.mjs, and the installed-product lifecycle smoke. Only the human proof-sweep gate is dropped. Two validations are re-attached so nothing is lost with promotion: the cross-platform runtime allowlist moves to the ungated publish path, where it now holds in every flag state instead of only when Windows publishes, and the Windows standalone bytes are cross-checked against the digests computed on the signing runner. A new check requires latest.yml to reference the installer actually published beside it, matching the macOS updater-feed contract. Proof collection survives as an opt-in mode: the windows_proof input is now threaded into release-core.yml and gates the proof staging, manifest generation, manifest validation and bundle upload. It reads no repository variable, so evidence can be collected before Windows is enabled and again as a regression check afterwards, which also resolves the circular gate. Remove promote-approved-win-proof and the four repository variables that only existed to bind it. Keeping them behind an opt-in would preserve a second, divergent asset-assembly path that reintroduces the cross-run provenance bug and would have to be validated forever. ADE_WINDOWS_PUBLIC_RELEASE_ENABLED is now the only Windows repository variable the release reads. Asset assembly collapses accordingly: everything is published from the current run, so the proof-internal split is gone and the ade-darwin-* provenance bug cannot reappear. The published asset names are unchanged in both flag states. Based-on: nsxdavid/ADE#999 (cherry picked from commit f79fa49101a0d971e3312a7634a094680f441c9a)
The /release skill grepped clean for windows, win-x64 and latest.yml. Its updater-correctness list and its required-assets verification were macOS-only, so it would happily verify and publish a release containing zero Windows assets. AGENTS.md described a release as producing only .dmg, .zip, blockmap and latest-mac.yml. Add the Windows assets to both the updater-correctness list and the required-assets inventory, add the latest.yml updater-feed check, and add a Phase 0 preflight that reads ADE_WINDOWS_PUBLIC_RELEASE_ENABLED and records the expected platform matrix so later phases know what the draft must contain. Add a gate/asset agreement check that fails in both directions: gate on with no Windows assets means the Windows build silently did not contribute, gate off with Windows assets means they reached a release that was not supposed to carry them. Both keep the release draft. Also correct the standalone runtime assets, which the required-assets list had never covered on any platform, and update the expected workflow shape, recovery rules and final report for the in-run Windows build. Based-on: nsxdavid/ADE#999 (cherry picked from commit 78762d2119eb4d9a9acb45dbfdc51e27d94b2acb)
Rewrite the release-pipeline bullets in ARCHITECTURE for the model that landed: Windows builds fresh on the tag mirroring build-mac-release, one repository variable gates it, promotion of a previously approved proof run and its four binding variables are gone, and the accepted tradeoff is stated plainly. Record that proof collection survives as an opt-in mode that reads no repository variable, that the evidence bundle's 90-day retention now bounds review time rather than tagging, and that every published asset comes from the run that built it. Based-on: nsxdavid/ADE#999 (cherry picked from commit c943ec73bdad8f439d8431b6719e4848dab4e666)
The Windows standalone checksum manifest is written by Git Bash on windows-latest, where sha256sum reads in binary mode and marks each name with a leading asterisk. Publishing runs on Linux, where the two-space form is required. Assert the normalization is present so a future edit cannot quietly drop it and turn the cross-runner digest check into a hard release failure. Based-on: nsxdavid/ADE#999 (cherry picked from commit adcc755de4aa0d5e49836a2a33058c9e31cab4de)
Three docs outside the signing playbook still instructed maintainers to set or reason about repository variables that no longer exist: the proof contract told them to bind the approved proof SHA, run id and build-manifest digest so a release could promote those bytes; the port lane told them to enable a separate signed-build variable and bind an approved proof run before enabling promotion; and the auto-update doc described release generation and public availability as two variables. Point all three at the single ADE_WINDOWS_PUBLIC_RELEASE_ENABLED gate and the windows_proof dispatch input, rename the evidence artifact to ade-win-proof-v<VERSION>, and state in the proof contract that a published installer carries its own signing timestamp and is therefore not the byte-identical artifact the proof indexes. Based-on: nsxdavid/ADE#999 (cherry picked from commit 356d365eeb6ae14326e553815b3be9ad0d6fb5fb)
The playbook was built entirely around approved-proof promotion. It told maintainers to set five repository variables, bind an approved proof SHA, run id and build-manifest digest, and re-prove every later release by first disabling the public gate. None of that exists any more. Reduce enablement to one variable and one command, and reframe the clean-host sweep as strongly recommended pre-enablement validation and an ongoing regression check rather than a gate. State the accepted tradeoff plainly and once: the pipeline enforces nothing about the sweep, so a release can publish Windows bytes no human installed on a clean machine. Keep the signing language exactly as strong as the code: signing is mandatory and machine-enforced at every layer, and there is no unsigned Windows publication path. Rename the downloaded evidence bundle to ade-win-proof-v<VERSION>, note that ade-win-release-v<VERSION> now holds only the installer set, restate the retention window as bounding evidence review rather than tagging, and replace the promotion failure modes with the ones that now matter: the Windows build failing while the gate is on, and the gate disagreeing with the draft's Windows assets in either direction. Also clarify in the proof contract that releaseGates describes the proof run rather than repository state, so a regression sweep collected after enablement still records publicReleaseEnabled false truthfully, and that signedBuildEnabled is a historical field name. Based-on: nsxdavid/ADE#999 (cherry picked from commit 32fefb97466156278866fb3bcc98be42ccb7d0ea)
ADE vendors cr-sqlite for darwin-arm64, darwin-x64 and win32-x64 only. The architecture and CRDT docs claimed Linux loads crsqlite.so, which has never been true: package-native-deps warn-and-skips linux, so every release ships a Linux brain with no CRR engine. Rather than close the gap by vendoring a Linux binary, this records the support line as it actually is. A Linux host is a remote runtime target that a macOS or Windows desktop drives over SSH -- a role that does not touch cr-sqlite at all -- and is not a sync peer. That keeps the capability we really have without claiming replication we do not. The packaging note now distinguishes an intentional exclusion from an unvendored target, so a Linux build no longer emits a WARNING that reads as a defect. Based-on: nsxdavid/ADE#999
The Windows packaging path assumed a password-protected PFX delivered through WINDOWS_CSC_LINK and WINDOWS_CSC_KEY_PASSWORD. That model cannot exist: since June 2023 CA/Browser Forum rules require code-signing private keys to live on FIPS-validated hardware, exportable .pfx delivery ended for OV and EV alike, and Azure Artifact Signing never releases the certificate at all - it is held in the service and reachable only at the moment of signing. Neither secret is set in this repository and neither ever will be. Rewire run-electron-builder.mjs onto electron-builder 26's native win.azureSignOptions. electron-builder selects the Azure signing manager above the single chokepoint every Windows artifact passes through, so one configuration covers the packaged channel executable, its bundled DLLs, the NSIS installer, and the uninstaller. A post-build signing step could only reach the finished installer, leaving the executable already embedded inside it unsigned. The signed path now requires the AZURE_TENANT_ID / AZURE_CLIENT_ID / AZURE_CLIENT_SECRET triple that Azure.Identity's EnvironmentCredential reads, and hands it to electron-builder only on that path so an unsigned dist:win can never reach the signing service. EnvironmentCredential is first in the DefaultAzureCredential chain, so a complete triple is resolved before any managed-identity probe against an instance-metadata endpoint a GitHub-hosted runner does not have. Pin the publisher by certificate Subject and nothing else. The service renews its certificate daily and expires it after 72 hours, so a pinned thumbprint would fail every release within days. WINDOWS_SIGNING_EXPECTED_THUMBPRINT is therefore refused outright rather than ignored, so a pin cannot quietly stop pinning. The same Subject is passed as electron-builder's publisherName, which electron-updater parses as a Distinguished Name before running a downloaded installer, so the updater and the release validator agree on one publisher. validate-win-artifacts.mjs stays exactly as strict: valid Authenticode status, a trusted RFC3161 timestamp, the pinned Subject, and one certificate shared by the installer and the ADE.exe it installs. Based-on: nsxdavid/ADE#999 (cherry picked from commit d49aa1ec698604a77d4cc54b52afd7f9a416bd17)
…act Signing The standalone runtime signer still imported a PFX from WINDOWS_CSC_LINK and WINDOWS_CSC_KEY_PASSWORD, so it would have failed for the same reason the desktop packaging path did: there is no exportable certificate to import, and Azure Artifact Signing never releases one. Sign through Invoke-TrustedSigning, the same mechanism electron-builder 26 uses for the desktop installer, so both Windows artifacts ADE publishes come from one signing path with one set of parameter names. All local key-material handling is gone; the only credential the script carries is the Entra service principal. Verification is unchanged in strictness: valid Authenticode status, a trusted RFC3161 timestamp against the service's own timestamp authority, and the pinned publisher Subject. WINDOWS_SIGNING_EXPECTED_THUMBPRINT is refused rather than ignored, because a certificate that expires after 72 hours cannot be pinned by thumbprint. Based-on: nsxdavid/ADE#999 (cherry picked from commit d8c26adf40837716d0ecd8e62a5498afc58d2623)
Both release workflows gated Windows on WINDOWS_CSC_LINK and WINDOWS_CSC_KEY_PASSWORD, secrets that are not set and never will be. Replace that job-level requirement with the Azure Artifact Signing service principal: AZURE_TENANT_ID, AZURE_CLIENT_ID, and AZURE_CLIENT_SECRET, the three names Azure.Identity's EnvironmentCredential reads. Microsoft's guidance for runners outside Azure is exactly this triple, because the credential chain otherwise falls through to a managed-identity probe a GitHub-hosted runner cannot answer and raises CredentialUnavailableException. verify keeps failing about a minute in rather than after a full package build, and now also refuses to start while WINDOWS_SIGNING_EXPECTED_THUMBPRINT exists. The service renews its certificate daily and expires it after 72 hours, so leaving a thumbprint pin accepted-but-ignored would look like a pin while pinning nothing. The macOS and Linux release paths are untouched; CSC_LINK and CSC_KEY_PASSWORD remain the macOS Developer ID secrets. Based-on: nsxdavid/ADE#999 (cherry picked from commit d07d802b627b210bf9d7f47f6673c96b97ab3742)
…t Signing The playbook documented a PFX flow that cannot exist and told maintainers to adapt the workflow themselves if they picked a service without one. The pipeline now signs through Azure Artifact Signing, so document that service and the exact resources behind it: the arulsigning account in rg-signing, the East US endpoint, the adePublicTrust certificate profile, and the ade-signing-ci service principal holding the Artifact Signing Certificate Profile Signer role. State plainly why there is no certificate to hold and why the publisher is pinned by Subject: the service holds the key, never releases the certificate, renews it daily, and expires it after 72 hours. Record that WINDOWS_SIGNING_EXPECTED_THUMBPRINT is refused at every layer rather than ignored, and that RFC3161 timestamping is what keeps a shipped installer verifiable past the certificate's three-day life. Correct the audited settings table to the four secrets that now exist, explain why electron-builder's native win.azureSignOptions was chosen over a post-build signing action, and add the failure modes a maintainer will actually hit: a 403 from a lost role assignment, a credential chain that fell past EnvironmentCredential, a build that straddled the daily certificate rotation, and a runner that could not reach PSGallery. Based-on: nsxdavid/ADE#999 (cherry picked from commit ce999120c28ecf3833cbd145cb8a00dc7f3c377d)
GitHub refused to start prepare-release.yml outright: Invalid workflow file: .github/workflows/prepare-release.yml#L112 The nested job 'publish-release' is requesting 'contents: write', but is only allowed 'contents: read'. release-core.yml is called by two entry points with different intents. release.yml is the real release and grants contents: write. prepare-release.yml is the non-publishing validation run and grants contents: read deliberately. GitHub validates a called workflow's job permissions statically, at parse time, before any job-level `if:` is evaluated, so publish-release's `permissions: contents: write` broke the dry run even though its `if: always() && inputs.publish && ...` meant it could never run there. Granting contents: write to prepare-release.yml would have fixed the parse error and destroyed the only thing that actually guarantees a dry run cannot create a release. Its header comment promises "It never creates or updates a GitHub Release"; the read-only token is what enforces that promise, not the hardcoded publish input. So the capability moves instead of the permission. publish-release now lives in release-publish.yml, a reusable workflow carrying its own contents: write, called only by release.yml. Nothing in release-core.yml requests more than contents: read any more, and the publish input is gone with it, so a dry run cannot reach the publishing path at all rather than being trusted to pass publish: false. Do not merge publish-release back into release-core.yml. That would reintroduce this exact failure the next time anyone dispatches the dry run. The publish gate is reconstructed, not relaxed. release-core.yml gains a build-results job with `if: always()` that republishes the three build results the gate reads as workflow outputs, and release.yml's publish-release job applies the same condition it always had: with the Windows gate on, a failed or skipped Windows build blocks the draft exactly as a failed macOS build does, and always() still lets the gate evaluate when build-win-release is legitimately skipped with the gate off. Artifacts are scoped to the run, not the workflow file, so the publish job still downloads ade-mac-release-*, ade-win-release-* and ade-runtime-* by name from the same run. Based-on: nsxdavid/ADE#999
040cf2a to
561e607
Compare
The main-ancestor check ran on every path, including windows_proof -- whose entire purpose is collecting clean-host packaging and signing evidence for a commit that has not merged yet. Requiring the commit be on main first made the mode unusable for what it exists to do. Skipped only for windows_proof. That mode is reachable only from prepare-release.yml, which holds contents: read and cannot create a release or tag, so nothing can ship from a commit this step did not vet. Every publishing path leaves windows_proof false and is still gated, as is the ordinary platform-neutral dry run. Based-on: nsxdavid/ADE#999
CI triggers on pull_request with branches: [main], so it only fires for the one PR in a stack whose base is main. The other four target their parent branch and therefore accumulate no check runs at all -- including ci-pass, which release-core.yml's verify job requires on the exact SHA it builds. That made a signed proof build of a stacked branch unreachable: the gate could never be satisfied. workflow_dispatch runs the same jobs against a given ref and produces the same ci-pass check run on the same SHA, so the release gate is satisfied by real CI rather than weakened to accommodate the stack. Based-on: nsxdavid/ADE#999
gitleaks-action scans only the PR range on pull_request, but the whole repository history on workflow_dispatch. Fingerprints are commit-scoped, so every finding already waived in .gitleaksignore reappears under the SHA of each later commit that touched the same line -- 19 of them here, all previously accepted false positives (a variable named secretPath, a STORAGE_KEY constant, a code comment, Clerk public client IDs). Pinning the dispatched scan to origin/main..HEAD gives it the same scope a pull_request run has, so identical code is judged identically however CI was started. The alternative -- appending 19 more commit-pinned waivers -- would need repeating every time one of those lines moves. Based-on: nsxdavid/ADE#999
gitleaks-action exposes no input for narrowing its commit range, so the previous attempt to pass one through the environment had no effect and the dispatched scan still walked all history. Call the pinned binary directly with --log-opts instead. Verified before pushing: of the six files carrying already-waived findings, five are untouched by this branch's 155 commits, and the change to the sixth adds windowsHide at line ~800 -- far from the flagged line 48. The range therefore contains no secret-shaped content. Based-on: nsxdavid/ADE#999
Same reasoning as the main-ancestor skip directly above it. windows_proof packages and signs a commit that has not merged, to prove the installer before review; requiring a green ci-pass on that exact SHA forces a full CI cycle per iteration and makes the mode unusable for what it is for. The mode is reachable only from prepare-release.yml, which holds contents: read and cannot create a release or tag, so nothing ships from a commit this step did not vet. Real releases leave windows_proof false and remain gated on both checks. Based-on: nsxdavid/ADE#999
Official Node.js releases are Authenticode-signed on Windows just as they are codesigned on macOS. postject rewrites the executable to embed NODE_SEA_BLOB, so a signature left in place covers bytes that no longer exist and signtool refuses to re-sign the result: SignTool Error: SignedCode::Sign returned error: 0x800700C1 0x800700C1 is ERROR_BAD_EXE_FORMAT. Node's SEA documentation requires removing the signature before injection on both platforms, but removeSignatureIfNeeded returned early for anything that was not darwin, so the step silently did nothing on Windows and every signed runtime build failed. Four platforms built; only win32-x64 died, at signing. signtool.exe ships with the Windows SDK and is not on PATH, so resolve it from the versioned SDK bin directories, newest first. A missing SDK warns rather than fails, keeping unsigned local builds working. Verified on this machine against a real signed node.exe: resolver located signtool 10.0.19041.0, 'remove /s' exited 0, and the binary went from SIGNED to UNSIGNED. Based-on: nsxdavid/ADE#999
Stack position
Windows 5/5 · cumulative head · parent: #1009 · stack root: #1006
This is the full-system head of stack #1011. No lower layer merges independently. Atomic merge is allowed only after every PR has a current ready-stacked state and this exact head SHA passes the complete proof matrix.
Based on the Windows implementation by @nsxdavid in #999.
Responsibilities
Exclusions
This PR does not publish a release or enable public Windows/website flags. WSL, ARM64, and native Windows computer use are excluded.
Validation
Draft checkpoint: implementation and proof-harness workers are still running. Final evidence must be bound to this PR's exact head/base/tree and is not yet complete.
Evidence and provenance