Skip to content

feat(passkey): SHORTCAKE_PASSKEY companion linking - #928

Merged
jlucaso1 merged 10 commits into
mainfrom
feat-passkey
Jul 1, 2026
Merged

feat(passkey): SHORTCAKE_PASSKEY companion linking#928
jlucaso1 merged 10 commits into
mainfrom
feat-passkey

Conversation

@jlucaso1

@jlucaso1 jlucaso1 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Fix: #930

Implements WhatsApp's SHORTCAKE_PASSKEY companion-linking flow (the passkey/WebAuthn device-link gate) end to end.

The companion answers the server's WebAuthn challenge, runs the ephemeral-identity handshake (commit/reveal nonce + X25519 shared key), and sends its rotated ADV secret in an AES-256-GCM envelope. Linking then completes through the normal pair-success path.

Layout

  • wacore::shortcake: pure crypto + protobuf (ephemeral identity, commitment, verification code, HKDF/AES-GCM envelope, handoff proof). No Tokio, wasm-buildable.
  • passkey: the PasskeyAuthenticator seam for the one non-reproducible step (a real WebAuthn assertion), plus the request/assertion parsers.
  • passkey::flow: the client driver. Routes the passkey_prologue_request and crsc_continuation notifications, runs the IQ handshake, emits PairPasskey{Request,Confirmation,Error} events, and exposes send_passkey_response / send_passkey_confirmation / set_passkey_authenticator. With an authenticator registered the flow runs automatically; otherwise the host drives it from the events.

Notes

  • The ADV secret is rotated per attempt and committed only after the primary receives it, so an abandoned attempt never leaves the device on an unshared secret.
  • The handshake drives against an injectable IO seam, so the full IQ sequence is unit-tested (a test decrypts the pairing request as a simulated primary and checks the committed secret is the delivered one).

Tests

Deterministic crypto/interop tests in wacore, plus flow tests: notification routing, ADV rotation, the full handshake, and error/edge paths.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds a public passkey module, client-side SHORTCAKE_PASSKEY flow wiring, shared SHORTCAKE crypto helpers, new passkey event variants, and pair-success device-identity extraction.

Changes

SHORTCAKE_PASSKEY linking flow and crypto

Layer / File(s) Summary
Passkey module surface and authenticator
src/lib.rs, src/passkey/mod.rs
Exposes the passkey module and adds request parsing, assertion building, error handling, authenticator abstraction, and tests.
Passkey client flow and notifications
src/client.rs, src/client/lifecycle.rs, src/handlers/notification/mod.rs, src/passkey/flow.rs
Adds passkey state to Client, routes passkey notifications and continuations, and implements the prologue, continuation, response, and confirmation flow.
Shortcake crypto core
wacore/src/lib.rs, wacore/src/pair_code.rs, wacore/src/shortcake.rs
Exposes Shortcake utilities and adds the companion-linking crypto, protobuf, encryption, handoff, and test coverage.
Passkey event kinds and payloads
wacore/src/types/events.rs
Adds passkey event kinds and payload structs and maps them through Event::kind.
Pair-success device identity extraction
wacore/src/pair.rs, src/pair.rs
Adds a helper to extract device-identity from pair-success and updates pairing success handling plus the regression test.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

  • oxidezap/whatsapp-rust#404: Introduces the DeviceCommand::SetAdvSecretKey path that this passkey flow uses when committing the rotated ADV secret.
  • oxidezap/whatsapp-rust#641: Touches the same src/pair.rs pairing-completion path that now uses extract_device_identity_bytes.

Suggested labels: api-design

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: adding SHORTCAKE_PASSKEY companion linking.
Description check ✅ Passed The PR description directly addresses issue #930 and clearly describes the SHORTCAKE_PASSKEY implementation with layout, design decisions, and testing approach.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-passkey

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

📦 Binary size report

Metric main PR Δ
bin size (stripped) 10.11 MiB 10.16 MiB +54.69 KiB (+0.53%) 🔺
bin .text 8.16 MiB 8.20 MiB +42.56 KiB (+0.51%) 🚨
bin allocated (text+data+bss) 10.11 MiB 10.16 MiB +48.99 KiB (+0.47%) 🔺
llvm-lines wacore 644,533 647,967 +3,434 (+0.53%) 🔺
llvm-lines wacore copies 17,879 17,951 +72 (+0.40%) 🔺
llvm-lines whatsapp-rust lib 658,435 669,566 +11,131 (+1.69%) ⚠️
llvm-lines whatsapp-rust lib copies 20,447 20,711 +264 (+1.29%) ⚠️
deps crates (Cargo.lock) 466 466 0
.text per crate
Crate main PR Δ
.text whatsapp_rust 1.49 MiB 1.51 MiB +25.19 KiB (+1.65%) ⚠️
.text wacore 527.81 KiB 538.13 KiB +10.32 KiB (+1.95%) ⚠️
.text wacore_binary 155.84 KiB 155.84 KiB 0
.text wacore_libsignal 165.86 KiB 165.86 KiB 0
.text wacore_appstate 144.24 KiB 144.29 KiB +57 B (+0.04%) 🔺
.text wacore_noise 27.71 KiB 27.71 KiB 0
.text waproto 871.99 KiB 871.99 KiB 0
.text whatsapp_rust_sqlite_storage 475.72 KiB 475.72 KiB 0
.text whatsapp_rust_tokio_transport 43.67 KiB 43.67 KiB 0
.text whatsapp_rust_ureq_http_client 8.81 KiB 8.81 KiB 0
.text std 998.85 KiB 1002.04 KiB +3.19 KiB (+0.32%) 🔺
.text other deps 3.28 MiB 3.29 MiB +3.43 KiB (+0.10%) 🔺
Top movers (cargo-bloat attribution)
Crate main PR Δ
whatsapp_rust 1.49 MiB 1.51 MiB +25.19 KiB (+1.65%)
wacore 527.81 KiB 538.13 KiB +10.32 KiB (+1.95%)
base64 1.49 KiB 5.36 KiB +3.87 KiB (+259.61%)
std 998.85 KiB 1002.04 KiB +3.19 KiB (+0.32%)
tokio_websockets 11.96 KiB 10.94 KiB -1.02 KiB (-8.54%)

🚨 Per-PR size budget exceeded (Δ stripped ≤ 64.00 KiB, Δ .text ≤ 32.00 KiB):

  • bin .text: +42.56 KiB (+0.51%) exceeds the 32.00 KiB per-PR budget

The size-increase-ok label is set, so the gate is not enforced for this PR.

