Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 8 additions & 2 deletions .github/workflows/desktop-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ jobs:
find "$rg_dir" -type f -name 'rg' -path '*-darwin/*' -exec \
codesign --force --sign "$APPLE_SIGNING_IDENTITY" \
--options runtime --timestamp \
--entitlements src-tauri/Entitlements.plist {} +
{} +
else
echo "::warning::Ripgrep vendor directory not found at $rg_dir; no ripgrep binaries signed."
fi
Expand All @@ -356,7 +356,7 @@ jobs:
if [ -f "$node_bin" ]; then
codesign --force --sign "$APPLE_SIGNING_IDENTITY" \
--options runtime --timestamp \
--entitlements src-tauri/Entitlements.plist "$node_bin"
--entitlements src-tauri/NodeEntitlements.plist "$node_bin"
else
echo "::warning::Node.js runtime binary not found at $node_bin; no Node.js binary signed."
fi
Expand Down Expand Up @@ -387,6 +387,10 @@ jobs:
app="$(find packages/desktop-shell/src-tauri/target/${{ matrix.rust_target }}/release/bundle/macos -maxdepth 1 -name '*.app' -print -quit)"
codesign --verify --deep --strict --verbose=2 "$app"
spctl --assess --type execute --verbose=2 "$app"
entitlements="$(mktemp)"
trap 'rm -f "$entitlements"' EXIT
codesign -d --entitlements - --xml "$app" > "$entitlements" 2>/dev/null
test "$(/usr/libexec/PlistBuddy -c 'Print :com.apple.security.device.audio-input' "$entitlements")" = true
Comment on lines +392 to +393

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] Release-time verification is asymmetric: the new artifact-level assertions check only the app's main executable (audio-input entitlement here, usage description in Contents/Info.plist). The PR's actual behavioral change — Node signed with NodeEntitlements.plist (allow-jit only), ripgrep signed with no entitlements — has no release-time artifact check; only the workflow-text/source-file unit assertions cover it. — Failure scenario: a future divergence between the source plists and the final bundle — a build step re-signing or replacing the embedded node/rg binaries, or the signing step dropping the NodeEntitlements.plist flag while the workflow-text test is updated in the same commit — ships Node carrying com.apple.security.device.audio-input (or ripgrep the full app entitlements), exactly the exposure this PR exists to prevent. codesign --verify --deep --strict checks validity, not entitlement content; every release gate stays green.

Suggested change
codesign -d --entitlements - --xml "$app" > "$entitlements" 2>/dev/null
test "$(/usr/libexec/PlistBuddy -c 'Print :com.apple.security.device.audio-input' "$entitlements")" = true
node_bin="$app/Contents/Resources/runtime/qwen-code/node/bin/node"
if [ -f "$node_bin" ]; then
node_entitlements="$(mktemp)"
codesign -d --entitlements - --xml "$node_bin" > "$node_entitlements" 2>/dev/null
test "$(/usr/libexec/PlistBuddy -c 'Print :com.apple.security.device.audio-input' "$node_entitlements")" != true
test "$(/usr/libexec/PlistBuddy -c 'Print :com.apple.security.cs.allow-jit' "$node_entitlements")" = true
rm -f "$node_entitlements"
fi
中文说明

发布期验证不对称:新增的产物级断言只检查 App 主可执行文件(此处的 audio-input 权限,以及 Contents/Info.plist 里的用途说明)。本 PR 真正的行为变更——Node 改用仅含 allow-jit 的 NodeEntitlements.plist 签名、ripgrep 不带任何权限签名——没有任何发布期产物检查,只有 workflow 文本/源文件级的单元断言在覆盖。— 失败场景:未来源 plist 与最终 bundle 出现偏差——某步构建重新签名或替换内嵌的 node/rg 二进制,或签名步骤丢掉 NodeEntitlements.plist 参数且同一提交里改了 workflow 文本测试——Node 带着 com.apple.security.device.audio-input(或 ripgrep 带着完整应用权限)发布,这正是本 PR 要防止的暴露。codesign --verify --deep --strict 只检查签名有效性,不检查权限内容;所有发布关卡仍然全绿。

— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)

