Skip to content

fix(release): normalize Live Host signing identity #8579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LaZzyMan merged 1 commit into from
Aug 6, 2026
Merged

fix(release): normalize Live Host signing identity #8579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/live-host-release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,9 @@ jobs:
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: }"

@qwen-code-ci-bot qwen-code-ci-bot Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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)

{
echo "CSC_NAME=$identity"
echo "CSC_NAME=$identity_name"
echo 'CSC_IDENTITY_AUTO_DISCOVERY=true'
} >> "$GITHUB_ENV"

Expand Down
5 changes: 5 additions & 0 deletions scripts/tests/release-workflow.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ describe('Live Host release workflow', () => {
expect(liveHostReleaseWorkflow).toContain(
'echo "APPLE_API_KEY_ID=$api_key_id"',
);
expect(liveHostReleaseWorkflow).toContain(
'identity_name="${identity#Developer ID Application: }"',
);
Comment on lines +118 to +120

@qwen-code-ci-bot qwen-code-ci-bot Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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)

expect(liveHostReleaseWorkflow).toContain('echo "CSC_NAME=$identity_name"');
expect(liveHostReleaseWorkflow).not.toContain('echo "CSC_NAME=$identity"');
});
});

Expand Down
Loading