From e213f1d929ff633d9831a18a7fcb70f5f3b7b454 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 22 Jul 2026 19:48:40 +0300 Subject: [PATCH 1/9] feat(wallet-cli): headless auto-approval + document the gas-fee slot Complete the two consumer-side pieces a wired `TransactionController` needs to process a transaction end-to-end in the daemon: - Headless auto-approval: subscribe to `ApprovalController:stateChanged` and accept every pending request via `ApprovalController:acceptRequest`, so an awaited `addRequest` (raised by a transaction/signature flow) resolves instead of hanging on the headless daemon's no-op `showApprovalRequest`. The subscription is torn down in the `createWallet` `dispose` path. - Gas-fee slot: the `gasFeeController` slot (`clientId: 'cli'`) is already wired; document it in `buildInstanceOptions` and rely on the wallet package's production-default EIP-1559 endpoint rather than re-specifying it. The auto-approval trust model is documented in `subscribeToAutoApproval` and the README: the daemon accepts every approval without a per-request prompt, so the trust boundary is its `0600` same-user Unix socket. Closes #9512 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/wallet-cli/CHANGELOG.md | 1 + packages/wallet-cli/README.md | 2 + .../src/daemon/auto-approval.test.ts | 196 ++++++++++++++++++ .../wallet-cli/src/daemon/auto-approval.ts | 138 ++++++++++++ .../daemon/wallet-factory.integration.test.ts | 31 +++ .../src/daemon/wallet-factory.test.ts | 52 +++++ .../wallet-cli/src/daemon/wallet-factory.ts | 82 ++++++-- 7 files changed, 486 insertions(+), 16 deletions(-) create mode 100644 packages/wallet-cli/src/daemon/auto-approval.test.ts create mode 100644 packages/wallet-cli/src/daemon/auto-approval.ts diff --git a/packages/wallet-cli/CHANGELOG.md b/packages/wallet-cli/CHANGELOG.md index d5dc94b9d8f..fd10cde19f5 100644 --- a/packages/wallet-cli/CHANGELOG.md +++ b/packages/wallet-cli/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Auto-accept pending approval requests in the daemon, so a transaction or signature flow resolves instead of hanging on the headless no-op `showApprovalRequest`; the daemon thus approves **every** request without a prompt (trust boundary is its `0600` same-user socket — see `subscribeToAutoApproval`), and the subscription is torn down on `dispose` ([#9512](https://github.com/MetaMask/core/pull/9512)) - Wire the `transactionController` slot in the daemon wallet's instance options, so the daemon runs the `TransactionController` with an explicit CLI-appropriate configuration (swaps processing disabled, no client hooks) rather than relying on the controller's implicit defaults ([#9509](https://github.com/MetaMask/core/pull/9509)) - Wire the `gasFeeController` slot in the daemon wallet's instance options, passing `clientId: 'cli'` so the CLI identifies itself to the gas estimation API, now that `@metamask/wallet` requires this option ([#9527](https://github.com/MetaMask/core/pull/9527)) - Add the `mm wallet unlock` command, which dispatches `KeyringController:submitPassword` over the daemon socket, allowing the keyring to be unlocked after a daemon start with no password or after a `mm daemon call KeyringController:setLocked` ([#8821](https://github.com/MetaMask/core/pull/8821)) diff --git a/packages/wallet-cli/README.md b/packages/wallet-cli/README.md index 6da5493e94b..fd83efa65f9 100644 --- a/packages/wallet-cli/README.md +++ b/packages/wallet-cli/README.md @@ -57,6 +57,8 @@ mm daemon purge # stop, then delete all daemon state files (--force to State (socket, PID file, log, and the SQLite database) lives in the per-user oclif data directory; override it with `MM_DATA_DIR`. +> **Security model — the daemon auto-approves everything.** Because it is headless, the daemon accepts every approval request (transactions and signatures included) with no per-request prompt; otherwise an awaited request would hang forever with no UI to resolve it. The trust boundary is therefore the daemon's `0600`, same-user Unix socket — anything that can reach the socket can move funds. A scoped/opt-in approval policy is planned for when a user-facing send command lands. + ## Troubleshooting ### Rebuilding `better-sqlite3` diff --git a/packages/wallet-cli/src/daemon/auto-approval.test.ts b/packages/wallet-cli/src/daemon/auto-approval.test.ts new file mode 100644 index 00000000000..a87656fcd82 --- /dev/null +++ b/packages/wallet-cli/src/daemon/auto-approval.test.ts @@ -0,0 +1,196 @@ +import { subscribeToAutoApproval } from './auto-approval.js'; + +type ApprovalStateChangeHandler = (state: { + pendingApprovals: Record; +}) => void; + +type MessengerArg = Parameters[0]; + +/** + * Build a fake root messenger that records `subscribe`/`unsubscribe`/`call` and + * captures the state-change handler `subscribeToAutoApproval` registers, so a + * test can drive it directly. + * + * @returns The fake messenger, its jest mocks, and a `handler` getter. + */ +function makeMessenger(): { + messenger: MessengerArg; + call: jest.Mock; + subscribe: jest.Mock; + unsubscribe: jest.Mock; + handler: () => ApprovalStateChangeHandler; +} { + let captured: ApprovalStateChangeHandler | undefined; + const call = jest.fn().mockResolvedValue({ value: undefined }); + const subscribe = jest.fn( + (_eventType: string, subscribed: ApprovalStateChangeHandler) => { + captured = subscribed; + }, + ); + const unsubscribe = jest.fn(); + + return { + messenger: { call, subscribe, unsubscribe } as unknown as MessengerArg, + call, + subscribe, + unsubscribe, + handler: (): ApprovalStateChangeHandler => { + if (!captured) { + throw new Error('handler was not registered'); + } + return captured; + }, + }; +} + +/** + * Drain pending microtasks so the `.catch(...).finally(...)` chain attached to + * each accept settles before assertions run. + */ +async function flushMicrotasks(): Promise { + for (let i = 0; i < 5; i++) { + await Promise.resolve(); + } +} + +describe('subscribeToAutoApproval', () => { + it('subscribes to ApprovalController:stateChanged', () => { + const { messenger, subscribe } = makeMessenger(); + + subscribeToAutoApproval(messenger); + + expect(subscribe).toHaveBeenCalledTimes(1); + expect(subscribe).toHaveBeenCalledWith( + 'ApprovalController:stateChanged', + expect.any(Function), + ); + }); + + it('accepts every pending request via ApprovalController:acceptRequest', () => { + const { messenger, call, handler } = makeMessenger(); + subscribeToAutoApproval(messenger); + + handler()({ pendingApprovals: { 'id-a': {}, 'id-b': {} } }); + + expect(call).toHaveBeenCalledWith( + 'ApprovalController:acceptRequest', + 'id-a', + ); + expect(call).toHaveBeenCalledWith( + 'ApprovalController:acceptRequest', + 'id-b', + ); + expect(call).toHaveBeenCalledTimes(2); + }); + + it('does nothing when there are no pending requests', () => { + const { messenger, call, handler } = makeMessenger(); + subscribeToAutoApproval(messenger); + + handler()({ pendingApprovals: {} }); + + expect(call).not.toHaveBeenCalled(); + }); + + it('accepts an id only once while its accept is in flight, and again after it settles', async () => { + const { messenger, call, handler } = makeMessenger(); + subscribeToAutoApproval(messenger); + + // Two state changes listing the same id before the first accept settles: + // the second is skipped so the request is not accepted twice. + handler()({ pendingApprovals: { 'id-a': {} } }); + handler()({ pendingApprovals: { 'id-a': {} } }); + expect(call).toHaveBeenCalledTimes(1); + + // Once the accept settles the id leaves the in-flight set, so a later state + // change is free to accept again. + await flushMicrotasks(); + handler()({ pendingApprovals: { 'id-a': {} } }); + expect(call).toHaveBeenCalledTimes(2); + }); + + it('does not re-accept ids surfaced by the re-entrant state change that accepting itself triggers', () => { + const { messenger, call, handler } = makeMessenger(); + subscribeToAutoApproval(messenger); + // Accepting `id-a` deletes it, which synchronously re-fires the state + // change; here the re-entrant change still lists both ids. Each must be + // accepted exactly once. + call.mockImplementation((_action: string, id: string) => { + if (id === 'id-a') { + handler()({ pendingApprovals: { 'id-a': {}, 'id-b': {} } }); + } + return Promise.resolve({ value: undefined }); + }); + + handler()({ pendingApprovals: { 'id-a': {}, 'id-b': {} } }); + + expect(call.mock.calls.filter(([, id]) => id === 'id-a')).toHaveLength(1); + expect(call.mock.calls.filter(([, id]) => id === 'id-b')).toHaveLength(1); + }); + + it('logs and swallows an async rejection from an accept', async () => { + const { messenger, call, handler } = makeMessenger(); + call.mockRejectedValueOnce(new Error('accept boom')); + const log = jest.fn(); + subscribeToAutoApproval(messenger, log); + + expect(() => handler()({ pendingApprovals: { 'id-a': {} } })).not.toThrow(); + await flushMicrotasks(); + + expect(log).toHaveBeenCalledWith( + expect.stringContaining( + 'Failed to auto-accept approval request id-a: Error: accept boom', + ), + ); + }); + + it('logs and swallows a synchronous throw from an accept, and can retry the id later', () => { + const { messenger, call, handler } = makeMessenger(); + call.mockImplementationOnce(() => { + throw new Error('sync boom'); + }); + const log = jest.fn(); + subscribeToAutoApproval(messenger, log); + + expect(() => handler()({ pendingApprovals: { 'id-a': {} } })).not.toThrow(); + expect(log).toHaveBeenCalledWith( + expect.stringContaining( + 'Failed to auto-accept approval request id-a: Error: sync boom', + ), + ); + + // The failed id was cleared from the in-flight set, so a subsequent state + // change accepts it again rather than skipping it forever. + handler()({ pendingApprovals: { 'id-a': {} } }); + expect(call).toHaveBeenCalledTimes(2); + }); + + it('falls back to console.error when no logger is supplied', async () => { + const { messenger, call, handler } = makeMessenger(); + call.mockRejectedValueOnce(new Error('accept boom')); + const consoleErrorSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => undefined); + subscribeToAutoApproval(messenger); + + handler()({ pendingApprovals: { 'id-a': {} } }); + await flushMicrotasks(); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('Failed to auto-accept approval request id-a'), + ); + }); + + it('unsubscribes the same handler it subscribed', () => { + const { messenger, subscribe, unsubscribe } = makeMessenger(); + + const dispose = subscribeToAutoApproval(messenger); + dispose(); + + const [, subscribedHandler] = subscribe.mock.calls[0]; + expect(unsubscribe).toHaveBeenCalledWith( + 'ApprovalController:stateChanged', + subscribedHandler, + ); + }); +}); diff --git a/packages/wallet-cli/src/daemon/auto-approval.ts b/packages/wallet-cli/src/daemon/auto-approval.ts new file mode 100644 index 00000000000..7430f8bd260 --- /dev/null +++ b/packages/wallet-cli/src/daemon/auto-approval.ts @@ -0,0 +1,138 @@ +import type { + DefaultActions, + DefaultEvents, + RootMessenger, +} from '@metamask/wallet'; + +/** + * The slice of `ApprovalController` state this module reads. The full + * state-change payload is the controller's `ApprovalControllerState`; auto + * approval only needs the ids of the pending requests, so it looks at + * `pendingApprovals` alone (the request bodies are irrelevant to accepting + * them). + */ +type PendingApprovalsState = { + pendingApprovals: Record; +}; + +/** + * Handler for `ApprovalController`'s state-change event, narrowed to the slice + * auto approval reads. + */ +type ApprovalStateChangeHandler = (state: PendingApprovalsState) => void; + +/** + * Subscribe the daemon to auto-accept every pending approval request. + * + * ## Trust model — the daemon accepts every approval without confirmation + * + * A send or signature flow raises an approval via + * `ApprovalController:addRequest` and **awaits** its resolution. A UI client + * resolves that request from a human decision; the headless daemon has no UI, + * so with nothing accepting the request the awaiting call hangs forever. (The + * injected `showApprovalRequest` hook only signals "a request needs + * attention" — it does not resolve anything.) + * + * This subscribes to `ApprovalController:stateChanged` and immediately accepts + * every pending request via `ApprovalController:acceptRequest`. The daemon + * therefore approves **everything** — transactions and signatures included — + * with no per-request prompt. That is the intended model for a headless + * daemon: it is driven only by its owner's local CLI over a `0600`, same-user + * Unix socket (see `startRpcSocketServer`), so the trust boundary is the + * socket, not a per-request confirmation. + * + * This is **not** "safe by default": a scoped/opt-in policy (a config flag, or + * accepting only specific approval types) is deferred until the user-facing + * send command exists. Until then, anything that can reach the socket can move + * funds. + * + * `acceptRequest` deletes the request as it resolves, which itself emits + * another state change; a single flow can also emit several changes in quick + * succession. The `inFlight` guard makes accepting each id idempotent across + * those re-entrant/rapid changes — without it the same id would be accepted + * twice and the second accept would reject because the request is already gone. + * Both synchronous throws and async rejections from an accept are logged and + * swallowed, so one bad request can neither crash the daemon nor wedge the + * subscription. + * + * @param messenger - The wallet root messenger. + * @param log - Optional logger for accept failures. Defaults to `console.error` + * (which a detached daemon's `stdio: 'ignore'` discards, so a daemon host + * should supply its own logger). + * @returns A function that unsubscribes the auto-approval handler. + */ +export function subscribeToAutoApproval( + messenger: Readonly>, + log?: (message: string) => void, +): () => void { + const logFn = + log ?? + ((message: string): void => { + console.error(message); + }); + + const inFlight = new Set(); + + const logFailure = (id: string, error: unknown): void => { + logFn(`Failed to auto-accept approval request ${id}: ${String(error)}`); + }; + + const acceptRequest = (id: string): void => { + if (inFlight.has(id)) { + return; + } + inFlight.add(id); + + try { + messenger + .call('ApprovalController:acceptRequest', id) + .catch((error: unknown) => logFailure(id, error)) + .finally(() => inFlight.delete(id)); + } catch (error) { + // A synchronous throw (e.g. the request was resolved between the state + // change and this call) never attaches the `finally` above, so clean up + // here and keep the throw inside the handler. + inFlight.delete(id); + logFailure(id, error); + } + }; + + const handler: ApprovalStateChangeHandler = (state) => { + for (const id of Object.keys(state.pendingApprovals)) { + acceptRequest(id); + } + }; + + return subscribeToApprovalStateChanged(messenger, handler); +} + +/** + * Subscribe a handler to `ApprovalController:stateChanged`. + * + * The wallet's typed event union only declares the deprecated `:stateChange` + * member, so — as in the persistence layer — this localizes the single cast + * needed to subscribe to the non-deprecated `:stateChanged` event behind a + * typed {@link ApprovalStateChangeHandler}, keeping the `state` payload + * compile-checked at the call site instead of erased by a statement-level + * `@ts-expect-error`. + * + * @param messenger - The wallet root messenger. + * @param handler - The state-change handler to register. + * @returns A function that unsubscribes the handler. + */ +function subscribeToApprovalStateChanged( + messenger: Readonly>, + handler: ApprovalStateChangeHandler, +): () => void { + const subscriber = messenger as unknown as { + subscribe: (eventType: string, handler: ApprovalStateChangeHandler) => void; + unsubscribe: ( + eventType: string, + handler: ApprovalStateChangeHandler, + ) => void; + }; + subscriber.subscribe('ApprovalController:stateChanged', handler); + return () => { + subscriber.unsubscribe('ApprovalController:stateChanged', handler); + }; +} diff --git a/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts b/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts index 6223dc66268..8ee2c72cdd4 100644 --- a/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts +++ b/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts @@ -64,6 +64,37 @@ describe('createWallet (real Wallet, in-memory)', () => { await dispose(); } }); + + it('auto-accepts a pending approval request instead of hanging', async () => { + const { wallet, dispose } = await createWallet({ + databasePath: ':memory:', + password: Password.from(TEST_PASSWORD), + srp: Srp.from(TEST_PHRASE), + infuraProjectId: INFURA_PROJECT_ID, + log: () => undefined, + }); + + try { + // `addRequest` awaits acceptance; on a headless daemon with no UI it would + // hang forever, so the auto-approval subscription must resolve it. + // `shouldShowRequest: false` keeps the no-op `showApprovalRequest` out of + // the picture — resolution comes purely from the state-change accept path. + // If auto-approval regressed, this would time out rather than resolve. + const accepted = await wallet.messenger.call( + 'ApprovalController:addRequest', + { origin: 'https://example.com', type: 'test:approval' }, + false, + ); + expect(accepted).toBeUndefined(); + + // Accepting deletes the request, so nothing is left pending. + expect( + wallet.messenger.call('ApprovalController:getState').pendingApprovals, + ).toStrictEqual({}); + } finally { + await dispose(); + } + }); }); describe('createWallet (integration)', () => { diff --git a/packages/wallet-cli/src/daemon/wallet-factory.test.ts b/packages/wallet-cli/src/daemon/wallet-factory.test.ts index d50a2b4fd78..f0ec6ab0065 100644 --- a/packages/wallet-cli/src/daemon/wallet-factory.test.ts +++ b/packages/wallet-cli/src/daemon/wallet-factory.test.ts @@ -12,6 +12,7 @@ import { join } from 'node:path'; import { KeyValueStore } from '../persistence/KeyValueStore.js'; import * as persistenceModule from '../persistence/persistence.js'; +import * as autoApprovalModule from './auto-approval.js'; import { Password, Srp } from './secrets.js'; import { createWallet } from './wallet-factory.js'; @@ -126,6 +127,21 @@ describe('createWallet', () => { await dispose(); }); + it('installs the headless auto-approval subscription on the real wallet messenger', async () => { + const autoApprovalSpy = jest + .spyOn(autoApprovalModule, 'subscribeToAutoApproval') + .mockReturnValue(() => undefined); + + const { wallet, dispose } = await createWallet(CONFIG); + + expect(autoApprovalSpy).toHaveBeenCalledWith( + wallet.messenger, + expect.any(Function), + ); + + await dispose(); + }); + it('reads metadata from a throwaway probe wallet and destroys it', async () => { const loadStateSpy = jest .spyOn(persistenceModule, 'loadState') @@ -501,6 +517,42 @@ describe('createWallet', () => { expect(closeSpy).toHaveBeenCalled(); }); + it('unsubscribes auto-approval before destroying the wallet', async () => { + const autoApprovalUnsubscribe = jest.fn(); + jest + .spyOn(autoApprovalModule, 'subscribeToAutoApproval') + .mockReturnValue(autoApprovalUnsubscribe); + + const { wallet, dispose } = await createWallet(CONFIG); + await dispose(); + + const destroyMock = wallet.destroy as jest.Mock; + expect(autoApprovalUnsubscribe).toHaveBeenCalledTimes(1); + expect(autoApprovalUnsubscribe.mock.invocationCallOrder[0]).toBeLessThan( + destroyMock.mock.invocationCallOrder[0], + ); + }); + + it('logs and continues when the auto-approval unsubscribe throws', async () => { + jest + .spyOn(autoApprovalModule, 'subscribeToAutoApproval') + .mockReturnValue(() => { + throw new Error('auto-approval unsub boom'); + }); + const log = jest.fn(); + const closeSpy = jest.spyOn(KeyValueStore.prototype, 'close'); + + const { dispose } = await createWallet({ ...CONFIG, log }); + await dispose(); + + expect(log).toHaveBeenCalledWith( + expect.stringContaining( + 'Auto-approval unsubscribe failed during teardown', + ), + ); + expect(closeSpy).toHaveBeenCalled(); + }); + it('logs and still closes the store when wallet.destroy rejects', async () => { const log = jest.fn(); const closeSpy = jest.spyOn(KeyValueStore.prototype, 'close'); diff --git a/packages/wallet-cli/src/daemon/wallet-factory.ts b/packages/wallet-cli/src/daemon/wallet-factory.ts index 832c16a155a..36e09e547be 100644 --- a/packages/wallet-cli/src/daemon/wallet-factory.ts +++ b/packages/wallet-cli/src/daemon/wallet-factory.ts @@ -16,6 +16,7 @@ import { rm } from 'node:fs/promises'; import { KeyValueStore } from '../persistence/KeyValueStore.js'; import { loadState, subscribeToChanges } from '../persistence/persistence.js'; +import { subscribeToAutoApproval } from './auto-approval.js'; import type { Password, Srp } from './secrets.js'; import type { Logger } from './types.js'; @@ -61,7 +62,13 @@ export type CreateWalletResult = { * - `remoteFeatureFlagController` — a `ClientConfigApiService` fetching real * flags over the network. * - `approvalController` — a no-op `showApprovalRequest` (the daemon is - * headless). + * headless); pending requests are accepted separately by the auto-approval + * subscription `createWallet` installs, not by this hook. + * - `gasFeeController` — only `clientId: 'cli'` (sent as `X-Client-Id` to the + * gas API); the API endpoints, poll interval, and compatibility callbacks are + * left at the wallet package's platform-agnostic defaults (the default + * `EIP1559APIEndpoint` is already the production URL, so the daemon does not + * re-specify it). * - `transactionController` — swaps processing disabled and no client hooks; * see the slot's inline comment for why the daemon relies on the * controller's defaults for everything else. @@ -77,7 +84,9 @@ function buildInstanceOptions( ): WalletOptions['instanceOptions'] { return { approvalController: { - // TODO: surface approval requests over the daemon transport. + // The daemon is headless, so there is no UI to open: requests are + // resolved by the auto-approval subscription (see `subscribeToAutoApproval`) + // rather than by this hook, which stays a no-op. showApprovalRequest: (): undefined => undefined, }, connectivityController: { @@ -174,7 +183,8 @@ export async function createWallet({ const logFn = log ?? ((message: string): void => console.error(message)); const store = new KeyValueStore(databasePath); let wallet: Wallet | undefined; - let unsubscribe: (() => void) | undefined; + let persistenceUnsubscribe: (() => void) | undefined; + let autoApprovalUnsubscribe: (() => void) | undefined; let wasFirstRun = false; try { @@ -195,13 +205,20 @@ export async function createWallet({ state, instanceOptions: buildInstanceOptions(infuraProjectId), }); - unsubscribe = subscribeToChanges( + persistenceUnsubscribe = subscribeToChanges( wallet.messenger, wallet.controllerMetadata, store, logFn, ); + // Accept pending approval requests headlessly, so an awaited + // `ApprovalController:addRequest` (raised by a transaction or signature + // flow) resolves instead of hanging. See `subscribeToAutoApproval` for the + // trust model. Installed before `init` so any request raised during + // initialization is covered too. + autoApprovalUnsubscribe = subscribeToAutoApproval(wallet.messenger, logFn); + // Complete post-construction controller setup before serving requests — // e.g. `NetworkController.init` applies the selected network so a provider // is available. `init` settles every step independently; a rejected step @@ -249,10 +266,22 @@ export async function createWallet({ return { wallet, dispose: async () => - (disposePromise ??= teardown(unsubscribe, wallet, store, logFn)), + (disposePromise ??= teardown( + persistenceUnsubscribe, + autoApprovalUnsubscribe, + wallet, + store, + logFn, + )), }; } catch (error) { - await teardown(unsubscribe, wallet, store, logFn); + await teardown( + persistenceUnsubscribe, + autoApprovalUnsubscribe, + wallet, + store, + logFn, + ); if (wasFirstRun && databasePath !== IN_MEMORY_DATABASE_PATH) { // Best-effort cleanup of the on-disk SQLite files (main, WAL, SHM) so a @@ -311,30 +340,51 @@ async function loadPersistedState( } } +/** + * Best-effort unsubscribe: run the function if present, logging (and + * swallowing) any error so a later teardown step still runs. + * + * @param unsubscribe - The unsubscribe function, if one was registered. + * @param label - Human-readable subscription name for the failure log. + * @param logFn - Logger for a failure. + */ +function runUnsubscribe( + unsubscribe: (() => void) | undefined, + label: string, + logFn: Logger, +): void { + if (!unsubscribe) { + return; + } + try { + unsubscribe(); + } catch (error) { + logFn(`${label} unsubscribe failed during teardown: ${String(error)}`); + } +} + /** * Persistence-safe teardown of a wallet and its store; see {@link * CreateWalletResult.dispose} for the ordering rationale. Each step is * best-effort, so a failure is logged and the remaining steps still run. * - * @param unsubscribe - The persistence-subscription unsubscribe function, if - * one was registered. + * @param persistenceUnsubscribe - The persistence-subscription unsubscribe + * function, if one was registered. + * @param autoApprovalUnsubscribe - The auto-approval-subscription unsubscribe + * function, if one was registered. * @param wallet - The wallet to destroy, if one was constructed. * @param store - The store to close. * @param logFn - Logger for step failures. */ async function teardown( - unsubscribe: (() => void) | undefined, + persistenceUnsubscribe: (() => void) | undefined, + autoApprovalUnsubscribe: (() => void) | undefined, wallet: Wallet | undefined, store: KeyValueStore, logFn: Logger, ): Promise { - if (unsubscribe) { - try { - unsubscribe(); - } catch (error) { - logFn(`Persistence unsubscribe failed during teardown: ${String(error)}`); - } - } + runUnsubscribe(persistenceUnsubscribe, 'Persistence', logFn); + runUnsubscribe(autoApprovalUnsubscribe, 'Auto-approval', logFn); if (wallet) { try { await wallet.destroy(); From 12c4c703aa80192caad69be696637eeb0d178a0c Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Wed, 22 Jul 2026 19:49:31 +0300 Subject: [PATCH 2/9] docs(wallet-cli): point changelog entry at PR #9612 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/wallet-cli/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/wallet-cli/CHANGELOG.md b/packages/wallet-cli/CHANGELOG.md index fd10cde19f5..68665afe75c 100644 --- a/packages/wallet-cli/CHANGELOG.md +++ b/packages/wallet-cli/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Auto-accept pending approval requests in the daemon, so a transaction or signature flow resolves instead of hanging on the headless no-op `showApprovalRequest`; the daemon thus approves **every** request without a prompt (trust boundary is its `0600` same-user socket — see `subscribeToAutoApproval`), and the subscription is torn down on `dispose` ([#9512](https://github.com/MetaMask/core/pull/9512)) +- Auto-accept pending approval requests in the daemon, so a transaction or signature flow resolves instead of hanging on the headless no-op `showApprovalRequest`; the daemon thus approves **every** request without a prompt (trust boundary is its `0600` same-user socket — see `subscribeToAutoApproval`), and the subscription is torn down on `dispose` ([#9612](https://github.com/MetaMask/core/pull/9612)) - Wire the `transactionController` slot in the daemon wallet's instance options, so the daemon runs the `TransactionController` with an explicit CLI-appropriate configuration (swaps processing disabled, no client hooks) rather than relying on the controller's implicit defaults ([#9509](https://github.com/MetaMask/core/pull/9509)) - Wire the `gasFeeController` slot in the daemon wallet's instance options, passing `clientId: 'cli'` so the CLI identifies itself to the gas estimation API, now that `@metamask/wallet` requires this option ([#9527](https://github.com/MetaMask/core/pull/9527)) - Add the `mm wallet unlock` command, which dispatches `KeyringController:submitPassword` over the daemon socket, allowing the keyring to be unlocked after a daemon start with no password or after a `mm daemon call KeyringController:setLocked` ([#8821](https://github.com/MetaMask/core/pull/8821)) From a699bf4008a0397cbc72c316e4944854233c9622 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Thu, 23 Jul 2026 11:49:49 +0300 Subject: [PATCH 3/9] fix(wallet-cli): address review findings from PR #9612 - Import `Logger` type alias in `auto-approval.ts` instead of redeclaring the inline shape, making compatibility with `wallet-factory.ts` explicit - Fix misleading catch-block comment that implied a rethrow; the error is intentionally suppressed - Clarify `subscribeToApprovalStateChanged` docstring: root cause is the missing `ControllerStateChangedEvent` in `ApprovalControllerEvents`, not dynamic string widening as in the persistence layer; add TODO pointing at the upstream fix - Add dropped-Patch[]-parameter note to `ApprovalStateChangeHandler` - Extend `runUnsubscribe` failure log to note the subscription may remain live - Replace fragile 5x Promise.resolve() flush with setImmediate pattern - Add missing startup-failure test: subscribeToAutoApproval throws means persistence listener is still cleaned up, wallet destroyed, store closed - Extend auto-approval unsubscribe test to also assert wallet.destroy ran - Add 5 s per-test timeout to the auto-approval integration test - Split dense CHANGELOG entry into lead sentence + nested security note Co-Authored-By: Claude Sonnet 4.6 --- packages/wallet-cli/CHANGELOG.md | 3 +- .../src/daemon/auto-approval.test.ts | 8 +++--- .../wallet-cli/src/daemon/auto-approval.ts | 28 +++++++++++++------ .../daemon/wallet-factory.integration.test.ts | 5 +++- .../src/daemon/wallet-factory.test.ts | 26 ++++++++++++++++- .../wallet-cli/src/daemon/wallet-factory.ts | 5 +++- 6 files changed, 59 insertions(+), 16 deletions(-) diff --git a/packages/wallet-cli/CHANGELOG.md b/packages/wallet-cli/CHANGELOG.md index 68665afe75c..36dd108a15c 100644 --- a/packages/wallet-cli/CHANGELOG.md +++ b/packages/wallet-cli/CHANGELOG.md @@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Auto-accept pending approval requests in the daemon, so a transaction or signature flow resolves instead of hanging on the headless no-op `showApprovalRequest`; the daemon thus approves **every** request without a prompt (trust boundary is its `0600` same-user socket — see `subscribeToAutoApproval`), and the subscription is torn down on `dispose` ([#9612](https://github.com/MetaMask/core/pull/9612)) +- Auto-accept pending approval requests in the daemon so a transaction or signature flow resolves instead of hanging on the headless no-op `showApprovalRequest`; the subscription is torn down on `dispose` ([#9612](https://github.com/MetaMask/core/pull/9612)) + - **Security note:** the daemon approves every request without a per-request prompt; the trust boundary is its `0600`, same-user Unix socket. - Wire the `transactionController` slot in the daemon wallet's instance options, so the daemon runs the `TransactionController` with an explicit CLI-appropriate configuration (swaps processing disabled, no client hooks) rather than relying on the controller's implicit defaults ([#9509](https://github.com/MetaMask/core/pull/9509)) - Wire the `gasFeeController` slot in the daemon wallet's instance options, passing `clientId: 'cli'` so the CLI identifies itself to the gas estimation API, now that `@metamask/wallet` requires this option ([#9527](https://github.com/MetaMask/core/pull/9527)) - Add the `mm wallet unlock` command, which dispatches `KeyringController:submitPassword` over the daemon socket, allowing the keyring to be unlocked after a daemon start with no password or after a `mm daemon call KeyringController:setLocked` ([#8821](https://github.com/MetaMask/core/pull/8821)) diff --git a/packages/wallet-cli/src/daemon/auto-approval.test.ts b/packages/wallet-cli/src/daemon/auto-approval.test.ts index a87656fcd82..d75f755bebd 100644 --- a/packages/wallet-cli/src/daemon/auto-approval.test.ts +++ b/packages/wallet-cli/src/daemon/auto-approval.test.ts @@ -45,12 +45,12 @@ function makeMessenger(): { /** * Drain pending microtasks so the `.catch(...).finally(...)` chain attached to - * each accept settles before assertions run. + * each accept settles before assertions run. Uses `setImmediate` to cross the + * I/O callback boundary, which flushes the entire microtask queue regardless + * of chain depth. */ async function flushMicrotasks(): Promise { - for (let i = 0; i < 5; i++) { - await Promise.resolve(); - } + await new Promise((resolve) => setImmediate(resolve)); } describe('subscribeToAutoApproval', () => { diff --git a/packages/wallet-cli/src/daemon/auto-approval.ts b/packages/wallet-cli/src/daemon/auto-approval.ts index 7430f8bd260..909f20698f8 100644 --- a/packages/wallet-cli/src/daemon/auto-approval.ts +++ b/packages/wallet-cli/src/daemon/auto-approval.ts @@ -4,6 +4,8 @@ import type { RootMessenger, } from '@metamask/wallet'; +import type { Logger } from './types.js'; + /** * The slice of `ApprovalController` state this module reads. The full * state-change payload is the controller's `ApprovalControllerState`; auto @@ -17,7 +19,9 @@ type PendingApprovalsState = { /** * Handler for `ApprovalController`'s state-change event, narrowed to the slice - * auto approval reads. + * auto approval reads. The `Patch[]` second argument from the raw event is + * intentionally absent — auto approval re-accepts on every state change and + * does not filter by patch. */ type ApprovalStateChangeHandler = (state: PendingApprovalsState) => void; @@ -63,7 +67,7 @@ type ApprovalStateChangeHandler = (state: PendingApprovalsState) => void; */ export function subscribeToAutoApproval( messenger: Readonly>, - log?: (message: string) => void, + log?: Logger, ): () => void { const logFn = log ?? @@ -89,9 +93,10 @@ export function subscribeToAutoApproval( .catch((error: unknown) => logFailure(id, error)) .finally(() => inFlight.delete(id)); } catch (error) { - // A synchronous throw (e.g. the request was resolved between the state - // change and this call) never attaches the `finally` above, so clean up - // here and keep the throw inside the handler. + // Synchronous throw means the Promise chain above was never built + // (no .catch/.finally attached), so clean up the in-flight guard here. + // The error is intentionally suppressed — a race-condition reject must + // not crash the daemon or permanently wedge the subscription. inFlight.delete(id); logFailure(id, error); } @@ -109,13 +114,19 @@ export function subscribeToAutoApproval( /** * Subscribe a handler to `ApprovalController:stateChanged`. * - * The wallet's typed event union only declares the deprecated `:stateChange` - * member, so — as in the persistence layer — this localizes the single cast - * needed to subscribe to the non-deprecated `:stateChanged` event behind a + * `ApprovalControllerEvents` only declares `ApprovalController:stateChange` + * (via `ControllerStateChangeEvent`), not the non-deprecated + * `ApprovalController:stateChanged` variant that `BaseController` publishes at + * runtime. This helper localizes the unavoidable cast — using the same + * technique as `subscribeToStateChanged` in the persistence layer — behind a * typed {@link ApprovalStateChangeHandler}, keeping the `state` payload * compile-checked at the call site instead of erased by a statement-level * `@ts-expect-error`. * + * TODO: Remove this cast once `ApprovalControllerEvents` includes + * `ControllerStateChangedEvent` (`:stateChanged`), matching the union already + * exported by `BaseController`. + * * @param messenger - The wallet root messenger. * @param handler - The state-change handler to register. * @returns A function that unsubscribes the handler. @@ -124,6 +135,7 @@ function subscribeToApprovalStateChanged( messenger: Readonly>, handler: ApprovalStateChangeHandler, ): () => void { + // TODO: Remove once ApprovalControllerEvents includes ControllerStateChangedEvent. const subscriber = messenger as unknown as { subscribe: (eventType: string, handler: ApprovalStateChangeHandler) => void; unsubscribe: ( diff --git a/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts b/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts index 8ee2c72cdd4..48c4c2ebd60 100644 --- a/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts +++ b/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts @@ -65,6 +65,9 @@ describe('createWallet (real Wallet, in-memory)', () => { } }); + // Short timeout: auto-approval must resolve in milliseconds once the wallet + // is up. A regression hangs, so fail fast rather than waiting the file-wide + // 30 s (set for KDF/SQLite-heavy tests). it('auto-accepts a pending approval request instead of hanging', async () => { const { wallet, dispose } = await createWallet({ databasePath: ':memory:', @@ -94,7 +97,7 @@ describe('createWallet (real Wallet, in-memory)', () => { } finally { await dispose(); } - }); + }, 5_000); }); describe('createWallet (integration)', () => { diff --git a/packages/wallet-cli/src/daemon/wallet-factory.test.ts b/packages/wallet-cli/src/daemon/wallet-factory.test.ts index f0ec6ab0065..238b2fbab59 100644 --- a/packages/wallet-cli/src/daemon/wallet-factory.test.ts +++ b/packages/wallet-cli/src/daemon/wallet-factory.test.ts @@ -542,7 +542,7 @@ describe('createWallet', () => { const log = jest.fn(); const closeSpy = jest.spyOn(KeyValueStore.prototype, 'close'); - const { dispose } = await createWallet({ ...CONFIG, log }); + const { wallet, dispose } = await createWallet({ ...CONFIG, log }); await dispose(); expect(log).toHaveBeenCalledWith( @@ -550,6 +550,7 @@ describe('createWallet', () => { 'Auto-approval unsubscribe failed during teardown', ), ); + expect((wallet.destroy as jest.Mock)).toHaveBeenCalledTimes(1); expect(closeSpy).toHaveBeenCalled(); }); @@ -662,6 +663,29 @@ describe('createWallet', () => { expect(closeSpy).toHaveBeenCalled(); }); + it('unsubscribes persistence, destroys the wallet, and closes the store when subscribeToAutoApproval throws', async () => { + const persistenceUnsubscribe = jest.fn(); + jest + .spyOn(persistenceModule, 'subscribeToChanges') + .mockReturnValue(persistenceUnsubscribe); + jest + .spyOn(autoApprovalModule, 'subscribeToAutoApproval') + .mockImplementation(() => { + throw new Error('auto-approval subscribe failed'); + }); + const closeSpy = jest.spyOn(KeyValueStore.prototype, 'close'); + + await expect(createWallet(CONFIG)).rejects.toThrow( + 'auto-approval subscribe failed', + ); + const realWallet = MockWallet.mock.results[1]?.value as Wallet; + // The persistence subscription was installed before the throw, so it + // must be torn down even though autoApprovalUnsubscribe was never set. + expect(persistenceUnsubscribe).toHaveBeenCalledTimes(1); + expect(realWallet.destroy).toHaveBeenCalledTimes(1); + expect(closeSpy).toHaveBeenCalled(); + }); + it('unsubscribes, destroys the wallet, and closes the store when SRP import rejects on first run', async () => { const failure = new Error('bad SRP'); mockImportSrp.mockRejectedValue(failure); diff --git a/packages/wallet-cli/src/daemon/wallet-factory.ts b/packages/wallet-cli/src/daemon/wallet-factory.ts index 36e09e547be..c1d3f22d2f4 100644 --- a/packages/wallet-cli/src/daemon/wallet-factory.ts +++ b/packages/wallet-cli/src/daemon/wallet-factory.ts @@ -359,7 +359,10 @@ function runUnsubscribe( try { unsubscribe(); } catch (error) { - logFn(`${label} unsubscribe failed during teardown: ${String(error)}`); + logFn( + `${label} unsubscribe failed during teardown — subscription may remain ` + + `live until the process exits: ${String(error)}`, + ); } } From 384c62f5e4d1a67181948ad781ccaf6953137b2b Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Thu, 23 Jul 2026 11:52:04 +0300 Subject: [PATCH 4/9] fix(wallet-cli): restore TODO for surfacing approvals over the transport The feat commit dropped the TODO when it replaced the old comment, but the future work (exposing approval requests over the daemon transport for proper user confirmation) is still planned. Co-Authored-By: Claude Sonnet 4.6 --- packages/wallet-cli/src/daemon/wallet-factory.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/wallet-cli/src/daemon/wallet-factory.ts b/packages/wallet-cli/src/daemon/wallet-factory.ts index c1d3f22d2f4..a4889c31e14 100644 --- a/packages/wallet-cli/src/daemon/wallet-factory.ts +++ b/packages/wallet-cli/src/daemon/wallet-factory.ts @@ -87,6 +87,7 @@ function buildInstanceOptions( // The daemon is headless, so there is no UI to open: requests are // resolved by the auto-approval subscription (see `subscribeToAutoApproval`) // rather than by this hook, which stays a no-op. + // TODO: surface approval requests over the daemon transport. showApprovalRequest: (): undefined => undefined, }, connectivityController: { From 1aea676daec2daef7a8da810c2be0d8c4f79fa5e Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Thu, 23 Jul 2026 11:52:50 +0300 Subject: [PATCH 5/9] fix(wallet-cli): trim over-verbose inline comment on auto-approval setup Keep only the ordering rationale; the rest is already in the function name and its JSDoc. Co-Authored-By: Claude Sonnet 4.6 --- packages/wallet-cli/src/daemon/wallet-factory.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/wallet-cli/src/daemon/wallet-factory.ts b/packages/wallet-cli/src/daemon/wallet-factory.ts index a4889c31e14..fd0d5113b69 100644 --- a/packages/wallet-cli/src/daemon/wallet-factory.ts +++ b/packages/wallet-cli/src/daemon/wallet-factory.ts @@ -213,11 +213,7 @@ export async function createWallet({ logFn, ); - // Accept pending approval requests headlessly, so an awaited - // `ApprovalController:addRequest` (raised by a transaction or signature - // flow) resolves instead of hanging. See `subscribeToAutoApproval` for the - // trust model. Installed before `init` so any request raised during - // initialization is covered too. + // Installed before `init` so any approval raised during initialization is covered. autoApprovalUnsubscribe = subscribeToAutoApproval(wallet.messenger, logFn); // Complete post-construction controller setup before serving requests — From e61e469a8ed337fa9e72d3b1e70f3f6a866ce18e Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Thu, 23 Jul 2026 11:57:33 +0300 Subject: [PATCH 6/9] fix(wallet-cli): remove over-commenting per review - Drop inline TODO that duplicated the JSDoc TODO in subscribeToApprovalStateChanged - Remove four in-test comments that restate their it() descriptions - Trim integration test comment block to only the non-obvious shouldShowRequest note Co-Authored-By: Claude Sonnet 4.6 --- packages/wallet-cli/src/daemon/auto-approval.test.ts | 9 --------- packages/wallet-cli/src/daemon/auto-approval.ts | 1 - .../src/daemon/wallet-factory.integration.test.ts | 8 ++------ 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/packages/wallet-cli/src/daemon/auto-approval.test.ts b/packages/wallet-cli/src/daemon/auto-approval.test.ts index d75f755bebd..5f13423ca1a 100644 --- a/packages/wallet-cli/src/daemon/auto-approval.test.ts +++ b/packages/wallet-cli/src/daemon/auto-approval.test.ts @@ -96,14 +96,10 @@ describe('subscribeToAutoApproval', () => { const { messenger, call, handler } = makeMessenger(); subscribeToAutoApproval(messenger); - // Two state changes listing the same id before the first accept settles: - // the second is skipped so the request is not accepted twice. handler()({ pendingApprovals: { 'id-a': {} } }); handler()({ pendingApprovals: { 'id-a': {} } }); expect(call).toHaveBeenCalledTimes(1); - // Once the accept settles the id leaves the in-flight set, so a later state - // change is free to accept again. await flushMicrotasks(); handler()({ pendingApprovals: { 'id-a': {} } }); expect(call).toHaveBeenCalledTimes(2); @@ -112,9 +108,6 @@ describe('subscribeToAutoApproval', () => { it('does not re-accept ids surfaced by the re-entrant state change that accepting itself triggers', () => { const { messenger, call, handler } = makeMessenger(); subscribeToAutoApproval(messenger); - // Accepting `id-a` deletes it, which synchronously re-fires the state - // change; here the re-entrant change still lists both ids. Each must be - // accepted exactly once. call.mockImplementation((_action: string, id: string) => { if (id === 'id-a') { handler()({ pendingApprovals: { 'id-a': {}, 'id-b': {} } }); @@ -159,8 +152,6 @@ describe('subscribeToAutoApproval', () => { ), ); - // The failed id was cleared from the in-flight set, so a subsequent state - // change accepts it again rather than skipping it forever. handler()({ pendingApprovals: { 'id-a': {} } }); expect(call).toHaveBeenCalledTimes(2); }); diff --git a/packages/wallet-cli/src/daemon/auto-approval.ts b/packages/wallet-cli/src/daemon/auto-approval.ts index 909f20698f8..f9340a080c4 100644 --- a/packages/wallet-cli/src/daemon/auto-approval.ts +++ b/packages/wallet-cli/src/daemon/auto-approval.ts @@ -135,7 +135,6 @@ function subscribeToApprovalStateChanged( messenger: Readonly>, handler: ApprovalStateChangeHandler, ): () => void { - // TODO: Remove once ApprovalControllerEvents includes ControllerStateChangedEvent. const subscriber = messenger as unknown as { subscribe: (eventType: string, handler: ApprovalStateChangeHandler) => void; unsubscribe: ( diff --git a/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts b/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts index 48c4c2ebd60..b3e1dacf452 100644 --- a/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts +++ b/packages/wallet-cli/src/daemon/wallet-factory.integration.test.ts @@ -78,11 +78,8 @@ describe('createWallet (real Wallet, in-memory)', () => { }); try { - // `addRequest` awaits acceptance; on a headless daemon with no UI it would - // hang forever, so the auto-approval subscription must resolve it. - // `shouldShowRequest: false` keeps the no-op `showApprovalRequest` out of - // the picture — resolution comes purely from the state-change accept path. - // If auto-approval regressed, this would time out rather than resolve. + // `shouldShowRequest: false` bypasses the no-op `showApprovalRequest` hook — + // acceptance must come purely from the auto-approval subscription. const accepted = await wallet.messenger.call( 'ApprovalController:addRequest', { origin: 'https://example.com', type: 'test:approval' }, @@ -90,7 +87,6 @@ describe('createWallet (real Wallet, in-memory)', () => { ); expect(accepted).toBeUndefined(); - // Accepting deletes the request, so nothing is left pending. expect( wallet.messenger.call('ApprovalController:getState').pendingApprovals, ).toStrictEqual({}); From 89edb0b70a8b6263a7e55b315c29042fe075442f Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Thu, 23 Jul 2026 12:31:09 +0300 Subject: [PATCH 7/9] fix(wallet-cli): trim subscribeToAutoApproval JSDoc trust model section Replace the long inline trust model with a one-paragraph summary and a reference to #9513 (the mm-send issue) for the planned scoped policy. Keep only the inFlight guard rationale and error-swallowing note, which are non-obvious implementation details an editor of this code needs. Co-Authored-By: Claude Sonnet 4.6 --- .../wallet-cli/src/daemon/auto-approval.ts | 42 +++++-------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/packages/wallet-cli/src/daemon/auto-approval.ts b/packages/wallet-cli/src/daemon/auto-approval.ts index f9340a080c4..b5c697479ad 100644 --- a/packages/wallet-cli/src/daemon/auto-approval.ts +++ b/packages/wallet-cli/src/daemon/auto-approval.ts @@ -28,41 +28,19 @@ type ApprovalStateChangeHandler = (state: PendingApprovalsState) => void; /** * Subscribe the daemon to auto-accept every pending approval request. * - * ## Trust model — the daemon accepts every approval without confirmation + * Without this, any `ApprovalController:addRequest` call hangs forever on a + * headless daemon. **Everything is approved without a prompt** — trust boundary + * is the `0600` same-user socket. A scoped policy is tracked in + * {@link https://github.com/MetaMask/core/issues/9513}. * - * A send or signature flow raises an approval via - * `ApprovalController:addRequest` and **awaits** its resolution. A UI client - * resolves that request from a human decision; the headless daemon has no UI, - * so with nothing accepting the request the awaiting call hangs forever. (The - * injected `showApprovalRequest` hook only signals "a request needs - * attention" — it does not resolve anything.) - * - * This subscribes to `ApprovalController:stateChanged` and immediately accepts - * every pending request via `ApprovalController:acceptRequest`. The daemon - * therefore approves **everything** — transactions and signatures included — - * with no per-request prompt. That is the intended model for a headless - * daemon: it is driven only by its owner's local CLI over a `0600`, same-user - * Unix socket (see `startRpcSocketServer`), so the trust boundary is the - * socket, not a per-request confirmation. - * - * This is **not** "safe by default": a scoped/opt-in policy (a config flag, or - * accepting only specific approval types) is deferred until the user-facing - * send command exists. Until then, anything that can reach the socket can move - * funds. - * - * `acceptRequest` deletes the request as it resolves, which itself emits - * another state change; a single flow can also emit several changes in quick - * succession. The `inFlight` guard makes accepting each id idempotent across - * those re-entrant/rapid changes — without it the same id would be accepted - * twice and the second accept would reject because the request is already gone. - * Both synchronous throws and async rejections from an accept are logged and - * swallowed, so one bad request can neither crash the daemon nor wedge the - * subscription. + * The `inFlight` guard makes accepting each id idempotent: `acceptRequest` + * deletes the request on resolve, which itself re-emits `stateChanged`. Without + * it the same id would be accepted twice and the second accept would reject. + * Both sync throws and async rejections are logged and swallowed so one bad + * request cannot crash the daemon or wedge the subscription. * * @param messenger - The wallet root messenger. - * @param log - Optional logger for accept failures. Defaults to `console.error` - * (which a detached daemon's `stdio: 'ignore'` discards, so a daemon host - * should supply its own logger). + * @param log - Optional logger for accept failures. Defaults to `console.error`. * @returns A function that unsubscribes the auto-approval handler. */ export function subscribeToAutoApproval( From 0118faf7359a28f319b169c91e9470e8061669bf Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Thu, 23 Jul 2026 16:27:13 +0300 Subject: [PATCH 8/9] fix(wallet-cli): remove redundant parens in type cast assertion Co-Authored-By: Claude Sonnet 4.6 --- packages/wallet-cli/src/daemon/wallet-factory.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/wallet-cli/src/daemon/wallet-factory.test.ts b/packages/wallet-cli/src/daemon/wallet-factory.test.ts index 238b2fbab59..fd6e3451a01 100644 --- a/packages/wallet-cli/src/daemon/wallet-factory.test.ts +++ b/packages/wallet-cli/src/daemon/wallet-factory.test.ts @@ -550,7 +550,7 @@ describe('createWallet', () => { 'Auto-approval unsubscribe failed during teardown', ), ); - expect((wallet.destroy as jest.Mock)).toHaveBeenCalledTimes(1); + expect(wallet.destroy as jest.Mock).toHaveBeenCalledTimes(1); expect(closeSpy).toHaveBeenCalled(); }); From 26e4183eb16d752f41938143e9edc2cae47b1ef5 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Thu, 23 Jul 2026 16:33:54 +0300 Subject: [PATCH 9/9] fix(wallet-cli): consolidate duplicate wallet bump entries and strengthen async rejection test Co-Authored-By: Claude Sonnet 4.6 --- packages/wallet-cli/CHANGELOG.md | 3 +-- packages/wallet-cli/src/daemon/auto-approval.test.ts | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/wallet-cli/CHANGELOG.md b/packages/wallet-cli/CHANGELOG.md index 36dd108a15c..cf1fee9105e 100644 --- a/packages/wallet-cli/CHANGELOG.md +++ b/packages/wallet-cli/CHANGELOG.md @@ -27,8 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `--password` / `MM_WALLET_PASSWORD` is now optional on `mm daemon start`; on subsequent runs, omitting it starts the daemon with a locked keyring, and the persisted vault is auto-unlocked when a password is supplied ([#8821](https://github.com/MetaMask/core/pull/8821)) - The daemon RPC server now validates `params` against each handler's superstruct before dispatch, returning a `-32602 invalidParams` error on mismatch instead of passing raw params to the handler ([#8846](https://github.com/MetaMask/core/pull/8846)) - Report daemon socket connection errors consistently across `mm daemon call` and `mm daemon list` ([#9339](https://github.com/MetaMask/core/pull/9339)) -- Bump `@metamask/wallet` from `^3.0.0` to `^8.1.0` ([#9218](https://github.com/MetaMask/core/pull/9218), [#9263](https://github.com/MetaMask/core/pull/9263), [#9349](https://github.com/MetaMask/core/pull/9349), [#9396](https://github.com/MetaMask/core/pull/9396), [#9470](https://github.com/MetaMask/core/pull/9470), [#9629](https://github.com/MetaMask/core/pull/9629)) +- Bump `@metamask/wallet` from `^3.0.0` to `^8.1.0` ([#9218](https://github.com/MetaMask/core/pull/9218), [#9263](https://github.com/MetaMask/core/pull/9263), [#9349](https://github.com/MetaMask/core/pull/9349), [#9396](https://github.com/MetaMask/core/pull/9396), [#9470](https://github.com/MetaMask/core/pull/9470), [#9609](https://github.com/MetaMask/core/pull/9609), [#9629](https://github.com/MetaMask/core/pull/9629)) - Wrap daemon password and SRP in opaque `Password` and `Srp` types that redact on logging; validated and unwrapped only at trust boundaries ([#8863](https://github.com/MetaMask/core/pull/8863)) -- Bump `@metamask/wallet` from `^7.0.1` to `8.0.0`. ([#9609](https://github.com/MetaMask/core/pull/9609)) [Unreleased]: https://github.com/MetaMask/core/ diff --git a/packages/wallet-cli/src/daemon/auto-approval.test.ts b/packages/wallet-cli/src/daemon/auto-approval.test.ts index 5f13423ca1a..00890c3745c 100644 --- a/packages/wallet-cli/src/daemon/auto-approval.test.ts +++ b/packages/wallet-cli/src/daemon/auto-approval.test.ts @@ -135,6 +135,10 @@ describe('subscribeToAutoApproval', () => { 'Failed to auto-accept approval request id-a: Error: accept boom', ), ); + + // Verify .finally() ran so the id is retryable after an async rejection. + handler()({ pendingApprovals: { 'id-a': {} } }); + expect(call).toHaveBeenCalledTimes(2); }); it('logs and swallows a synchronous throw from an accept, and can retry the id later', () => {