Skip to content

Commit 78462b0

Browse files
authored
fix(desktop): correct the Windows and macOS release verification (#167)
## Related Issue No issue — this is a live release failure. The `desktop-v0.2.1` tag build failed on both platforms ([run 32647843142](https://github.com/PyModel/pythinker-code/actions/runs/32647843142)), leaving `v0.2.1` as an empty draft with no installers. ## Problem **Windows** — the job passes `--config.win.publisherName`. Electron Builder 26 removed that option in favour of `signtoolOptions.publisherName`, and `WindowsConfiguration` sets `additionalProperties: false`, so the unknown key invalidates the whole `win` object: ``` ⨯ Invalid configuration object. electron-builder 26.15.3 ... - configuration.win should be one of these: null ``` The build dies during schema validation, before packaging. This path had never run in CI: it only emits those flags when Windows signing is configured, and the `AZURE_*` secrets were set for the first time today. The unit tests compared the generated argument array against a hand-written expected array, so they encoded the bug rather than catching it. **macOS** — the verify step runs `xcrun stapler validate` against the `.dmg`. Electron Builder notarizes and staples the `.app`, then packs the already-stapled bundle into the disk image; the image itself never receives a ticket, so that assertion can never pass. Notarization had actually succeeded — the same run logged `source=Notarized Developer ID` for the mounted bundle immediately before failing. ## What changed - `windowsSigningArgs` drops the duplicate publisher argument on the Azure path (the publisher is already carried in `azureSignOptions.publisherName`) and moves the certificate path to `--config.win.signtoolOptions.publisherName`. - The macOS verify step validates the staple on the app inside the disk image instead of on the image. - `package-win.spec.ts` now resolves the installed `app-builder-lib/scheme.json` through `electron-builder` and walks every emitted `--config.win.*` path against it, so an option Electron Builder does not declare fails the suite instead of the release. No new dependency. Verified by reintroducing `--config.win.publisherName`: 3 tests fail with `Electron Builder has no option 'publisherName' under win`; restored, 16 pass. ## Verification - `pnpm exec vitest run tests/package-win.spec.ts` — 16 passed - `pnpm run typecheck` (apps/desktop) — clean, both tsconfigs - `oxlint --type-aware` on the touched files — 0 warnings, 0 errors - `scripts/check-no-comments.mjs` — OK - Reproduced the original failure locally with the exact flags, and confirmed the corrected flags validate and reach `signing with Azure Trusted Signing` ## Checklist - [x] I have read the CONTRIBUTING document. - [ ] I have linked a related issue — none; this fixes a live release failure. - [x] I have added tests that prove my feature works. - [x] Ran `gen-changesets` skill, or this PR needs no changeset — no changeset: release-pipeline verification only, nothing users can perceive, and `desktop-v0.2.1` is being re-cut at the same version. - [x] Ran `gen-docs` skill, or this PR needs no doc update. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved macOS release validation by checking the signed application within the mounted disk image. * Updated Windows signing configuration to correctly support certificate-based and Azure signing options. * **Tests** * Added validation to ensure Windows signing settings match supported configuration options. * Expanded coverage for valid signing configurations and rejection of unsupported options. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent f7a233c commit 78462b0

3 files changed

Lines changed: 78 additions & 10 deletions

File tree

.github/workflows/desktop-release.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,10 @@ jobs:
246246
exit 1
247247
fi
248248
verify_app "$dmg_app"
249-
xcrun stapler validate "$dmg_path"
249+
# electron-builder notarizes and staples the .app, then packs the
250+
# already-stapled bundle into the disk image; the image itself never
251+
# receives a ticket. Validate the staple where it actually lives.
252+
xcrun stapler validate "$dmg_app"
250253
cleanup_mount
251254
trap - EXIT
252255

apps/desktop/scripts/package-win.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,12 @@ export function windowsSigningArgs(env: NodeJS.ProcessEnv): readonly string[] {
7373
)
7474
}
7575

76-
const publisherName = hasAzureValue
77-
? trimmedValue(env['AZURE_SIGNING_PUBLISHER_NAME'])!
78-
: trimmedValue(env['WINDOWS_SIGNING_PUBLISHER_NAME'])!
79-
if (!hasAzureValue) args.length = 0
80-
args.push('--config.win.publisherName', publisherName)
81-
return args
76+
// Electron Builder 26 removed `win.publisherName`, and `win` rejects unknown
77+
// keys, so passing it fails schema validation before any build work. The
78+
// Azure path already carries the publisher in `azureSignOptions`; the
79+
// certificate path now sets it where signtool reads it.
80+
if (hasAzureValue) return args
81+
return ['--config.win.signtoolOptions.publisherName', trimmedValue(env['WINDOWS_SIGNING_PUBLISHER_NAME'])!]
8282
}
8383

8484
/** Require one complete signing method for a tagged Windows release. */

apps/desktop/tests/package-win.spec.ts

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,56 @@
1+
import { createRequire } from 'node:module'
12
import { describe, expect, it } from 'vitest'
23
import {
34
requireWindowsReleaseSigning,
45
windowsPackageInvocation,
56
windowsSigningArgs,
67
} from '../scripts/package-win'
78

