Skip to content

Sync: Write & verify device_info via dev settings - #9369

Merged
CDRussell merged 3 commits into
developfrom
feature/craig/sync_unified_device_list_device_info_patch
Aug 4, 2026
Merged

Sync: Write & verify device_info via dev settings#9369
CDRussell merged 3 commits into
developfrom
feature/craig/sync_unified_device_list_device_info_patch

Conversation

@CDRussell

@CDRussell CDRussell commented Jul 31, 2026

Copy link
Copy Markdown
Member

Task/Issue URL: https://app.asana.com/1/137249556945/project/608920331025315/task/1217068100697970?focus=true
Tech Design URL (if applicable):
API Proposals URL(s) (if applicable):

Description

Dev-only plumbing for the unified device list's device_info capability. Accessible only from Sync Dev Settings screen for now.

Steps to test this PR

Rename via device_info PATCH works

  • Fresh install internal build
  • Open Sync Dev Settings, and tap Create account to quickly sign into sync
  • Tap Create & Register account_info Key button
  • Tap Rename device (PATCH device_info) button. Call it test and verify it shows PATCH ok with the chosen name
  • Tap Fetch & decrypt devices and verify both legacy and device_info has name=test

#### 2nd device confirmation

  • Install on a 2nd device, and sync with the first device
  • Visit production Sync & Backup screen and verify you see test as the other device's name

Note

High Risk
Touches sync cryptography, protected key unwrap, and server-side device identity; incorrect encryption or PATCH semantics could break cross-device names or leak mishandled key material paths.

Overview
Adds the unified device list write path: when this device’s name changes, the client encrypts a cross-credential device_info JWE (under the cached account_info public key) and still encrypts legacy name/type for older clients, then PATCHes /sync/devices with all three fields every time (the server clears info if omitted).

New sync-impl pieces include AccountInfoPrivateKeyProvider (unwrap account_info private key for decrypt), DeviceInfoEncryptor/DeviceInfoDecryptor (RSA-OAEP JWE round-trip with a decrypt session), DeviceFieldEncryptor (mirror of existing field decrypt), and DeviceInfoUpdater orchestrating the PATCH. DeviceV2 gains an info field; SyncJweCrypto adds rsaSpkiFromJwkComponents for JWK→SPKI encryption. SyncGzipInterceptor skips gzip on PATCH /sync/devices until the backend accepts it.

Sync Dev Settings exposes rename (dialog → PATCH) and fetch & decrypt devices (legacy vs device_info side by side). Unit tests cover the new encrypt/decrypt/update and remote PATCH behavior.

Reviewed by Cursor Bugbot for commit 71bcd4a. Configure here.

CDRussell commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Comment on lines +57 to +61
// The /sync/devices endpoint rejects gzip bodies as invalid_json, so skip compression here until the backend supports it.
val isDevicesEndpoint = chain.request().url.encodedPath.endsWith("/sync/devices")

// check if it's http operation is PATCH
if (chain.request().method == "PATCH" && syncFeature.gzipPatchRequests().isEnabled()) {
if (chain.request().method == "PATCH" && !isDevicesEndpoint && syncFeature.gzipPatchRequests().isEnabled()) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is temporary; should be able to remove this soon as backend will soon support it

fun openSession(): Result<Session>

/** Decrypts `device_info` blobs with a private key held only for this session's lifetime. */
interface Session {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Adding Session as a performance improvement instead of doing a GET network call for every device, instead it can be done once and that will last to get all the devices decrypted.

A better solution is to cache this so a network call isn't needed at all. Will confirm this is possible and do that in a later branch if so.

@CDRussell
CDRussell force-pushed the feature/craig/sync_unified_device_list_device_info_patch branch 2 times, most recently from 2a7c6f2 to 05dde12 Compare July 31, 2026 16:53
Base automatically changed from feature/craig/sync_unified_device_list_crypto_transport_refactors to develop August 3, 2026 09:19
Dev-only plumbing for the cross-credential device_info blob, exercised
from the internal Sync dev-settings screen. No production wiring, no
feature flag.

- DeviceInfoEncryptor/Decryptor: JWE of {name,type} under the account_info
  key (RSA-OAEP-256 + A256GCM). SyncJweCrypto.rsaSpkiFromJwkComponents
  bridges the cached JWK public key to the SPKI form the encrypter wants.
- DeviceFieldEncryptor: legacy per-credential name/type encryption (the
  inverse of DeviceFieldDecryptor) so clients that don't read device_info
  yet still see the new name.
- AccountInfoPrivateKeyProvider: fetch and cache the unwrapped account_info
  private key for decryption.
- DeviceInfoUpdater + PATCH /sync/devices transport (patchThisDevice).
- SyncGzipInterceptor: skip gzip for /sync/devices (backend rejects it).
- Dev settings: rename-device (PATCH device_info) and fetch/decrypt-devices
  buttons, plus tests for the non-UI units.
@CDRussell
CDRussell force-pushed the feature/craig/sync_unified_device_list_device_info_patch branch from 05dde12 to 71bcd4a Compare August 3, 2026 15:34
@CDRussell
CDRussell marked this pull request as ready for review August 3, 2026 15:48
val type: String,
)

@SingleInstanceIn(AppScope::class)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've noticed some stateless classes in this PR are marked with @SingleInstanceIn while others aren't, even though there isn't an obvious cost to creating them. This class is one example, and RealDeviceInfoDecryptor is another that goes the opposite way. Is there a convention or reasoning we're following for when to apply @SingleInstanceIn?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good observation on the inconsistency.

Looking again, it's being applied here when it isn't needed, and in a few other of these related classes too. Unless there's shared state in the class, it's guarding access around a shared resource, or there is a real cost for construction, it's probably not needing to be a singleton.

Will remove it from a few of these classes.

@CDRussell
CDRussell merged commit 5f9a1c0 into develop Aug 4, 2026
14 checks passed

Copy link
Copy Markdown
Member Author

Merge activity

@CDRussell
CDRussell deleted the feature/craig/sync_unified_device_list_device_info_patch branch August 4, 2026 09:04
CDRussell added a commit that referenced this pull request Aug 6, 2026
…pports it (#9405)

Task/Issue URL: https://app.asana.com/1/137249556945/project/72649045549333/task/1217193238272991?focus=true
Tech Design URL (if applicable): 
API Proposals URL(s) (if applicable):

### Description
The backend now accepts gzipped `PATCH` bodies on `/sync/devices`. When we [first integrated](#9369) with this endpoint `gzip` wasn't supported, so `SyncGzipInterceptor` explicitly skipped compression for it. This removes that exclusion so devices `PATCH` requests are compressed like every other sync `PATCH`.

### Steps to test this PR
- QA optional

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes how device PATCH payloads are sent to the sync API; compression failures still fall back to the uncompressed request via existing error handling.
> 
> **Overview**
> **`SyncGzipInterceptor`** no longer skips gzip for PATCH requests to **`/sync/devices`**. Those calls now follow the same rule as other sync PATCH traffic when **`gzipPatchRequests`** is enabled.
> 
> The workaround for backend **`invalid_json`** on gzipped device PATCH bodies is removed, along with the **`isDevicesEndpoint`** check and related comment.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 40717bc. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants