Skip to content

fix(build): preserve identifiers in strip-and-sync framework fallback#1259

Merged
ErikBjare merged 1 commit intoActivityWatch:masterfrom
TimeToBuildBob:fix/framework-signing-strip-and-sync
Apr 13, 2026
Merged

fix(build): preserve identifiers in strip-and-sync framework fallback#1259
ErikBjare merged 1 commit intoActivityWatch:masterfrom
TimeToBuildBob:fix/framework-signing-strip-and-sync

Conversation

@TimeToBuildBob
Copy link
Copy Markdown
Contributor

Summary

  • preserve the original codesign identifier before stripping signatures in the non-standard framework fallback
  • sign the canonical binary with that identifier, then sync the signed copy to duplicate framework paths
  • keep the existing strip-and-sync approach, but avoid falling back to path-derived identifiers like Python

Why

Build Tauri on master is still failing on macOS in run 24240587094 after #1258. The notarization rejection log shows all embedded Python.framework copies are still rejected as The signature of the binary is invalid.

The current strip-and-sync fallback fixed divergent signatures, but it re-signs the canonical binary without preserving the original identifier. For these embedded Python binaries, that means codesign uses the path/basename-derived identifier (Python) instead of the original binary identifier, which is exactly the regression fixed earlier in #1255.

This patch reapplies that lesson to the strip-and-sync fallback: capture the original identifier before removing the signature, then pass it explicitly during re-signing.

Verification

  • bash -n scripts/package/build_app_tauri.sh
  • inspected notarization rejection log from run 24240587094 / job 70773894191

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 11, 2026

Greptile Summary

