ci(win): sign the Windows installer via SignPath (free for OSS) - #283
Closed
EtienneLescot wants to merge 2 commits into
Closed
ci(win): sign the Windows installer via SignPath (free for OSS)#283EtienneLescot wants to merge 2 commits into
EtienneLescot wants to merge 2 commits into
Conversation
SmartScreen keys an installer's reputation to the signing identity when it is signed and to the file hash when it is not, so the unsigned NSIS installer restarts from zero reputation at every release: users who had stopped seeing the "Windows protected your PC" interstitial on one version meet it again on the next, forever. Signing is gated on the secrets existing, like the macOS job, so this is inert until an Azure Trusted Signing account is configured — builds keep producing an unsigned installer meanwhile, and forks are unaffected. A partial configuration fails the job instead, since it is a typo rather than a choice and the quiet alternative is publishing unsigned. The key stays in Microsoft's HSM, so nothing is imported on the runner and no certificate material lives in a secret. Arguments go through a bash array because publisherName must match the certificate subject exactly and legal names contain spaces. Only the NSIS job changes; the Store package is re-signed by Microsoft during certification and is left alone.
📝 WalkthroughWalkthroughThe Windows workflow now supports optional SignPath signing. It rejects partial configuration, preserves unsigned builds when secrets are absent, exchanges installer artifacts for signing, and verifies Authenticode signatures and timestamps. Documentation defines the signing process and policy. ChangesWindows installer signing
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Windows Build
participant SignPath
participant Authenticode Verification
GitHub Actions->>Windows Build: Produce unsigned installer
GitHub Actions->>SignPath: Submit installer artifact
SignPath-->>GitHub Actions: Return signed installer artifact
GitHub Actions->>Windows Build: Replace installer
GitHub Actions->>Authenticode Verification: Validate signature and timestamp
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Azure Artifact Signing is not reachable here: Microsoft restricts individual developer identity validation to the United States and Canada, and the EU path requires a registered legal entity. SignPath Foundation issues OV certificates free of charge to OSI-licensed projects, which OpenScreen qualifies for. The shape of the integration changes with it. SignPath signs out of band rather than inside electron-builder: the build produces an unsigned installer, uploads it as a short-lived workflow artifact, and SignPath pulls it by artifact id, signs it on its own HSM and hands the file back. It is swapped over the build output so the published openscreen-windows artifact keeps its name and shape whether or not signing ran, and the release publisher is unaffected. Hence the new actions: read permission — SignPath reads the run's artifacts with the job's own GITHUB_TOKEN. Foundation release policies require a human to approve each request, so the wait is raised to an hour; the action's 600 s default expires while the approver is still reading the notification mail. The gate and the signature verification are unchanged from the Azure version: inert with no secrets, hard failure on a partial configuration, and an assertion on the artifact itself since a signing step that no-ops still exits 0. Adds CODE_SIGNING_POLICY.md, which the Foundation's conditions require to be published and to credit SignPath. It leaves one TODO for the privacy policy URL.
EtienneLescot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CODE_SIGNING_POLICY.md`:
- Around line 77-79: Replace the TODO placeholder in the policy document with
the maintained public privacy-policy URL, reusing the same URL declared for the
Microsoft Store submission. Ensure the URL is presented as an explicit
privacy-policy reference before SignPath onboarding or renewal.
- Around line 40-51: The code-signing policy must require signing workflows to
run for an immutable tag matching the requested release tag. Update the release
gate in build.yml to allow signing only when github.ref equals
refs/tags/${input_release_tag}, or explicitly revise the policy to exclude
manual-dispatch releases from tagged-source attestations.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 812bf86c-bb2a-4b7a-8e42-6ea6dafeb280
📒 Files selected for processing (3)
.github/workflows/build.ymlCODE_SIGNING_POLICY.mdtechnical-documentation/engineering/release-and-secrets.md
🚧 Files skipped from review as they are similar to previous changes (1)
- technical-documentation/engineering/release-and-secrets.md
Comment on lines
+40
to
+51
| 1. A tagged release triggers the `Build Electron App` workflow on GitHub Actions. | ||
| 2. The workflow builds the installer from the tagged source and uploads it as a | ||
| workflow artifact. | ||
| 3. SignPath retrieves that artifact directly from the workflow run, verifying it | ||
| came from this repository's CI rather than from an uploaded file. | ||
| 4. The maintainer approves the signing request in the SignPath dashboard. | ||
| 5. The signed installer is returned to the workflow, its signature is verified, | ||
| and it is published as the release artifact. | ||
|
|
||
| Because the artifact is pulled from the workflow run rather than submitted by | ||
| hand, a signature attests that the binary was built by this repository's CI from | ||
| tagged source. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect workflow triggers and any ref checks around the signing step.
sed -n '1,175p' .github/workflows/build.yml
rg -n -C5 'workflow_dispatch|refs/tags|github\.ref|Sign installer via SignPath' .github/workflowsRepository: getopenscreen/openscreen
Length of output: 29484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the macOS signing job, Windows Store signing job, and release-job gating for ref/tag checks.
sed -n '170,250p' .github/workflows/build.yml
sed -n '250,400p' .github/workflows/build.yml
sed -n '400,510p' .github/workflows/build.yml
sed -n '570,620p' .github/workflows/build.yml
# Verify local YAML event expression forms do not enforce ref tag constraints besides the publish/release step.
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/build.yml')
text = p.read_text()
events = []
for line in text.splitlines():
line = line.strip()
if line.startswith(('push:', 'release:', 'workflow_dispatch:', 'schedule:', 'pull_request:')):
events.append(line)
print("workflow event declarations in build.yml:")
for e in events:
print("-", e)
print("contains refs/tags check:", "refs/tags" in text)
print("macOS signing step names:", [line.strip() for line in text.splitlines() if "Signing" in line or "notarize" in line.lower() or "SignPath" in line])
PYRepository: getopenscreen/openscreen
Length of output: 19954
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm manual dispatch has no release-specific trigger constraint and inspect Linux build context.
sed -n '1,15p' .github/workflows/build.yml
sed -n '200,240p' .github/workflows/build.yml
sed -n '500,585p' .github/workflows/build.ymlRepository: getopenscreen/openscreen
Length of output: 5168
Require an immutable tag before signing.
build.yml runs from any manual dispatch to build-windows and build-macos, and those jobs sign when secrets are present. The release gate accepts workflow_dispatch with any release_tag, even if that tag has no refs/tags/ ref. Gate signing on github.ref == "refs/tags/${input_release_tag}" or amend the policy to state that manual-dispatch releases are not tagged-source attestations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CODE_SIGNING_POLICY.md` around lines 40 - 51, The code-signing policy must
require signing workflows to run for an immutable tag matching the requested
release tag. Update the release gate in build.yml to allow signing only when
github.ref equals refs/tags/${input_release_tag}, or explicitly revise the
policy to exclude manual-dispatch releases from tagged-source attestations.
Comment on lines
+77
to
+79
| <!-- TODO(maintainer): link the published privacy policy URL here — the SignPath | ||
| Foundation conditions require this page to reference it, and the same URL is | ||
| already declared in the Microsoft Store submission. --> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Replace the required privacy-policy placeholder.
This TODO leaves the required privacy-policy link absent. The text states that the SignPath Foundation requires this reference. Add the maintained public URL before using this policy for SignPath onboarding or renewal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CODE_SIGNING_POLICY.md` around lines 77 - 79, Replace the TODO placeholder in
the policy document with the maintained public privacy-policy URL, reusing the
same URL declared for the Microsoft Store submission. Ensure the URL is
presented as an explicit privacy-policy reference before SignPath onboarding or
renewal.
Collaborator
Author
|
Superseded: distribution strategy changed rather than the signing setup. Verified on the actual CI artifacts that both the NSIS installer and the AppX come out unsigned — the Microsoft signature is applied during Store certification and only ever exists on the copy hosted in the Store: So rather than buying a certificate to smooth the GitHub Paying for a certificate stays open as a later, optional call once install traffic shows how much the fallback path actually matters. |
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
1 participant
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.
Why
The NSIS installer published on GitHub releases is unsigned: there is no
signtool, certificate or signing configuration anywhere in the repo, while macOS has had a full sign + notarize + verify path for a while.The cost is not just "Unknown publisher" in the UAC prompt. SmartScreen keys reputation to the signing identity for a signed binary, and to the file hash for an unsigned one. Every release therefore starts from zero: users who had stopped seeing "Windows protected your PC" on 1.8.0 meet the full interstitial again on 1.9.0, and again on 1.10.0. Signed, that reputation accumulates across versions instead of resetting.
The Store package is unaffected either way — Microsoft re-signs it during certification — so this PR touches only the NSIS job.
Why not Azure
The first version of this PR used Azure Trusted Signing. It is not reachable here. Microsoft's current docs state:
The maintainer is in France, so the individual path is closed, and the EU organization path needs a registered legal entity with a business identifier. SignPath Foundation issues OV certificates free of charge to OSI-licensed projects, which OpenScreen qualifies for — and it costs nothing rather than $9.99/month.
What changes
SignPath signs out of band, not inside electron-builder, so the pipeline shape differs from the Azure version:
Step 4 is what keeps the blast radius small: the published
openscreen-windowsartifact keeps its name, path and shape whether or not signing ran, so the release publisher needs no change. Step 3 is why the workflow now grantsactions: read— SignPath reads the run's artifacts with the job's ownGITHUB_TOKEN.Because SignPath fetches from the workflow run rather than accepting an upload, a signature attests the binary came from this repository's CI on tagged source, not from someone's laptop.
A release can block on a human. Foundation release policies require manual approval of every signing request. The wait is raised to 3600 s; the action's 600 s default expires while the approver is still reading the notification mail.
Unchanged from the Azure version
Gated on the secrets being present, mirroring the macOS job's
steps.signing.outputs.enabledidiom:Verify installer signatureasserts on the artifact rather than on the pipeline having run, because a signing step that no-ops still exits 0. It requiresGet-AuthenticodeSignatureto reportValidand a timestamp to be present — without one, the signature stops validating the day the certificate expires, retroactively invalidating every release already in users' hands.Verification
The gate is branchy, so it is tested by extracting the real script out of
build.yml— including the list of secret names from its ownrequired=(...)array, so the test cannot drift from what CI runs:Action inputs were checked against
action.ymlat tagv2rather thanmain.Also adds
CODE_SIGNING_POLICY.mdThe Foundation's conditions require a published code signing policy crediting SignPath and listing team roles — it is an onboarding prerequisite, not documentation for its own sake. It states the single-maintainer reality, that the key never leaves SignPath's HSM, and that an OV certificate means SmartScreen reputation still builds over time rather than vanishing at once.
It carries one
TODOfor the privacy policy URL, which the conditions also require and which I could not source from the repo.Merging this does not sign anything yet
It is inert until the SignPath onboarding completes and the four secrets exist. Merging now means the first release built afterwards is signed automatically, with no further code change.
Summary by CodeRabbit
New Features
Documentation