Comment on lines +390 to +393

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] Dry-run rehearsal silently skips the app-side mic-entitlement gate: the audio-input check landed in 'Verify macOS signature' (if: runner.os == 'macOS' && inputs.dry_run == false), while the sibling NSMicrophoneUsageDescription check went into 'Smoke packaged application' (ungated). Since dry_run defaults to true and this workflow is workflow_dispatch-only, the codesign-dump path executes nowhere — not in CI, not in a dry-run rehearsal — until the first real signed release. Separately, tauri.conf.json's bundle.macOS.entitlements: "Entitlements.plist" is asserted by no test. — Failure scenario: an operator triggers the default dry-run to rehearse a release; the smoke step's Info.plist check passes green but the audio-input assertion is skipped, so a regression such as bundle.macOS.entitlements being repointed at NodeEntitlements.plist sails through the dry-run and is first caught on the real release run — whose codesign-dump assertion has never executed anywhere before.

中文说明

干跑排练会静默跳过应用侧麦克风权限关卡:audio-input 检查放在了 'Verify macOS signature'(if: runner.os == 'macOS' && inputs.dry_run == false),而同族的 NSMicrophoneUsageDescription 检查放在了 'Smoke packaged application'(不设门控)。由于 dry_run 默认是 true,且该 workflow 仅由 workflow_dispatch 触发,codesign 导出路径在任何地方都不会执行——CI 不执行、干跑也不执行——直到第一次真实签名发布。另外,tauri.conf.json 中的 bundle.macOS.entitlements: "Entitlements.plist" 没有任何测试断言。— 失败场景:操作员触发默认干跑来排练发布;smoke 步骤的 Info.plist 检查通过,但 audio-input 断言被跳过,于是诸如把 bundle.macOS.entitlements 改指向 NodeEntitlements.plist 之类的回归可以安然通过干跑,直到真实发布时才被发现——而那条 codesign 导出断言此前从未在任何地方执行过。

— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)


- name: 'Verify Windows signature'
if: "runner.os == 'Windows' && inputs.dry_run == false"
Expand Down Expand Up @@ -427,6 +431,8 @@ jobs:
run: |
set -euo pipefail
executable="$(find src-tauri/target/${{ matrix.rust_target }}/release/bundle/macos -path '*.app/Contents/MacOS/*' -type f -perm -111 -print -quit)"
info_plist="$(dirname "$(dirname "$executable")")/Info.plist"
/usr/libexec/PlistBuddy -c 'Print :NSMicrophoneUsageDescription' "$info_plist" >/dev/null
npm run smoke:packaged -- "$executable"

- name: 'Smoke packaged application'
Expand Down
63 changes: 62 additions & 1 deletion packages/desktop-shell/scripts/test-release.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,61 @@ function testDesktopReleaseSigningWorkflow() {
),
'Unsigned Windows installers are only allowed when no signing config exists',
);
const ripgrepStart = workflow.indexOf('# ripgrep vendor binaries');
const ripgrepEnd = workflow.indexOf('# Node.js runtime binary');
assert.ok(
workflow.includes('--entitlements src-tauri/Entitlements.plist {} +'),
ripgrepStart !== -1 && ripgrepEnd > ripgrepStart,
'the vendor signing step must keep its ripgrep/Node section markers',
);
const ripgrepSigningBlock = workflow.slice(ripgrepStart, ripgrepEnd);
assert.doesNotMatch(
ripgrepSigningBlock,
/--entitlements/,
'ripgrep must not inherit the app entitlements',
);
Comment thread
yiliang114 marked this conversation as resolved.
assert.match(
workflow,
/--options runtime --timestamp \\\n\s+\{\} \+/,
'ripgrep codesign failures must fail the signing step',
Comment on lines +128 to 129

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 regex /--options runtime --timestamp \\n\s+\{\} \+/ pins the rg codesign command's exact flag sequence AND its multi-line continuation shape; any semantically-correct future edit — a flag reorder or a new flag inserted before {} — makes the assertion fail, breaking CI on correct changes rather than on regressions (probe-verified: inserting --verbose=4 between --timestamp and {} fails the suite; revert restores green). — Failure scenario: a future maintainer adds a flag to the ripgrep codesign (e.g. --verbose=4) or reorders flags while keeping the command fully valid → assert.match fails → CI goes red on a correct change, and the author must discover that this whitespace-sensitive regex — not the signing — is what broke.

中文说明

正则 /--options runtime --timestamp \\n\s+\{\} \+/ 同时固定了 rg codesign 命令的精确参数顺序和多行续行形态;任何语义正确的未来改动——调整参数顺序或在 {} 前插入新参数——都会让断言失败,CI 因正确改动而非回归变红(探针实测:在 --timestamp 与 {} 之间插入 --verbose=4 会让套件失败,还原后恢复绿色)。— 失败场景:未来维护者给 ripgrep codesign 加一个参数(如 --verbose=4)或调整参数顺序但命令完全合法 → assert.match 失败 → CI 因正确改动变红,作者还得查明是这个对空白敏感的 regex——而不是签名本身——坏了事。

— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)

);
Comment on lines +126 to 130

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 assertion message 'ripgrep codesign failures must fail the signing step' (carried over from the pre-diff assertion) does not match what the regex tests: /--options runtime --timestamp \\n\s+\{\} \+/ only pins the codesign command's syntactic continuation shape. The property the message names actually lives in set -euo pipefail at the top of the 'Sign bundled vendor binaries (macOS)' step — and no assertion in test-release.js checks it. — Failure scenario: set -e (or set -euo pipefail) is removed from that step, or the codesign command is detached from that shell context → a failing codesign becomes a non-fatal warning → unsigned/partially-signed helper binaries ship in the release, while this test, whose stated purpose is exactly to prevent that, still passes.