Baseline: 4ac1c81d2 (latest main run) · Head: d8eb38999 · Graphs

@codspeed-hq

codspeed-hq Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 179 untouched benchmarks


Comparing feat-passkey (2ba1fce) with main (4ac1c81)

Open in CodSpeed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/passkey/mod.rs`:
- Around line 48-55: The UserVerification::parse helper currently treats any
unknown or malformed userVerification string as Preferred, which silently
weakens verification; change this parsing logic to fail closed by rejecting
unknown values instead of defaulting. Update the UserVerification parsing path
and the second matching site mentioned in the review so both handle invalid
input explicitly, returning an error or otherwise blocking the passkey flow when
the value is not one of the supported variants.
- Around line 154-166: The allowCredentials parsing in passkey::mod:: should not
silently ignore malformed entries, because skipping non-array values or
missing/non-string id fields can turn an invalid request into an empty
allow_credentials list and make it discoverable. Update the allow_credentials
collection logic to validate every descriptor in the allowCredentials array and
return PasskeyError::InvalidOptions when any entry is not a valid object with a
string id, instead of filtering it out; keep the existing BASE64_URL_SAFE_NO_PAD
decode path for valid ids.
- Around line 100-106: The wasm callback seam still hard-codes Send on the boxed
future, which prevents !Send browser authenticator implementations from working.
Update the future alias used by CallbackAuthenticator and its constructor path
in PasskeyAuthenticator/CallbackAuthenticator so the Send bound is only required
off wasm, matching the existing async_trait(?Send) behavior. Keep the fix
localized to AssertionFuture, CallbackAuthenticator::cb, and
CallbackAuthenticator::new so the wasm path accepts non-Send futures while
native behavior stays unchanged.

In `@wacore/src/pair.rs`:
- Around line 747-760: The test currently bypasses the real pair-success
handling path by manually extracting device-identity and calling
PairUtils::do_pair_crypto(), so it does not verify the production behavior when
encryption-metadata is present. Update the test in wacore/src/pair.rs to invoke
the actual pair-success stanza-handling entrypoint used by the Pair/PairUtils
flow, and assert that it successfully ignores extra passkey children while still
processing device-identity correctly; keep the fixture setup but remove the
direct manual crypto shortcut.
- Around line 733-735: The test fixture in NodeBuilder::new("device") uses a
plausible real-looking JID, so replace that attr("jid", ...) value with an
obviously fictitious, non-user number/JID everywhere this fixture appears in
pair.rs. Keep the change limited to the test data around NodeBuilder::new and
ensure the replacement cannot be mistaken for real PII.

In `@wacore/src/shortcake.rs`:
- Around line 81-85: The public Shortcake helpers are accepting arbitrary byte
slices where the protocol requires fixed 32-byte crypto fields, so tighten the
API boundary in the affected functions such as
build_companion_ephemeral_identity and the other HKDF/protobuf builders
mentioned in the comment. Change the signatures to use fixed-size array
parameters where appropriate, or validate exact lengths up front and return
ShortcakeError::Length on mismatch instead of producing outputs from truncated
inputs. Add tests that cover non-32-byte public_key, nonce, commitment hash, and
ADV secret inputs, not just the existing primary_nonce < 5 case.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9c0040d1-b9be-47e3-b384-a55e97eab9e9

📥 Commits

Reviewing files that changed from the base of the PR and between 645e2ba and 2eedf89.

📒 Files selected for processing (6)
  • src/lib.rs
  • src/passkey/mod.rs
  • wacore/src/lib.rs
  • wacore/src/pair.rs
  • wacore/src/pair_code.rs
  • wacore/src/shortcake.rs

Comment thread src/passkey/mod.rs
Comment thread src/passkey/mod.rs
Comment thread src/passkey/mod.rs
Comment thread wacore/src/pair.rs
Comment thread wacore/src/pair.rs Outdated
Comment thread wacore/src/shortcake.rs

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found across 6 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/passkey/mod.rs Outdated
Comment thread wacore/src/pair.rs Outdated
Comment thread src/passkey/mod.rs Outdated
Comment thread wacore/src/shortcake.rs
Comment thread wacore/src/pair.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/passkey/mod.rs (1)

193-199: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Keep raw_options_json truly raw.

Line 199 reserializes through serde_json::Value, so whitespace is stripped and object keys can be reordered. That means this field no longer matches the original server payload even though the API exposes it as raw/verbatim options, which is a bad boundary for the host authenticator. Store the input string directly instead.

Proposed fix
     Ok(AssertionRequest {
         challenge,
         rp_id,
         allow_credentials,
         user_verification,
         timeout_ms,
-        raw_options_json: json.to_string(),
+        raw_options_json: json.to_owned(),
     })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/passkey/mod.rs` around lines 193 - 199, The AssertionRequest construction