9+
interface SchemaNode {
10+
readonly $ref?: string
11+
readonly anyOf?: readonly SchemaNode[]
12+
readonly properties?: Readonly<Record<string, SchemaNode>>
13+
}
14+
15+
const requireFromTests = createRequire(import.meta.url)
16+
const schema = requireFromTests(
17+
requireFromTests.resolve('app-builder-lib/scheme.json', {
18+
paths: [requireFromTests.resolve('electron-builder')],
19+
}),
20+
) as { readonly definitions: Readonly<Record<string, SchemaNode>> }
21+
22+
function referencedDefinition(node: SchemaNode): SchemaNode | undefined {
23+
const reference = node.$ref ?? node.anyOf?.find(branch => branch.$ref !== undefined)?.$ref
24+
return reference === undefined ? undefined : schema.definitions[reference.replace('#/definitions/', '')]
25+
}
26+
27+
/**
28+
* Assert that a `--config.<path>` option exists in the installed Electron Builder schema.
29+
*
30+
* `WindowsConfiguration` sets `additionalProperties: false`, so an option that
31+
* the schema does not declare fails validation before any build work and takes
32+
* the whole `win` object down with it. Comparing against the real schema keeps
33+
* these arguments honest across Electron Builder upgrades, which is what an
34+
* expected-array assertion cannot do.
35+
* @param path - Dotted option path with the `--config.` prefix removed.
36+
*/
37+
function assertSchemaOption(path: string): void {
38+
const [root, ...rest] = path.split('.')
39+
expect(root).toBe('win')
40+
let definition = schema.definitions['WindowsConfiguration']!
41+
rest.forEach((segment, index) => {
42+
const property = definition.properties?.[segment]
43+
if (property === undefined) {
44+
throw new Error(`Electron Builder has no option '${rest.slice(0, index + 1).join('.')}' under win`)
45+
}
46+
const next = referencedDefinition(property)
47+
if (next !== undefined) definition = next
48+
else if (index !== rest.length - 1) {
49+
throw new Error(`Electron Builder option 'win.${rest.slice(0, index + 1).join('.')}' has no nested options`)
50+
}
51+
})
52+
}
53+
854
const signingEnvironment: NodeJS.ProcessEnv = {
955
AZURE_TENANT_ID: 'tenant-id',
1056
AZURE_CLIENT_ID: 'client-id',
@@ -26,7 +72,6 @@ describe('Windows Azure signing configuration', () => {
2672
'--config.win.azureSignOptions.codeSigningAccountName', 'signing-account',
2773
'--config.win.azureSignOptions.certificateProfileName', 'certificate-profile',
2874
'--config.win.azureSignOptions.publisherName', 'CN=Example Publisher, O=Example Publisher, L=Redmond, S=WA, C=US',
29-
'--config.win.publisherName', 'CN=Example Publisher, O=Example Publisher, L=Redmond, S=WA, C=US',
3075
])
3176
})
3277

@@ -56,6 +101,27 @@ describe('Windows Azure signing configuration', () => {
56101
})
57102
})
58103

104+
describe('Electron Builder option names', () => {
105+
const certificateEnvironment: NodeJS.ProcessEnv = {
106+
WIN_CSC_LINK: 'certificate.p12',
107+
WIN_CSC_KEY_PASSWORD: 'password',
108+
WINDOWS_SIGNING_PUBLISHER_NAME: 'CN=Example Publisher, O=Example Publisher',
109+
}
110+
111+
it('emits only options the installed Electron Builder schema declares', () => {
112+
for (const environment of [signingEnvironment, certificateEnvironment]) {
113+
const options = windowsSigningArgs(environment).filter(argument => argument.startsWith('--config.'))
114+
expect(options.length).toBeGreaterThan(0)
115+
for (const option of options) assertSchemaOption(option.slice('--config.'.length))
116+
}
117+
})
118+
119+
it('rejects an option the schema does not declare', () => {
120+
expect(() => { assertSchemaOption('win.publisherName') })
121+
.toThrow("Electron Builder has no option 'publisherName' under win")
122+
})
123+
})
124+
59125
describe('Windows tagged-release signing', () => {
60126
it('rejects an unsigned tagged release', () => {
61127
expect(() => requireWindowsReleaseSigning({})).toThrow('Windows release signing is not configured')
@@ -70,7 +136,7 @@ describe('Windows tagged-release signing', () => {
70136

71137
expect(requireWindowsReleaseSigning(environment)).toBe('CN=Example Publisher, O=Example Publisher')
72138
expect(windowsSigningArgs(environment)).toEqual([
73-
'--config.win.publisherName', 'CN=Example Publisher, O=Example Publisher',
139+
'--config.win.signtoolOptions.publisherName', 'CN=Example Publisher, O=Example Publisher',
74140
])
75141
})
76142

@@ -111,7 +177,6 @@ describe('Windows package invocation', () => {
111177
'--config.win.azureSignOptions.codeSigningAccountName', 'signing-account',
112178
'--config.win.azureSignOptions.certificateProfileName', 'certificate-profile',
113179
'--config.win.azureSignOptions.publisherName', 'CN=Example Publisher, O=Example Publisher, L=Redmond, S=WA, C=US',
114-
'--config.win.publisherName', 'CN=Example Publisher, O=Example Publisher, L=Redmond, S=WA, C=US',
115180
])
116181
})
117182

0 commit comments

Comments
 (0)