fix(release): normalize Live Host signing identity - #8579
Merged
Conversation
Contributor
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. |
LaZzyMan
marked this pull request as ready for review
August 5, 2026 10:20
Collaborator
Collaborator
|
Thanks for the PR! Template looks good ✓ — all sections present, complete bilingual translation. Problem: observed, not theoretical. Release run 30993064678 imported the certificate and configured notarization successfully, then Electron Builder rejected the signing qualifier with Direction: aligned. The Live Host signing path this fixes landed in #8574 (merged earlier today); this is the correction that makes that already-shipped direction actually complete a published release. It is release infrastructure, but the direction was decided by the merged predecessors, and the author is the owner of that work. Size: not applicable — no core package paths touched. 3 production lines ( Approach: scope is exactly right. The workflow still discovers and validates the full Risk: no elevated risk signals — no high-risk paths matched, and the downstream "Verify signing and notarization" step fails closed if the wrong identity ends up on the app. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 各部分齐全，中文翻译完整。 问题：已观测到的真实问题，不是理论性加固。release run 30993064678 已成功导入证书并配置公证，随后 Electron Builder 以 方向：对齐。本 PR 修复的 Live Host 签名链路刚在 #8574（今天早些时候合并）落地，本修复让该已合入方向真正能完成正式发布。虽然属于 release 基础设施，但方向已由先前合入的 PR 确定，且作者就是该工作的负责人。 规模：不适用 —— 未触及核心包路径。3 行生产代码（ 方案：范围恰到好处。工作流仍然发现并校验完整的 风险：无升级风险信号 —— 未命中高风险路径，且下游 "Verify signing and notarization" 步骤在 identity 不正确时会 fail-closed。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewClean pass — no blockers. My independent take on this fix before reading the diff: strip the reserved type prefix at export time, keep the full-identity discovery and validation upstream of it. That is exactly what the PR does, and I don't see a simpler path. A few things I checked specifically:
Test evidenceUnattended CI run — this review quotes the PR's own CI checks on the reviewed commit via the API; no PR code was built or executed here. The unit suite (
The central claim — Electron Builder accepts the normalized qualifier and signs both apps — is not substantiated by PR CI: the unit tests pin the workflow text, not the signing run, and the signing path needs real Apple credentials on a 中文说明代码审查：通过，无阻塞问题。我在看 diff 之前的独立方案就是在导出时去掉保留类型前缀、保留上游的完整 identity 发现与校验——PR 的做法与此一致，也没有更简路径。具体核对了几处：bash 前缀剥离语义正确（模式为字面量，上游 sed 保证 identity 以该前缀开头，空格与 测试证据：本次为无人值守 CI 运行，仅通过 API 引用该 PR 自身在受审 commit 上的 CI 结果，未构建或执行任何 PR 代码。单测套件（含改动的 release-workflow 测试）通过，Live Host macOS 构建/准备任务通过；macOS/Windows 单测与 CLI 集成测试被路径过滤跳过；真正的签名链路只在 核心声明（Electron Builder 接受规范化后的 qualifier 并完成双架构签名）无法由 PR CI 证实：单测固定的只是 workflow 文本；签名链路需要真实 Apple 凭据。此 PR 上没有任何沙箱通道能补齐这一环（ — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 5/5 — observed failure, exact root cause, minimal correct fix, pinned test, green CI, fail-closed downstream. This one is easy to be sure about. The failure was real (run 30993064678 died inside Electron Builder with The change is one bash prefix-strip plus three test assertions; the quoting is right, the mismatch case degrades loudly instead of silently, and the release workflow's signing/notarization verification still fails closed if the wrong identity ever lands on an app. Everything CI can prove is green on the reviewed commit. The only thing no PR review can prove — the actual signing run — needs the Approving. The definitive confirmation will be the next published Live Host release run. 中文说明这个 PR 很容易确信。失败是真实观测到的（run 30993064678 在 Electron Builder 中以 改动只有一处 bash 前缀剥离和三条测试断言；引号正确，不匹配时会显式报错而非静默失败，发布工作流的签名/公证校验在 identity 错误时仍然 fail-closed。受审 commit 上 CI 能证明的部分全部为绿。唯一任何 PR 审查都无法证明的——真实签名运行——需要从 予以批准。最终确认将是下一次 Live Host 正式发布运行。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
approved these changes
Aug 5, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
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/live-host-release.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
已审查——无阻断问题。 建议见行内评论。 未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/live-host-release.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| security set-key-partition-list -S apple-tool:,apple:,codesign: -s -k "$keychain_password" "$keychain" | ||
| identity="$(security find-identity -v -p codesigning "$keychain" | sed -n 's/.*"\(Developer ID Application:.*\)"/\1/p' | head -n 1)" | ||
| if [ -z "$identity" ]; then echo '::error::Developer ID Application identity was not found.'; exit 1; fi | ||
| identity_name="${identity#Developer ID Application: }" |
Collaborator
There was a problem hiding this comment.
[Suggestion] The why of this prefix strip is undocumented here — Electron Builder rejects a prefixed CSC_NAME (Please remove prefix "Developer ID Application:" — verified against the pinned electron-builder 26.4.0-series source) and selects the certificate type itself, while codesign consumers elsewhere in the repo want the prefixed form. Both sibling implementations of the same strip carry an explanatory comment (packages/desktop/apps/electron/scripts/build-dmg.sh:147, packages/desktop/scripts/build/darwin.ts:26); this workflow copy does not. — Concrete cost: a maintainer reconciling the three signing paths reads this as a pointless round trip (the sed above deliberately captures Developer ID Application:.*, and this line discards exactly that prefix), "simplifies" the workflow back to the prefixed form, and updates the contract test to match; nothing catches it in PR CI (the signing steps only run on workflow_dispatch with dry_run=false), so the regression surfaces mid-release at the Electron Builder step, after dependency install and full build time.
Suggested change
| identity_name="${identity#Developer ID Application: }" | |
| # Strip the certificate type prefix: electron-builder errors if CSC_NAME | |
| # starts with it and selects the certificate type automatically. | |
| identity_name="${identity#Developer ID Application: }" |
中文说明
这里没有说明为什么要去掉证书类型前缀 —— Electron Builder 会拒绝带前缀的 CSC_NAME（Please remove prefix "Developer ID Application:"，已对照锁定的 electron-builder 26.4.0 系列源码确认），并自行选择证书类型；而仓库中其他 codesign 消费方需要带前缀的完整形式。同一去前缀逻辑的两个兄弟实现都带解释性注释（packages/desktop/apps/electron/scripts/build-dmg.sh:147、packages/desktop/scripts/build/darwin.ts:26），工作流这份没有。—— 具体代价：后续维护者对齐三条签名路径时，会把这步看成无意义的往返（上面的 sed 专门捕获 Developer ID Application:.*，这一行又把该前缀丢掉），于是把 workflow「简化」回带前缀的形式并同步更新契约测试；PR CI 无法发现（签名步骤只在 workflow_dispatch 且 dry_run=false 时运行），回归会在依赖安装和完整构建之后、发布中途的 Electron Builder 步骤才暴露。
— qwen3.8-max via Qwen Code /review (v0.21.5)
Comment on lines
+118
to
+120
| expect(liveHostReleaseWorkflow).toContain( | ||
| 'identity_name="${identity#Developer ID Application: }"', | ||
| ); |
Collaborator
There was a problem hiding this comment.
[Suggestion] The three new assertions are order-insensitive substring checks, so they do not pin that identity_name is assigned before the $GITHUB_ENV export block that reads it. Probed by mutation: moving the identity_name=... workflow line below the } >> "$GITHUB_ENV" block leaves all three assertions green. This file already uses multi-line pinning elsewhere (the if: |- three-clause gate assertion). — Concrete cost: a future refactor of the Import macOS certificate step moves the assignment below the export block → this test stays green → the next published release aborts at release time with identity_name: unbound variable under set -euo pipefail (loud failure — the downstream verify step still gates artifacts, so not a silent mis-ship). The pin below kills that mutation and passes on the current code (both directions probed); the now-redundant standalone CSC_NAME=$identity_name assertion right after can stay or be dropped.
Suggested change
| expect(liveHostReleaseWorkflow).toContain( | |
| 'identity_name="${identity#Developer ID Application: }"', | |
| ); | |
| expect(liveHostReleaseWorkflow).toContain( | |
| 'identity_name="${identity#Developer ID Application: }"\n' + | |
| ' {\n' + | |
| ' echo "CSC_NAME=$identity_name"', | |
| ); |
中文说明
这三个新断言都是不敏感于顺序的子串检查，没有固定 identity_name 必须在读取它的 $GITHUB_ENV 导出块之前赋值。已做变异验证：把 workflow 中的 identity_name=... 行移到 } >> "$GITHUB_ENV" 块之后，三个断言仍然全部通过。本文件其他地方已使用多行固定模式（如 if: |- 三子句门控断言）。—— 具体代价：未来重构 Import macOS certificate 步骤时把赋值移到导出块下方 → 测试仍然为绿 → 下一次正式发布在 set -euo pipefail 下以 identity_name: unbound variable 于发布时中止（失败是显式的 —— 下游校验步骤仍会拦截产物，不会静默发出未签名产物）。下面的固定方式能捕获该变异，且对当前代码通过（两个方向均已实测）；其后那条独立的 CSC_NAME=$identity_name 断言会因此变得冗余，可留可删。
— qwen3.8-max via Qwen Code /review (v0.21.5)
DragonnZhang
approved these changes
Aug 6, 2026
Collaborator
|
Released in v0.21.7. |
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
3 participants
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.
What this PR does
Normalizes the validated Developer ID certificate name before passing it to Electron Builder during a published Qwen Live Host release.
Why it's needed
The first credential-compatible release run successfully imported the repository certificate and configured notarization, but Electron Builder rejected the signing qualifier because it included the reserved
Developer ID Application:type prefix. Electron Builder expects only the certificate-name remainder and selects the certificate type itself.Reviewer Test Plan
How to verify
Run the release-workflow unit test and validate the workflow with actionlint. Confirm that the workflow still discovers and validates a full Developer ID identity, strips only the exact type prefix, and exports the normalized certificate name rather than the full identity.
Evidence (Before & After)
Before: run 30993064678 passed certificate import and notarization setup, then failed in Electron Builder with
Please remove prefix "Developer ID Application:". After: a local two-architecture Electron Builder 26.4.0 run used the normalized qualifier, selected the expected Developer ID identity, signed both apps, and produced both ZIP and DMG artifacts.Tested on
Environment (optional)
macOS; Vitest release workflow 5/5, installer 7/7, release architecture 2/2, Prettier, actionlint, and the PR dry-run passed. Both built apps and both extracted ZIP apps passed strict deep signature verification; arm64/x64 executable architecture, the universal native Appshot module, both DMG checksums, and both manifest ZIP hashes were verified.
Risk & Scope
main; their configuration, fail-closed verification, artifact contract, stable feed, and installer consumption path were reviewed end to end before this PR was marked ready.Linked Issues
Follow-up to #7859 and #8574.
中文说明
本 PR 的改动
在正式发布 Qwen Live Host 时，将已验证的 Developer ID 证书名称规范化后再传给 Electron Builder。
为什么需要
首次兼容仓库现有凭据的发布已经成功导入证书并配置公证，但 Electron Builder 拒绝了包含保留类型前缀
Developer ID Application:的签名 qualifier。Electron Builder 只接受证书名称剩余部分，并自行选择证书类型。Reviewer Test Plan
验证方式
运行发布工作流单测并使用 actionlint 校验工作流。确认工作流仍然发现并验证完整 Developer ID identity，只移除精确的类型前缀，并导出规范化后的证书名称而不是完整 identity。
前后证据
修改前：run 30993064678 已通过证书导入和公证配置，随后 Electron Builder 报错
Please remove prefix "Developer ID Application:"。修改后：本机 Electron Builder 26.4.0 双架构构建使用规范化 qualifier，选中了预期 Developer ID identity，签署了两个 App，并生成了两个 ZIP 和两个 DMG。测试平台
环境
macOS；发布工作流 Vitest 5/5、安装器 7/7、发布架构 2/2、Prettier、actionlint 和 PR dry-run 通过。两个构建目录 App 和两个 ZIP 解压后的 App 均通过严格深层签名校验；同时核验了 arm64/x64 主程序架构、universal 原生 Appshot、两个 DMG 校验和以及 manifest 中两个 ZIP 的哈希。
风险与范围
main完成最终工作流；在将 PR 标记为 ready 前，已端到端审查其配置、fail-closed 校验、产物契约、stable feed 和安装器消费链路。关联事项
#7859 和 #8574 的后续修复。