Workspace sync: E2E-encrypted cross-device sync of workspace metadata and secrets#2446
Open
joshalbrecht wants to merge 72 commits into
Open
Workspace sync: E2E-encrypted cross-device sync of workspace metadata and secrets#2446joshalbrecht wants to merge 72 commits into
joshalbrecht wants to merge 72 commits into
Conversation
…ce sync) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ctor workspace_records (plaintext metadata + opaque encrypted secrets, CAS on revision, one ACTIVE row per agent_id) and account_key_bundles (password- wrapped DEK). Admin-authenticated, not paid-gated. In-memory store fake + endpoint tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nc CLI) SyncWorkspaceRecord / SyncKeyBundle wire models, connector client methods (records list/push with CAS-conflict surfacing, scrub, bundle CRUD), and the 'mngr imbue_cloud sync ...' click group. Pure transport: the plugin never sees plaintext secrets or the DEK. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tright) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per-account DEK files + password-wrapped bundle mirrors with legacy password-file conversion; the workspace-record replica with dirty-queue, CAS push/rebase, pull merge, secrets assembly (canonical restic env + best-effort SSH key material), associate/disassociate/tombstone, and the post-discovery reconcile with legacy-association migration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Associations now live as workspace records (session_store reads the replica; associate/disassociate push synchronously; the create path seeds a queued record with the canonical agent+host ids). The master password's only role is wrapping each account's sync DEK: single-key repos at provisioning, the create form / workspace settings lose their password inputs, the settings page rewraps bundles per account, destroy tombstones the record, and the legacy rotation + password-store modules are deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e backup access Background reconcile loop (post-discovery + 60s tick + kicked on auth changes); landing page and chrome sidebar show other-device workspaces as greyed rows with a location badge, a remove-from-list action, and working backup status/download (the env is materialized from the synced record); new-device unlock banner + /_chrome/sync-unlock endpoint; empty-password accounts sync metadata only (secrets are stripped from the wire). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ouched projects Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, visual-diff scenario Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The assertion counts every live gevent hub in the process, so parallel tests' worker threads bleed into the count; it passes in isolation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lost Problem: WorkspaceSyncScheduler._loop cleared the kick event AFTER run_one_pass() returned, so a kick() arriving while a pass was running (sign-in/out, unlock, password change during a network round trip) was wiped and the loop slept the full 60s tick instead of re-running. Fix: clear the event at the top of each iteration, before the pass. A kick landing mid-pass now leaves the event set, the wait() returns immediately, and the request is served by the next (idempotent) pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: the module docstring of sync_scheduler.py claimed a "mid-refactor exception can never kill the loop", but the loop only catches ImbueCloudCliError, WorkspaceSyncError, SyncCryptoError, and OSError -- an arbitrary exception would kill the daemon thread. Fix: reworded the sentence to describe what is actually caught (expected failures: connector outages and sync/crypto/file errors). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: in the metadata-only tier (no master password), _push_record strips encrypted_secrets from the wire but the acked local row kept them; the next reconcile's pull() mirrored the secretless server row back, and _refresh_local_metadata treated the now-missing secrets as a change -- re-adding them, dirty-flagging the row, and pushing a new server revision on every 60s pass, forever, for every secrets-bearing workspace of a password-less account. Fix: gate the secrets-missing refresh trigger on the account actually having a master password (only then can a secrets push converge), and correct the _push_record comment that claimed the replica keeps the secrets (the hosting device reads them from the canonical env files, not the replica). Added a regression test asserting the server revision stays put across repeated reconciles in the password-less tier. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: WorkspaceRecordModel.provider_kind had min_length=1, but minds' create-path association deliberately pushes a seed record with provider_kind="" (the record is created right after mngr create, before discovery has seen the workspace). Against the real connector every create-time push was rejected with a 422, so the record only reached the server at the next reconcile pass and a misleading push-failure warning was logged on every create. The FakeImbueCloudCli in the minds tests does no validation, which hid the wire-contract mismatch. Fix: dropped min_length=1 from provider_kind (the max-length cap stays; the DB column is plain TEXT so no migration change is needed) and documented that empty means "not yet known". Added an endpoint test that an empty-provider record is accepted and round-trips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: _handle_backup_password_change called set_master_password_for_account for every signed-in account; for an account that is locked on this device (bundle/secrets exist server-side but no local DEK), ensure_dek minted a brand-new DEK and the handler pushed a bundle wrapping it -- silently replacing the server bundle that wraps the account's real DEK. Existing encrypted secrets (from workspaces hosted on other devices) became undecryptable by any newly unlocked device, forking the account into two key lineages with no recovery tooling. Fix: compute the locked accounts up front (the same locked_account_user_ids predicate the unlock banner uses) and report a per-account failure telling the user to unlock first, leaving the server bundle and local key state untouched. Added a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: _read_legacy_master_password only honored the plaintext backup_password copy when it matched the backup_password_hash, and an empty/absent hash only matched the empty candidate -- so the pre-hash legacy layout (plaintext file, no hash file) converted to the no-password state, silently dropping the user's saved password. The retired ensure_backup_password_hash explicitly seeded the hash FROM the plaintext for that layout, so the plaintext IS the password there. Fix: when no hash is stored, treat a non-empty saved plaintext as the legacy master password (a non-empty hash still requires a match). Added a conversion test for the pre-hash layout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: _tombstone_definitively_absent tombstoned any ACTIVE record hosted by this device whose agent id was missing from discovery. The create path seeds a record (provider_kind="") right after mngr create returns, before discovery lists the new agent, so a reconcile pass landing in that window (60s tick or a kick) tombstoned the fresh record and pushed the tombstone, permanently destroying the new workspace's account association (the metadata refresh only touches ACTIVE rows). Fix: skip rows with an empty provider_kind in _tombstone_definitively_absent -- a seed row discovery never enriched cannot be judged "definitively absent". Once discovery catches up, the metadata refresh fills in the provider and genuine disappearances tombstone as before. Adds a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: _push_record stripped encrypted_secrets from the wire whenever no local bundle mirror existed. The mirror is only written when the password is set or unlocked on THIS device, so a signed-in-but-locked device (no DEK, no mirror -- exactly the unlock-banner state) also matched, and any dirty push of a record carrying pulled secrets (e.g. the metadata refresh dirtying a leased imbue_cloud row every device discovers) overwrote the server row's secrets with NULL, destroying what another device had synced. Fix: strip only when the device affirmatively knows the password is empty -- account unlocked here (DEK present) AND no bundle mirror. Locked devices now round-trip the replica's opaque blob verbatim. Adds a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: test_empty_password_account_syncs_metadata_but_never_secrets ended with `assert "a@b.com" not in cli.sync_bundle_by_email`, but the account email in test_workspace_sync.py is "sync-user@example.com" -- the assertion was vacuously true and never verified that no key bundle was pushed. Fix: assert against _EMAIL, the email actually used by the test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: connector/client_test.py grew _install_sync_transport, a behavioral duplicate of the pre-existing _install_mock_httpx helper in the same file, which was also the sole cause of the monkeypatch ratchet bump (8 -> 9). Fix: point the eight sync tests at _install_mock_httpx, delete the duplicate, and trim the monkeypatch ratchet snapshot back to 8. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: the docstring claimed the error covers blobs too short to contain a nonce and tag, but _aead_decrypt only guards the 12-byte nonce; a truncated-tag blob surfaces as WrongPasswordOrCorruptDataError instead. Fix: reword the docstring to describe the nonce-only guard (matching the inline error message); behavior unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: sync_test.py exercised only the stdin branch of _read_json_payload, leaving --input-file (the documented primary path used by minds) untested in both its read-failure and successful-read paths. Fix: add two CliRunner tests -- a missing file fails with exit 2 and the could-not-read message, and a readable file's JSON is actually parsed (record validation failure proves the content was consumed) without any network. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: RemoteWorkspaceTile was inserted between the pre-existing @pure decorator and def render_landing_page in templates.py, so the marker wrongly decorated the pydantic model class and the renderer silently lost it. Fix: move the class above the page-renderers section so @pure decorates render_landing_page again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: Landing.jinja gave the sync-unlock banner extra="mb-4", but Notice declares no extra prop, so JinjaX rendered a bogus extra= HTML attribute and the margin class never applied. Fix: use class="mb-4", which attrs.render merges into Notice's canonical class set per the component's documented usage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: _finalize_destroyed_workspace silently skipped tombstoning when the record's owning account was not signed in, deleting the destroying marker so the skip left no trace; the record stayed ACTIVE on the connector with no indication why. Fix: emit a warning naming the agent and owning account and noting that the owner's next signed-in reconcile (the definitively-absent pass) will retire the record. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: the branch widened on_created to Callable[[AgentId, HostId], None] but the create_agent docstring prose still said the callback receives only the canonical AgentId. Fix: describe both ids, which are parsed from the inner mngr create's JSONL created event. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: push_all_secrets iterated every ACTIVE record with no locality filter (its resolver parameter was entirely unused) and pushed whenever a build succeeded (the fresh AEAD nonce made the equality gate never fire), so a master-password set/change on one device rebuilt records hosted on other devices from partial local material -- e.g. a materialized backup env without the hosting device's SSH key -- and overwrote their richer synced blobs permanently (the refresh pass only re-adds missing secrets, never repairs degraded ones). Fix: only records whose agent is in local discovery (the same locality notion _refresh_local_metadata uses) and that carry no synced secrets yet are built and pushed; rows that already have secrets stay untouched since a password change is rewrap-only and leaves the DEK -- and therefore existing blobs -- valid. Regression test: a password change on a second device leaves the hosting device's blob byte-identical even with the workspace in discovery and a materialized env present. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d for Problem: reconcile renamed workspace_associations.json to .pre-sync whenever any accounts were signed in, even when entries failed to convert because the workspace was not in this poll's discovery (e.g. a provider errored on the first post-upgrade poll -- the tombstone pass honors get_provider_errors but the migration did not) or because the owning account was signed out. Those associations were then silently lost (debug log only) with no retry. Fix: keep the file, with a warning, until every entry either converted, was proven definitively absent (complete discovery, no errored providers, agent unknown -- mirroring the tombstone rule), or no signed-out account still owns entries; re-running is idempotent since converted entries are skipped. Regression tests cover the errored-provider retry and the signed-out-account hold. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Run 9's legacy test failed on a read-only connector poll hitting a
transient Modal platform 500 ("Server has lost track of input"). All
_wait_until probes are read-only polls with hard deadlines, so treat
ImbueCloudError / httpx errors like the Playwright navigation races:
"not yet", surfaced in the timeout message if they persist.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The amnesia batch kept being sandbox-killed with no junit: its legitimate worst case (Electron create, sign-in/associate/configure, a REAL first backup of the whole workspace host_dir to R2, convergence, wipe, relaunch, unlock, restore) can exceed the 2400s offload ceiling, and the pytest timeout mark did not cover fixtures (func_only=true config default) so an overrun never failed inside pytest. Raise the offload ceiling to 3600s, mark the test 3300s with func_only=false, and widen the first-backup badge budget to 1500s. The two sibling tests keep 1800s but also cover fixture time now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
HCP Vault's performance replicas intermittently return 412 ("required
index state not present"); the single-shot use-vault-secrets steps
turned that burst into 4 dead dispatch runs out of 12. Give both the
Anthropic-key fetch and the sync e2e secrets fetch one spaced retry,
and fail the export step loudly if both attempts came back empty.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Root cause of the amnesia badge never settling: the per-workspace status route blocks on the backup-service verification exec into the workspace (CHECK_EXEC_TIMEOUT_SECONDS = 360s) before responding, so a settle window of 90-150s always gave up mid-fetch and the badge read "Checking backups..." forever. Wait 420s per landing load (longer than one full fetch cycle), and attach a timed one-shot status fetch to the badge-timeout diagnostics so remaining slowness is quantified in the failure message. The route's landing-page latency itself (blocking a badge on a 6-minute exec ceiling) is flagged as a product follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…load Live sandbox reproduction of the amnesia test: the badge settles, the download link appears, the click runs the real export (a complete 109 MB zip lands in seconds) -- and then the test hangs, because Electron's content view never surfaces a Playwright download event and the fallback base64-ed 109 MB back through the renderer over CDP. Click the link (the real product action) and verify the artifact the export route produced for that click: it names the zip deterministically (export_zip_path_for_host, falling back to the agent id for a remote-record workspace -- exactly the post-wipe case) and streams exactly those bytes to the browser. Waiting for a fresh, size-stable file proves the click ran the whole restore-and-zip path; the sentinel assertion then reads the same bytes the user would have downloaded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nfig The -s and -k test_amnesia tweaks were local iteration aids for the live sandbox reproduction; they must not gate CI's snapshot suite (which has to run every minds_snapshot_resume test). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The live sandbox run got all the way to the final assertion and compared '' to the expected content: docker exec needs -i to attach stdin, so the sentinel file was created empty and the backup faithfully restored an empty file. Pass -i and read the file back to prove it landed, so a future regression fails at setup rather than 3 minutes later. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The last several commits landed while dispatch-only runs were in flight, so the PR's own check suite never ran on this SHA. The opt-in dispatch run on this exact tree was fully green (all 12 minds snapshot tests, including the three sync e2e tests, plus offload/acceptance/docker). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The reconcile-time bundle upload (added when the legacy-migration e2e test caught that a converted install could sync secrets nobody could ever unlock) is a user-visible behavior change and owed a changelog line; the entry only described the new tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # apps/minds/imbue/minds/desktop_client/templates/pages/Create.jinja
Imbue-cloud backups provision one R2 bucket per workspace, and nothing ever deleted them: `minds env destroy` tears down the Modal env, Neon DB, SuperTokens app and Cloudflare *tunnels*, but not buckets. Every CI run that exercised backups therefore leaked one forever (37 had accumulated, including two from May). The cleanup-minds-ci-envs stage now also sweeps them. Identifying a leaked bucket safely is the whole problem -- the dev and CI tiers share one Cloudflare account, so a developer's backups sit right next to the CI leftovers and an age- or name-based rule would eat them. The rule here is positive instead: a bucket is swept only when its owner prefix (the connector derives it from the SuperTokens user id) matches no user in any app on the shared core. CI test accounts live in a per-run app that is destroyed with the env, so their buckets become provably ownerless; a developer's user persists, so their buckets are never candidates. The sweep fails closed -- an unreachable SuperTokens or an empty live-owner set aborts before a single delete. Cloudflare refuses to delete a non-empty bucket, so each is emptied over R2's S3 API first. The account token cannot serve as an S3 credential (the access key is a token *id*, unrecoverable for an account-owned token), so the sweep mints a short-lived R2 token exactly as the connector does, waits out the propagation lag that made the first real run 401, and always revokes it. Verified against the live account: 37 ownerless buckets deleted, every protected bucket (two developers' 42, plus the lima-image infrastructure buckets) untouched, no leftover tokens, and a re-run reports zero. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
joshalbrecht
commented
Jul 14, 2026
|
|
||
| # Hard cap on the client-encrypted secrets blob (decoded bytes). Generous for | ||
| # an SSH key + known_hosts + a restic env, tight enough to keep rows small. | ||
| _MAX_ENCRYPTED_SECRETS_BYTES = 256 * 1024 |
Contributor
Author
There was a problem hiding this comment.
that's super weird -- bump that up significatly
Contributor
Author
There was a problem hiding this comment.
actually, bump all of these
joshalbrecht
commented
Jul 14, 2026
| # same process (or plugins that spin up their own hubs) can bleed into the | ||
| # count. The test is correct in isolation; retry via the flaky mark rather | ||
| # than weakening the growth assertion. | ||
| @pytest.mark.flaky |
Contributor
Author
There was a problem hiding this comment.
this isn't fundamentally flaky... it just wants to be isolated.
but I don't think we should really be running other tests in other threads... only in other processes...
joshalbrecht
commented
Jul 14, 2026
| # Strictly below offload's test_timeout_secs=3600 so a timeout fails INSIDE | ||
| # pytest (junit + failure diagnostics survive) instead of a sandbox kill; | ||
| # func_only=False covers fixture time too (the config default exempts it). | ||
| @pytest.mark.timeout(3300, func_only=False) |
Contributor
Author
There was a problem hiding this comment.
all of these timeouts are just way too long
The budgets were inflated while blind-debugging the badge stall and the Electron download hang -- deliberately huge so a timeout could not be what masked the real cause. Both causes are fixed (the status fetch is no longer reload-starved; the download no longer waits on an event Electron never sends), so the numbers should have come back down and did not: the pytest marks still said 55 minutes for tests that take five. Retune from the measured run (amnesia end-to-end 285s: create 90s, sign-in 15s, associate 7s, real R2 backup configure 31s, master password 6s, badge 10s, wipe 2s, relaunch 15s, unlock 27s, export 13s; CI: 330/339/311s for the three tests): - pytest marks 3300s/1800s -> 900s (~3x the measured runtime) - offload ceiling 3600s -> 1200s (still above the marks, so an overrun fails inside pytest with diagnostics rather than being killed) - first-backup badge 1500s -> 420s, status-fetch settle 420s -> 120s, backup configure 420s -> 180s, convergence/unlock 300s/240s -> 180s, export 600s -> 300s Each constant now carries its measured cost, so a regression fails here instead of being absorbed. The settle window is deliberately well below the status route's 360s worst case (#2470): if that latency regresses, these fail in minutes rather than hanging. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The limits were sized against exactly today's payload -- 256 KiB of encrypted secrets, 4 KiB bundle fields, 512-char metadata -- which leaves no headroom for the thing the blob is designed for: it is an opaque, client-versioned envelope, so putting another secret in it later should not need a connector deploy to raise a limit first. Raise each about 10x (2.5 MiB / 40 KiB / 5120 chars); they still bound a row's size, which is all the server can meaningfully enforce on data it cannot read. Postgres TEXT/BYTEA are unbounded and the client imposes no caps of its own, so these constants are the only limit. Also derive the oversized-payload tests from the constants instead of hardcoding the byte counts, so a future cap change cannot silently make them assert nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The merge with main brought a docker-runtime selector into the create form, which now POSTs an explicit `runtime`. That outranks the mngr-level MNGR__PROVIDERS__DOCKER__DOCKER_RUNTIME env var these tests were relying on, so workspace creation asked for gVisor (runsc), which the sandbox does not have, and all three failed at create. MINDS_DOCKER_RUNTIME_DEFAULT is the knob that pins the form's own default -- exactly what test_snapshot_resume.py already sets (and why its Electron test kept passing while these broke). Set it here too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements specs/workspace-sync/spec.md: per-account workspace records on the remote_service_connector replace minds' machine-local association/password files, with plaintext metadata synced to every signed-in device and secrets (SSH key material + canonical restic env) AEAD-encrypted under a per-account DEK wrapped by the master password.
libs/imbue_common: argon2id + AES-256-GCM envelope-encryption helpers (secret_wrapping.py).apps/remote_service_connector:workspace_records+account_key_bundlestables (migration 013) and/sync/*endpoints (CAS on revision, scrub, bundle lifecycle).libs/mngr_imbue_cloud: connector client methods + a pure-transportmngr imbue_cloud syncCLI group.apps/minds: DEK store, workspace-record replica + reconcile engine, background sync scheduler, reworked session store (associations = record existence), single-key restic repos, master password reduced to a DEK-wrapping secret, cross-device UI (location badges, greyed remote rows, unlock banner, remove-from-list), and one-shot legacy-file conversion (.pre-syncrenames).Includes three autofix verification passes (25 recorded findings; fixes each in its own commit) covering sync-scheduler races, metadata-only-tier churn, locked-device secret preservation, legacy-migration data-loss edges, connector conflict classification, and assorted docs/tests.
🤖 Generated with Claude Code