fix(build): preserve binary identifier when signing via temp-path copy#1255
Conversation
The temp-path codesign workaround for non-standard Python.framework bundles signed each binary without --identifier, so codesign derived the identifier from the random temp filename (e.g. 'tmp.XXXXXX'). Apple's notarization service then rejected those binaries with 'The signature of the binary is invalid' -- the certificate chain and code hashes are valid, but the identifier doesn't match what the binary originally carried. Fix: extract the existing identifier from the binary (set by PyInstaller's codesign_identity step, typically 'org.python.python') before copying to the temp path, then pass --identifier to the codesign invocation. Falls back to basename if the binary is unsigned.
Greptile SummaryThis PR fixes Apple notarization rejection for PyInstaller-embedded Confidence Score: 5/5Safe to merge — the fix is correct and directly addresses the notarization rejection. All findings are P2: one is a defensive awk pattern for a case (identifier containing No files require special attention.
|
| Filename | Overview |
|---|---|
| scripts/package/build_app_tauri.sh | Adds identifier extraction (codesign -d + awk) before the temp-path signing workaround so Apple's notarizer sees the original identifier (e.g. org.python.python) rather than a random tmp.XXXXXX basename; core logic is correct, two minor P2 style concerns (awk truncation, temp-file leak on copy failure). |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Find Mach-O files inside ambiguous .framework] --> B["codesign -d fw_bin 2>&1 | awk -F= '/^Identifier=/{print $2}'"]
B --> C{existing_id empty?}
C -- Yes --> D["existing_id = basename(fw_bin)"]
C -- No --> E["existing_id = extracted value\ne.g. org.python.python"]
D --> F["tmp_binary = mktemp"]
E --> F
F --> G["cp fw_bin → tmp_binary"]
G --> H["codesign --identifier existing_id\n--sign APPLE_PERSONALID tmp_binary"]
H --> I{codesign OK?}
I -- Fail --> J["rm tmp_binary; exit 1"]
I -- Success --> K["cp tmp_binary → fw_bin"]
K --> L["rm tmp_binary"]
L --> M[signed_count++]
M --> N{More Mach-O files?}
N -- Yes --> B
N -- No --> O[Report count; continue to next bundle]
Reviews (1): Last reviewed commit: "fix(build): preserve binary identifier w..." | Re-trigger Greptile
| existing_id=$(codesign -d "$fw_bin" 2>&1 \ | ||
| | awk -F= '/^Identifier=/{print $2; exit}' || true) |
There was a problem hiding this comment.
awk
-F= truncates identifiers containing =
awk -F= '{print $2}' splits on every = and discards everything after the second field. If an identifier ever contained = (e.g. a future PyInstaller/custom build), existing_id would be silently truncated. Using cut -d= -f2- preserves the full value after the first =:
| existing_id=$(codesign -d "$fw_bin" 2>&1 \ | |
| | awk -F= '/^Identifier=/{print $2; exit}' || true) | |
| existing_id=$(codesign -d "$fw_bin" 2>&1 \ | |
| | grep '^Identifier=' | cut -d= -f2- || true) |
In practice org.python.python contains no = so this won't cause an issue today, but it's a more defensive approach.
There was a problem hiding this comment.
Fixed in 399c6ad — switched to sed -n 's/^Identifier=//p' which preserves any = characters in the identifier value.
| tmp_binary=$(mktemp) | ||
| cp "$fw_bin" "$tmp_binary" | ||
| sign_binary "$tmp_binary" || { rm -f "$tmp_binary"; exit 1; } | ||
| codesign --force --options runtime --timestamp \ | ||
| --entitlements "$ENTITLEMENTS" \ | ||
| --identifier "$existing_id" \ | ||
| --sign "$APPLE_PERSONALID" \ | ||
| "$tmp_binary" || { rm -f "$tmp_binary"; exit 1; } | ||
| cp "$tmp_binary" "$fw_bin" | ||
| rm -f "$tmp_binary" |
There was a problem hiding this comment.
Temp file leaked if
cp back to $fw_bin fails
If cp "$tmp_binary" "$fw_bin" fails (e.g. disk full, permission denied), set -e at the top of the script causes an immediate exit before rm -f "$tmp_binary" runs, leaking the temp file. A trap would ensure cleanup:
tmp_binary=$(mktemp)
trap 'rm -f "$tmp_binary"' EXIT
cp "$fw_bin" "$tmp_binary"
codesign --force --options runtime --timestamp \
--entitlements "$ENTITLEMENTS" \
--identifier "$existing_id" \
--sign "$APPLE_PERSONALID" \
"$tmp_binary" || exit 1
cp "$tmp_binary" "$fw_bin"
trap - EXIT
rm -f "$tmp_binary"This is a minor robustness concern only — the OS will reclaim the file at process exit regardless.
There was a problem hiding this comment.
Fixed in 399c6ad — added || { rm -f "$tmp_binary"; exit 1; } to the cp back to $fw_bin so the temp file is cleaned up on copy failure.
…ction, clean up temp on cp failure
ActivityWatch#1255) * fix(build): preserve binary identifier when signing via temp-path copy The temp-path codesign workaround for non-standard Python.framework bundles signed each binary without --identifier, so codesign derived the identifier from the random temp filename (e.g. 'tmp.XXXXXX'). Apple's notarization service then rejected those binaries with 'The signature of the binary is invalid' -- the certificate chain and code hashes are valid, but the identifier doesn't match what the binary originally carried. Fix: extract the existing identifier from the binary (set by PyInstaller's codesign_identity step, typically 'org.python.python') before copying to the temp path, then pass --identifier to the codesign invocation. Falls back to basename if the binary is unsigned. * fix(build): address P2 review findings — use sed for identifier extraction, clean up temp on cp failure
…igning fallback Rebases on top of ActivityWatch#1255-ActivityWatch#1258 and adds two correctness improvements: 1. Strip all existing signatures before comparison, so content hashing identifies true duplicates rather than nonce-only signature differences from PyInstaller's pre-sign codesign_identity step. 2. Group binaries by SHA-256 content hash instead of treating the whole framework as one duplicate set. This correctly handles the (unlikely but possible) case where a framework contains genuinely different Mach-O files — only true duplicates share a signed payload. Both changes preserve master's existing patterns: identifier preservation via temp copy, codesign --force --options runtime, and the ambiguous- framework fallback structure.
…igning fallback (#1259) Rebases on top of #1255-#1258 and adds two correctness improvements: 1. Strip all existing signatures before comparison, so content hashing identifies true duplicates rather than nonce-only signature differences from PyInstaller's pre-sign codesign_identity step. 2. Group binaries by SHA-256 content hash instead of treating the whole framework as one duplicate set. This correctly handles the (unlikely but possible) case where a framework contains genuinely different Mach-O files — only true duplicates share a signed payload. Both changes preserve master's existing patterns: identifier preservation via temp copy, codesign --force --options runtime, and the ambiguous- framework fallback structure.
Problem
After #1254, the temp-path codesign workaround for non-standard
Python.frameworkbundles signs each binary without--identifier. This causescodesignto derive the identifier from the random temp filename (e.g.tmp.XXXXXX).Apple's notarization service then rejects those 9 binaries (3
Python.frameworkpaths × 3 watchers) with:Even though the certificate chain is valid and the code-hash pages are correct, the identifier
tmp.XXXXXXdoesn't match the original identifier the binary carried (e.g.org.python.pythonfrom PyInstaller'scodesign_identitystep). Apple's notarizer flags this as an invalid signature.Evidence: post-#1254 master CI run 24204948489 — macOS-latest Package dmg rejection log shows exactly 9 issues, all
"The signature of the binary is invalid."for thePython.framework/Python,Versions/Current/Python, andVersions/3.9/Pythonbinaries.Fix
Before copying the binary to the temp path, extract its existing code-signing identifier with
codesign -d:Then pass
--identifier "$existing_id"to thecodesigninvocation at the temp path. This preservesorg.python.python(or whatever identifier PyInstaller embedded) in the re-signed binary, satisfying Apple's notarization check.Validation
bash -n scripts/package/build_app_tauri.shpassesAPPLE_PERSONALIDsecret), but the notarization rejection is visible in post-merge master CI