-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
What is this about?
Context
MetaMask Mobile historically serialized encrypted vault payloads with a cipher
field in the local Encryptor implementation. MetaMask controller and Metamask Extension
interfaces, especially @metamask/browser-passworder-style consumers, expect
the encrypted payload field to be named data.
This mismatch created an adaptation layer in the
SeedlessOnboardingController
that translated between the mobile cipher shape and the controller-facing
data shape. That adapter added complexity and created a persistence hazard:
some vaults could be written with data, while legacy mobile decrypt paths
still assumed cipher.
The risk was highest for persisted vaults that crossed process boundaries:
KeyringControllervault stateSeedlessOnboardingControllervault state
Without a unified format, old and new code paths could read and write different
payload shapes for the same encrypted data.
Recommendation
We are standardizing the encrypted payload field name on data.
The implementation makes three coordinated changes:
Encryptornow writesdatainstead ofcipher.Encryptorstill reads legacy payloads that only containcipher.- Persisted vault state is migrated so stored vault JSON gains
datawhen it
previously only hadcipher.
As part of this, the seedless-specific encryptor adapter is removed and the
shared Encryptor is used directly.
Changes
1. Shared encryptor contract updated
[app/core/Encryptor/types.ts]
EncryptionResultnow usesdataas the canonical encrypted payload field.cipherremains optional only for legacy compatibility during migration.
[app/core/Encryptor/Encryptor.ts]
encryptWithKey()now returns{ data, iv, ... }.encrypt()andencryptWithDetail()now serialize vault JSON withdata.decryptWithKey()accepts eitherdataor legacycipher.decrypt()anddecryptWithDetail()inherit this compatibility because they
route throughdecryptWithKey().
2. Seedless adapter removed
[app/core/Engine/controllers/seedless-onboarding-controller/index.ts]
- Removed the controller-specific adapter that mapped
cipher <-> data. - Removed the vault text normalization helper previously used to inject
cipherintodata-only payloads. SeedlessOnboardingControllernow receives the sharedEncryptordirectly.
This reduces one-off compatibility code and makes seedless follow the same
encryption contract as the rest of the app.
3. Persisted vault migration added
- Adds migration.
- Rewrites persisted
vaultJSON for:KeyringControllerSeedlessOnboardingController
- If a payload contains
cipherbut notdata, the migration adds
data: payload.cipher. - Invalid JSON and non-string vault values are left unchanged.
4. Supporting call sites updated
The following code now reads data as the canonical encrypted payload field:
- [
app/components/Views/AesCryptoTestForm/AesCryptoTestForm.tsx] - [
app/util/validators/index.ts]
Consequences
Positive
- Removes controller-specific encryption glue code.
- Aligns mobile vault payloads with the broader MetaMask/browser-passworder
convention. - Reduces the chance that a vault written by one path cannot be read by another.
- Allows seedless and keyring vault handling to share the same lower-level
compatibility logic.
Negative
- This changes the serialized output shape of newly written vaults.
- Any code that still directly expects
cipherfrom fresh encryption results
may break if not updated.
Risks
1. Incomplete call-site migration
Some mobile-only code may still assume encrypt() or encryptWithKey()
returns cipher. If a caller was missed, it may fail at runtime or silently
mis-handle encrypted output.
Mitigation:
- Updated the known direct call sites found in this change.
- Added unit test updates for the shared
Encryptor. - Kept legacy read support in
Encryptorso decryption remains tolerant.
2. Persisted vaults skipped by migration
If a vault payload is malformed JSON or stored in an unexpected shape, migration
intentionally leaves it unchanged. That avoids destructive rewriting, but
it means some corrupted or unusual payloads may remain unmigrated.
Mitigation:
Encryptorstill supports legacycipherreads.- Migration is additive, not destructive: it adds
dataand preserves
cipher.
Rollout Notes
- The migration is intentionally additive: migrated payloads keep
cipherand
gaindata. - New encryptor writes produce
dataas canonical output. - Legacy decrypt support should remain in place until confidence is high that
old persistedcipher-only payloads are no longer present.
Follow-up
- Audit any upstream or external consumers of
EncryptionResultfor hard-coded
cipherassumptions. - After rollout confidence is high, consider removing legacy
cipherread
support fromEncryptorif the mobile app no longer needs to open old vaults.
Scenario
No response
Design
No response
Technical Details
No response
Threat Modeling Framework
No response
Acceptance Criteria
No response
Stakeholder review needed before the work gets merged
- Engineering (needed in most cases)
- Design
- Product
- QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
- Security
- Legal
- Marketing
- Management (please specify)
- Other (please specify)
References
No response