in mod.rs is reserializing the passkey options via json.to_string(), which makes
raw_options_json no longer preserve the original server payload. Update the
AssertionRequest builder to pass through the original input string directly for
raw_options_json, and keep the rest of the fields in the
AssertionRequest/AssertionRequest::new flow unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/passkey/mod.rs`:
- Around line 193-199: The AssertionRequest construction in mod.rs is
reserializing the passkey options via json.to_string(), which makes
raw_options_json no longer preserve the original server payload. Update the
AssertionRequest builder to pass through the original input string directly for
raw_options_json, and keep the rest of the fields in the
AssertionRequest/AssertionRequest::new flow unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 19a1cb00-5030-4c75-a222-53a6b593a63d

📥 Commits

Reviewing files that changed from the base of the PR and between 2eedf89 and e73fd72.

📒 Files selected for processing (4)
  • src/pair.rs
  • src/passkey/mod.rs
  • wacore/src/pair.rs
  • wacore/src/shortcake.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/passkey/mod.rs (1)

160-163: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Hey, rpId is the one field still leaking. Let's make it fail closed too.

Look, we just went through and made challenge, userVerification, and allowCredentials all reject garbage — that's the right call. But rpId here still does .and_then(|r| r.as_str()), so a present-but-non-string value silently becomes None. For a passkey gate the RP ID is what scopes the credential and gives us phishing resistance; quietly dropping a malformed one and falling back to default is exactly the kind of soft-downgrade we just killed everywhere else. Distinguish "absent" from "present-but-invalid" so the whole parser is consistent. Move fast, but this part has to be airtight.

Proposed fix
-    let rp_id = v
-        .get("rpId")
-        .and_then(|r| r.as_str())
-        .map(|s| s.to_string());
+    let rp_id = match v.get("rpId") {
+        None => None,
+        Some(r) => Some(
+            r.as_str()
+                .ok_or_else(|| PasskeyError::InvalidOptions("rpId must be a string".into()))?
+                .to_string(),
+        ),
+    };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/passkey/mod.rs` around lines 160 - 163, The `rpId` parsing in
`src/passkey/mod.rs` still soft-fails by turning a present-but-non-string value
into `None`; update the parser to fail closed just like the `challenge`,
`userVerification`, and `allowCredentials` handling. In the same passkey request
parsing path that reads `rpId` from the input map, distinguish missing from
invalid values and return a parse error when `rpId` is present but not a string,
instead of silently mapping it away.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/passkey/mod.rs`:
- Around line 160-163: The `rpId` parsing in `src/passkey/mod.rs` still
soft-fails by turning a present-but-non-string value into `None`; update the
parser to fail closed just like the `challenge`, `userVerification`, and
`allowCredentials` handling. In the same passkey request parsing path that reads
`rpId` from the input map, distinguish missing from invalid values and return a
parse error when `rpId` is present but not a string, instead of silently mapping
it away.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2fcdc61e-9ed6-46b4-9d9c-00e9e7897cc7

📥 Commits

Reviewing files that changed from the base of the PR and between e73fd72 and 7361469.

📒 Files selected for processing (4)
  • src/pair.rs
  • src/passkey/mod.rs
  • wacore/src/pair.rs
  • wacore/src/shortcake.rs

jlucaso1 added a commit that referenced this pull request Jul 1, 2026
Complete the #928 foundation into a functional, integrated flow mirroring
whatsmeow's pair-passkey.go:

- src/passkey/flow.rs: the Client driver (send_passkey_response /
  send_passkey_confirmation / set_passkey_authenticator with optional
  authenticator auto-drive), notification handlers for
  passkey_prologue_request and crsc_continuation (both gated on from ==
  SERVER_JID), ADV-secret rotation, and the pairing-handoff-vs-verification
  -code branch.
- wacore::shortcake: parse_primary_ephemeral_identity with 32-byte length
  validation (which now uses ShortcakeError::Length).
- Three PairPasskey{Request,Confirmation,Error} events.

Fold in the outstanding #928 review items: PasskeyAuthenticator and
CallbackAuthenticator use MaybeSendSync instead of a hardcoded Send + Sync
bound, and parse_request_options rejects an empty challenge / credential id.

Tests: a full end-to-end interop test drives both the companion and a
simulated primary through the real primitives and proves the primary can
decrypt the companion's PairingRequest (key agreement + verification-code
agreement + handoff-proof verification); plus notification-routing,
rotation, error-path, and prologue-wire-shape tests.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/passkey/flow.rs`:
- Around line 356-369: The passkey flow is persisting the rotated ADV secret too
early in the confirmation path; keep the new secret pending in the passkey
attempt state instead of calling
PersistenceManager::process_command(DeviceCommand::SetAdvSecretKey) before the
commit point. Update src/passkey/flow.rs around send_passkey_confirmation so the
new ADV secret is stored only after the encrypted_pairing_request send succeeds,
and ensure the handoff key/rotation logic in the same flow remains tied to the
commit step rather than the pre-confirmation setup.
- Around line 245-249: The linking cache update in the passkey flow can attach
the encryption key to the wrong attempt after an await because state.linking may
have changed before the mutation. Revalidate that the current linking attempt is
still the same one captured earlier in the flow before setting encryption_key,
using the relevant passkey flow state in the passkey_state lock section so only
the intended continuation is armed.
- Around line 160-189: Clear the stale linking state if sending the passkey
prologue fails: in the `passkey_prologue` flow around `state.linking` and
`send_iq` in `Flow`, make sure a failed `self.send_iq(...)` rolls back or
removes the cached `LinkingCache` so a later continuation cannot reuse an
unaccepted pairing state. Update the logic in this branch to preserve the
existing `handoff_proof` behavior while ensuring the `passkey_state` lock does
not keep `keypair`, `companion_nonce`, or `pairing_ref` after `send_iq` returns
an error.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c58eca99-969c-433f-bb91-7396d61ec6b4

📥 Commits

Reviewing files that changed from the base of the PR and between 7361469 and e743918.

📒 Files selected for processing (7)
  • src/client.rs
  • src/client/lifecycle.rs
  • src/handlers/notification/mod.rs
  • src/passkey/flow.rs
  • src/passkey/mod.rs
  • wacore/src/shortcake.rs
  • wacore/src/types/events.rs

Comment thread src/passkey/flow.rs Outdated
Comment thread src/passkey/flow.rs Outdated
Comment thread src/passkey/flow.rs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 7 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/passkey/flow.rs">

<violation number="1" location="src/passkey/flow.rs:166">
P2: Flow state is mutated before `passkey_prologue` send succeeds. On send failure, retry/continuation state becomes inconsistent.</violation>

<violation number="2" location="src/passkey/flow.rs:247">
P2: Revalidate that the linking cache still belongs to this attempt before storing the encryption key. Between the initial copy (lines 204-217) and this mutation, async work occurs (IQ round-trip + key derivation). A concurrent `send_passkey_response` can replace `state.linking` with a new attempt, causing this continuation to arm the wrong cache with an unrelated encryption key. Check `pairing_ref`/`companion_nonce` match before writing.</violation>

