refactor(wallet-cli): Wrap daemon password/SRP in opaque Password/Srp types

packages/wallet-cli/CHANGELOG.md

@@ -15,4 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - `mm daemon stop`, `mm daemon status`, `mm daemon purge` manage daemon lifecycle and state.
 - Persist daemon state to a SQLite database at `<dataDir>/wallet.db`; subsequent `daemon start` runs reuse the persisted KeyringController vault instead of re-importing the SRP ([#8446](https://github.com/MetaMask/core/pull/8446)).
 
+### Changed
+
+- Daemon password and secret recovery phrase are now wrapped in opaque `Password` and `Srp` classes that redact themselves under `util.inspect`, `JSON.stringify`, `toString`, and template-literal interpolation; the underlying string is reachable only via `unwrap()` at trust boundaries ([#8778](https://github.com/MetaMask/core/issues/8778)).
+  - `Srp.from` validates word count (12/15/18/21/24) and that every word is in the BIP-39 English wordlist, surfacing typos at the CLI boundary instead of producing a malformed mnemonic downstream.
+
 [Unreleased]: https://github.com/MetaMask/core/

packages/wallet-cli/package.json

@@ -49,6 +49,7 @@
     "@inquirer/confirm": "^6.0.11",
     "@metamask/remote-feature-flag-controller": "^4.2.0",
     "@metamask/rpc-errors": "^7.0.2",
+    "@metamask/scure-bip39": "^2.1.1",
     "@metamask/utils": "^11.9.0",
     "@metamask/wallet": "^0.0.0",
     "@oclif/core": "^4.10.5",

packages/wallet-cli/src/commands/daemon/start.test.ts

@@ -6,13 +6,15 @@ jest.mock('../../daemon/daemon-spawn');
 
 const mockEnsureDaemon = jest.mocked(ensureDaemon);
 
+const SRP = 'test test test test test test test test test test test ball';
+
 const FLAGS = [
   '--infura-project-id',
   'key',
   '--password',
   'pw',
   '--srp',
-  'phrase',
+  SRP,
 ];
 
 describe('daemon start', () => {

packages/wallet-cli/src/commands/daemon/start.ts

@@ -1,6 +1,7 @@
 import { Command, Flags } from '@oclif/core';
 
 import { ensureDaemon } from '../../daemon/daemon-spawn';
+import { Password, Srp } from '../../daemon/secrets';
 
 export default class DaemonStart extends Command {
   static override description = 'Start the wallet daemon';
@@ -33,7 +34,8 @@ export default class DaemonStart extends Command {
   public async run(): Promise<void> {
     const { flags } = await this.parse(DaemonStart);
     const infuraProjectId = flags['infura-project-id'];
-    const { password, srp } = flags;
+    const password = Password.from(flags.password);
+    const srp = Srp.from(flags.srp);
 
     const { state, socketPath } = await ensureDaemon({
       dataDir: this.config.dataDir,

packages/wallet-cli/src/daemon/daemon-entry.test.ts

@@ -185,13 +185,21 @@ describe('daemon-entry', () => {
       recursive: true,
       mode: 0o700,
     });
-    expect(mockCreateWallet).toHaveBeenCalledWith({
-      databasePath: '/tmp/wallet.db',
-      infuraProjectId: 'key',
-      password: 'pass',
-      srp: 'test test test test test test test test test test test ball',
-      log: expect.any(Function),
-    });
+    expect(mockCreateWallet).toHaveBeenCalledWith(
+      expect.objectContaining({
+        databasePath: '/tmp/wallet.db',
+        infuraProjectId: 'key',
+        log: expect.any(Function),
+      }),
+    );
+    // The Password/Srp instances are constructed in an isolated module scope
+    // (jest.isolateModulesAsync), so their class identity differs from any
+    // import in this test file. Verify structurally via `.unwrap()`.
+    const passedConfig = mockCreateWallet.mock.calls[0][0];
+    expect(passedConfig.password.unwrap()).toBe('pass');
+    expect(passedConfig.srp.unwrap()).toBe(
+      'test test test test test test test test test test test ball',
+    );
     expect(mockWriteFile).toHaveBeenCalledWith(
       '/tmp/daemon.pid',
       expect.stringMatching(new RegExp(`^${process.pid}\\n\\d+\\n$`, 'u')),

packages/wallet-cli/src/daemon/daemon-entry.ts

@@ -8,6 +8,7 @@ import { pingDaemon } from './daemon-client';
 import { getDaemonPaths } from './paths';
 import { startRpcSocketServer } from './rpc-socket-server';
 import type { RpcSocketServerHandle } from './rpc-socket-server';
+import { Password, Srp } from './secrets';
 import type { DaemonStatusInfo, RpcHandlerMap } from './types';
 import { isErrorWithCode, isProcessAlive, readPidFile } from './utils';
 import { createWallet } from './wallet-factory';
@@ -33,15 +34,17 @@ async function main(): Promise<void> {
     throw new Error('INFURA_PROJECT_ID environment variable is required');
   }
 
-  const password = process.env.MM_WALLET_PASSWORD;
-  if (!password) {
+  const passwordRaw = process.env.MM_WALLET_PASSWORD;
+  if (!passwordRaw) {
     throw new Error('MM_WALLET_PASSWORD environment variable is required');
   }
+  const password = Password.from(passwordRaw);
 
-  const srp = process.env.MM_WALLET_SRP;
-  if (!srp) {
+  const srpRaw = process.env.MM_WALLET_SRP;
+  if (!srpRaw) {
     throw new Error('MM_WALLET_SRP environment variable is required');
   }
+  const srp = Srp.from(srpRaw);
 
   // 0o700: owner-only. The daemon exposes the full wallet messenger over
   // the socket inside this directory, so anyone who can traverse the dir

packages/wallet-cli/src/daemon/daemon-spawn.test.ts

@@ -4,6 +4,7 @@ import { existsSync } from 'node:fs';
 import { pingDaemon } from './daemon-client';
 import { ensureDaemon } from './daemon-spawn';
 import { getDaemonPaths } from './paths';
+import { Password, Srp } from './secrets';
 import type { DaemonSpawnConfig } from './types';
 
 jest.mock('node:child_process');
@@ -16,11 +17,13 @@ const mockExistsSync = jest.mocked(existsSync);
 const mockPingDaemon = jest.mocked(pingDaemon);
 const mockGetDaemonPaths = jest.mocked(getDaemonPaths);
 
+const SRP = 'test test test test test test test test test test test ball';
+
 const CONFIG: DaemonSpawnConfig = {
   dataDir: '/tmp/data',
   infuraProjectId: 'test-key',
-  password: 'test-pass',
-  srp: 'test test test test test test test test test test test ball',
+  password: Password.from('test-pass'),
+  srp: Srp.from(SRP),
   packageRoot: '/pkg',
 };
 
@@ -134,8 +137,7 @@ describe('ensureDaemon', () => {
           MM_DAEMON_SOCKET_PATH: '/tmp/test.sock',
           INFURA_PROJECT_ID: 'test-key',
           MM_WALLET_PASSWORD: 'test-pass',
-          MM_WALLET_SRP:
-            'test test test test test test test test test test test ball',
+          MM_WALLET_SRP: SRP,
         }),
       }),
     );

packages/wallet-cli/src/daemon/daemon-spawn.ts

@@ -71,8 +71,8 @@ export async function ensureDaemon(
       MM_DAEMON_DATA_DIR: config.dataDir,
       MM_DAEMON_SOCKET_PATH: socketPath,
       INFURA_PROJECT_ID: config.infuraProjectId,
-      MM_WALLET_PASSWORD: config.password,
-      MM_WALLET_SRP: config.srp,
+      MM_WALLET_PASSWORD: config.password.unwrap(),
+      MM_WALLET_SRP: config.srp.unwrap(),
     },
   });
 

packages/wallet-cli/src/daemon/secrets.test.ts

@@ -0,0 +1,140 @@
+import { inspect } from 'node:util';
+
+import { Password, Srp } from './secrets';
+
+const VALID_SRP_12 =
+  'test test test test test test test test test test test ball';
+const VALID_SRP_24 =
+  'test test test test test test test test test test test test ' +
+  'test test test test test test test test test test test ball';
+
+describe('Password', () => {
+  describe('from', () => {
+    it('wraps a non-empty string', () => {
+      const password = Password.from('hunter2');
+      expect(password).toBeInstanceOf(Password);
+    });
+
+    it('throws on an empty string', () => {
+      expect(() => Password.from('')).toThrow(
+        'Password must be a non-empty string',
+      );
+    });
+  });
+
+  describe('unwrap', () => {
+    it('returns the original value', () => {
+      expect(Password.from('hunter2').unwrap()).toBe('hunter2');
+    });
+  });
+
+  describe('redaction', () => {
+    const SECRET = 'do-not-log-me';
+    let password: Password;
+
+    beforeEach(() => {
+      password = Password.from(SECRET);
+    });
+
+    it('redacts under util.inspect', () => {
+      const inspected = inspect(password);
+      expect(inspected).toBe('[redacted]');
+      expect(inspected).not.toContain(SECRET);
+    });
+
+    it('redacts inside an inspected object', () => {
+      const inspected = inspect({ password });
+      expect(inspected).toContain('[redacted]');
+      expect(inspected).not.toContain(SECRET);
+    });
+
+    it('redacts under JSON.stringify', () => {
+      const serialized = JSON.stringify({ password });
+      expect(serialized).toBe('{"password":"[redacted]"}');
+      expect(serialized).not.toContain(SECRET);
+    });
+
+    it('redacts under String() conversion', () => {
+      expect(String(password)).toBe('[redacted]');
+    });
+
+    it('redacts under template-literal interpolation', () => {
+      // eslint-disable-next-line @typescript-eslint/restrict-template-expressions -- We are intentionally exercising the redacting toString().
+      const message = `password is ${password}`;
+      expect(message).toBe('password is [redacted]');
+      expect(message).not.toContain(SECRET);
+    });
+  });
+});
+
+describe('Srp', () => {
+  describe('from', () => {
+    it.each([12, 15, 18, 21, 24])(
+      'accepts a %i-word mnemonic of valid words',
+      (count) => {
+        const phrase = Array.from({ length: count - 1 }, () => 'test')
+          .concat('ball')
+          .join(' ');
+        expect(Srp.from(phrase)).toBeInstanceOf(Srp);
+      },
+    );
+
+    it('throws when the word count is invalid', () => {
+      expect(() => Srp.from('test test test')).toThrow(
+        /must be 12, 15, 18, 21, or 24 words \(got 3\)/u,
+      );
+    });
+
+    it('throws when a word is not in the BIP-39 wordlist', () => {
+      const phrase =
+        'notabip39word test test test test test test test test test test ball';
+      expect(() => Srp.from(phrase)).toThrow(
+        'Secret recovery phrase contains a word not in the BIP-39 English wordlist',
+      );
+    });
+  });
+
+  describe('unwrap', () => {
+    it('returns the original phrase', () => {
+      expect(Srp.from(VALID_SRP_12).unwrap()).toBe(VALID_SRP_12);
+      expect(Srp.from(VALID_SRP_24).unwrap()).toBe(VALID_SRP_24);
+    });
+  });
+
+  describe('redaction', () => {
+    let srp: Srp;
+
+    beforeEach(() => {
+      srp = Srp.from(VALID_SRP_12);
+    });
+
+    it('redacts under util.inspect', () => {
+      const inspected = inspect(srp);
+      expect(inspected).toBe('[redacted]');
+      expect(inspected).not.toContain('ball');
+    });
+
+    it('redacts inside an inspected object', () => {
+      const inspected = inspect({ srp });
+      expect(inspected).toContain('[redacted]');
+      expect(inspected).not.toContain('ball');
+    });
+
+    it('redacts under JSON.stringify', () => {
+      const serialized = JSON.stringify({ srp });
+      expect(serialized).toBe('{"srp":"[redacted]"}');
+      expect(serialized).not.toContain('ball');
+    });
+
+    it('redacts under String() conversion', () => {
+      expect(String(srp)).toBe('[redacted]');
+    });
+
+    it('redacts under template-literal interpolation', () => {
+      // eslint-disable-next-line @typescript-eslint/restrict-template-expressions -- We are intentionally exercising the redacting toString().
+      const message = `srp is ${srp}`;
+      expect(message).toBe('srp is [redacted]');
+      expect(message).not.toContain('ball');
+    });
+  });
+});