Repository navigation
Skip a keychain item that cannot be read, instead of ending the export - #103
Merged
Merged
Conversation
Paula's traceback pinned it, which is what the stack trace was added for. The fault is one inconsistent guard rather than anything about her account: she has five owned AirTags, all visible under Items, which rules out both of the alternatives I had asked her to check. `service_keys_from_der` converts a DerError to ServiceKeyError for the outer `parse_one` and not for the descent that follows. `_from_v1` opens with `element.children()`, so a payload whose first element parses but whose contents are not DER raises a raw DerError - and `session.py` only catches `(ItemError, ServiceKeyError)`, so the skip-and-continue written for exactly this case is bypassed on which exception type came out. The proof is three consecutive lines of her log: two items failing this way were caught, logged and skipped, and the third took the whole export down. One item in sixty-one. Reproduced without an account in three lines, and the report carries it: an outer TLV with an honest length and content that is not DER gives DerError where the docstring promises ServiceKeyError.
acd17c5 -> 47c4f89, which makes service_keys_from_der answer in its own vocabulary for the whole body rather than only the outer parse_one, and catches der.DerError alongside ServiceKeyError in the caller that skips unreadable items. Verified against the new pin rather than assumed. The payload from the report - an outer TLV with an honest length wrapping content that is not DER - now raises ServiceKeyError where it raised DerError before, so the item is skipped like the two beside it instead of ending the run. 441 tests pass, flake8 clean. Worth recording what the fix is expected to recover, because it is more than one item. `unlock` reads Manatee and then ProtectedCloudStorage, accumulating across both, and the reporter's log shows Manatee completing with 141 elliptic-curve keys before the crash. The exception unwound the whole call, so those 141 were found and then thrown away. Surviving the item returns them. Not yet confirmed to fix the reported export: whether the one fatal item was itself a key is not visible from the log, and only their next run answers it.
saamdotexe
pushed a commit
to saamdotexe/OpenTagViewer
that referenced
this pull request
Oct 22, 2026
…der-escapes Skip a keychain item that cannot be read, instead of ending the export
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Takes the FindMy.py fix for the crash reported in #89, and keeps the diagnosis that found it.
What was wrong
service_keys_from_derconverted aDerErrorintoServiceKeyErrorfor the outerparse_oneand not for the descent that follows.
_from_v1opens withelement.children(), so a payloadwhose first element parses but whose contents are not DER raised a raw
DerError— and the callerin
session.pyonly caught(ItemError, ServiceKeyError).So the skip-and-continue written for exactly this case was bypassed on which exception type came
out. The proof is three consecutive lines of the reporter's log:
Those hex accounts are
PCSBoundanddefault. Two items failed the same way and were skippedcorrectly; the third ended the export. One item in sixty-one.
What it actually cost
More than one item.
unlockreadsManateeand thenProtectedCloudStorage, accumulating keysacross both, and the reporter's log shows Manatee completing with 141 elliptic-curve keys
before the crash. The exception unwound the whole call, so those were found and then discarded.
The fix
Upstream,
acd17c5→47c4f89: the whole body ofservice_keys_from_derinside the guard ratherthan only
parse_one— both arms descend, so both could raise it — plusder.DerErroralongsideServiceKeyErrorin the caller that skips unreadable items. That second half is a consistency fixrather than defensiveness:
pcs.py's key walk already caught both, and_pcs_keys_inhad notcarried it across.
Only
python/uv.lockchanges here.Verified against the new pin
DerError→ServiceKeyError, item skipped, run survivesServiceKeyError, no regressionDerError_pcs_keys_inReproduction, no account needed:
What is not claimed
This is not confirmed to fix the reported export, which is why it links #89 rather than closing
it. Whether the one fatal item was itself a key is not visible from the log — only the reporter's
next run answers that. What is certain is that the 61-item view now survives it, and the 141 keys
already recovered from Manatee are no longer thrown away.
If it still comes up short, the line to read is the
holds no keyDEBUG for the item that used tobe fatal.
Reported by @paula-coder-646 in #89, with a traceback that pinned the failure to the line.
PR description summarised by Claude Code.