<violation number="3" location="src/passkey/flow.rs:368">
P1: Persisting the rotated ADV secret here creates a window for state divergence. If any subsequent step fails (assertion, IQ send, encryption, confirmation), the local device has a rotated secret the primary never received. Consider deferring the persist until after `send_passkey_confirmation` succeeds, or stashing the new secret in the linking cache and committing only on success.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/passkey/flow.rs Outdated
Comment thread src/passkey/flow.rs Outdated
Comment thread src/passkey/flow.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/passkey/flow.rs`:
- Around line 327-337: The fallback IQ in the passkey flow is bypassing the
standard WhatsApp spec executor by calling send_iq directly. Update the
passkey_request_options path in the flow.rs logic to route through the
client.execute(Spec::new(&jid)).await? pattern instead, keeping the existing
InfoQuery payload but using the appropriate IQ spec constructor with a borrowed
Jid. Make sure the change stays within the same passkey flow method so the
fallback remains consistent with the rest of the IQ request handling.
- Around line 641-657: The test in
passkey_prologue_request_without_inline_options_falls_back_to_fetch currently
only checks for a non-continuation PairPasskeyError, which can pass for the
wrong reason. Tighten the assertion to verify the error text or path corresponds
to the IQ fetch fallback so it proves the fallback branch in process_node and
the passkey prologue flow actually ran, rather than an immediate
missing-inline-options failure.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c550232f-4cac-486b-b6d0-3116f18eada5

📥 Commits

Reviewing files that changed from the base of the PR and between e743918 and 668183f.

📒 Files selected for processing (1)
  • src/passkey/flow.rs

Comment thread src/passkey/flow.rs Outdated
Comment thread src/passkey/flow.rs
@jlucaso1 jlucaso1 added the size-increase-ok Accepted binary-size increase: downgrades the per-PR size gate to a warning label Jul 1, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (3)
src/passkey/flow.rs (3)

393-407: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Don’t persist the rotated ADV secret before the flow commits.

drive_passkey_request writes new_adv as soon as options are received. If assertion, prologue, user confirmation, or <encrypted_pairing_request> send fails, this device has already moved to an ADV secret the primary never accepted. Keep the rotated secret pending and commit it only after the encrypted pairing request succeeds.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/passkey/flow.rs` around lines 393 - 407, `drive_passkey_request` is
persisting the rotated ADV secret too early by calling `SetAdvSecretKey` before
the flow is committed. Keep the newly generated ADV secret pending in
`drive_passkey_request` (and any related passkey state) and only write it
through `persistence_manager.process_command` after the encrypted pairing
request is successfully sent and the flow has committed. Use the existing
`drive_passkey_request` and `passkey_state` flow to locate the commit point and
move the persistence there.

653-656: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the fallback fetch failure, not just any request error.

This predicate would also pass if the handler emitted an immediate initial-stage error. Check the error path so the test proves the IQ fallback ran.

Proposed test tightening
-            |e| matches!(e, Event::PairPasskeyError(err) if !err.continuation),
+            |e| {
+                matches!(
+                    e,
+                    Event::PairPasskeyError(err)
+                        if !err.continuation
+                            && err.error.contains("passkey_request_options iq failed")
+                )
+            },
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/passkey/flow.rs` around lines 653 - 656, The test in passkey flow is too
broad because the wait_for predicate matches any non-continuation
PairPasskeyError, so it can pass on an immediate initial-stage failure instead
of the IQ fallback path. Tighten the assertion around wait_for and
Event::PairPasskeyError by checking the specific fallback fetch failure
condition emitted after the IQ fallback runs, using the existing collector/event
matching in the passkey flow test to ensure the fallback path is what failed.

327-337: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Route this fallback IQ through the standard executor.

This fallback still bypasses the repo’s IQ spec path by calling send_iq directly. Keep this on client.execute(Spec::new(&jid)).await? so the md IQ path stays consistent. As per coding guidelines, “Use client.execute(Spec::new(&jid)).await? pattern for IQ requests. IqSpec constructors take &Jid not Jid.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/passkey/flow.rs` around lines 327 - 337, The fallback in passkey flow is
still sending the IQ directly, so route it through the standard IQ executor
instead of calling send_iq in this block. Update the passkey_request_options
path in the passkey flow to use client.execute(Spec::new(&jid)).await? with the
existing md InfoQuery/IQ spec flow, keeping the Jid passed by reference as
required by the repo pattern. Make sure the response handling stays the same
after switching to the executor-based path.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@src/passkey/flow.rs`:
- Around line 393-407: `drive_passkey_request` is persisting the rotated ADV
secret too early by calling `SetAdvSecretKey` before the flow is committed. Keep
the newly generated ADV secret pending in `drive_passkey_request` (and any
related passkey state) and only write it through
`persistence_manager.process_command` after the encrypted pairing request is
successfully sent and the flow has committed. Use the existing
`drive_passkey_request` and `passkey_state` flow to locate the commit point and
move the persistence there.
- Around line 653-656: The test in passkey flow is too broad because the
wait_for predicate matches any non-continuation PairPasskeyError, so it can pass
on an immediate initial-stage failure instead of the IQ fallback path. Tighten
the assertion around wait_for and Event::PairPasskeyError by checking the
specific fallback fetch failure condition emitted after the IQ fallback runs,
using the existing collector/event matching in the passkey flow test to ensure
the fallback path is what failed.
- Around line 327-337: The fallback in passkey flow is still sending the IQ
directly, so route it through the standard IQ executor instead of calling
send_iq in this block. Update the passkey_request_options path in the passkey
flow to use client.execute(Spec::new(&jid)).await? with the existing md
InfoQuery/IQ spec flow, keeping the Jid passed by reference as required by the
repo pattern. Make sure the response handling stays the same after switching to
the executor-based path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6a84bcde-345d-40d8-81a2-87ca9514ec53

📥 Commits

Reviewing files that changed from the base of the PR and between e743918 and 668183f.

📒 Files selected for processing (1)
  • src/passkey/flow.rs

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/passkey/flow.rs Outdated
Comment thread src/passkey/flow.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/passkey/flow.rs (1)

286-345: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Keep the staged ADV secret tied to one linking attempt.

drive_passkey_request can overwrite pending_adv_secret while an earlier linking cache is waiting for confirmation, and send_passkey_confirmation then reads the current pending secret independently of the cache it is confirming. Put the pending secret into LinkingCache when arming the prologue, or abort/clear the previous attempt before staging a new one, then revalidate the attempt before clearing state after the IQ await.

Also applies to: 411-424

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/passkey/flow.rs` around lines 286 - 345, The confirmation flow is using a
shared pending ADV secret that can be overwritten by a newer attempt, so tie the
staged secret to the specific linking attempt. Update the
`LinkingCache`/`passkey_state` logic in `drive_passkey_request` and
`send_passkey_confirmation` so the secret is stored with the cache (or any
previous attempt is explicitly cleared/aborted before staging a new one), and
recheck that the same attempt is still active after the IQ await before clearing
state and committing the secret.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/passkey/flow.rs`:
- Around line 128-136: The new passkey IQ requests in the flow are bypassing the
required spec-executor path; update the ref, prologue, nonce, and
encrypted-pairing-request logic in the passkey flow to use
client.execute(Spec::new(&jid)).await? instead of send_iq directly. Refactor the
relevant helpers and call sites in PasskeyFlow so the IqSpec constructors
receive &Jid (not Jid), and keep the existing flow/error handling intact while
routing all these IQ exchanges through the spec executor.

---

Outside diff comments:
In `@src/passkey/flow.rs`:
- Around line 286-345: The confirmation flow is using a shared pending ADV
secret that can be overwritten by a newer attempt, so tie the staged secret to
the specific linking attempt. Update the `LinkingCache`/`passkey_state` logic in
`drive_passkey_request` and `send_passkey_confirmation` so the secret is stored
with the cache (or any previous attempt is explicitly cleared/aborted before
staging a new one), and recheck that the same attempt is still active after the
IQ await before clearing state and committing the secret.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6ff03fe1-fdd3-4454-839f-dca5c0a96ef6

📥 Commits

Reviewing files that changed from the base of the PR and between 668183f and ccfc4b6.

📒 Files selected for processing (5)
  • src/handlers/notification/mod.rs
  • src/passkey/flow.rs
  • src/passkey/mod.rs
  • wacore/src/shortcake.rs
  • wacore/src/types/events.rs

Comment thread src/passkey/flow.rs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 5 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/passkey/flow.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/passkey/flow.rs`:
- Around line 409-416: The passkey handoff logic is always setting
passkey_state.handoff_key from get_device_snapshot() and
derive_pairing_handoff_hmac_key, which makes fresh links look like re-links.
Update flow.rs so this assignment in the passkey flow is gated by an explicit
re-link/continuity flag coming from the notification/request path, and otherwise
leave handoff_key as None; use the existing passkey_state and
send_passkey_response path to wire the signal through.
- Around line 49-52: The ADV confirmation flow in passkey/flow.rs still allows
out-of-order commits because the confirmation path snapshots new_adv_secret,
awaits the IQ, then commits and clears shared state. Update the confirmation
logic around the relevant attempt/state handling to serialize commits: add an
explicit confirming state or attempt token in the flow so only one
encrypted-pairing-request commit/rollback can run at a time, and make concurrent
confirmations reject or wait until the current one finishes. Ensure the commit
and rollback paths use the attempt identity to prevent stale confirmations from
persisting the wrong ADV secret.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 388aff8d-d1bb-4f09-9ca2-56fe2291a8f1

