Replace three silent failures on the headless-export path with errors - #2
Open
gregakespret wants to merge 1 commit into
Open
Replace three silent failures on the headless-export path with errors#2gregakespret wants to merge 1 commit into
gregakespret wants to merge 1 commit into
Conversation
`MobileMeDelegateResponse::config` is `#[serde(default)]` because the iosbuddy
endpoint omits it. Three places treated its absence as impossible:
- `TokenProvider::get_storage_info` reached through it with
`.expect("No Quota?")`, so an account whose delegate has no mobileme config
panicked instead of returning an error. It now returns
`PushError::MobileMeConfigMissing`.
- `KeychainClientState::new` and `SharedStreamsState::new` returned a bare
`None`, indistinguishable from an account that genuinely has no keychain or
shared streams, and reached the key through `.as_dictionary().unwrap()`. Both
now go through `mme_config_url`, which names the missing key in a `warn!`.
`set_mme_delegate` stamped `SystemTime::now()` regardless of when the delegate
was actually obtained. Today's only caller logs in immediately beforehand, so
this is latent rather than live, but a delegate restored from disk would re-arm
the week-long freshness window and hand out expired tokens until the timer
fired — at which point `refresh_mme` needs a live PET that a restored-session
process does not have. The refresh time is now a parameter. Since it is
caller-supplied, `get_mme_token` no longer unwraps `duration_since`; a bogus
future timestamp forces a refresh rather than panicking.
`BeaconRatchet::default()` yields an empty secret, which ratchets to another
empty secret, so an accessory built with defaulted ratchets derived keys that
decrypt nothing and reported no error. `get_current` now rejects it with
`PushError::BeaconRatchetUninitialized`. The derive stays — the export path
constructs `BeaconAccessory` only to read `master_record` and `naming` — and
the comment says so.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017VmSAa6yRzmHW6kK55g57w
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.
Follow-up to #1, from the same review pass. #1 fixed the two findings that lived in its own diff; these three are in
findmy-export-supportitself.MobileMeDelegateResponse::configis#[serde(default)]because the iosbuddy endpoint omits it — the same reasonnew_with_hostexists as a fallback. Three places still treat its absence as impossible:TokenProvider::get_storage_inforeaches through it with.expect("No Quota?"). An account whose delegate carries tokens but no mobileme config panics rather than returning an error. It now returnsPushError::MobileMeConfigMissing, naming the key it wanted.KeychainClientState::newandSharedStreamsState::newreturn a bareNone— indistinguishable at the call site from an account that genuinely has no keychain or no shared streams — and reach the URL through.as_dictionary().unwrap(). Both now go through onemme_config_urlhelper that logs which dataclass and key were missing.TokenProvider::set_mme_delegatestampedSystemTime::now()whatever the delegate's real age. This is latent, not live: the only caller today logs in immediately beforehand, so the timestamp is honest. It stops being honest the moment a delegate is restored from disk — the week-long freshness window re-arms, expired tokens go out until the timer fires, andrefresh_mmethen needs a live PET that a restored-session process does not have, failing as a bareTokenMissing. The refresh time is now a parameter. Because it is caller-supplied,get_mme_tokenno longer unwrapsduration_since; a timestamp in the future forces a refresh instead of panicking.BeaconRatchet::default()produces an empty secret, and an empty secret ratchets to another empty secret — so an accessory assembled with defaulted ratchets derived keys that decrypt nothing, with no error anywhere.get_currentnow rejects that state withPushError::BeaconRatchetUninitialized. The derive stays: the export path builds aBeaconAccessoryonly to readmaster_recordandnaming, never its ratchets, and the comment on the derive says so.Downstream:
set_mme_delegategains a second argument, soexport-findmyneedsset_mme_delegate(mobileme, SystemTime::now())atsrc/pipeline.rs:313. The delegate there comes from alogin_apple_delegatescall twenty lines up, sonow()is the correct value and behaviour is unchanged.cargo check --locked --libpasses, both with default features and with--no-default-features --features remote-anisette-v3. No new warnings (215 before and after).🤖 Generated with Claude Code
https://claude.ai/code/session_017VmSAa6yRzmHW6kK55g57w