Fix web onboarding device verification#114
Conversation
There was a problem hiding this comment.
Pull request overview
Enables device verification during onboarding in web builds by wiring up device listing, SAS verification initiation/acceptance flow, and ensuring UI caches refresh when SAS completes. This closes a functional gap where web sessions couldn’t complete cross-signing verification during onboarding.
Changes:
- Add web transport + web-server route for
list_devices(GET /api/devices) to support web device listing. - Mount
VerificationOverlayduring onboarding and invalidate device/cross-signing queries after SAS completion. - Improve Rust verification/device logic to support the initiator/acceptor SAS flow and to sign (verify) the bootstrap device after cross-signing setup.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/matrixTransport.ts | Adds web transport mapping for list_devices to /api/devices. |
| src/lib/matrixTransport.test.ts | Extends transport mapping tests to cover list_devices. |
| src/features/verification/VerificationOverlay.tsx | Invalidates device + cross-signing queries on SAS completion. |
| src/features/verification/VerificationOverlay.test.tsx | Wraps overlay tests with React Query provider to support invalidation. |
| src/features/settings/useDevices.ts | Exports query keys so other features can invalidate consistently. |
| src/features/settings/SettingsScreen.tsx | Enables Devices tab on web and mounts Devices panel for all builds. |
| src/features/settings/SettingsScreen.shellMode.test.tsx | Updates shell-mode expectations now that Devices tab is present. |
| src/features/onboarding/VerifyDevicePane.tsx | Adds “verify with another session” flow during onboarding and cache invalidation. |
| src/features/onboarding/VerifyDevicePane.test.tsx | Adds tests for outgoing SAS verification during onboarding. |
| src/features/onboarding/OnboardingScreen.tsx | Runs cross-signing/device status queries on web builds and uses them to gate panes. |
| src/features/onboarding/OnboardingScreen.test.tsx | Updates mocks for new verification-related calls used by onboarding. |
| src/App.tsx | Mounts VerificationOverlay during onboarding so SAS UI is available there. |
| src/App.test.tsx | Extends mocks to cover verification listeners and stubs the overlay. |
| src-tauri/src/matrix/verification.rs | Verifies the local device after cross-signing bootstrap; fixes SAS acceptor path. |
| src-tauri/src/matrix/devices.rs | Refactors device listing into an impl helper and improves “current device verified” lookup. |
| e2e/live-web-e2ee.spec.ts | Adds a live Playwright E2EE test for web verification + decryption. |
| crates/charm-web-server/src/routes.rs | Adds /api/devices endpoint for web device listing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 102a6470ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Web preview: https://pr-114-charm-preview.justin-tech.workers.dev Built with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d670194841
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cab701247
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sentry Snapshot Testing
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b76b01aa6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4116375 to
87edbd3
Compare
87edbd3 to
07d3c7b
Compare
Signed-off-by: Evie Gauthier <evie@gauthier.id>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8e729e3ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Left over from migrating this test to the shared renderWithProviders helper — QueryClient/QueryClientProvider/render/ReactNode were no longer used, tripping oxlint's no-unused-vars and cascading to fail the frontend build (and every platform build that depends on it).
…dentity gap in DevicesPanel - handleBootstrap now invalidates the devices query alongside cross-signing status, so the current device's verified badge doesn't serve stale data for up to 30s after a successful cross-signing setup. - DeviceRow's row actions (Sign out / Manage in account settings) are now gated on usesOAuth being resolved (not the loading default of false), so an OAuth-managed web session can't briefly expose an in-app Sign out that its password-only UIA retry can never satisfy. - DevicesPanel's isBootstrapped check now also considers has_identity, same as VerifyDevicePane already does for onboarding, so a web session whose account has cross-signing identity but lacks local private keys offers verification from a trusted session instead of a misleading setup/reset path.
PR/merge_group builds previously ran sccache with empty AWS credentials (deliberately, to avoid exposing the write key outside a trusted main push). But an anonymous request to the DO Spaces bucket gets a 400 InvalidArgument at sccache server startup, which SCCACHE_IGNORE_SERVER_IO_ERROR doesn't catch, hard-failing every Rust CI job repo-wide. Now that a read-only DO Spaces key exists, use it for every non-main ref instead of empty strings, restoring cache-read benefit without exposing the write-capable key. Also fixes unrelated oxfmt drift in docs/ci-tiers.md caught by the same CI run.
sentry[bot] flagged that fork/Dependabot PRs get all secrets withheld by GitHub, so the new read-only key added in the previous commit would also resolve to '' there — reproducing the exact anonymous- request 400 InvalidArgument this PR set out to fix. Gate RUSTC_WRAPPER itself on whether the resolved AWS key is non-empty, falling back to plain local compilation (no sccache at all) when neither the write nor read-only credentials are available, instead of invoking sccache with empty credentials.
Formatting drift left over from the has_identity/usesOAuth fixes.
Same class of bug already fixed in DevicesPanel/VerifyDevicePane: isVerified only checked local cross-signing keys, not has_identity. On a web session verifying against an account whose cross-signing was already bootstrapped elsewhere, SAS verification makes the current device verified without ever populating local keys, so isVerified stayed false and the verify pane never dismissed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 780324175e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| {verifierDevices.length > 0 | ||
| ? "Choose a session you already trust, then compare emojis there to verify this sign-in." | ||
| : "Open Charm on a trusted session, then come back here to start verification."} |
There was a problem hiding this comment.
Offer reset when no trusted verifier exists
When an OIDC account already has a cross-signing identity but this sign-in has no trusted verifier devices (verifierDevices.length === 0), this branch ignores the resetUrl fetched above and only tells the user to open a trusted session. If the old trusted sessions are gone, the account-management reset URL is the only recovery path; onboarding leaves the user with just Check again/Not now and they must skip to settings to recover. Fresh evidence: the new isBootstrapped branch still renders this no-verifier copy without rendering resetUrl.
Useful? React with 👍 / 👎.
Summary
Notes
Test plan
CHARM_LIVE_WEB_E2EE=1 CHARM_LIVE_HOMESERVER=http://localhost:8008 VITE_CHARM_BUILD_TARGET=web VITE_CHARM_WEB_API_BASE_URL=http://localhost:8787 pnpm exec playwright test e2e/live-web-e2ee.spec.ts --project=chromium --workers=1 --reporter=linepnpm test:run src/lib/matrixTransport.test.ts src/features/settings/SettingsScreen.shellMode.test.tsx src/features/onboarding/OnboardingScreen.test.tsx src/features/onboarding/VerifyDevicePane.test.tsx src/features/verification/VerificationOverlay.test.tsx src/App.test.tsxcargo fmt --manifest-path src-tauri/Cargo.toml --checkCARGO_TARGET_DIR=/tmp/charm-web-onboarding-target cargo check -p charm-web-servergit diff --checkpnpm lint && pnpm fmt:check && pnpm typecheck && pnpm test:coverage && pnpm knip && pnpm build