Repository navigation
Test the escrow metadata decode, and actually run tests in CI - #3
Open
gregakespret wants to merge 2 commits into
Open
gregakespret wants to merge 2 commits into
gregakespret wants to merge 2 commits into
Conversation
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
gregakespret
force-pushed
the
test/escrow-decode-seam
branch
from
October 17, 2026 12:40
3bcc628 to
b0ad0b8
Compare
export-findmy's allowlist is exactly two prefixes, "Escrow lookup returned " and "Escrow metadata schema mismatch: ", and it consults them only for records below `warn`. The test pinned the mismatch prefix plus the two "Discarding escrow metadata..." lines, which are `warn!` and on no list, so it protected nothing extra. It missed the lookup summary, the one `info!` the filter would drop if reworded. Move that line's text into `escrow_lookup_summary` (output unchanged) and pin exactly the two listed prefixes. Rewording the lookup line now fails the test. Build the fixture blob as a binary plist, which is what `enroll` writes, instead of XML. Correct the shape assertion's comment. The shape string carries no values, but the printed line does: serde's error ahead of it quotes the mismatched value. Bounding that changes log output, so it is left to its own change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nad993B5gQ8biZVfwe1S9K
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.
What
#1 changed two things about how one escrow record's metadata blob is decoded — a missing
passcodeGenerationis 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.get_viable_bottlesintodecode_escrow_metadata, so all four outcomes are reachable without a liveKeychainClient. The label-join miss stays in the loop; it is about the join, not the blob.Displayimpl on a small error enum, and the lookup summary intoescrow_lookup_summary. Two of these lines are a cross-repo interface. export-findmy raises this module toinfobehind a redaction guard and prints records belowwarnonly if they start with a prefix on its allowlist, which holds exactlyEscrow lookup returnedandEscrow metadata schema mismatch:. If the lookup line (aninfo!) is reworded, it disappears from those logs. One test pins both prefixes in the repo that owns the strings.cargo teststep to.github/workflows/build.yml. CI rancargo build --releaseand nothing else, so none of this would have executed, and neither have the TLK-share signing tests Sign a TLK share's extra record fields when verifying it #4 added (tlkshare_signing_tests). This step runs both. Same profile as the build step, so it reuses those artifacts.Log output is byte-for-byte unchanged. The tests live in
escrow_metadata_tests, named by subject like #4's module beside it.Two behaviours the tests turned up
plist::from_bytesaccepts plain text. It also reads the ASCII/NeXTSTEP format, so arbitrary bytes parse as a bareValue::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 theNotPlistarm needs bytes that break all three formats, e.g. a truncatedbplist00.invalid type: string "7"), so a device serial can reach the log. Fixing that changes log output, so it is left for a separate change.Verification
cargo test --release --features 'macos-validation-data,remote-anisette-v3' --package rustpush --lib— the exact command CI now runs — 17 passed (7 here, 9 from #4, 1 older).Mutation-checked that the tests can fail: dropping
#[serde(default)]frompasscode_generation, swappingbase64_decode_checkedback tobase64_decode, and rewording the lookup line each break the test that covers them.The test blobs are binary plists, which is what
enrollwrites.🤖 Generated with Claude Code
https://claude.ai/code/session_01Nad993B5gQ8biZVfwe1S9K