📥 Commits

Reviewing files that changed from the base of the PR and between ccfc4b6 and ff8e826.

📒 Files selected for processing (1)
  • src/passkey/flow.rs

Comment thread src/passkey/flow.rs Outdated
Comment thread src/passkey/flow.rs

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/passkey/flow.rs Outdated
jlucaso1 added 6 commits June 30, 2026 23:22
Complete the #928 foundation into a functional, integrated flow mirroring
whatsmeow's pair-passkey.go:

- src/passkey/flow.rs: the Client driver (send_passkey_response /
  send_passkey_confirmation / set_passkey_authenticator with optional
  authenticator auto-drive), notification handlers for
  passkey_prologue_request and crsc_continuation (both gated on from ==
  SERVER_JID), ADV-secret rotation, and the pairing-handoff-vs-verification
  -code branch.
- wacore::shortcake: parse_primary_ephemeral_identity with 32-byte length
  validation (which now uses ShortcakeError::Length).
- Three PairPasskey{Request,Confirmation,Error} events.

Fold in the outstanding #928 review items: PasskeyAuthenticator and
CallbackAuthenticator use MaybeSendSync instead of a hardcoded Send + Sync
bound, and parse_request_options rejects an empty challenge / credential id.

Tests: a full end-to-end interop test drives both the companion and a
simulated primary through the real primitives and proves the primary can
decrypt the companion's PairingRequest (key agreement + verification-code
agreement + handoff-proof verification); plus notification-routing,
rotation, error-path, and prologue-wire-shape tests.
Mirror whatsmeow's getPasskeyRequestOptions fallback: when a
passkey_prologue_request notification arrives without an inline
<passkey_request_options>, fetch it via an "md" IQ get before rotating the
ADV secret and emitting PairPasskeyRequest, instead of erroring out.

Extract the rotate + dispatch + auto-drive path into drive_passkey_request
so the inline and fetched cases share it, and update the missing-options
test to assert the fallback runs (it surfaces a non-continuation error when
offline).
Defer the ADV-secret rotation to send_passkey_confirmation (staged pending
in the flow state) so a failed or abandoned attempt never leaves the device
rotated to a secret the primary never received. Clear the linking cache when
the prologue IQ fails, and revalidate the attempt still owns the cache before
storing the derived key.

Hoist the IQ namespace, stanza tags, and notification routing keys into named
constants, derive the verification-code split offset, and trim the comments.
Move the not-yet-committed rotated secret out of the shared flow state into
the per-attempt LinkingCache, generated in send_passkey_response. Two
concurrent passkey requests can no longer cross-contaminate which secret a
confirmation commits.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/passkey/mod.rs`:
- Around line 179-182: Reject malformed rpId values in the passkey parsing logic
instead of converting them to None. In the parsing path that builds
AssertionRequest::rp_id from the rpId field, distinguish between a missing key
and a present non-string value: keep missing as absent, but return an error for
invalid types so non-Android backends cannot silently default the RP. Update the
rpId handling in the passkey module around the AssertionRequest parser to fail
closed on malformed input.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bdaafd08-3a40-4e87-8879-e5638229f7dc

📥 Commits

Reviewing files that changed from the base of the PR and between ff8e826 and 212ea52.

📒 Files selected for processing (12)
  • src/client.rs
  • src/client/lifecycle.rs
  • src/handlers/notification/mod.rs
  • src/lib.rs
  • src/pair.rs
  • src/passkey/flow.rs
  • src/passkey/mod.rs
  • wacore/src/lib.rs
  • wacore/src/pair.rs
  • wacore/src/pair_code.rs
  • wacore/src/shortcake.rs
  • wacore/src/types/events.rs

Comment thread src/passkey/mod.rs Outdated
Extract the handshake into a ShortcakeSession state machine (explicit Stage
enum) that drives against a ShortcakeIo trait instead of the concrete Client,
so the full IQ sequence is unit-testable with a scripted stand-in. Client
implements ShortcakeIo; the public API and notification handlers are unchanged.

Add a full-handshake test that runs the ref -> prologue -> companion_nonce ->
encrypted_pairing_request sequence and decrypts the pairing request as a
simulated primary, proving the committed ADV secret is exactly the one
delivered.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/passkey/flow.rs`:
- Around line 391-401: The `send_passkey_confirmation` flow in `PasskeyFlow` is
taking and dropping the active session too early when confirmation is triggered
before the continuation reaches the right stage. Update
`send_passkey_confirmation()` to check that `Stage::AwaitingConfirmation` is
active before calling `take()` on `passkey_state.session`, and only consume the
session after that validation so premature confirmation does not break the link
flow.
- Around line 193-197: `on_primary_identity` is emitting
`PairPasskeyConfirmation` before the passkey session is restored, so synchronous
listeners can observe “confirmation without an active session,” and the `?` path
can also drop the session on error. Refactor `on_primary_identity` and
`drive_continuation` so `on_primary_identity` only returns the confirmation
payload, the session is restored first, and `PairPasskeyConfirmation` is
dispatched afterward; keep the restore step outside the fallible path and
preserve the existing `skip_handoff_ux` value when building the payload.
- Around line 382-385: The send_passkey_response flow currently leaves the
passkey session empty while ShortcakeSession::open awaits, allowing concurrent
manual/auto responses to start overlapping handshakes. Update
send_passkey_response in the Assertion handling path to reserve the attempt
before the first await by introducing an explicit Opening state or attempt token
in passkey_state, and have subsequent responses reject or wait if an open is
already in progress. Ensure the reservation is checked and set around the
handoff_key/session transition so only one active open can complete and assign
passkey_state.session.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 220d0255-ef43-4851-9616-eeda1b03bc0f

