Skip to content

Test the escrow metadata decode, and actually run tests in CI - #3

Open
gregakespret wants to merge 1 commit into
findmy-export-supportfrom
test/escrow-decode-seam
Open

Test the escrow metadata decode, and actually run tests in CI#3
gregakespret wants to merge 1 commit into
findmy-export-supportfrom
test/escrow-decode-seam

Conversation

@gregakespret

Copy link
Copy Markdown
Owner

What

#1 changed two things about how one escrow record's metadata blob is decoded — a missing passcodeGeneration is now tolerated, and a blob that is not standard base64 is reported rather than unwrapped — and neither has a test that can fail if it regresses. Both regress silently: the user's account simply reports no viable bottles.

  • Extract the per-record decode out of get_viable_bottles into decode_escrow_metadata, so all four outcomes are reachable without a live KeychainClient. The label-join miss stays in the loop; it is about the join, not the blob.
  • Move the three log strings into a Display impl on a small error enum. They are a cross-repo interface: export-findmy raises this module to info behind a redaction guard and prints only the records it recognises by prefix, so a reworded message disappears from those logs rather than reading differently. One test pins the prefixes in the repo that owns the strings.
  • Add a cargo test step to .github/workflows/build.yml. CI ran cargo build --release and nothing else, so none of this would have executed. Same profile as the build step, so it reuses those artifacts.

Log output is byte-for-byte unchanged.

Two behaviours the tests turned up

  • plist::from_bytes accepts plain text. It also reads the ASCII/NeXTSTEP format, so arbitrary bytes parse as a bare Value::String. A garbled blob therefore surfaces as a schema mismatch with shape [string], not as "not a valid plist" — worth knowing when reading these logs. Reaching the NotPlist arm needs bytes that break all three formats, e.g. a truncated bplist00.
  • The shape dump carries key names and value types only, never a value — asserted, since it is printed at warn and ends up pasted into bug reports.

Verification

cargo test --release --features 'macos-validation-data,remote-anisette-v3' --package rustpush --lib — the exact command CI now runs — 8 passed.

Mutation-checked that the tests can fail: dropping #[serde(default)] from passcode_generation and swapping base64_decode_checked back to base64_decode each break the test that covers them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Nad993B5gQ8biZVfwe1S9K

The two fixes in #1 — tolerating a missing `passcodeGeneration` and reporting
a bad base64 blob instead of unwrapping it — landed with no test able to fail
if they regress, and both regress silently: the account just reports no viable
bottles.

Extract the per-record decode out of `get_viable_bottles` into
`decode_escrow_metadata`, so the four outcomes are reachable without a live
`KeychainClient`, and move the three log strings into a `Display` impl. Those
strings are a cross-repo interface — export-findmy prints this module's
sub-`warn` records only when it matches them by prefix — so one test pins the
prefixes in the repo that owns them.

Two behaviours the tests turned up and now record:

- `plist::from_bytes` also reads the ASCII/NeXTSTEP format, so arbitrary text
  parses as a bare `Value::String`. A garbled blob surfaces as a schema
  mismatch with shape `[string]`, not as "not a valid plist"; that arm needs
  bytes that break all three formats, e.g. a truncated `bplist00`.
- The shape dump carries key names and value types only, never a value.

CI ran `cargo build --release` and nothing else, so none of this would have
executed. Add a `cargo test` step on the same profile, which reuses the
artifacts the build step already produced.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nad993B5gQ8biZVfwe1S9K
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