Suggested change
assert.match(
workflow,
/--options runtime --timestamp \\\n\s+\{\} \+/,
'ripgrep codesign failures must fail the signing step',
);
const signingStep = workflow.slice(
workflow.indexOf("name: 'Sign bundled vendor binaries (macOS)'"),
workflow.indexOf("name: 'Build desktop installers'"),
);
assert.match(
signingStep,
/set -euo pipefail/,
'the signing step must keep failing closed on codesign errors',
);
assert.match(
workflow,
/--options runtime --timestamp \\n\s+\{\} \+/,
'the ripgrep codesign command must keep its hardened-runtime flags',
);
中文说明

断言消息 "ripgrep codesign failures must fail the signing step"(沿用自改动前的断言)与正则实际测试的内容不符:/--options runtime --timestamp \\n\s+\{\} \+/ 只固定了 codesign 命令的语法续行形态。消息所指的属性实际存在于 'Sign bundled vendor binaries (macOS)' 步骤顶部的 set -euo pipefail——而 test-release.js 中没有任何断言检查它。— 失败场景:该步骤删掉 set -e(或 set -euo pipefail),或 codesign 命令脱离该 shell 上下文 → 失败的 codesign 变成非致命警告 → 未签名/部分签名的辅助二进制随发布流出,而这个声称正是要阻止该情况的测试仍然通过。

— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)

assert.ok(
workflow.includes(
'--entitlements src-tauri/NodeEntitlements.plist "$node_bin"',
),
Comment on lines +131 to +134

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 test pins the ripgrep sibling's full invocation (--options runtime --timestamp \ + {} +) but pins only the entitlements argument for the Node sibling — the hardened-runtime flags on the Node codesign (--options runtime --timestamp) are asserted nowhere. Asymmetric guard (probe-verified: deleting those flags from the Node line leaves the suite green). — Failure scenario: a future edit drops --options runtime --timestamp from the Node codesign line (leaving --entitlements src-tauri/NodeEntitlements.plist "$node_bin" intact) → the assertion still passes on the substring → the suite greenlights a Node binary signed without hardened runtime, and the regression surfaces only at release (child-process launch failure under the hardened parent, or notarization rejection).

Suggested change
assert.ok(
workflow.includes(
'--entitlements src-tauri/NodeEntitlements.plist "$node_bin"',
),
assert.match(
workflow,
/codesign --force --sign "\$APPLE_SIGNING_IDENTITY" \\n\s+--options runtime --timestamp \\n\s+--entitlements src-tauri\/NodeEntitlements\.plist "\$node_bin"/,
'Node.js must keep its hardened-runtime flags and minimal entitlements',
);
中文说明

测试固定了 ripgrep 兄弟命令的完整调用形态(--options runtime --timestamp \ + {} +),但对 Node 兄弟只固定了 entitlements 参数——Node codesign 行上的 hardened-runtime 标志(--options runtime --timestamp)没有任何断言。守卫不对称(探针实测:删掉 Node 行的这些标志后套件仍然全绿)。— 失败场景:未来某次编辑从 Node codesign 行删掉 --options runtime --timestamp(保留 --entitlements src-tauri/NodeEntitlements.plist "$node_bin")→ 断言仍能匹配子串 → 套件给一个没有 hardened runtime 签名的 Node 二进制放行,回归要到发布时才暴露(在 hardened 父进程下子进程启动失败,或公证被拒)。

— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)

'Node.js must use its minimal helper entitlements',
);
const nodeEntitlements = fs.readFileSync(
path.join(packageDir, 'src-tauri', 'NodeEntitlements.plist'),
'utf8',
);
Comment thread
yiliang114 marked this conversation as resolved.
const appEntitlements = fs.readFileSync(
path.join(packageDir, 'src-tauri', 'Entitlements.plist'),
'utf8',
);
assert.match(
appEntitlements,
/<key>com\.apple\.security\.device\.audio-input<\/key>\s*<true\/>/,
'the app bundle must keep microphone access for voice dictation',
);
Comment thread
yiliang114 marked this conversation as resolved.
const infoPlist = fs.readFileSync(
path.join(packageDir, 'src-tauri', 'Info.plist'),
'utf8',
);
assert.match(
infoPlist,
/NSMicrophoneUsageDescription<\/key>\s*<string>.+<\/string>/,
'the app bundle must declare a non-empty microphone usage description',
);
assert.match(
nodeEntitlements,
/<key>com\.apple\.security\.cs\.allow-jit<\/key>\s*<true\/>/,
'the bundled Node.js runtime must keep its JIT entitlement',
);
assert.doesNotMatch(
nodeEntitlements,
/com\.apple\.security\.device\.audio-input/,
'Node.js must not receive microphone access',
);
assert.match(
workflow,
/Ripgrep vendor directory not found at \$rg_dir/,
Expand All @@ -125,6 +176,16 @@ function testDesktopReleaseSigningWorkflow() {
/Node\.js runtime binary not found at \$node_bin/,
'missing Node.js runtime binary must be visible in release logs',
);
assert.match(
workflow,
/Print :com\.apple\.security\.device\.audio-input/,
'the macOS signature check must keep verifying the audio-input entitlement',
);
Comment on lines +179 to +183

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 regex pins only the PlistBuddy key name, not the load-bearing = true comparison on the same workflow line. Mutation-verified: deleting = true from that workflow line leaves the unit suite fully green. — Failure scenario: a packaged app whose com.apple.security.device.audio-input entitlement is <false/> (or a non-boolean value) then passes the release gate that this test exists to guard. (The missing-key case is still caught via the empty output; the false-value case is not.)

Suggested change
assert.match(
workflow,
/Print :com\.apple\.security\.device\.audio-input/,
'the macOS signature check must keep verifying the audio-input entitlement',
);
assert.match(
workflow,
/Print :com\.apple\.security\.device\.audio-input' "\$entitlements"\)" = true/,
'the macOS signature check must keep verifying the audio-input entitlement',
);
中文说明

正则只固定了 PlistBuddy 的键名,没有固定同一行上承载核心语义的 = true 比较。变异实测:从 workflow 那行删掉 = true 后,单元套件仍然全绿。— 失败场景:打包应用里 com.apple.security.device.audio-input 权限被设为 <false/>(或非布尔值)时,本测试要守护的发布关卡仍然放行。(键缺失的情况仍会因空输出被抓到;false 值的情况抓不到。)

— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)

assert.match(
workflow,
/Print :NSMicrophoneUsageDescription/,
'the packaged smoke must keep verifying the microphone usage description',
);
assert.ok(
workflow.indexOf("name: 'Prepare bundled runtime'") <
workflow.indexOf("name: 'Sign bundled vendor binaries (macOS)'"),
Expand Down
2 changes: 2 additions & 0 deletions packages/desktop-shell/src-tauri/Entitlements.plist
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
<dict>
<key>com.apple.security.cs.allow-jit</key>
<true/>
<key>com.apple.security.device.audio-input</key>
<true/>
<key>com.apple.security.network.client</key>
<true/>
<key>com.apple.security.network.server</key>
Expand Down
8 changes: 8 additions & 0 deletions packages/desktop-shell/src-tauri/Info.plist
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>NSMicrophoneUsageDescription</key>
<string>Qwen Code uses the microphone for voice dictation in the prompt composer.</string>
</dict>
</plist>
8 changes: 8 additions & 0 deletions packages/desktop-shell/src-tauri/NodeEntitlements.plist
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>com.apple.security.cs.allow-jit</key>
<true/>
</dict>
</plist>
Loading