📥 Commits

Reviewing files that changed from the base of the PR and between 212ea52 and 4bba686.

📒 Files selected for processing (1)
  • src/passkey/flow.rs

Comment thread src/passkey/flow.rs Outdated
Comment thread src/passkey/flow.rs
Comment thread src/passkey/flow.rs

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/passkey/flow.rs
Comment thread src/passkey/flow.rs Outdated
Comment thread src/passkey/flow.rs Outdated
Address review of the event-driven flow:

- reject a non-string rpId instead of silently dropping the RP binding, matching
  the other parse_request_options validations
- return the confirmation payload from on_primary_identity and restore the
  session before publishing PairPasskeyConfirmation, so a synchronous listener
  that confirms from the event sees an active session
- validate the confirmation stage before taking the session, so a premature
  send_passkey_confirmation errors without dropping the in-flight attempt
- reserve the single attempt slot before the opening awaits, rejecting a
  concurrent open that would clobber the nonce/ref and commit the wrong secret

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/passkey/flow.rs`:
- Around line 404-422: The send_passkey_confirmation method still uses a
production unwrap when consuming state.session, which violates the repo rule.
Update the match in send_passkey_confirmation to match directly on
state.session.take() (or otherwise handle the taken value without unwrap) so the
AwaitingConfirmation path remains safe and the premature-call branches still
preserve the session correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2803a411-5efc-47bb-81eb-60d7bc48e6d3

📥 Commits

Reviewing files that changed from the base of the PR and between 4bba686 and 02b1196.

📒 Files selected for processing (2)
  • src/passkey/flow.rs
  • src/passkey/mod.rs

Comment thread src/passkey/flow.rs

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 2 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/passkey/flow.rs Outdated
Comment thread src/passkey/flow.rs Outdated
…ncellation-safe

send_passkey_confirmation now matches on session.take() and restores the session on the wrong-stage arm instead of unwrapping. An OpeningGuard releases the opening reservation on drop, so a send_passkey_response cancelled mid-open can no longer leave the flow permanently blocked.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/passkey/flow.rs (1)

469-474: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Restore the session on the on_primary_identity error path too.

This still takes the session before the await and drops it on ?. A duplicate/stale continuation that hits the stage check can delete the active AwaitingConfirmation session before the user confirms. That’s not reliable enough for this gate.

Proposed fix
-        let confirmation = session.on_primary_identity(self, &primary_bytes).await?;
+        let confirmation = match session.on_primary_identity(self, &primary_bytes).await {
+            Ok(confirmation) => confirmation,
+            Err(err) => {
+                self.passkey_state.lock().await.session = Some(session);
+                return Err(err);
+            }
+        };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/passkey/flow.rs` around lines 469 - 474, The `on_primary_identity` call
in `flow.rs` still consumes the `AwaitingConfirmation` session before the await
and loses it on the `?` error path. Update the `on_primary_identity` flow so the
session is restored back into `passkey_state.lock().await.session` not only
after a successful `confirmation`, but also when `on_primary_identity(self,
&primary_bytes)` returns an error; use the surrounding `session`,
`confirmation`, and `passkey_state` logic to ensure a stale continuation cannot
delete the active session.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/passkey/flow.rs`:
- Around line 272-280: The OpeningGuard cleanup in drop is still racy because
OpeningGuard::drop uses passkey_state.try_lock(), so a contended lock can leave
state.opening stuck true and block later passkey flows. Update the cleanup in
OpeningGuard and the surrounding passkey_state/opening reservation logic to use
a cancellation-safe approach that cannot be skipped under contention, such as an
owned async cleanup path or a wait-free reservation flag, and make sure the
opening flag is always cleared. Add a regression test around the passkey
response path that holds passkey_state long enough to force contention and
verifies the opening state is still reset.

---

Duplicate comments:
In `@src/passkey/flow.rs`:
- Around line 469-474: The `on_primary_identity` call in `flow.rs` still
consumes the `AwaitingConfirmation` session before the await and loses it on the
`?` error path. Update the `on_primary_identity` flow so the session is restored
back into `passkey_state.lock().await.session` not only after a successful
`confirmation`, but also when `on_primary_identity(self, &primary_bytes)`
returns an error; use the surrounding `session`, `confirmation`, and
`passkey_state` logic to ensure a stale continuation cannot delete the active
session.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 23cc33f8-3f39-47ab-b6af-44315ac802b4

📥 Commits

Reviewing files that changed from the base of the PR and between 4bba686 and dcf085d.

📒 Files selected for processing (2)
  • src/passkey/flow.rs
  • src/passkey/mod.rs

Comment thread src/passkey/flow.rs Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/passkey/flow.rs Outdated
Move the open-in-flight flag out of the async-locked state into a plain AtomicBool on the client, released by the guard's Drop with a synchronous store. This is cancellation-safe even under lock contention, unlike the previous try_lock() cleanup, which could fail on a contended drop and leave the reservation stuck.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/passkey/flow.rs`:
- Around line 420-421: Keep an active attempt sentinel across the async passkey
flow so the slot is never temporarily empty while work is in flight. Update the
passkey state handling around ShortcakeSession::open, the confirmation path, and
the continuation path so send_passkey_response cannot start a second attempt or
accept a continuation before the current attempt is tracked. Use an explicit
in-progress state or attempt token in passkey_state instead of clearing session
during awaited phases, and only transition to None once the attempt is fully
finished or failed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: edee0532-8f9a-41c3-8398-f3f9df737f20

📥 Commits

Reviewing files that changed from the base of the PR and between dcf085d and 2ba1fce.

📒 Files selected for processing (3)
  • src/client.rs
  • src/client/lifecycle.rs
  • src/passkey/flow.rs

