From e66446edc3837c2fee08a9021b2562f0850d105f Mon Sep 17 00:00:00 2001 From: Patricio <38959764+patriciobcs@users.noreply.github.com> Date: Fri, 31 Jul 2026 17:11:11 +0200 Subject: [PATCH] fix: derive raw vs extrinsic signing from the request channel (#1624) The signing popup decided whether to show the message view or the transaction view by inspecting fields on the payload (`data`, `blockNumber`), while the background picks the signing behaviour from the message channel the request arrived on. Those two are independent and the payload comes from the dapp, so they can disagree. `RequestSign` becomes a discriminated union on `channel`, set by the background from the message type. Correlating the discriminant with the payload type means consumers narrow through `isExtrinsicRequest` rather than casting, so the display and the signing path can no longer drift apart. `Tabs.extrinsicSign` also rejects a payload carrying a truthy `data`, since that field has no meaning on that channel. Also branches the signing render on the request rather than on decoded state. Paging between two queued requests of different shapes could otherwise commit one frame using the previous request's decoded payload. Tests added under `signer channel` in `Extension.spec.ts`. --- .../src/background/RequestBytesSign.ts | 5 +- .../src/background/RequestExtrinsicSign.ts | 5 +- .../src/background/handlers/Extension.spec.ts | 75 +++++++++++++++++++ .../src/background/handlers/Extension.ts | 12 ++- .../src/background/handlers/Tabs.ts | 4 + .../extension-base/src/background/types.ts | 20 ++++- packages/extension-base/src/utils/index.ts | 1 + .../src/utils/isExtrinsicRequest.ts | 8 ++ .../src/Popup/Signing/Request/index.tsx | 51 +++++++------ .../src/Popup/Signing/Signing.spec.tsx | 3 + .../extension-ui/src/Popup/Signing/index.tsx | 6 +- 11 files changed, 151 insertions(+), 39 deletions(-) create mode 100644 packages/extension-base/src/utils/isExtrinsicRequest.ts diff --git a/packages/extension-base/src/background/RequestBytesSign.ts b/packages/extension-base/src/background/RequestBytesSign.ts index f732672c..1f9c5a24 100644 --- a/packages/extension-base/src/background/RequestBytesSign.ts +++ b/packages/extension-base/src/background/RequestBytesSign.ts @@ -5,11 +5,12 @@ import type { KeyringPair } from '@polkadot/keyring/types'; import type { TypeRegistry } from '@polkadot/types'; import type { SignerPayloadRaw } from '@polkadot/types/types'; import type { HexString } from '@polkadot/util/types'; -import type { RequestSign } from './types.js'; +import type { RequestSignBytes } from './types.js'; import { u8aToHex, u8aWrapBytes } from '@polkadot/util'; -export default class RequestBytesSign implements RequestSign { +export default class RequestBytesSign implements RequestSignBytes { + public readonly channel = 'bytes' as const; public readonly payload: SignerPayloadRaw; constructor (payload: SignerPayloadRaw) { diff --git a/packages/extension-base/src/background/RequestExtrinsicSign.ts b/packages/extension-base/src/background/RequestExtrinsicSign.ts index 156fed88..a36f00d0 100644 --- a/packages/extension-base/src/background/RequestExtrinsicSign.ts +++ b/packages/extension-base/src/background/RequestExtrinsicSign.ts @@ -5,9 +5,10 @@ import type { KeyringPair } from '@polkadot/keyring/types'; import type { TypeRegistry } from '@polkadot/types'; import type { SignerPayloadJSON } from '@polkadot/types/types'; import type { HexString } from '@polkadot/util/types'; -import type { RequestSign } from './types.js'; +import type { RequestSignExtrinsic } from './types.js'; -export default class RequestExtrinsicSign implements RequestSign { +export default class RequestExtrinsicSign implements RequestSignExtrinsic { + public readonly channel = 'extrinsic' as const; public readonly payload: SignerPayloadJSON; constructor (payload: SignerPayloadJSON) { diff --git a/packages/extension-base/src/background/handlers/Extension.spec.ts b/packages/extension-base/src/background/handlers/Extension.spec.ts index d13f286b..68b759cb 100644 --- a/packages/extension-base/src/background/handlers/Extension.spec.ts +++ b/packages/extension-base/src/background/handlers/Extension.spec.ts @@ -15,6 +15,7 @@ import type { KeypairType } from '@polkadot/util-crypto/types'; import { TypeRegistry } from '@polkadot/types'; import keyring from '@polkadot/ui-keyring'; +import { stringToHex } from '@polkadot/util'; import { cryptoWaitReady } from '@polkadot/util-crypto'; import { AccountsStore } from '../../stores/index.js'; @@ -475,4 +476,78 @@ describe('Extension', () => { expect(res).toEqual(true); }); }); + + describe('signer channel', () => { + let address: string, payload: SignerPayloadJSON; + + beforeEach(async () => { + address = await createAccount(); + payload = { + address, + blockHash: '0xe1b1dda72998846487e4d858909d4f9a6bbd6e338e4588e5d809de16b1317b80', + blockNumber: '0x00000393', + era: '0x3601', + genesisHash: '0x242a54b35e1aad38f37b884eddeb71f6f9931b02fac27bf52dfb62ef754e5e62', + method: '0x040105fa8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a4882380100', + nonce: '0x0000000000000000', + signedExtensions: ['CheckSpecVersion', 'CheckTxVersion', 'CheckGenesis', 'CheckMortality', 'CheckNonce', 'CheckWeight', 'ChargeTransactionPayment'], + specVersion: '0x00000026', + tip: '0x00000000000000000000000000000000', + transactionVersion: '0x00000005', + version: 4 + }; + }); + + // `data` has no meaning on the extrinsic channel and does not affect what is + // signed, so it must not reach the UI and steer how the request is shown. + it('rejects a signPayload request carrying raw data', async () => { + const { blockNumber: _omitted, ...rest } = payload; + + await expect(tabs.handle('1615191860871.6', 'pub(extrinsic.sign)', { + ...rest, + data: stringToHex('Sign in to EvilDapp') + } as unknown as SignerPayloadJSON, 'http://localhost:3000', {} as chrome.runtime.Port)) + .rejects.toThrow(/Unexpected raw data/); + }); + + // a falsy `data` never selected the message view, so it must keep signing + it('accepts a signPayload request with a falsy data field', async () => { + // eslint-disable-next-line jest/valid-expect-in-promise + tabs.handle('1615191860871.7', 'pub(extrinsic.sign)', { + ...payload, + data: null + } as unknown as SignerPayloadJSON, 'http://localhost:3000', {} as chrome.runtime.Port) + .catch((err) => console.log(err)); + + const queued = state.allSignRequests[state.allSignRequests.length - 1]; + + const res = await extension.handle('1615192062290.7', 'pri(signing.approve.password)', { + id: queued.id, + password, + savePass: false + }, {} as chrome.runtime.Port); + + expect(res).toEqual(true); + }); + + // The UI reads this instead of sniffing the payload, so a request cannot be + // displayed as one kind and signed as the other. + it('tags a request with the channel it arrived on', async () => { + // eslint-disable-next-line jest/valid-expect-in-promise + tabs.handle('1615191860871.8', 'pub(extrinsic.sign)', payload, 'http://localhost:3000', {} as chrome.runtime.Port) + .catch((err) => console.log(err)); + + const queued = state.allSignRequests[state.allSignRequests.length - 1]; + + expect(queued.request.channel).toEqual('extrinsic'); + + const res = await extension.handle('1615192062290.8', 'pri(signing.approve.password)', { + id: queued.id, + password, + savePass: false + }, {} as chrome.runtime.Port); + + expect(res).toEqual(true); + }); + }); }); diff --git a/packages/extension-base/src/background/handlers/Extension.ts b/packages/extension-base/src/background/handlers/Extension.ts index ccbd9f6e..0d0e34c2 100644 --- a/packages/extension-base/src/background/handlers/Extension.ts +++ b/packages/extension-base/src/background/handlers/Extension.ts @@ -5,7 +5,7 @@ import type { MetadataDef } from '@polkadot/extension-inject/types'; import type { KeyringPair, KeyringPair$Json, KeyringPair$Meta } from '@polkadot/keyring/types'; -import type { Registry, SignerPayloadJSON, SignerPayloadRaw } from '@polkadot/types/types'; +import type { Registry } from '@polkadot/types/types'; import type { SubjectInfo } from '@polkadot/ui-keyring/observable/types'; import type { KeypairType } from '@polkadot/util-crypto/types'; import type { AccountJson, AllowedPath, AuthorizeRequest, MessageTypes, MetadataRequest, RequestAccountBatchExport, RequestAccountChangePassword, RequestAccountCreateExternal, RequestAccountCreateHardware, RequestAccountCreateSuri, RequestAccountEdit, RequestAccountExport, RequestAccountForget, RequestAccountShow, RequestAccountTie, RequestAccountValidate, RequestActiveTabsUrlUpdate, RequestAuthorizeApprove, RequestBatchRestore, RequestDeriveCreate, RequestDeriveValidate, RequestJsonRestore, RequestMetadataApprove, RequestMetadataReject, RequestSeedCreate, RequestSeedValidate, RequestSigningApprovePassword, RequestSigningApproveSignature, RequestSigningCancel, RequestSigningIsLocked, RequestTypes, RequestUpdateAuthorizedAccounts, ResponseAccountExport, ResponseAccountsExport, ResponseAuthorizeList, ResponseDeriveValidate, ResponseJsonGetAccountInfo, ResponseSeedCreate, ResponseSeedValidate, ResponseSigningIsLocked, ResponseType, SigningRequest } from '../types.js'; @@ -20,6 +20,7 @@ import { accounts as accountsObservable } from '@polkadot/ui-keyring/observable/ import { assert, isHex } from '@polkadot/util'; import { keyExtractSuri, mnemonicGenerate, mnemonicValidate } from '@polkadot/util-crypto'; +import { isExtrinsicRequest } from '../../utils/index.js'; import { withErrorLog } from './helpers.js'; import { createSubscription, unsubscribe } from './subscriptions.js'; @@ -35,10 +36,6 @@ function getSuri (seed: string, type?: KeypairType): string { : seed; } -function isJsonPayload (value: SignerPayloadJSON | SignerPayloadRaw): value is SignerPayloadJSON { - return (value as SignerPayloadJSON).genesisHash !== undefined; -} - export default class Extension { readonly #cachedUnlocks: CachedUnlocks; @@ -368,9 +365,10 @@ export default class Extension { // construct a new registry (avoiding pollution), between requests let registry: Registry; - const { payload } = request; - if (isJsonPayload(payload)) { + if (isExtrinsicRequest(request)) { + const payload = request.payload; + // Get the metadata for the genesisHash const metadata = this.#state.knownMetadata.find(({ genesisHash }) => genesisHash === payload.genesisHash); diff --git a/packages/extension-base/src/background/handlers/Tabs.ts b/packages/extension-base/src/background/handlers/Tabs.ts index 79096244..bbd3b82c 100644 --- a/packages/extension-base/src/background/handlers/Tabs.ts +++ b/packages/extension-base/src/background/handlers/Tabs.ts @@ -141,6 +141,10 @@ export default class Tabs { } private extrinsicSign (url: string, request: SignerPayloadJSON): Promise { + // matches the predicate the UI used to key off, so payloads that were + // never ambiguous (absent, null, empty) keep working + assert(!(request as unknown as SignerPayloadRaw).data, 'Unexpected raw data in a signPayload payload'); + const address = request.address; const pair = this.getSigningPair(address); diff --git a/packages/extension-base/src/background/types.ts b/packages/extension-base/src/background/types.ts index b707ecf6..a5a0f57c 100644 --- a/packages/extension-base/src/background/types.ts +++ b/packages/extension-base/src/background/types.ts @@ -398,12 +398,26 @@ export type SubscriptionMessageTypes = NoUndefinedValues<{ export type MessageTypesWithSubscriptions = keyof SubscriptionMessageTypes; export type MessageTypesWithNoSubscriptions = Exclude -export interface RequestSign { - readonly payload: SignerPayloadJSON | SignerPayloadRaw; - +interface RequestSignBase { sign (registry: Registry, pair: KeyringPair): { signature: HexString }; } +// `channel` is set by the background from the message type the request arrived +// on, never derived from the payload - the payload comes from the dapp and +// cannot be trusted to describe what will actually be signed. Discriminating on +// it also correlates the payload type, so consumers narrow instead of casting. +export interface RequestSignBytes extends RequestSignBase { + readonly channel: 'bytes'; + readonly payload: SignerPayloadRaw; +} + +export interface RequestSignExtrinsic extends RequestSignBase { + readonly channel: 'extrinsic'; + readonly payload: SignerPayloadJSON; +} + +export type RequestSign = RequestSignBytes | RequestSignExtrinsic; + export interface RequestJsonRestore { file: KeyringPair$Json; password: string; diff --git a/packages/extension-base/src/utils/index.ts b/packages/extension-base/src/utils/index.ts index 1bc96ad3..b024b852 100644 --- a/packages/extension-base/src/utils/index.ts +++ b/packages/extension-base/src/utils/index.ts @@ -2,3 +2,4 @@ // SPDX-License-Identifier: Apache-2.0 export { canDerive } from './canDerive.js'; +export { isExtrinsicRequest } from './isExtrinsicRequest.js'; diff --git a/packages/extension-base/src/utils/isExtrinsicRequest.ts b/packages/extension-base/src/utils/isExtrinsicRequest.ts new file mode 100644 index 00000000..2c87c954 --- /dev/null +++ b/packages/extension-base/src/utils/isExtrinsicRequest.ts @@ -0,0 +1,8 @@ +// Copyright 2019-2026 @polkadot/extension-base authors & contributors +// SPDX-License-Identifier: Apache-2.0 + +import type { RequestSign, RequestSignExtrinsic } from '../background/types.js'; + +export function isExtrinsicRequest (request: RequestSign): request is RequestSignExtrinsic { + return request.channel === 'extrinsic'; +} diff --git a/packages/extension-ui/src/Popup/Signing/Request/index.tsx b/packages/extension-ui/src/Popup/Signing/Request/index.tsx index df91772e..7ba56c3c 100644 --- a/packages/extension-ui/src/Popup/Signing/Request/index.tsx +++ b/packages/extension-ui/src/Popup/Signing/Request/index.tsx @@ -3,11 +3,11 @@ import type { AccountJson, RequestSign } from '@polkadot/extension-base/background/types'; import type { ExtrinsicPayload } from '@polkadot/types/interfaces'; -import type { SignerPayloadJSON, SignerPayloadRaw } from '@polkadot/types/types'; import type { HexString } from '@polkadot/util/types'; import React, { useCallback, useContext, useEffect, useState } from 'react'; +import { isExtrinsicRequest } from '@polkadot/extension-base/utils'; import { TypeRegistry } from '@polkadot/types'; import { ActionContext, Address, VerticalSpace, Warning } from '../../../components/index.js'; @@ -39,18 +39,16 @@ export const CMD_SIGN_MESSAGE = 3; // keep it global, we can and will re-use this across requests const registry = new TypeRegistry(); -function isRawPayload (payload: SignerPayloadJSON | SignerPayloadRaw): payload is SignerPayloadRaw { - return !!(payload as SignerPayloadRaw).data; -} - export default function Request ({ account: { accountIndex, addressOffset, genesisHash: accountGenesisHash, isExternal, isHardware, type }, buttonText, isFirst, request, signId, url }: Props): React.ReactElement | null { const onAction = useContext(ActionContext); const [{ hexBytes, payload }, setData] = useState({ hexBytes: null, payload: null }); const [error, setError] = useState(null); const { t } = useTranslation(); + // Raw vs extrinsic follows the channel the request arrived on, never the + // payload fields - those are dapp-supplied and can describe either shape. // Use payload genesis for transaction-signing flow. Account genesis can be null // for allow-any accounts and should not drive payload decoding/signing setup. - const payloadGenesisHash = !isRawPayload(request.payload) + const payloadGenesisHash = isExtrinsicRequest(request) ? request.payload.genesisHash : null; const chain = useMetadata(payloadGenesisHash); @@ -58,25 +56,25 @@ export default function Request ({ account: { accountIndex, addressOffset, genes useEffect((): void => { // When the chain and request are ready, configure the chain's registry. // This will be picked up by LedgerSign. - if (chain && !isRawPayload(request.payload)) { + if (chain && isExtrinsicRequest(request)) { chain.registry.setSignedExtensions(request.payload.signedExtensions, chain.definition.userExtensions); } }, [chain, request]); useEffect((): void => { - const payload = request.payload; + if (isExtrinsicRequest(request)) { + const json = request.payload; - if (isRawPayload(payload)) { - setData({ - hexBytes: payload.data, - payload: null - }); - } else { - registry.setSignedExtensions(payload.signedExtensions); + registry.setSignedExtensions(json.signedExtensions); setData({ hexBytes: null, - payload: registry.createType('ExtrinsicPayload', payload, { version: payload.version }) + payload: registry.createType('ExtrinsicPayload', json, { version: json.version }) + }); + } else { + setData({ + hexBytes: request.payload.data, + payload: null }); } }, [request]); @@ -93,8 +91,15 @@ export default function Request ({ account: { accountIndex, addressOffset, genes [onAction, signId] ); - if (payload !== null) { - const json = request.payload as SignerPayloadJSON; + // Branch on the request itself rather than on the decoded state, so a render + // that lands before the effect has caught up shows nothing instead of feeding + // the previous request's view a payload of the other shape. + if (isExtrinsicRequest(request)) { + const json = request.payload; + + if (payload === null) { + return null; + } return ( <> @@ -147,8 +152,12 @@ export default function Request ({ account: { accountIndex, addressOffset, genes /> ); - } else if (hexBytes !== null) { - const { address, data } = request.payload as SignerPayloadRaw; + } else { + const { address, data } = request.payload; + + if (hexBytes === null) { + return null; + } return ( <> @@ -197,6 +206,4 @@ export default function Request ({ account: { accountIndex, addressOffset, genes ); } - - return null; } diff --git a/packages/extension-ui/src/Popup/Signing/Signing.spec.tsx b/packages/extension-ui/src/Popup/Signing/Signing.spec.tsx index b3d220ff..a69e5d30 100644 --- a/packages/extension-ui/src/Popup/Signing/Signing.spec.tsx +++ b/packages/extension-ui/src/Popup/Signing/Signing.spec.tsx @@ -95,6 +95,7 @@ describe('Signing requests', () => { }, id: '1607347015530.2', request: { + channel: 'extrinsic', payload: { address: '5D4bqjQRPgdMBK8bNvhX4tSuCtSGZS7rZjD5XH5SoKcFeKn5', blockHash: '0x661f57d206d4fecda0408943427d4d25436518acbff543735e7569da9db6bdd7', @@ -132,6 +133,7 @@ describe('Signing requests', () => { }, id: '1607356155395.3', request: { + channel: 'extrinsic', payload: { address: '5Ggap6soAPaP5UeNaiJsgqQwdVhhNnm6ez7Ba1w9jJ62LM2Q', blockHash: '0xcf69b7935b785f90b22d2b36f2227132ef9c5dd33db1dbac9ecdafac05bf9476', @@ -221,6 +223,7 @@ describe('Signing requests', () => { }, id: '1607357806151.5', request: { + channel: 'extrinsic', payload: { address: '5Cf1CGZas62RWwce3d2EPqUvSoi1txaXKd9M5w9bEFSsQtRe', blockHash: '0xd2f2dfb56c16af1d0faf5b454153d3199aeb6647537f4161c26a34541c591ec8', diff --git a/packages/extension-ui/src/Popup/Signing/index.tsx b/packages/extension-ui/src/Popup/Signing/index.tsx index bc01d7cb..8c2a6157 100644 --- a/packages/extension-ui/src/Popup/Signing/index.tsx +++ b/packages/extension-ui/src/Popup/Signing/index.tsx @@ -1,10 +1,10 @@ // Copyright 2019-2026 @polkadot/extension-ui authors & contributors // SPDX-License-Identifier: Apache-2.0 -import type { SignerPayloadJSON } from '@polkadot/types/types'; - import React, { useCallback, useContext, useEffect, useState } from 'react'; +import { isExtrinsicRequest } from '@polkadot/extension-base/utils'; + import { Loading, SigningReqContext } from '../../components/index.js'; import { useTranslation } from '../../hooks/index.js'; import { Header } from '../../partials/index.js'; @@ -42,7 +42,7 @@ export default function Signing (): React.ReactElement { : requests[requests.length - 1] : requests[0] : null; - const isTransaction = !!((request?.request?.payload as SignerPayloadJSON)?.blockNumber); + const isTransaction = !!request && isExtrinsicRequest(request.request); return request ? (