Skip to content

Remove Windows PFX signing fallback#3638

Draft
mokagio wants to merge 2 commits into
trunkfrom
mokagio/remove-windows-pfx-signing-fallback
Draft

Remove Windows PFX signing fallback#3638
mokagio wants to merge 2 commits into
trunkfrom
mokagio/remove-windows-pfx-signing-fallback

Conversation

@mokagio
Copy link
Copy Markdown
Contributor

@mokagio mokagio commented May 29, 2026

Related issues

Follow-up to the Studio Azure Trusted Signing migration (AINFRA-2233): removing the PFX fallback now that Azure signing is confirmed in shipped builds.

How AI was used in this PR

Drafted with Claude Code (Opus 4.8). I reviewed the full diff, ran the type checker and lint, and verified the pipeline/PowerShell/JS syntax locally; the Windows signing path itself only runs on CI.

Proposed Changes

Studio's Windows build kept a PFX certificate as a fallback behind the USE_AZURE_TRUSTED_SIGNING switch. Azure Trusted Signing has shipped successfully (v1.8.2, v1.9.0) and CI hardcoded the switch to 1 everywhere, so the PFX branches were dead code. This removes them and makes Azure the only Windows signing path.

windowsSign.ts now gates on CI && process.platform === 'win32' instead of the env var, so the Forge config — loaded on every platform — stays inert on Mac/Linux instead of throwing where the Azure env vars are absent.

Testing Instructions

  • CI: the Windows dev and release build jobs must produce Azure-signed studio-setup.exe / .appx artifacts (this is where the signing path actually executes).
  • Local sanity: npm run typecheck and npx eslint apps/studio/windowsSign.ts apps/studio/forge.config.ts scripts/package-appx.mjs pass.

Azure Trusted Signing has been validated in shipped builds (v1.8.2, v1.9.0)
and was already the only path CI ever took — `USE_AZURE_TRUSTED_SIGNING` was
hardcoded to `1` in both pipelines, so the PFX branches were dead fallback.

`windowsSign.ts` now gates on `CI && win32` rather than the env var, since the
config is loaded by Forge on every platform and an unconditional Azure config
would throw on Mac/Linux CI where the Azure env vars are absent.

---

Generated with the help of Claude Code, https://claude.com/claude-code

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mokagio mokagio self-assigned this May 29, 2026
@mokagio mokagio marked this pull request as ready for review May 29, 2026 05:15
Copilot AI review requested due to automatic review settings May 29, 2026 05:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the legacy Windows PFX signing fallback and makes Azure Trusted Signing the only intended signing path for Studio Windows build artifacts.

Changes:

  • Removes USE_AZURE_TRUSTED_SIGNING from Buildkite Windows build jobs.
  • Updates the Windows build script to always run Azure signing setup.
  • Removes PFX fallback branches from Forge/Squirrel and AppX packaging paths.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.buildkite/pipeline.yml Removes the Azure signing feature flag from Windows dev builds.
.buildkite/release-build-and-distribute.yml Removes the Azure signing feature flag from Windows release builds.
.buildkite/commands/build-for-windows.ps1 Always initializes Azure Trusted Signing before building Windows artifacts.
apps/studio/windowsSign.ts Switches signing activation from an env flag to Windows CI detection.
apps/studio/forge.config.ts Removes Squirrel PFX signing fallback and only passes Azure signing config when present.
scripts/package-appx.mjs Removes AppX PFX fallback and always signs sideload AppX output with Azure signing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/studio/windowsSign.ts Outdated
Comment on lines 15 to 19
if ( ! process.env.CI || process.platform !== 'win32' ) {
return undefined;
}

if ( ! process.env.AZURE_CODE_SIGNING_DLIB || ! process.env.AZURE_METADATA_JSON || ! process.env.SIGNTOOL_PATH ) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — confirmed. The Windows E2E job (run-e2e-tests.sh) runs electron-forge package for an intentionally-unsigned bundle and never runs setup_azure_trusted_signing.ps1, so a CI && win32 gate would have thrown at config import there.

Digging in, USE_AZURE_TRUSTED_SIGNING turned out to be load-bearing rather than redundant: it's the "sign this build" signal that the make jobs set and E2E leaves unset. So I've kept it as the gate (in windowsSign.ts and the pipelines) and removed only the dead PFX fallback branches. Addressed in 7dea0c6.

Posted by Claude (Opus 4.8) on behalf of @mokagio with approval.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

`USE_AZURE_TRUSTED_SIGNING` is the signal that a build should be signed, not a
redundant flag: it is set on the `make` build jobs and left unset on the
Windows E2E job, which runs `electron-forge package` to produce an unsigned
bundle. Removing it (and gating signing on `CI && win32` instead) would throw
at config import on that E2E job and would leave the real build jobs producing
unsigned installers. Retain the env var and the gate; remove only the dead PFX
fallback branches.

Surfaced by Copilot review on the PR.

---

Generated with the help of Claude Code, https://claude.com/claude-code

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mokagio mokagio marked this pull request as draft May 29, 2026 05:49
@wpmobilebot
Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing 7dea0c6 vs trunk

app-size

Metric trunk 7dea0c6 Diff Change
App Size (Mac) 1335.91 MB 1335.91 MB +0.00 MB ⚪ 0.0%

site-editor

Metric trunk 7dea0c6 Diff Change
load 1737 ms 1727 ms 10 ms ⚪ 0.0%

site-startup

Metric trunk 7dea0c6 Diff Change
siteCreation 9609 ms 9592 ms 17 ms ⚪ 0.0%
siteStartup 4918 ms 4925 ms +7 ms ⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants