fix(desktop): codesign ripgrep and node binaries before tauri build - #8518
Conversation
macOS notarization rejects the app bundle because Tauri only signs the main binary, not the embedded ripgrep and Node.js runtime binaries under Contents/Resources/runtime/qwen-code/. Add a pre-build codesign step that signs all native macOS executables in the bundled runtime with the Developer ID identity, hardened runtime, and the existing entitlements.
The Tauri release workflow threw when WINDOWS_CERTIFICATE was missing, blocking the entire release (including macOS). Mirror the old Electron workflow behavior: warn and continue unsigned when no cert is configured. Also add fallback to legacy WIN_CSC_LINK/WIN_CSC_KEY_PASSWORD secrets so existing Electron-era credentials still work if present.
|
Thanks for the quick follow-up on the notarization failure! Template: the three main sections are present with real content. The formal subsections (Risk & Scope, Linked Issues, Tested-on table, Chinese summary) are skipped — not gating on that. One gap though: the description only covers the macOS codesign change. The second commit ("allow Windows build without signing certificate") landed mid-review and isn't described in the body at all — please add the what/why for it. Problem: confirmed observed, not theoretical. Today's 0.0.6 release run (run 30886803205, main @ Direction: commit 1 is squarely in scope — macOS desktop releases don't ship until notarization passes. Commit 2 turns Windows signing from fail-closed into warn-and-continue for published releases; that's a release-policy change, and as written it contradicts the existing Size: single workflow file, +49/−8. No core-package paths touched. Approach: commit 1's scope is right — one question on the Risk: no elevated risk-path signals. One verification heads-up: the new signing step only runs when Flagging these for discussion before approval. 中文说明感谢对公证(notarization)失败的快速跟进! 模板:三个主要章节都有实质内容。正式的小节(Risk & Scope、Linked Issues、Tested-on 表格、中文摘要)缺失——不因此卡住。但有一个缺口:PR 描述只覆盖了 macOS 签名改动。第二个 commit("允许 Windows 在没有签名证书的情况下构建")是在 review 过程中新推上来的,描述里完全没有提及——请补充它的 what/why。 问题:已确认是真实观测到的问题,不是理论问题。今天的 0.0.6 release 运行(run 30886803205,main @ 方向:commit 1 完全在范围内——macOS 桌面版不过公证就无法发布。commit 2 把 Windows 签名从"失败即停"改成了"警告并继续"(针对正式发布),这是一个 release 策略变更,而且与现有的 规模:单个 workflow 文件,+49/−8,未触及核心包路径。 方案:commit 1 的范围合适——对 风险:无高风险路径信号。一个验证上的提醒:新签名步骤只在 先提出这些问题讨论,再谈批准。 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewReviewed the full diff at the reviewed commit. My independent take on "notarization rejects the bundle because embedded binaries are unsigned": add a macOS-only step after the runtime is prepared and before Commit 1 —
Commit 2 —
Either way, please document commit 2 in the PR body (what/why/how-to-verify) — it changes published-release behavior and currently isn't mentioned. Minor: the two pfx-import branches in commit 2 are identical 5-line blocks; picking CI evidence (this PR's own runs, via API — no PR code was executed)The first Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Not verified, and why: the central claims here are release-runtime behaviour (notarization passes; Windows build proceeds without a cert) — neither is reachable from the unit suite, and this workflow cannot be exercised pre-merge: 中文说明Commit 1(macOS 供应商二进制签名):正确。 我逐项核过:步骤位置在 Commit 2(允许无证书 Windows 构建):名不副实,这是阻塞项。 legacy CI 证据:通过 API 读取本 PR 自己的 CI(未执行任何 PR 代码)。第一次 CI 运行(旧 head 未能验证的部分及原因:本 PR 的核心主张是 release 运行时行为(公证通过;无证书 Windows 构建继续),单元测试够不着,且该 workflow 合并前无法演练: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 2/5 — the macOS half is solid, evidence-backed, and needed; the Windows half as written cannot do what its title says and changes release policy without saying so. Stepping back: commit 1 is exactly the fix I would have proposed — right place in the pipeline, right identity/entitlements, matching the notarization log item by item, and I'd merge it happily on its own (with the optional Also weighing on me: the whole change is only truly testable by the next published release from Requesting changes on the commit 2 contradiction; everything else above is minor or optional. 中文说明置信度:2/5 —— macOS 部分扎实、有证据支撑、且是必需的;Windows 部分按现状无法实现其标题所述,并且在没有说明的情况下改变了 release 策略。 退一步看:commit 1 完全就是我也会提出的修复——流水线位置正确、identity/entitlements 正确、与公证日志逐条对应,单看它我愿意直接合并(可选地加上 另一点顾虑:整个改动只能靠下一次从 就 commit 2 的矛盾请求修改;其余都是次要或可选项。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
The Windows change in commit 2 can't do what its title says — the pre-existing Verify Windows signature step throws on an unsigned installer, so the warn-and-continue branch just moves the failure to after a full build (and publish still needs all build legs). See my notes above for the two clean ways out. The macOS codesign half looks right. 🙏
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
The 'Verify Windows signature' step threw on any non-Valid status, including NotSigned. With no Windows code signing certificate configured, this blocked the Windows build job, which in turn blocked the publish job (needs: [prepare, build]). Allow NotSigned with a warning instead of throwing, matching the fallback behavior of the 'Import Windows certificate' step. A genuinely invalid signature (HashMismatch, etc.) still throws.
- Scope ripgrep codesign find to *-darwin/* paths so Linux ELF binaries (built in the same matrix) aren't targeted. - Unify the two pfx-import branches into a single code path to eliminate duplicated write/import/configure logic.
Good catch — this was fixed in commit
Fixed in
Deduplicated in PR description also updated to document all three changes. |
|
@qwen-code /review |
| _Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/30893155251)._ |
Review —
|
doudouOUC
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/desktop-release.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
— qwen3.7-max via Qwen Code /review (v0.21.4)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/desktop-release.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
— qwen3.8-max via Qwen Code /review (v0.21.5)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/desktop-release.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
Addressed the Windows signing closeout issues and the small macOS vendor-signing reliability items. Changed:
Verified: Intentionally not changed: regenerating |
Review:
|
chiga0
left a comment
There was a problem hiding this comment.
Review Overview
I performed a blind independent review before reading existing reviews.
Verdict: APPROVE. The PR fixes the blocking 0.0.6 release issues (macOS notarization of vendor binaries, Windows optional signing) and the current HEAD addresses the Critical feedback from earlier reviewers. I found no Critical correctness or security issue.
Cross-Validation
| Finding | Other Reviewer | My Assessment |
|---|---|---|
Empty WINDOWS_CONFIG passed as --config "" to Tauri |
doudouOUC (Critical) | Fixed in current HEAD — build step now guards with [ -n "$WINDOWS_CONFIG" ]. |
| Incomplete certificate pair silently degrades to unsigned | qwen-code-ci-bot (Critical) | Fixed in current HEAD — $primaryIncomplete / $legacyIncomplete checks throw. |
find -exec ... \; swallows codesign failures |
qwen-code-ci-bot (Suggestion) | Fixed in current HEAD — uses + form. |
| Missing tests for release-contract branches | qwen-code-ci-bot (Suggestion) | Fixed in current HEAD — testDesktopReleaseSigningWorkflow() added. |
NotSigned tolerance unconditional |
qwen-code-ci-bot (Suggestion) | Fixed in current HEAD — now conditional on -not $env:WINDOWS_CONFIG. |
| Missing runtime paths silently no-op | qwen-code-ci-bot (Suggestion) | Fixed in current HEAD — else branches emit warnings. |
checksums.json sealed before signing modifies binaries |
qwen-code-ci-bot (Suggestion) | Still present — see inline comment. |
Unique Findings
| ID | Severity | File:Line | Summary |
|---|---|---|---|
| U1 | Major | .github/workflows/desktop-release.yml:330-361 |
Sign bundled vendor binaries mutates node/bin/node and rg after prepare-runtime.js wrote checksums.json and smoke:runtime verified it. The published macOS bundle ships an integrity manifest that no longer matches the signed binaries. Regenerate checksums.json after codesign, or move signing before writeChecksums(). |
| U2 | Minor | packages/desktop-shell/scripts/test-release.js:83-140 |
The workflow-contract tests are string-fragile (exact YAML fragments, indexOf on step names). A formatting-only refactor would break them without changing behavior. Consider parsing the YAML once and asserting against the AST. |
| U3 | Minor | .github/workflows/desktop-release.yml:390-404 |
Verify Windows signature only checks the first nsis/*.exe. If Tauri ever produces additional Windows installer formats (e.g., .msi) or per-target artifacts, they are not attested. |
Additional Audit Coverage
Areas I independently checked beyond existing findings:
- Entitlements (
allow-jit, network) are appropriate for signed Node.js runtime. - Fail-closed behavior for incomplete primary/legacy certificate pairs is correct.
NotSignedtolerance is now correctly scoped to the no-certificate-configured case.- Step ordering matches the PR's intent: prepare → test → sign → build → verify.
| working-directory: 'packages/desktop-shell' | ||
| run: 'npm run test:release' | ||
|
|
||
| - name: 'Sign bundled vendor binaries (macOS)' |
There was a problem hiding this comment.
[Major] Signed macOS binaries invalidate the bundled checksums.json
prepare-runtime.js writes checksums.json at the end of Prepare bundled runtime (line 314) and smoke:runtime verifies it immediately after. This step then calls codesign --force on node/bin/node and the rg binaries, changing their SHA-256. The published macOS app bundle therefore ships an integrity manifest whose listed hashes no longer match the actual signed binaries.
Suggested fix: regenerate checksums.json after the codesign calls (extract the checksum-writing logic into a reusable script), or move the signing calls before writeChecksums() inside prepare-runtime.js when APPLE_SIGNING_IDENTITY is available.
| } | ||
| } | ||
|
|
||
| function testDesktopReleaseSigningWorkflow() { |
There was a problem hiding this comment.
[Minor] String-fragile workflow-contract tests
testDesktopReleaseSigningWorkflow asserts exact YAML string fragments and uses indexOf on step names. A formatting-only refactor (e.g., renaming a step, rewrapping a command) would break these tests without changing workflow behavior.
Consider parsing the workflow YAML once and asserting against the parsed AST, or at least isolating the literal fragments into a single source of truth.
| @@ -338,10 +390,18 @@ jobs: | |||
| - name: 'Verify Windows signature' | |||
There was a problem hiding this comment.
[Minor] Windows signature verification only covers NSIS .exe
The step checks nsis/*.exe | Select-Object -First 1. If Tauri is later configured to produce .msi installers or multiple per-target artifacts, this step will silently ignore them.
Consider iterating over all Windows installer artifacts produced by the build, or documenting that NSIS is the only attested format.
|
Released in v0.21.6. |
What this PR does
This PR fixes three issues that blocked the 0.0.6 desktop release:
1. macOS: Sign bundled vendor binaries (commit 1)
Tauri only signs the main app binary, not embedded ripgrep and Node.js runtime binaries under
Contents/Resources/runtime/qwen-code/. The 0.0.6 notarization failed because Apple rejects unsigned executables in the bundle.Added a "Sign bundled vendor binaries" step (after
Prepare bundled runtime, beforeBuild desktop installers) that codesigns ripgrep and Node.js binaries with the Developer ID identity, hardened runtime, and existingEntitlements.plist. Thefindscope is narrowed to*-darwin/*paths so Linux ELF binaries built in the same matrix job are not targeted.2. Windows: Allow build without signing certificate (commit 2)
The "Import Windows certificate" step threw when no
WINDOWS_CERTIFICATEorWIN_CSC_LINKsecret was set, killing the Windows build job. Sincepublishdepends on all build jobs (needs: [prepare, build]), a Windows failure blocks macOS publish too.Refactored to select the available credential pair (preferring
WINDOWS_CERTIFICATE, falling back to legacyWIN_CSC_LINK), then run a single pfx import path. Without credentials, emits a warning and continues unsigned — matching the 0.0.5 Electron-era behavior where Windows signing was optional.3. Windows: Allow unsigned artifacts in verify step (commit 3)
The "Verify Windows signature" step threw on any non-
Validstatus, includingNotSigned. Without a certificate, this killed the build even after commit 2 allowed import to pass.Now
Validpasses,NotSignedwarns (no certificate configured; SmartScreen will warn users), and any other status (HashMismatch, etc.) still throws.Why it's needed
0.0.6 failed at notarization (macOS) and certificate import (Windows). macOS needs every executable signed; Windows needs the workflow to tolerate missing certificates so publish can proceed with macOS-only signing.
Reviewer Test Plan
How to verify: trigger a Desktop Release workflow run with
dry_run=false. macOS notarization should succeed (ripgrep/node now signed). Windows build should complete without certificate secrets (unsigned with warning). Thepublishjob should run for all platforms.