Take the FindMy.py fix for the peer id the shares are fetched for

[parawanderer]

Opening this so @jamorenom has a branch to test. Not for merge until it has been run against an affected account.

Pin moves 337381de → 4f940158, carrying the fix for #140 plus the three things reviewing it turned up.

The bug

RecoveredPeer.peer_id returned the escrow record's label suffix. Cuttlefish addresses peers by the SHA256:-prefixed hash on the sealed bottle, and on some accounts those are different strings — so FetchRecoverableTlkShares asked for a peer that does not exist, and Apple answered with every view's key set and no shares at all rather than with an error. The export died at zone_keys() reporting that no keychain keys are held, which reads as an account or permissions problem and is not one.

Nothing in this repo changes but the revision string. exporter/ never touches peer_id — it asks for recovery_options and zone_keys and takes what it is given, which is why the whole failure arrived here as an empty result.

What came in beyond the reported fix

• The peer id now comes from the trust circle, with the bottle's ids as the fallback for a peer the circle no longer lists. That also fixes a case where the directory lists the inner id and not the envelope's, where preferring the envelope addresses a peer nobody knows while the sponsor check passed against the other one.
• join_trust_circle refuses a sponsor the circle does not contain. That was the third use of this property and it is the one that writes: a voucher naming an unknown sponsor is signed, sent and permanent.
• An end-to-end harness over a synthetic account in both shapes — label suffix agreeing with the circle id, and diverging. Reintroducing iCloud route: RecoveredPeer.peer_id uses the escrow label, not the Cuttlefish peer hash — every share comes back unreadable #140 fails the divergent variant and passes the agreeing one, which is what happened in the world.

That last part is what makes this safe to take. This project's own account is an agreeing one, so it takes the branch the fallback still takes — which is why nothing offline caught the bug for so long, and why a fix that always preferred the bottle id would have looked fine here and broken it.

Testing

508 exporter tests pass against the new pin.

The fix itself is unverified from here. It needs an account whose escrow label diverges from its circle id, and neither this side nor the FindMy.py side has one. @jamorenom reports 0/22 shares before and 21/21 after on two unrelated accounts and offered to re-run — that is the only evidence it works, and it is theirs.

To test this branch:

git fetch origin fix/icloud-peer-id-140
git checkout fix/icloud-peer-id-140
cd python && uv sync --frozen
uv run python -m exporter.cli --source icloud    # plus whatever you normally pass

The log line to look for is Addressing the recovered peer as SHA256:…, followed by a non-zero share count.

---

PR description summarised by Claude Code.

[jamorenom]

Tested fix/icloud-peer-id-140 at 905815f (FindMy.py 4f940158) against the diverging account. It works.

? Which device's passcode do you have? <iPhone 11, escrowed 2023-10-12>
INFO  findmy.keychain.session: Addressing the recovered peer as SHA256:<REDACTED>; its escrow label says <REDACTED, no SHA256: prefix>
INFO  findmy.keychain.session: Shares for SHA256:<REDACTED>: 21/21 unwrapped; views: ApplePay, Applications,
      AutoUnlock, Backstop, Contacts, CreditCards, DevicePairing, Engram, FindMy, Groups, Health, Home,
      LimitedPeersAllowed, MFi, Mail, Manatee, Passwords, Photos, ProtectedCloudStorage, SecureObjectSync, WiFi
INFO  findmy.keychain.session: The Manatee view holds 106 elliptic-curve key(s)
INFO  findmy.cloudkit.pcs: The zone yields 3 key(s) for its records (2 from identities, 1 derived)
Exporting 3

Same account, same escrow record, on the pin this PR replaces: 0 of 22, every view reporting
carries no readable share record, then No keychain keys are held.

The two ids are genuinely different strings on this account — the label suffix is 26 characters
with no SHA256: prefix and is absent from the peer directory, while the id the circle answers
to is the usual digest. Redacted above since they are stable and account-linked, but happy to
confirm any property of them that would help.

The part of your change I could not have got to

addressable_peer_id deriving the sponsor and the address from one lookup is a real fix to
what I sent, not a tidy-up. Mine kept them separate —

sponsor = directory.get(sealed.peer_id) or directory.get(inner.peer_id)
cuttlefish_peer_id = sealed.peer_id or inner.peer_id

— so on an account where the circle lists the inner id and not the envelope's, it would have
checked the signature against one peer and requested shares for another, reproducing #140 one
shape over. I had no way to see that: my account resolves on the envelope id, so both branches
agree here and the bug is invisible from where I was standing. Same blind spot as yours, pointed
the other way.

The join_trust_circle hardening is the one I am most glad you caught. I only ever read the two
read paths, and that is the one that signs something permanent.

Two smaller notes from using it

Neither is a blocker; both are about the failure being legible when it happens rather than about
this fix.

1. decrypt_records dropping records per-record is invisible to a caller. MissingKeyError
   and PCSError are caught and logged, and the function returns a short list with no signal
   that it is short. For an exporter that shows the user a checklist this is fine. For anything
   that treats the result as authoritative inventory it is not, and I had to count raw
   MasterBeaconRecords against the returned groups to notice. Worth a returned count or a
   strict= flag at some point.

2. The two halves 401 differently. fetch_raw_reports catches a 401 and re-logs in, which on
   a session restored without a password raises ValueError("No username or password specified")
   rather than UnauthorizedError; the CloudKit half raises UnauthorizedError directly. Anything
   trying to tell "needs re-auth" from "transient" has to catch both shapes. Again fine for the
   exporter, which has the password in hand.

Thanks for turning a one-line report into something with a test harness behind it — and for
saying plainly in the PR body which part was still unverified. That is why I ran it.

[parawanderer]

@jamorenom, both notes are fixed upstream, in ddc7f234 on feat/icloud-keychain-export, and pinned here in #152.

1. decrypt_records now returns DecryptedRecords — a list subclass carrying skipped, skipped_total and first_miss, so nothing that already treats it as a list has to change. You asked for a count or a strict= flag and got the counts; the PCSError branch was only being logged, so that was a second hole in the same tally.

The exporter's fetch() now builds from it rather than undercounting. It reports the number separately from the per-accessory list, and deliberately not as a failure — a zone holds other parties' records legitimately, so the only thing that makes it worth a second look is it being close to the number of tags you expected. first_miss is carried for the reason your report implies: the count alone cannot tell "somebody else's records" from "wrong key encoding".

2. A 401 on a session with no password now raises UnauthorizedError before attempting re-login, rather than ValueError("No username or password specified") three frames down.

And item 3 turned out to be yours as well. The alignment question from #139 was settled while chasing this, and the mechanism was worse than either side had described: _fetch_accessory_reports walked sorted(key_to_ind[key], reverse=True), so a match took the highest index the key could be. Measured on the real fetch path — primary matches exact every time, secondary matches up to +191 indices, 47.8 hours ahead, permanently, because alignment never moves back. Fixed with min.

Thanks for running it, and for the two notes. The peer-id fix was verifiable because you had the account; these two were findable because you looked at what the library was doing rather than only at whether your export worked.