Skip to content

feat: reconcile legacy sync factor cleanup with Secure Enclave signer - #446

Open
noamtfh wants to merge 3 commits into
mainfrom
mcore-1924-signer-backed-sync-factor-cleanup
Open

noamtfh wants to merge 3 commits into
mainfrom
mcore-1924-signer-backed-sync-factor-cleanup

Conversation

@noamtfh

@noamtfh noamtfh commented Sep 9, 2026

Copy link
Copy Markdown

Summary

  • add a Bedrock-owned TurnkeyManager.reconcile_legacy_sync_factor_user operation for MCORE-1924 / MCORE-595
  • use the replacement Secure Enclave-backed P256Signer for the read-and-delete workflow; no private key bytes cross the FFI boundary
  • make cleanup convergent: if an earlier delete was accepted but its response was lost, a later invocation observes the legacy user is already absent and succeeds
  • validate the target is a UUID and an existing sync-factor user before deleting it; reject main, break-glass, and unknown users
  • preserve stable retryable/pending errors so iOS keeps the legacy local credential and retries reconciliation

This intentionally replaces the generic signer-backed stamping approach from #438. The use case is one migration cleanup operation, with the backup/recovery workflow kept in Bedrock as Paolo requested.

Testing

  • git diff --check
  • Added unit coverage for successful verified cleanup, already-absent convergence, non-sync-factor rejection, invalid input, and a transient 503 retry.
  • Not run: cargo fmt / cargo test (Rust toolchain is unavailable in this environment).

Note

Medium Risk
Deletes Turnkey users with safeguards (role check, UUID validation), but mistaken targeting or policy gaps could remove the wrong principal; convergent retries reduce duplicate-delete risk.

Overview
Adds a Bedrock-owned TurnkeyManager.reconcile_legacy_sync_factor_user flow to remove a legacy sync-factor Turnkey user after migration, stamped only by the replacement P256Signer (no private key bytes over FFI).

The API client lists users, no-ops if the legacy user is already gone (safe retries after lost responses), refuses non–sync-factor targets and invalid UUIDs, then submits delete_users with a fixed activity timestamp for idempotent retries. Failures surface as a new opaque TurnkeyCleanupError (Failed / Retryable / Pending) so native clients can keep the legacy credential until cleanup succeeds.

Unit tests cover happy path, uppercase iOS UUIDs, transient 503 retry, already-absent convergence, wrong user role, and invalid ID without network calls.

Reviewed by Cursor Bugbot for commit 1bf050c. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 85a192e. Configure here.

Comment thread bedrock/src/backup/turnkey/api.rs
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.

1 participant