Skip to content

Replace three silent failures on the headless-export path with errors - #2

Open
gregakespret wants to merge 1 commit into
findmy-export-supportfrom
fix/base-branch-review-fixes
Open

Replace three silent failures on the headless-export path with errors#2
gregakespret wants to merge 1 commit into
findmy-export-supportfrom
fix/base-branch-review-fixes

Conversation

@gregakespret

Copy link
Copy Markdown
Owner

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-support itself.

MobileMeDelegateResponse::config is #[serde(default)] because the iosbuddy endpoint omits it — the same reason new_with_host exists as a fallback. Three places still treat its absence as impossible:

  • TokenProvider::get_storage_info reaches through it with .expect("No Quota?"). An account whose delegate carries tokens but no mobileme config panics rather than returning an error. It now returns PushError::MobileMeConfigMissing, naming the key it wanted.
  • KeychainClientState::new and SharedStreamsState::new return a bare None — 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 one mme_config_url helper that logs which dataclass and key were missing.

TokenProvider::set_mme_delegate stamped SystemTime::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, and refresh_mme then needs a live PET that a restored-session process does not have, failing as a bare TokenMissing. The refresh time is now a parameter. Because it is caller-supplied, get_mme_token no longer unwraps duration_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_current now rejects that state with PushError::BeaconRatchetUninitialized. The derive stays: the export path builds a BeaconAccessory only to read master_record and naming, never its ratchets, and the comment on the derive says so.

Downstream: set_mme_delegate gains a second argument, so export-findmy needs set_mme_delegate(mobileme, SystemTime::now()) at src/pipeline.rs:313. The delegate there comes from a login_apple_delegates call twenty lines up, so now() is the correct value and behaviour is unchanged.

cargo check --locked --lib passes, 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

`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
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.

1 participant