This PR extends the strip-and-sync fallback (introduced in #1258) for non-standard PyInstaller-embedded Python.framework bundles to preserve the original codesign identifier before stripping pre-existing signatures. Without this, re-signing the canonical binary would let codesign derive the identifier from the path/basename (e.g., Python), reintroducing the same notarization rejection fixed in #1255.

Confidence Score: 5/5

Safe to merge — the fix is logically correct and the only remaining findings are minor style suggestions.

The core invariant of the patch (capture identifier → strip → re-sign with original identifier → sync) is implemented in the right order with a sensible fallback. No P0/P1 issues were found.

No files require special attention.

Important Files Changed

Filename Overview
scripts/package/build_app_tauri.sh Adds canonical_identifier capture via codesign -d before stripping in the ambiguous-framework fallback, then passes it to sign_binary_with_identifier; logic is correct and order of capture→strip→sign is sound.

Sequence Diagram

sequenceDiagram
    participant S as build_app_tauri.sh
    participant CS as codesign
    participant FS as Filesystem

    S->>FS: find fw -type f | xargs file | grep Mach-O
    FS-->>S: [fw_bin1, fw_bin2, fw_bin3, ...]

    Note over S: First iteration (canonical)
    S->>CS: codesign -d fw_bin1 (capture identifier)
    CS-->>S: Identifier=com.python.python
    S->>CS: codesign --remove-signature fw_bin1
    S->>CS: sign_binary_with_identifier fw_bin1 com.python.python
    CS-->>S: Signed canonical binary

    Note over S: Subsequent iterations (duplicates)
    S->>CS: codesign --remove-signature fw_bin2
    S->>FS: cp -p fw_bin1 fw_bin2
    S->>CS: codesign --remove-signature fw_bin3
    S->>FS: cp -p fw_bin1 fw_bin3

    Note over S: All copies now byte-identical and share the same signature
Loading

Reviews (1): Last reviewed commit: "fix(build): preserve identifiers in stri..." | Re-trigger Greptile

Comment on lines +178 to +184
while IFS= read -r fw_bin; do
if [ -z "$canonical_identifier" ]; then
canonical_identifier=$(codesign -d "$fw_bin" 2>&1 | awk -F= '/^Identifier=/{print substr($0, index($0, "=") + 1)}')
if [ -z "$canonical_identifier" ]; then
canonical_identifier="$(basename "$fw_bin")"
fi
fi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Identifier captured from first find result — order is non-deterministic

find "$fw" -type f produces filesystem-order output with no ordering guarantee, so the binary whose identifier is captured as the canonical one is unpredictable. In the normal case this is harmless — all PyInstaller copies of Python carry the same original identifier — but if a prior aborted run left one copy re-signed with a degraded identifier (e.g., Python), that copy could surface first and its bad identifier would be propagated to all others.

A small defensive improvement would be to prefer the deepest versioned path (e.g., Versions/3.x/Python) over the shallow symlink alias, or at least log which binary's identifier was selected so it's visible in CI output.

Comment thread scripts/package/build_app_tauri.sh Outdated
while IFS= read -r fw_bin; do
if [ -z "$canonical_identifier" ]; then
canonical_identifier=$(codesign -d "$fw_bin" 2>&1 | awk -F= '/^Identifier=/{print substr($0, index($0, "=") + 1)}')
if [ -z "$canonical_identifier" ]; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 -F= field separator is set but unused

awk -F= splits on = into fields, but the print statement uses substr($0, index($0,"=") + 1) (positional) rather than $2. The -F= flag has no effect here and can be dropped for clarity.

Suggested change
if [ -z "$canonical_identifier" ]; then
canonical_identifier=$(codesign -d "$fw_bin" 2>&1 | awk '/^Identifier=/{print substr($0, index($0, "=") + 1)}')

@TimeToBuildBob
Copy link
Copy Markdown
Contributor Author

I pushed one more fix instead of just restating the same theory.

What I found

The current #1259 branch was still over-collapsing binaries inside the ambiguous Python.framework fallback.

The fallback logic did this:

  • capture one canonical_identifier from the first Mach-O it sees in the framework
  • strip signatures from all Mach-O files in that framework
  • sign the first binary once with that single identifier
  • copy that signed binary to every other Mach-O path in the framework

That is too aggressive.

Inside these PyInstaller Python.framework trees, not every Mach-O is necessarily the same logical binary just because it lives under the same framework directory. The current loop was effectively treating the whole framework as one duplicate set.

So even if preserving the identifier fixed the Python -> org.python.python regression, the fallback still risked copying the wrong signed payload across distinct Mach-O files within the framework. That's exactly the kind of thing Apple would still reject as The signature of the binary is invalid.

What I changed

New commit on this PR:

  • 50875e1fix(build): group framework fallback by content hash

The fallback now:

  1. strips any pre-existing signature from each Mach-O
  2. computes a SHA-256 of the stripped binary content
  3. groups binaries by content hash
  4. signs the first binary in each hash-group with its own preserved identifier
  5. only syncs that signed canonical binary to other paths in the same content group

So the dedup invariant is now much tighter:

  • same stripped bytes → can share the same signed payload
  • different stripped bytes → must be signed independently

Why this is better

This matches the real problem more closely.

The bug isn't "everything inside Python.framework is one duplicate set".
The bug is "some paths inside these non-standard framework copies are byte-identical duplicates with signature noise".

Grouping by stripped-content hash preserves that distinction.

Verification

  • bash -n scripts/package/build_app_tauri.sh

If this still fails after merge/test on master, then I agree we should stop guessing from Linux and move the loop onto Erik's Mac so I can inspect the signed bundle directly with codesign, spctl, file, and notarytool log locally.

…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.
@TimeToBuildBob TimeToBuildBob force-pushed the fix/framework-signing-strip-and-sync branch from 50875e1 to 563dbdf Compare April 13, 2026 01:20
@TimeToBuildBob
Copy link
Copy Markdown
Contributor Author

Rebased on master + consolidated

Rebased onto latest master (which includes #1255-#1258) and consolidated the 4 previous commits into 1 clean commit (563dbdf).

What changed from the previous version:

Net diff: +50/-52 lines in build_app_tauri.sh. The fallback block now:

  1. Strips all pre-existing signatures (PyInstaller codesign_identity remnants)
  2. Computes SHA-256 of each stripped Mach-O binary
  3. Signs one canonical per content-hash group via temp copy (preserving identifiers)
  4. Syncs signed payload only within same hash group

This is strictly more correct than the current "always sync all" approach, and maintains all the hard-won fixes from #1255-#1258.

Next step: If notarization still fails after this merge, the remaining debug loop should happen on a macOS machine with codesign/notarytool inspection of the actual built artifacts.

@ErikBjare ErikBjare merged commit c9c256b into ActivityWatch:master Apr 13, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants