Auth: revoking an app password fails (Unavailable) while a verification is queued #1041

Closed
opened 2026-10-04 08:04:12 +00:00 by kayg · 4 comments
Owner

Finding (2026-10-04, dev 6074f71d1)

cargo test -p calternal-auth app_password_revoke_rejects_queued_verification fails on dev, alone and in the full suite:

thread 'store::tests::app_password_revoke_rejects_queued_verification' panicked at crates/calternal-auth/src/store.rs:4055:45:
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 88 filtered out; finished in 0.92s

revoke_app_password(...) returns Unavailable while a verification for the same app password is queued. Found by the #1035 job (it fails identically on its branch, unchanged test).

Why it matters

Revoking an app password is a security action. If revoke can fail with Unavailable while a client (for example a CalDAV/IMAP client retrying in a loop) keeps verifications queued, the User may be unable to revoke a leaked password, or the UI may report failure. The test's intent: revoke must win and the queued verification must be rejected.

Wanted

  • Root cause (lock ordering, a busy timeout, a writer queue that rejects instead of waiting, or a test that depends on timing) and a fix where revoke always succeeds and every queued or later verification with that password fails.
  • Find when it broke (git bisect over recent auth changes) and say so.
  • The test passes 50/50 runs in a loop and under --test-threads=4; the full cargo test -p calternal-auth is green.
  • Check the same pattern for passkey and session revocation.
## Finding (2026-10-04, dev 6074f71d1) `cargo test -p calternal-auth app_password_revoke_rejects_queued_verification` fails on dev, alone and in the full suite: ```text thread 'store::tests::app_password_revoke_rejects_queued_verification' panicked at crates/calternal-auth/src/store.rs:4055:45: test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 88 filtered out; finished in 0.92s ``` `revoke_app_password(...)` returns `Unavailable` while a verification for the same app password is queued. Found by the #1035 job (it fails identically on its branch, unchanged test). ## Why it matters Revoking an app password is a security action. If revoke can fail with `Unavailable` while a client (for example a CalDAV/IMAP client retrying in a loop) keeps verifications queued, the User may be unable to revoke a leaked password, or the UI may report failure. The test's intent: revoke must win and the queued verification must be rejected. ## Wanted - Root cause (lock ordering, a busy timeout, a writer queue that rejects instead of waiting, or a test that depends on timing) and a fix where revoke always succeeds and every queued or later verification with that password fails. - Find when it broke (`git bisect` over recent auth changes) and say so. - The test passes 50/50 runs in a loop and under `--test-threads=4`; the full `cargo test -p calternal-auth` is green. - Check the same pattern for passkey and session revocation.
Author
Owner

Started on branch job/apprevoke-1041, base 6074f71d18abe73b2b4255acb564f275a9acc851. I will trace the queued verification/revoke failure, collect git bisect evidence, check passkey and session revocation, and run the auth gates plus 50 focused repetitions. No push or deployment.

Started on branch `job/apprevoke-1041`, base `6074f71d18abe73b2b4255acb564f275a9acc851`. I will trace the queued verification/revoke failure, collect git bisect evidence, check passkey and session revocation, and run the auth gates plus 50 focused repetitions. No push or deployment.
Author
Owner

Root cause: the initial diagnosis attributed Unavailable to revoke, but the unchanged base test fails at store.rs:4055, the follower's Result::unwrap(). The revoke at line 4051 succeeds. A follower scheduled during the cache-change window returns 503 before joining its credential's existing flight.

git bisect start 6074f71d1 457d9e87a -- crates/calternal-auth/src/store.rs identifies 317020c287f5af25d1aad500943eaf519d996bab (Share credential flight limits and retry unrelated authority changes). The original focused test fails on 4c0ba10ca, d4683f3d7 and 317020c28; it passes on parent 457d9e87a. No test expectation was changed.

Fix: check change contention after bounded per-credential flight admission. Extend the regression to hold a change window open until the follower is queued, so host scheduling cannot hide the race. Retain unrelated-change retry responses and all admission bounds.

Root cause: the initial diagnosis attributed `Unavailable` to revoke, but the unchanged base test fails at `store.rs:4055`, the follower's `Result::unwrap()`. The revoke at line 4051 succeeds. A follower scheduled during the cache-change window returns 503 before joining its credential's existing flight. `git bisect start 6074f71d1 457d9e87a -- crates/calternal-auth/src/store.rs` identifies `317020c287f5af25d1aad500943eaf519d996bab` (Share credential flight limits and retry unrelated authority changes). The original focused test fails on 4c0ba10ca, d4683f3d7 and 317020c28; it passes on parent 457d9e87a. No test expectation was changed. Fix: check change contention after bounded per-credential flight admission. Extend the regression to hold a change window open until the follower is queued, so host scheduling cannot hide the race. Retain unrelated-change retry responses and all admission bounds.
Author
Owner

READY FOR MERGE: yes, for #1041.

Built: move the active-change check after bounded per-credential flight admission. A queued follower now waits for its leader and rejects the revoked credential. Revoke itself was already successful: the original failure at store.rs:4055 was the follower returning Unavailable. Preserve all queue bounds and unrelated-change retry semantics.

Files: crates/calternal-auth/src/store.rs. Added deterministic coverage for followers arriving before and during a change window, plus session/passkey revocation while App Password hashing is paused. Existing assertions were retained. Module and changed-function comments were re-read.

Head: 1193b325a6636dded03568aa5f15c534d27b89e2 on job/apprevoke-1041. Base: 6074f71d18abe73b2b4255acb564f275a9acc851. The required fetch and merge of origin/dev reported “Already up to date.” No push or deployment.

Bisect: git bisect start 6074f71d1 457d9e87a -- crates/calternal-auth/src/store.rs found first bad commit 317020c287f5af25d1aad500943eaf519d996bab. The unchanged focused test failed on 4c0ba10ca, d4683f3d7 and 317020c28; it passed on parent 457d9e87a. This commit added the premature active-change check.

Gate output (verbatim summaries; complete output remains in artifacts/gate-*.log):

cargo fmt --check: no stdout/stderr, exit 0.

cargo clippy -p calternal-auth --all-targets -- -D warnings:

    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1m 22s

cargo test -p calternal-auth -- --test-threads=4:

test result: ok. 89 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 47.74s
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

cargo test -p calternal-auth app_password_revoke_rejects_queued_verification -- --test-threads=4, run in a sequential loop 50 times; every run exited 0. Final run and loop summary:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 89 filtered out; finished in 3.28s
Focused regression: 50/50 passed (--test-threads=4)

Known gaps: the ignored test is the existing timing diagnostic. Separate code inspection found that passkey login/assertion authority writes lack a final credential-presence condition; filed #1043. Its complete WebAuthn interleaving was not reproduced here. This is separate from the fixed App Password race. No server auth wiring or web code changed, so those gates do not apply.

Decisions: no product or DESIGN changes. Keep the bounded credential queue and the existing retry behavior for unrelated active changes; let followers wait before that check, as #1041 requires.

UX gaps closed/left: not applicable; no UI change.

For the merge round: bash tests/adversarial/run.sh must verify the real-server authorization and robustness matrices, including revoked App Password rejection across protocol surfaces. Deferred under the owner’s verify-once policy. No performance measurement because this issue is not about performance.

Cleanup: cargo clean finished:

     Removed 5280 files, 2.0GiB total

No web build output was produced.

READY FOR MERGE: yes, for #1041. Built: move the active-change check after bounded per-credential flight admission. A queued follower now waits for its leader and rejects the revoked credential. Revoke itself was already successful: the original failure at store.rs:4055 was the follower returning Unavailable. Preserve all queue bounds and unrelated-change retry semantics. Files: crates/calternal-auth/src/store.rs. Added deterministic coverage for followers arriving before and during a change window, plus session/passkey revocation while App Password hashing is paused. Existing assertions were retained. Module and changed-function comments were re-read. Head: `1193b325a6636dded03568aa5f15c534d27b89e2` on `job/apprevoke-1041`. Base: `6074f71d18abe73b2b4255acb564f275a9acc851`. The required fetch and merge of origin/dev reported “Already up to date.” No push or deployment. Bisect: `git bisect start 6074f71d1 457d9e87a -- crates/calternal-auth/src/store.rs` found first bad commit `317020c287f5af25d1aad500943eaf519d996bab`. The unchanged focused test failed on 4c0ba10ca, d4683f3d7 and 317020c28; it passed on parent 457d9e87a. This commit added the premature active-change check. Gate output (verbatim summaries; complete output remains in artifacts/gate-*.log): `cargo fmt --check`: no stdout/stderr, exit 0. `cargo clippy -p calternal-auth --all-targets -- -D warnings`: ```text Finished `dev` profile [unoptimized + debuginfo] target(s) in 1m 22s ``` `cargo test -p calternal-auth -- --test-threads=4`: ```text test result: ok. 89 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 47.74s test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s ``` `cargo test -p calternal-auth app_password_revoke_rejects_queued_verification -- --test-threads=4`, run in a sequential loop 50 times; every run exited 0. Final run and loop summary: ```text test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 89 filtered out; finished in 3.28s Focused regression: 50/50 passed (--test-threads=4) ``` Known gaps: the ignored test is the existing timing diagnostic. Separate code inspection found that passkey login/assertion authority writes lack a final credential-presence condition; filed #1043. Its complete WebAuthn interleaving was not reproduced here. This is separate from the fixed App Password race. No server auth wiring or web code changed, so those gates do not apply. Decisions: no product or DESIGN changes. Keep the bounded credential queue and the existing retry behavior for unrelated active changes; let followers wait before that check, as #1041 requires. UX gaps closed/left: not applicable; no UI change. For the merge round: `bash tests/adversarial/run.sh` must verify the real-server authorization and robustness matrices, including revoked App Password rejection across protocol surfaces. Deferred under the owner’s verify-once policy. No performance measurement because this issue is not about performance. Cleanup: `cargo clean` finished: ```text Removed 5280 files, 2.0GiB total ``` No web build output was produced.
Author
Owner

Deployed to production (2026-10-04 ~11:33 IST, small round 5 = d0fc1f463)

Round: dev 6074f71d1 + job/apprevoke-1041 + job/passkeybind-1043 + job/lease-1042. Gates on the round: cargo fmt --check clean; clippy clean and tests green for calternal-auth (92 passed), calternal-db (45), calternal-plugin-notes (193), calternal-plugin-mail (46), calternal-server (163), 0 failed. Staging healthy first; production healthy in 9 s, no panics or errors.

## Deployed to production (2026-10-04 ~11:33 IST, small round 5 = d0fc1f463) Round: dev 6074f71d1 + job/apprevoke-1041 + job/passkeybind-1043 + job/lease-1042. Gates on the round: `cargo fmt --check` clean; clippy clean and tests green for calternal-auth (92 passed), calternal-db (45), calternal-plugin-notes (193), calternal-plugin-mail (46), calternal-server (163), 0 failed. Staging healthy first; production healthy in 9 s, no panics or errors.
kayg closed this issue 2026-10-04 09:33:25 +00:00
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal#1041
No description provided.