Skip to content

fix(desktop): two independent CI flakes in the Rust suite - #19

Merged
mattbalza merged 2 commits into
mainfrom
fix/passphrase-separator-in-word
Aug 6, 2026
Merged

fix(desktop): two independent CI flakes in the Rust suite#19
mattbalza merged 2 commits into
mainfrom
fix/passphrase-separator-in-word

Conversation

@mattbalza

Copy link
Copy Markdown
Owner

What broke

Windows Rust (x86_64-pc-windows-msvc) failed on main @ 3cabe1078:

thread 'key_backup::tests::generated_passphrase_clamps_word_count' panicked at src\key_backup_tests.rs:219:5:
assertion `left == right` failed
  left: 11
 right: 10

Nothing platform-specific about it — the job just happened to lose the coin flip.

Root cause

The EFF short wordlist 2.0 has exactly one entry containing a hyphen:

$ grep -nvE '^[a-z]+$' desktop/src-tauri/src/assets/eff_short_wordlist_2_0.txt
1281:yo-yo

generate_passphrase(n, "-") joins its draws with -, so a phrase holding yo-yo splits into n + 1 parts. Two tests assert phrase.split(separator).count() == count, and both draw from real OS entropy — combined odds ≈ 1% per CI run (~0.77% from the 10-word draw, ~0.23% from the 3-word one). It has nothing to do with the desktop change that commit carried.

Fix

At the shared function, not at the assertions: the rejection-sampling draw loop skips any word containing the separator, so no caller can be handed a phrase whose separator count disagrees with its word count — including the human retyping it. Dropping 1 of 1296 words costs ~0.001 bits per word.

An empty separator delimits nothing and is explicitly exercised by the existing tests, so it excludes nothing — load-bearing, since "".contains("") is true and the while chosen.len() < word_count loop would otherwise never terminate.

Test plan

  • hyphenated_wordlist_entry_is_excluded_from_hyphen_joined_phrases — pins the behaviour per separator and asserts the wordlist's hyphenated set is exactly ["yo-yo"], so a future entry fails loudly instead of resurrecting the flake.
  • generated_phrases_never_hide_a_word_boundary — 512 × 10-word draws through the real generator. Verified red against the unfixed draw loop (left: 11, right: 10), green with it.
  • cargo test --lib key_backup from desktop/src-tauri: 30 passed, 0 failed.
  • cargo fmt --check clean; cargo clippy --lib --all-targets -- -D warnings clean.

…sphrases

The EFF short wordlist 2.0 holds one entry containing a hyphen (`yo-yo`).
Drawn into a `-`-joined phrase it hides a word boundary: the phrase reads
as one more word than it has, to a human retyping it and to anything
counting the parts.

That is what turned `Windows Rust` red on 3cabe10 —
`generated_passphrase_clamps_word_count` saw 11 parts where it asked for
10. Both that test and `generated_passphrase_respects_word_count_and_separator`
draw against real entropy, so it fires roughly one CI run in a hundred on
any platform, not just Windows.

Fixed in the generator rather than in the assertions: the draw loop now
skips words containing the separator, so no caller can be handed a phrase
whose separator count disagrees with its word count. Cost is ~0.001 bits
per word. An empty separator delimits nothing, so nothing is excluded —
without that guard the loop would spin forever, since `"".contains("")`.
@mattbalza

Copy link
Copy Markdown
Owner Author

User-visible too, not just a test artifact: the backup-password generator popover (desktop/src/features/settings/ui/EncryptedBackupCreator.tsx:23) offers Hyphens as one of four separator choices, so ~1 in 130 hyphen-joined 10-word phrases would render as 11 words to whoever is writing it down.

`relay::tests::oversized_hint_is_capped_in_relay_error_message_string`
drives a real 429 through `relay_error_message`, which arms the
process-wide `GATE_EXPIRY` for the 300s cap. It took neither the serial
lock the `relay_admission` tests use nor reset the gate afterwards, and
Rust runs tests in parallel threads.

A waiter parked on its own 2s expiry re-reads the static when it wakes,
so it adopted the 300s window:

    concurrent_429_extends_the_window_for_parked_waiters
    assertion `left == right` failed: waiter must respect the extension
    armed mid-sleep (1s + 4s)
      left: 300.001s
     right: 5s

Fixed the contract rather than that one test: `test_support::lock_gate()`
returns an RAII guard that holds the lock and clears the gate on both
acquire and drop. A test can now neither inherit another's armed window
nor bequeath its own, and the "remember to reset at the end" step several
gate tests already skipped is gone.
@mattbalza mattbalza changed the title fix(desktop): stop a hyphenated wordlist entry breaking generated passphrases fix(desktop): two independent CI flakes in the Rust suite Aug 6, 2026
@mattbalza

Copy link
Copy Markdown
Owner Author

Second, unrelated flake in the same suite

The first CI run on this branch went red — but on a different test, and the passphrase tests passed (2024 passed; 1 failed):

relay_admission::tests::concurrent_429_extends_the_window_for_parked_waiters
assertion `left == right` failed: waiter must respect the extension armed mid-sleep (1s + 4s)
  left: 300.001s
 right: 5s

300s is MAX_HINT_SECONDS — the window belonged to a different test entirely.

relay::tests::oversized_hint_is_capped_in_relay_error_message_string drives a real 429 through relay_error_message, which by design arms the process-wide GATE_EXPIRY. That test took neither the serial lock the relay_admission tests share nor cleared the gate afterwards, and Rust runs tests in parallel threads. wait_for_rate_limit re-reads the static after its own expiry (that recheck loop is the feature under test), so the parked waiter adopted the 300s window.

Fixed the contract, not the one offender: test_support::lock_gate() returns an RAII guard holding the lock and clearing the gate on both acquire and drop. No test can inherit another's window or bequeath its own, and the manual reset_rate_limit_gate() trailer that four gate tests already skipped is gone.

an_armed_window_does_not_outlive_its_guard pins it. Verified red with both resets removed — the two Instants land exactly 300s apart (tv_sec: 5143 vs 4843).

cargo test --lib: 2015 passed, 0 failed, 18 ignored. cargo fmt --check and cargo clippy --all-targets -- -D warnings clean.

@mattbalza
mattbalza merged commit fe14dcc into main Aug 6, 2026
23 checks passed
@mattbalza
mattbalza deleted the fix/passphrase-separator-in-word branch August 6, 2026 11:42
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