Comment thread src/passkey/flow.rs
@jlucaso1 jlucaso1 changed the title feat: Add SHORTCAKE_PASSKEY linking foundation feat(passkey): SHORTCAKE_PASSKEY companion linking Jul 1, 2026
@jlucaso1
jlucaso1 merged commit 3fb4729 into main Jul 1, 2026
18 checks passed
@jlucaso1
jlucaso1 deleted the feat-passkey branch July 1, 2026 22:18
jlucaso1 added a commit that referenced this pull request Jul 2, 2026
Ports the post-merge additions to the buffa API: shortcake.rs and the passkey
flow (#928) move from prost to buffa (decode_from_slice, encode_to_vec via
buffa::Message, typed PlatformType through buffa::Enumeration), media options
(#930) convert Option<Box<ContextInfo>> into MessageField at the build sites,
and history_sync keeps main's smoothutf8 validation (#932).

Also swaps the temporary buffa git pin for crates.io 0.8.1, which carries the
inline-by-default sub-message representation and inlined Tag::new the pin
existed to measure.
JordanTheJet added a commit to JordanTheJet/zeroclaw that referenced this pull request Aug 18, 2026
WhatsApp began a server-side rollout on 2026-06-30 that requires a
passkey/WebAuthn step during companion linking. The pinned revision
(cbcdd2a, 2026-05-27) does not handle the server's
`passkey_prologue_request` notification, so linking never completes: the
daemon re-emits QR codes indefinitely and the phone reports "Couldn't
link device, try again later" with nothing in the logs (zeroclaw-labs#8627).

Upstream fixed this in oxidezap/whatsapp-rust#928 (merged 2026-07-01).
Move the six git pins onto b5daf757, which contains that fix.

b5daf757 rather than upstream HEAD is deliberate: one day after the
passkey fix, upstream migrated protobuf codegen from prost to buffa
(a6fb1174), rewriting every generated waproto field from Option<Box<T>>
to MessageField<T>. At HEAD this crate fails with 46 errors; at
b5daf757 it fails with 18 mechanical ones, fixed here:

- implement MsgSecretStore for RusqliteStore (a new Backend supertrait)
  with a msg_secrets table, matching upstream's documented conflict
  merge: the later expiry wins with 0 ("never") beating any finite
  deadline, and a known parent timestamp is never clobbered by 0
- add SignalStore::mark_prekeys_uploaded (UPDATE-only, so a pre-key
  consumed since the upload snapshot is not resurrected) and
  AppSyncStore::clear_mutation_macs
- UploadResponse crypto fields are plain [u8; 32], not accessors
- Device::account is Arc-wrapped; LidPnMapping.phone_number is Arc<str>
- get_device_snapshot() is sync; Bot::run() consumes self, so use
  Bot::spawn() for the handle this channel needs
- BotBuilder::with_backend takes `impl Backend`, so an Arc backend now
  goes through with_backend_arc

Also stop dropping the passkey events on the floor: PairPasskeyRequest,
PairPasskeyConfirmation and PairPasskeyError previously fell into the
`_ => {}` arm, which is why the failure was silent. They now emit login
lifecycle events and logs.

This restores visibility and drives the ceremony, but does not by itself
make a headless link succeed: the flow still needs a WebAuthn assertion
from a passkey registered to the account, which upstream abstracts behind
`PasskeyAuthenticator` and does not ship a default for. Wiring an
authenticator source is follow-up work.

Refs zeroclaw-labs#8627
JordanTheJet added a commit to JordanTheJet/zeroclaw that referenced this pull request Aug 18, 2026
WhatsApp began a server-side rollout on 2026-06-30 that requires a
passkey/WebAuthn step during companion linking. The pinned revision
(cbcdd2a, 2026-05-27) does not handle the server's
`passkey_prologue_request` notification, so linking never completes: the
daemon re-emits QR codes indefinitely and the phone reports "Couldn't
link device, try again later" with nothing in the logs (zeroclaw-labs#8627).

Upstream fixed this in oxidezap/whatsapp-rust#928 (merged 2026-07-01).
Move the six git pins onto b5daf757, which contains that fix.

b5daf757 rather than upstream HEAD is deliberate: one day after the
passkey fix, upstream migrated protobuf codegen from prost to buffa
(a6fb1174), rewriting every generated waproto field from Option<Box<T>>
to MessageField<T>. At HEAD this crate fails with 46 errors; at
b5daf757 it fails with 18 mechanical ones, fixed here:

- implement MsgSecretStore for RusqliteStore (a new Backend supertrait)
  with a msg_secrets table, matching upstream's documented conflict
  merge: the later expiry wins with 0 ("never") beating any finite
  deadline, and a known parent timestamp is never clobbered by 0
- add SignalStore::mark_prekeys_uploaded (UPDATE-only, so a pre-key
  consumed since the upload snapshot is not resurrected) and
  AppSyncStore::clear_mutation_macs
- UploadResponse crypto fields are plain [u8; 32], not accessors
- Device::account is Arc-wrapped; LidPnMapping.phone_number is Arc<str>
- get_device_snapshot() is sync; Bot::run() consumes self, so use
  Bot::spawn() for the handle this channel needs
- BotBuilder::with_backend takes `impl Backend`, so an Arc backend now
  goes through with_backend_arc

Also stop dropping the passkey events on the floor: PairPasskeyRequest,
PairPasskeyConfirmation and PairPasskeyError previously fell into the
`_ => {}` arm, which is why the failure was silent. They now emit login
lifecycle events and logs.

This restores visibility and drives the ceremony, but does not by itself
make a headless link succeed: the flow still needs a WebAuthn assertion
from a passkey registered to the account, which upstream abstracts behind
`PasskeyAuthenticator` and does not ship a default for. Wiring an
authenticator source is follow-up work.

Refs zeroclaw-labs#8627
JordanTheJet added a commit to JordanTheJet/zeroclaw that referenced this pull request Aug 20, 2026
WhatsApp began a server-side rollout on 2026-06-30 that requires a
passkey/WebAuthn step during companion linking. The pinned revision
(cbcdd2a, 2026-05-27) does not handle the server's
`passkey_prologue_request` notification, so linking never completes: the
daemon re-emits QR codes indefinitely and the phone reports "Couldn't
link device, try again later" with nothing in the logs (zeroclaw-labs#8627).

Upstream fixed this in oxidezap/whatsapp-rust#928 (merged 2026-07-01).
Move the six git pins onto b5daf757, which contains that fix.

b5daf757 rather than upstream HEAD is deliberate: one day after the
passkey fix, upstream migrated protobuf codegen from prost to buffa
(a6fb1174), rewriting every generated waproto field from Option<Box<T>>
to MessageField<T>. At HEAD this crate fails with 46 errors; at
b5daf757 it fails with 18 mechanical ones, fixed here:

- implement MsgSecretStore for RusqliteStore (a new Backend supertrait)
  with a msg_secrets table, matching upstream's documented conflict
  merge: the later expiry wins with 0 ("never") beating any finite
  deadline, and a known parent timestamp is never clobbered by 0
- add SignalStore::mark_prekeys_uploaded (UPDATE-only, so a pre-key
  consumed since the upload snapshot is not resurrected) and
  AppSyncStore::clear_mutation_macs
- UploadResponse crypto fields are plain [u8; 32], not accessors
- Device::account is Arc-wrapped; LidPnMapping.phone_number is Arc<str>
- get_device_snapshot() is sync; Bot::run() consumes self, so use
  Bot::spawn() for the handle this channel needs
- BotBuilder::with_backend takes `impl Backend`, so an Arc backend now
  goes through with_backend_arc

Also stop dropping the passkey events on the floor: PairPasskeyRequest,
PairPasskeyConfirmation and PairPasskeyError previously fell into the
`_ => {}` arm, which is why the failure was silent. They now emit login
lifecycle events and logs.

This restores visibility and drives the ceremony, but does not by itself
make a headless link succeed: the flow still needs a WebAuthn assertion
from a passkey registered to the account, which upstream abstracts behind
`PasskeyAuthenticator` and does not ship a default for. Wiring an
authenticator source is follow-up work.

Refs zeroclaw-labs#8627
JordanTheJet added a commit to JordanTheJet/zeroclaw that referenced this pull request Aug 20, 2026
WhatsApp began a server-side rollout on 2026-06-30 that requires a
passkey/WebAuthn step during companion linking. The pinned revision
(cbcdd2a, 2026-05-27) does not handle the server's
`passkey_prologue_request` notification, so linking never completes: the
daemon re-emits QR codes indefinitely and the phone reports "Couldn't
link device, try again later" with nothing in the logs (zeroclaw-labs#8627).

Upstream fixed this in oxidezap/whatsapp-rust#928 (merged 2026-07-01).
Move the six git pins onto b5daf757, which contains that fix.

b5daf757 rather than upstream HEAD is deliberate: one day after the
passkey fix, upstream migrated protobuf codegen from prost to buffa
(a6fb1174), rewriting every generated waproto field from Option<Box<T>>
to MessageField<T>. At HEAD this crate fails with 46 errors; at
b5daf757 it fails with 18 mechanical ones, fixed here:

- implement MsgSecretStore for RusqliteStore (a new Backend supertrait)
  with a msg_secrets table, matching upstream's documented conflict
  merge: the later expiry wins with 0 ("never") beating any finite
  deadline, and a known parent timestamp is never clobbered by 0
- add SignalStore::mark_prekeys_uploaded (UPDATE-only, so a pre-key
  consumed since the upload snapshot is not resurrected) and
  AppSyncStore::clear_mutation_macs
- UploadResponse crypto fields are plain [u8; 32], not accessors
- Device::account is Arc-wrapped; LidPnMapping.phone_number is Arc<str>
- get_device_snapshot() is sync; Bot::run() consumes self, so use
  Bot::spawn() for the handle this channel needs
- BotBuilder::with_backend takes `impl Backend`, so an Arc backend now
  goes through with_backend_arc

Also stop dropping the passkey events on the floor: PairPasskeyRequest,
PairPasskeyConfirmation and PairPasskeyError previously fell into the
`_ => {}` arm, which is why the failure was silent. They now emit login
lifecycle events and logs.

This restores visibility and drives the ceremony, but does not by itself
make a headless link succeed: the flow still needs a WebAuthn assertion
from a passkey registered to the account, which upstream abstracts behind
`PasskeyAuthenticator` and does not ship a default for. Wiring an
authenticator source is follow-up work.

Refs zeroclaw-labs#8627
JordanTheJet added a commit to JordanTheJet/zeroclaw that referenced this pull request Aug 20, 2026
WhatsApp began a server-side rollout on 2026-06-30 that requires a
passkey/WebAuthn step during companion linking. The pinned revision
(cbcdd2a, 2026-05-27) does not handle the server's
`passkey_prologue_request` notification, so linking never completes: the
daemon re-emits QR codes indefinitely and the phone reports "Couldn't
link device, try again later" with nothing in the logs (zeroclaw-labs#8627).

Upstream fixed this in oxidezap/whatsapp-rust#928 (merged 2026-07-01).
Move the six git pins onto b5daf757, which contains that fix.

b5daf757 rather than upstream HEAD is deliberate: one day after the
passkey fix, upstream migrated protobuf codegen from prost to buffa
(a6fb1174), rewriting every generated waproto field from Option<Box<T>>
to MessageField<T>. At HEAD this crate fails with 46 errors; at
b5daf757 it fails with 18 mechanical ones, fixed here:

- implement MsgSecretStore for RusqliteStore (a new Backend supertrait)
  with a msg_secrets table, matching upstream's documented conflict
  merge: the later expiry wins with 0 ("never") beating any finite
  deadline, and a known parent timestamp is never clobbered by 0
- add SignalStore::mark_prekeys_uploaded (UPDATE-only, so a pre-key
  consumed since the upload snapshot is not resurrected) and
  AppSyncStore::clear_mutation_macs
- UploadResponse crypto fields are plain [u8; 32], not accessors
- Device::account is Arc-wrapped; LidPnMapping.phone_number is Arc<str>
- get_device_snapshot() is sync; Bot::run() consumes self, so use
  Bot::spawn() for the handle this channel needs
- BotBuilder::with_backend takes `impl Backend`, so an Arc backend now
  goes through with_backend_arc

Also stop dropping the passkey events on the floor: PairPasskeyRequest,
PairPasskeyConfirmation and PairPasskeyError previously fell into the
`_ => {}` arm, which is why the failure was silent. They now emit login
lifecycle events and logs.

This restores visibility and drives the ceremony, but does not by itself
make a headless link succeed: the flow still needs a WebAuthn assertion
from a passkey registered to the account, which upstream abstracts behind
`PasskeyAuthenticator` and does not ship a default for. Wiring an
authenticator source is follow-up work.

Refs zeroclaw-labs#8627
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-design size-increase-ok Accepted binary-size increase: downgrades the per-PR size gate to a warning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature/Bug: Unhandled notification type passkey_prologue_request during login

1 participant