MetaMask / MetaMask/metamask-mobile

Vault Encryption Payload Rename from `cipher` to `data`

Open
#27,471 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
3k
Forks
1.7k
Avg merge
1d 14h
Merged PRs (30d)
669

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`](/app/core/Engine/controllers/seedless-onboarding-controller/index.ts)
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:

- `KeyringController` vault state
- `SeedlessOnboardingController` vault 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:

1. `Encryptor` now writes `data` instead of `cipher`.
2. `Encryptor` still reads legacy payloads that only contain `cipher`.
3. Persisted vault state is migrated so stored vault JSON gains `data` when it
previously only had `cipher`.

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`]

- `EncryptionResult` now uses `data` as the canonical encrypted payload field.
- `cipher` remains optional only for legacy compatibility during migration.

[`app/core/Encryptor/Encryptor.ts`]

- `encryptWithKey()` now returns `{ data, iv, ... }`.
- `encrypt()` and `encryptWithDetail()` now serialize vault JSON with `data`.
- `decryptWithKey()` accepts either `data` or legacy `cipher`.
- `decrypt()` and `decryptWithDetail()` inherit this compatibility because they
route through `decryptWithKey()`.

### 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
`cipher` into `data`-only payloads.
- `SeedlessOnboardingController` now receives the shared `Encryptor` directly.

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 `vault` JSON for:
- `KeyringController`
- `SeedlessOnboardingController`
- If a payload contains `cipher` but not `data`, 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 `cipher` from 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 `Encryptor` so 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:

- `Encryptor` still supports legacy `cipher` reads.
- Migration is additive, not destructive: it adds `data` and preserves
`cipher`.

## Rollout Notes

- The migration is intentionally additive: migrated payloads keep `cipher` and
gain `data`.
- New encryptor writes produce `data` as canonical output.
- Legacy decrypt support should remain in place until confidence is high that
old persisted `cipher`-only payloads are no longer present.

## Follow-up

- Audit any upstream or external consumers of `EncryptionResult` for hard-coded
`cipher` assumptions.
- After rollout confidence is high, consider removing legacy `cipher` read
support from `Encryptor` if 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

- [x] Engineering (needed in most cases)
- [ ] Design
- [x] 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)
- [x] Security
- [ ] Legal
- [ ] Marketing
- [ ] Management (please specify)
- [ ] Other (please specify)

### References

_No response_

Contributor guide

Open the contributing guide

Research direction

Start with app/core/Encryptor/types.ts and app/core/Encryptor/Encryptor.ts, then inspect the SeedlessOnboardingController and the listed supporting call sites. Verify that new encryption writes use data, legacy cipher payloads still decrypt, and persisted KeyringController and SeedlessOnboardingController vaults gain data without changing invalid or non-string values. Run the shared Encryptor and migration tests, if present.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
security
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.