diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 46131ea3fee..79caefd17d3 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -2240,6 +2240,11 @@ "count": 2 } }, + "packages/transaction-pay-controller/src/strategy/bridge/bridge-submit.ts": { + "no-restricted-syntax": { + "count": 1 + } + }, "packages/transaction-pay-controller/src/strategy/relay/hyperliquid-withdraw.ts": { "no-restricted-syntax": { "count": 1 diff --git a/packages/wallet-cli/CHANGELOG.md b/packages/wallet-cli/CHANGELOG.md index e33b8e09ec8..d8295d12e18 100644 --- a/packages/wallet-cli/CHANGELOG.md +++ b/packages/wallet-cli/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- 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 `^7.0.1` ([#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)) diff --git a/packages/wallet-cli/package.json b/packages/wallet-cli/package.json index 93b9f587718..d45908f167f 100644 --- a/packages/wallet-cli/package.json +++ b/packages/wallet-cli/package.json @@ -49,6 +49,7 @@ "@metamask/remote-feature-flag-controller": "^4.2.2", "@metamask/rpc-errors": "^7.0.2", "@metamask/storage-service": "^1.0.2", + "@metamask/superstruct": "^3.1.0", "@metamask/utils": "^11.11.0", "@metamask/wallet": "^7.0.1", "@oclif/core": "^4.10.5", diff --git a/packages/wallet-cli/src/daemon/daemon-entry.test.ts b/packages/wallet-cli/src/daemon/daemon-entry.test.ts index ce32f142ebc..72528b26040 100644 --- a/packages/wallet-cli/src/daemon/daemon-entry.test.ts +++ b/packages/wallet-cli/src/daemon/daemon-entry.test.ts @@ -1,3 +1,4 @@ +import { validate } from '@metamask/superstruct'; import { appendFile, readFile, rm, writeFile } from 'node:fs/promises'; import { pingDaemon } from './daemon-client'; @@ -489,7 +490,7 @@ describe('daemon-entry', () => { const callArgs = mockStartRpcSocketServer.mock.calls[0][0]; const { handlers } = callArgs; - const status = (await handlers.getStatus(null)) as { + const status = (await handlers.getStatus.run(null)) as { pid: number; uptime: number; }; @@ -512,7 +513,7 @@ describe('daemon-entry', () => { await importDaemonEntry(); const { handlers } = mockStartRpcSocketServer.mock.calls[0][0]; - const actions = await handlers.listActions(null); + const actions = await handlers.listActions.run(null); expect(actions).toStrictEqual([ 'NetworkController:getState', @@ -520,6 +521,28 @@ describe('daemon-entry', () => { ]); }); + it('getStatus paramsStruct rejects non-null params', async () => { + mockCreateWallet.mockResolvedValue(createMockWallet()); + mockStartRpcSocketServer.mockResolvedValue(createMockHandle()); + + await importDaemonEntry(); + + const { handlers } = mockStartRpcSocketServer.mock.calls[0][0]; + const [error] = validate(['unexpected'], handlers.getStatus.paramsStruct); + expect(error).toBeDefined(); + }); + + it('listActions paramsStruct rejects non-null params', async () => { + mockCreateWallet.mockResolvedValue(createMockWallet()); + mockStartRpcSocketServer.mockResolvedValue(createMockHandle()); + + await importDaemonEntry(); + + const { handlers } = mockStartRpcSocketServer.mock.calls[0][0]; + const [error] = validate(['unexpected'], handlers.listActions.paramsStruct); + expect(error).toBeDefined(); + }); + it('logs to file via makeLogger', async () => { mockCreateWallet.mockResolvedValue(createMockWallet()); mockStartRpcSocketServer.mockResolvedValue(createMockHandle()); @@ -625,6 +648,28 @@ describe('daemon-entry', () => { ); }); + it('logs dispose error during shutdown without aborting cleanup', async () => { + const result = createMockWallet(); + (result.dispose as jest.Mock).mockRejectedValue( + new Error('dispose failed'), + ); + mockCreateWallet.mockResolvedValue(result); + const handle = createMockHandle(); + mockStartRpcSocketServer.mockResolvedValue(handle); + + await importDaemonEntry(); + + const callArgs = mockStartRpcSocketServer.mock.calls[0][0]; + const onShutdown = callArgs.onShutdown as () => Promise; + await onShutdown(); + + expect(handle.close).toHaveBeenCalled(); + expect(mockAppendFile).toHaveBeenCalledWith( + '/tmp/daemon.log', + expect.stringContaining('dispose() failed during shutdown'), + ); + }); + it('handles rm rejection during shutdown cleanup gracefully', async () => { const result = createMockWallet(); mockCreateWallet.mockResolvedValue(result); @@ -714,13 +759,18 @@ describe('daemon-entry', () => { describe('call handler', () => { /** - * Import the daemon entry and extract the `call` handler from the - * handlers map, along with the mock wallet for assertions. + * Import the daemon entry and extract the `call` handler definition from + * the handlers map, along with the mock wallet for assertions. * - * @returns The call handler function and mock wallet result. + * @returns The call handler definition and mock wallet result. */ async function setupCallHandler(): Promise<{ - callHandler: (params: unknown) => Promise; + callHandler: { + paramsStruct: import('@metamask/superstruct').Struct< + [string, ...unknown[]] + >; + run: (params: [string, ...unknown[]]) => Promise; + }; result: MockCreateWalletResult; }> { const result = createMockWallet(); @@ -730,20 +780,25 @@ describe('daemon-entry', () => { await importDaemonEntry(); const callArgs = mockStartRpcSocketServer.mock.calls[0][0]; - const callHandler = callArgs.handlers.call as ( - params: unknown, - ) => Promise; + const callHandler = callArgs.handlers.call as unknown as { + paramsStruct: import('@metamask/superstruct').Struct< + [string, ...unknown[]] + >; + run: (params: [string, ...unknown[]]) => Promise; + }; return { callHandler, result }; } - it('registers a call handler', async () => { + it('registers a call handler definition', async () => { mockCreateWallet.mockResolvedValue(createMockWallet()); mockStartRpcSocketServer.mockResolvedValue(createMockHandle()); await importDaemonEntry(); const callArgs = mockStartRpcSocketServer.mock.calls[0][0]; - expect(typeof callArgs.handlers.call).toBe('function'); + const callDefinition = callArgs.handlers.call; + expect(callDefinition).toHaveProperty('paramsStruct'); + expect(typeof callDefinition.run).toBe('function'); }); it('forwards action and args to messenger.call', async () => { @@ -751,7 +806,7 @@ describe('daemon-entry', () => { const mockCall = result.wallet.messenger.call as jest.Mock; mockCall.mockReturnValue({ accounts: [] }); - const callResult = await callHandler([ + const callResult = await callHandler.run([ 'Controller:action', 'arg1', 'arg2', @@ -770,7 +825,7 @@ describe('daemon-entry', () => { const mockCall = result.wallet.messenger.call as jest.Mock; mockCall.mockReturnValue('ok'); - await callHandler(['Controller:action']); + await callHandler.run(['Controller:action']); expect(mockCall).toHaveBeenCalledWith('Controller:action'); }); @@ -780,7 +835,7 @@ describe('daemon-entry', () => { const mockCall = result.wallet.messenger.call as jest.Mock; mockCall.mockResolvedValue({ async: true }); - const callResult = await callHandler(['Controller:asyncAction']); + const callResult = await callHandler.run(['Controller:asyncAction']); expect(callResult).toStrictEqual({ async: true }); }); @@ -792,33 +847,29 @@ describe('daemon-entry', () => { throw new Error('A handler for Unknown:action has not been registered'); }); - await expect(callHandler(['Unknown:action'])).rejects.toThrow( + await expect(callHandler.run(['Unknown:action'])).rejects.toThrow( 'A handler for Unknown:action has not been registered', ); }); - it('throws when params is null', async () => { - const { callHandler } = await setupCallHandler(); - - await expect(callHandler(null)).rejects.toThrow( - 'Expected params to be an array with an action name', - ); - }); - - it('throws when params is an empty array', async () => { + it.each([ + ['null', null], + ['empty array', []], + ['non-string first element', [42]], + ['non-array', { foo: 'bar' }], + ])('paramsStruct rejects invalid params (%s)', async (_label, value) => { const { callHandler } = await setupCallHandler(); - - await expect(callHandler([])).rejects.toThrow( - 'Expected params to be an array with an action name', - ); + const [error] = validate(value, callHandler.paramsStruct); + expect(error).toBeDefined(); }); - it('throws when action name is not a string', async () => { + it('paramsStruct accepts a non-empty array starting with a string', async () => { const { callHandler } = await setupCallHandler(); - - await expect(callHandler([42])).rejects.toThrow( - 'Expected params to be an array with an action name', + const [error] = validate( + ['Controller:action', 1, 'two'], + callHandler.paramsStruct, ); + expect(error).toBeUndefined(); }); }); }); diff --git a/packages/wallet-cli/src/daemon/daemon-entry.ts b/packages/wallet-cli/src/daemon/daemon-entry.ts index a4b8f89d928..34c89f87dcd 100644 --- a/packages/wallet-cli/src/daemon/daemon-entry.ts +++ b/packages/wallet-cli/src/daemon/daemon-entry.ts @@ -1,3 +1,4 @@ +import { define, literal } from '@metamask/superstruct'; import type { Json } from '@metamask/utils'; import type { Wallet } from '@metamask/wallet'; import { appendFile, readFile, rm, writeFile } from 'node:fs/promises'; @@ -7,10 +8,37 @@ import { ensureOwnerOnlyDirectory } from './data-dir'; import { getDaemonPaths } from './paths'; import { startRpcSocketServer } from './rpc-socket-server'; import type { RpcSocketServerHandle } from './rpc-socket-server'; -import type { DaemonStatusInfo, Logger, RpcHandlerMap } from './types'; +import { defineHandler } from './types'; +import type { + DaemonStatusInfo, + Logger, + RpcDispatcher, + RpcHandlerMap, +} from './types'; import { isErrorWithCode, isProcessAlive, readPidFile } from './utils'; import { createWallet } from './wallet-factory'; +/** + * Params struct for the `call` RPC method. `params` must be a non-empty array + * whose first element is the messenger action name; remaining elements are + * positional action arguments forwarded as-is to `messenger.call`. + */ +const callParamsStruct = define<[string, ...unknown[]]>( + 'CallParams', + (value) => { + if (!Array.isArray(value)) { + return 'Expected an array'; + } + if (value.length === 0) { + return 'Expected a non-empty array'; + } + if (typeof value[0] !== 'string') { + return 'Expected the first element to be a string action name'; + } + return true; + }, +); + const startTime = Date.now(); main().catch((error: unknown) => { @@ -98,39 +126,37 @@ async function main(): Promise { })); const constructedWallet = wallet; + // Arbitrary messenger dispatch is intentional: the CLI exposes the full + // messenger surface over a Unix socket inside the per-user oclif data + // directory. The dataDir is chmodded to 0o700 above and the socket to + // 0o600 by the RPC server on bind, so only the owning user can open them, + // but there is no in-process auth check beyond that filesystem-permission + // barrier. The messenger is strongly typed by action name; we narrow it + // once here to the RpcDispatcher shape the `call` handler needs. + const dispatch = constructedWallet.messenger.call.bind( + constructedWallet.messenger, + ) as unknown as RpcDispatcher; + const handlers: RpcHandlerMap = { - getStatus: async (): Promise => ({ - pid: process.pid, - uptime: Math.floor((Date.now() - startTime) / 1000), + getStatus: defineHandler( + literal(null), + async (): Promise => ({ + pid: process.pid, + uptime: Math.floor((Date.now() - startTime) / 1000), + }), + ), + call: defineHandler(callParamsStruct, async (params) => { + const [action, ...args] = params; + return await dispatch(action, ...(args as Json[])); }), // Exposes the callable surface for discovery: it grows silently as // controllers are wired, so consumers need a way to see it without a // hand-kept catalog that would rot. - listActions: async (): Promise => - constructedWallet.messenger.getRegisteredActionTypes(), - // Arbitrary messenger dispatch is intentional: the CLI exposes the full - // messenger surface over a Unix socket inside the per-user oclif data - // directory. The dataDir is chmodded to 0o700 above and the socket to - // 0o600 by the RPC server on bind, so only the owning user can open - // them, but there is no in-process auth check beyond that - // filesystem-permission barrier. - call: async (params) => { - if (!Array.isArray(params) || typeof params[0] !== 'string') { - throw new Error('Expected params to be an array with an action name'); - } - const [action, ...args] = params as [string, ...Json[]]; - // The messenger's `call` is typed to a literal action-name union; the - // daemon dispatches arbitrary action names from RPC. Cast to a - // string-keyed `call` (which preserves arity) rather than to `any`, so - // the only untyped value is the `unknown` result narrowed below. - type ArbitraryDispatch = { - call: (actionName: string, ...callArgs: Json[]) => unknown; - }; - const result = ( - constructedWallet.messenger as unknown as ArbitraryDispatch - ).call(action, ...args); - return (result instanceof Promise ? await result : result) as Json; - }, + listActions: defineHandler( + literal(null), + async (): Promise => + constructedWallet.messenger.getRegisteredActionTypes(), + ), }; // `startRpcSocketServer` restricts the socket to the owner (chmod 0o600) @@ -182,7 +208,11 @@ async function main(): Promise { } catch (closeError) { log(`handle.close() failed: ${String(closeError)}`); } - await activeDispose(); + try { + await activeDispose(); + } catch (disposeError) { + log(`dispose() failed during shutdown: ${String(disposeError)}`); + } await Promise.all([ removeOwnedPidFile(pidPath, pidFileContents).catch( (rmError: unknown) => { diff --git a/packages/wallet-cli/src/daemon/rpc-socket-server.test.ts b/packages/wallet-cli/src/daemon/rpc-socket-server.test.ts index 150819e17c1..aeea4e8d236 100644 --- a/packages/wallet-cli/src/daemon/rpc-socket-server.test.ts +++ b/packages/wallet-cli/src/daemon/rpc-socket-server.test.ts @@ -1,10 +1,19 @@ +import { any, literal } from '@metamask/superstruct'; import { EventEmitter } from 'node:events'; import { chmod, unlink } from 'node:fs/promises'; import { createServer } from 'node:net'; import type { Server, Socket } from 'node:net'; import { startRpcSocketServer } from './rpc-socket-server'; -import type { RpcHandlerMap } from './types'; +import type { RpcHandlerDefinition, RpcHandlerMap } from './types'; + +// any() paramsStruct so the struct guard never rejects test inputs. +function asHandler(run: jest.Mock): RpcHandlerDefinition { + return { + paramsStruct: any(), + run: run as unknown as RpcHandlerDefinition['run'], + }; +} jest.mock('node:fs/promises'); jest.mock('node:net'); @@ -224,7 +233,7 @@ describe('startRpcSocketServer', () => { it('dispatches valid request to handler and returns result', async () => { const { simulateConnection } = createMockServer(); const handlers: RpcHandlerMap = { - getStatus: jest.fn().mockResolvedValue({ status: 'ok' }), + getStatus: asHandler(jest.fn().mockResolvedValue({ status: 'ok' })), }; await startRpcSocketServer({ @@ -252,7 +261,7 @@ describe('startRpcSocketServer', () => { it('returns null result when handler returns undefined', async () => { const { simulateConnection } = createMockServer(); const handlers: RpcHandlerMap = { - noop: jest.fn().mockResolvedValue(undefined), + noop: asHandler(jest.fn().mockResolvedValue(undefined)), }; await startRpcSocketServer({ @@ -367,7 +376,9 @@ describe('startRpcSocketServer', () => { it('returns -32603 when handler throws an Error', async () => { const { simulateConnection } = createMockServer(); const handlers: RpcHandlerMap = { - failing: jest.fn().mockRejectedValue(new Error('handler failed')), + failing: asHandler( + jest.fn().mockRejectedValue(new Error('handler failed')), + ), }; await startRpcSocketServer({ @@ -393,7 +404,7 @@ describe('startRpcSocketServer', () => { const { simulateConnection } = createMockServer(); const rpcError = { code: -32001, message: 'custom rpc' }; const handlers: RpcHandlerMap = { - failing: jest.fn().mockRejectedValue(rpcError), + failing: asHandler(jest.fn().mockRejectedValue(rpcError)), }; await startRpcSocketServer({ @@ -416,7 +427,7 @@ describe('startRpcSocketServer', () => { it('returns Internal error when handler throws a non-Error value', async () => { const { simulateConnection } = createMockServer(); const handlers: RpcHandlerMap = { - failing: jest.fn().mockRejectedValue('string error'), + failing: asHandler(jest.fn().mockRejectedValue('string error')), }; await startRpcSocketServer({ @@ -540,7 +551,7 @@ describe('startRpcSocketServer', () => { it('accumulates partial data across multiple events', async () => { const { simulateConnection } = createMockServer(); const handlers: RpcHandlerMap = { - test: jest.fn().mockResolvedValue('ok'), + test: asHandler(jest.fn().mockResolvedValue('ok')), }; await startRpcSocketServer({ @@ -619,7 +630,7 @@ describe('startRpcSocketServer', () => { const circular: Record = {}; circular.self = circular; const handlers: RpcHandlerMap = { - bad: jest.fn().mockResolvedValue(circular), + bad: asHandler(jest.fn().mockResolvedValue(circular)), }; await startRpcSocketServer({ @@ -691,10 +702,99 @@ describe('startRpcSocketServer', () => { jest.useRealTimers(); }); + it('returns -32602 when params fail the registered struct', async () => { + const { simulateConnection } = createMockServer(); + const run = jest.fn(); + const handlers: RpcHandlerMap = { + strict: { + paramsStruct: literal('expected'), + run: run as unknown as RpcHandlerMap[string]['run'], + }, + }; + + await startRpcSocketServer({ + socketPath: '/tmp/test.sock', + handlers, + }); + + const socket = createMockSocket(); + simulateConnection(socket); + sendRequest(socket, { + jsonrpc: '2.0', + id: '1', + method: 'strict', + params: ['something else'], + }); + + await flushPromises(); + + expect(getResponse(socket).error).toStrictEqual( + expect.objectContaining({ + code: -32602, + message: expect.stringContaining('Invalid params for strict'), + }), + ); + expect(run).not.toHaveBeenCalled(); + }); + + it('returns -32602 when params are absent and struct rejects null', async () => { + const { simulateConnection } = createMockServer(); + const run = jest.fn(); + const handlers: RpcHandlerMap = { + strict: { + paramsStruct: literal('expected'), + run: run as unknown as RpcHandlerMap[string]['run'], + }, + }; + + await startRpcSocketServer({ + socketPath: '/tmp/test.sock', + handlers, + }); + + const socket = createMockSocket(); + simulateConnection(socket); + sendRequest(socket, { jsonrpc: '2.0', id: '1', method: 'strict' }); + + await flushPromises(); + + expect(getResponse(socket).error).toStrictEqual( + expect.objectContaining({ + code: -32602, + message: expect.stringContaining('Invalid params for strict'), + }), + ); + expect(run).not.toHaveBeenCalled(); + }); + + it('logs the method name when a handler throws', async () => { + const { simulateConnection } = createMockServer(); + const log = jest.fn(); + const handlers: RpcHandlerMap = { + failing: asHandler( + jest.fn().mockRejectedValue(new Error('handler failed')), + ), + }; + + await startRpcSocketServer({ + socketPath: '/tmp/test.sock', + handlers, + log, + }); + + const socket = createMockSocket(); + simulateConnection(socket); + sendRequest(socket, { jsonrpc: '2.0', id: '1', method: 'failing' }); + + await flushPromises(); + + expect(log).toHaveBeenCalledWith(expect.stringContaining('failing')); + }); + it('wraps thrown object with code but no message as internal error', async () => { const { simulateConnection } = createMockServer(); const handlers: RpcHandlerMap = { - failing: jest.fn().mockRejectedValue({ code: 42 }), + failing: asHandler(jest.fn().mockRejectedValue({ code: 42 })), }; await startRpcSocketServer({ diff --git a/packages/wallet-cli/src/daemon/rpc-socket-server.ts b/packages/wallet-cli/src/daemon/rpc-socket-server.ts index 89170be5fef..a221235e150 100644 --- a/packages/wallet-cli/src/daemon/rpc-socket-server.ts +++ b/packages/wallet-cli/src/daemon/rpc-socket-server.ts @@ -1,4 +1,5 @@ import { rpcErrors } from '@metamask/rpc-errors'; +import { validate as validateStruct } from '@metamask/superstruct'; import type { JsonRpcId, JsonRpcParams, @@ -229,7 +230,23 @@ async function handleRequest( }; } - const result = await handler(coerceHandlerParams(params)); + const [structError, validatedParams] = validateStruct( + coerceHandlerParams(params), + handler.paramsStruct, + ); + if (structError !== undefined) { + return { + jsonrpc: '2.0', + id, + error: rpcErrors + .invalidParams({ + message: `Invalid params for ${method}: ${structError.message}`, + }) + .serialize(), + }; + } + + const result = await handler.run(validatedParams); return { jsonrpc: '2.0', id, result: result ?? null }; } catch (error) { log(`RPC handler "${method}" failed: ${String(error)}`); diff --git a/packages/wallet-cli/src/daemon/socket-integration.test.ts b/packages/wallet-cli/src/daemon/socket-integration.test.ts index c06bbe008de..98c619f2ded 100644 --- a/packages/wallet-cli/src/daemon/socket-integration.test.ts +++ b/packages/wallet-cli/src/daemon/socket-integration.test.ts @@ -1,3 +1,4 @@ +import { any } from '@metamask/superstruct'; import { stat } from 'node:fs/promises'; import { createConnection } from 'node:net'; import { tmpdir } from 'node:os'; @@ -6,6 +7,17 @@ import { join } from 'node:path'; import { pingDaemon, sendCommand } from './daemon-client'; import { startRpcSocketServer } from './rpc-socket-server'; import type { RpcSocketServerHandle } from './rpc-socket-server'; +import type { RpcHandlerDefinition } from './types'; + +// any() paramsStruct so integration test inputs are never rejected by the struct guard. +function handlerDefinition( + run: (params: unknown) => Promise, +): RpcHandlerDefinition { + return { + paramsStruct: any(), + run: run as unknown as RpcHandlerDefinition['run'], + }; +} /** * End-to-end integration tests for the daemon's IPC layer: real @@ -54,7 +66,7 @@ describe('socket integration', () => { await startServer({ socketPath, handlers: { - getStatus: async () => ({ pid: 42, uptime: 7 }), + getStatus: handlerDefinition(async () => ({ pid: 42, uptime: 7 })), }, }); @@ -74,7 +86,7 @@ describe('socket integration', () => { await startServer({ socketPath, handlers: { - getStatus: async () => ({ pid: 1, uptime: 0 }), + getStatus: handlerDefinition(async () => ({ pid: 1, uptime: 0 })), }, }); @@ -88,7 +100,7 @@ describe('socket integration', () => { await startServer({ socketPath, handlers: { - getStatus: async () => ({ pid: 1, uptime: 0 }), + getStatus: handlerDefinition(async () => ({ pid: 1, uptime: 0 })), }, }); @@ -105,9 +117,9 @@ describe('socket integration', () => { await startServer({ socketPath, handlers: { - boom: async () => { + boom: handlerDefinition(async () => { throw new Error('handler exploded'); - }, + }), }, }); @@ -130,7 +142,7 @@ describe('socket integration', () => { await startServer({ socketPath, handlers: { - getStatus: async () => ({ pid: 1, uptime: 0 }), + getStatus: handlerDefinition(async () => ({ pid: 1, uptime: 0 })), }, }); @@ -150,7 +162,7 @@ describe('socket integration', () => { await startServer({ socketPath, handlers: { - echo: async (params) => ({ params }), + echo: handlerDefinition(async (params) => ({ params })), }, }); @@ -199,7 +211,7 @@ describe('socket integration', () => { await startServer({ socketPath, handlers: { - getStatus: async () => ({ pid: 1, uptime: 0 }), + getStatus: handlerDefinition(async () => ({ pid: 1, uptime: 0 })), }, }); diff --git a/packages/wallet-cli/src/daemon/types.ts b/packages/wallet-cli/src/daemon/types.ts index eddedc8b336..0f46714535f 100644 --- a/packages/wallet-cli/src/daemon/types.ts +++ b/packages/wallet-cli/src/daemon/types.ts @@ -1,3 +1,4 @@ +import type { Struct } from '@metamask/superstruct'; import type { Json } from '@metamask/utils'; /** @@ -7,16 +8,74 @@ import type { Json } from '@metamask/utils'; export type Logger = (message: string) => void; /** - * A function that handles a JSON-RPC method call. + * A function that handles a JSON-RPC method call after its params have been + * validated by the corresponding {@link RpcHandlerDefinition.paramsStruct}. + */ +export type RpcHandler = ( + params: TParams, +) => Promise; + +/** + * Definition for a single JSON-RPC method: the struct that validates + * incoming `params` plus the handler that runs once `params` is known to + * match. + * + * The server (see `rpc-socket-server.ts`) validates the raw `params` against + * `paramsStruct` before invoking `run`, so each handler body can trust the + * shape of its input without re-checking. + */ +export type RpcHandlerDefinition = { + paramsStruct: Struct; + run: RpcHandler; +}; + +/** + * A map of RPC method names to their handler definitions. * - * The `params` argument will be `null` if the client did not provide params. + * `TParams` is erased to `unknown` here so definitions with different narrow + * params types (e.g. `null` vs. a tuple) can coexist in the same map. Consumers + * therefore see each `run` as accepting `unknown` and must validate `params` + * against the paired `paramsStruct` before invoking it — which is exactly what + * the server (see `rpc-socket-server.ts`) does. The concrete `TParams` is + * captured inside {@link defineHandler}, where the struct and handler are bound + * together. */ -export type RpcHandler = (params: Json) => Promise; +export type RpcHandlerMap = Record< + string, + RpcHandlerDefinition +>; /** - * A map of RPC method names to their handler functions. + * Bundle a params struct with the handler that runs once `params` is + * validated. The server invokes `run` only after `paramsStruct` accepts the + * value, so `run` can trust the type of its argument. + * + * The returned definition erases `TParams` to `unknown` so heterogeneous + * handlers can share an {@link RpcHandlerMap}. + * + * @param paramsStruct - Struct that validates `params` for this method. + * @param run - Handler invoked with the validated params. + * @returns An {@link RpcHandlerDefinition} suitable for an {@link RpcHandlerMap}. + */ +export function defineHandler( + paramsStruct: Struct, + run: RpcHandler, +): RpcHandlerDefinition { + return { paramsStruct, run } as unknown as RpcHandlerDefinition< + unknown, + TResult + >; +} + +/** + * Typed wrapper around `wallet.messenger.call` used by the `call` RPC. + * + * The messenger is strongly typed by action name; the daemon exposes the full + * messenger surface over the socket and dispatches by string, so we consolidate + * the unsafe cast into a single, documented escape hatch instead of repeating + * it at each call site. */ -export type RpcHandlerMap = Record; +export type RpcDispatcher = (action: string, ...args: Json[]) => Promise; /** * Resolved paths for daemon state files. diff --git a/yarn.lock b/yarn.lock index c439f3e2cf8..eec9edfd6ea 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9046,6 +9046,7 @@ __metadata: "@metamask/remote-feature-flag-controller": "npm:^4.2.2" "@metamask/rpc-errors": "npm:^7.0.2" "@metamask/storage-service": "npm:^1.0.2" + "@metamask/superstruct": "npm:^3.1.0" "@metamask/utils": "npm:^11.11.0" "@metamask/wallet": "npm:^7.0.1" "@oclif/core": "npm:^4.10.5"