diff --git a/extension/src/popup/components/__tests__/Operations.test.tsx b/extension/src/popup/components/__tests__/Operations.test.tsx index a7082ca3fc..2d74b5d4d9 100644 --- a/extension/src/popup/components/__tests__/Operations.test.tsx +++ b/extension/src/popup/components/__tests__/Operations.test.tsx @@ -2,6 +2,7 @@ import React from "react"; import { render, waitFor, screen } from "@testing-library/react"; import { Address, + Asset, Operation, OperationRecord, xdr, @@ -346,4 +347,100 @@ describe("Operations", () => { expect(screen.getByText("Issuer")).toBeDefined(); }); }); + + describe("value-bearing operations show the asset issuer", () => { + const renderOp = (op: Operation) => + render( + + + , + ); + + const valueOf = (label: string) => + screen + .getByText(label) + .parentNode?.querySelector("[data-testid='OperationKeyVal__value']"); + + // Regression test for HackerOne #3768317: the signing UI must show the + // issuer for value-bearing assets so a counterfeit USDC: is + // distinguishable from a non-native asset using the same code. + it("renders the issuer for the non-native asset in a manageSellOffer", async () => { + const op = { + offerId: "0", + selling: Asset.native(), + buying: new Asset("USDC", TEST_PUBLIC_KEY), + amount: "5000", + price: "1", + type: "manageSellOffer", + } as Operation.ManageSellOffer; + + renderOp(op); + + await waitFor(() => screen.getAllByTestId("OperationKeyVal")); + + expect(valueOf("Selling")).toHaveTextContent("XLM"); + expect(valueOf("Buying")).toHaveTextContent("USDC"); + + // Native XLM has no issuer, so exactly one issuer row is rendered, and it + // carries the (truncated, copyable) issuer of the non-native buying asset. + const issuerRows = screen.getAllByText("Asset Issuer"); + expect(issuerRows).toHaveLength(1); + expect(valueOf("Asset Issuer")).toHaveTextContent("GBTY…JZOF"); + }); + + it("renders the issuer for a payment of a non-native asset", async () => { + const op = { + destination: TEST_PUBLIC_KEY, + asset: new Asset("USDC", TEST_PUBLIC_KEY), + amount: "100", + type: "payment", + } as Operation.Payment; + + renderOp(op); + + await waitFor(() => screen.getAllByTestId("OperationKeyVal")); + + // master renamed the payment asset-code row to "Token Code"; the issuer + // row (added here) is unaffected by that rename. + expect(valueOf("Token Code")).toHaveTextContent("USDC"); + expect(valueOf("Asset Issuer")).toHaveTextContent("GBTY…JZOF"); + }); + + it("does not render an issuer row for a native (XLM) payment", async () => { + const op = { + destination: TEST_PUBLIC_KEY, + asset: Asset.native(), + amount: "100", + type: "payment", + } as Operation.Payment; + + renderOp(op); + + await waitFor(() => screen.getAllByTestId("OperationKeyVal")); + + expect(valueOf("Token Code")).toHaveTextContent("XLM"); + expect(screen.queryByText("Asset Issuer")).toBeNull(); + }); + }); }); diff --git a/extension/src/popup/components/signTransaction/Operations/__tests__/index.test.tsx b/extension/src/popup/components/signTransaction/Operations/__tests__/index.test.tsx new file mode 100644 index 0000000000..c09a5f1126 --- /dev/null +++ b/extension/src/popup/components/signTransaction/Operations/__tests__/index.test.tsx @@ -0,0 +1,233 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; +import { Provider } from "react-redux"; +import { + Account, + Asset, + BASE_FEE, + Networks, + Operation, + OperationRecord, + StrKey, + TransactionBuilder, + xdr, +} from "stellar-sdk"; + +import { makeDummyStore } from "popup/__testHelpers__"; +import { Operations } from "../index"; + +// setOptions never triggers the asset scanner, but mock it so the component's +// effect can never reach the network in the test environment. +jest.mock("popup/helpers/blockaid", () => ({ + scanAsset: jest.fn().mockResolvedValue(undefined), +})); + +// Build a setOptions transaction with the bundled SDK, serialize it to XDR and +// decode it back — the exact path Freighter uses to obtain the operation object +// it renders on the signing-approval screen. A present-but-zero Uint32 field +// (e.g. masterWeight: 0) decodes to the JS number 0. +// Valid ed25519 strkeys minted from fixed bytes — avoids curve math (and the +// crypto RNG, which is unavailable in this test environment). +const SOURCE_KEY = StrKey.encodeEd25519PublicKey(Buffer.alloc(32, 1)); +const ADDED_SIGNER = StrKey.encodeEd25519PublicKey(Buffer.alloc(32, 7)); + +type SetOptionsOptions = Parameters[0]; + +const decodeOperation = (operation: xdr.Operation) => { + const account = new Account(SOURCE_KEY, "0"); + const tx = new TransactionBuilder(account, { + fee: BASE_FEE, + networkPassphrase: Networks.TESTNET, + }) + .addOperation(operation) + .setTimeout(0) + .build(); + + return TransactionBuilder.fromXDR(tx.toXDR(), Networks.TESTNET) + .operations as Operation[]; +}; + +const decodeSetOptions = (options: SetOptionsOptions) => + decodeOperation(Operation.setOptions(options)); + +const renderOps = (operations: Operation[]) => + render( + + + , + ); + +// Read the value rendered next to a given operation-detail label. +const rowValue = (key: string) => { + const keyEl = screen + .getAllByTestId("OperationKeyVal__key") + .find((el) => el.textContent === key); + return keyEl?.parentElement + ?.querySelector('[data-testid="OperationKeyVal__value"]') + ?.textContent?.trim(); +}; + +const MASTER_KEY_WARNING = /disables your account's master key/i; + +describe("Operations — setOptions field visibility", () => { + it("decoder yields numeric 0 (falsy) for masterWeight/thresholds", () => { + const [op] = decodeSetOptions({ + masterWeight: 0, + lowThreshold: 0, + medThreshold: 0, + highThreshold: 0, + }) as any[]; + + expect(op.masterWeight).toBe(0); + expect(op.lowThreshold).toBe(0); + expect(op.medThreshold).toBe(0); + expect(op.highThreshold).toBe(0); + }); + + it("renders masterWeight 0, zeroed thresholds, and a signer change, with a master-key warning", () => { + renderOps( + decodeSetOptions({ + masterWeight: 0, + lowThreshold: 0, + medThreshold: 0, + highThreshold: 0, + signer: { ed25519PublicKey: ADDED_SIGNER, weight: 1 }, + }), + ); + + expect(screen.getByText("Set Options")).toBeInTheDocument(); + expect(screen.getByText("Signer")).toBeInTheDocument(); + expect(rowValue("Master Weight")).toBe("0"); + expect(rowValue("High Threshold")).toBe("0"); + expect(rowValue("Medium Threshold")).toBe("0"); + expect(rowValue("Low Threshold")).toBe("0"); + expect(screen.getByText(MASTER_KEY_WARNING)).toBeInTheDocument(); + }); + + it("renders masterWeight 0 on its own with the warning, never an empty operation", () => { + renderOps(decodeSetOptions({ masterWeight: 0 })); + + expect(rowValue("Master Weight")).toBe("0"); + expect(screen.getByText(MASTER_KEY_WARNING)).toBeInTheDocument(); + }); + + it("non-zero masterWeight/threshold render and do not warn", () => { + renderOps(decodeSetOptions({ masterWeight: 2, highThreshold: 3 })); + + expect(rowValue("Master Weight")).toBe("2"); + expect(rowValue("High Threshold")).toBe("3"); + expect(screen.queryByText(MASTER_KEY_WARNING)).not.toBeInTheDocument(); + }); + + it("HOME DOMAIN: clearing the home domain is surfaced as a status badge, not hidden", () => { + renderOps(decodeSetOptions({ homeDomain: "" })); + + expect(rowValue("Home Domain")).toBe("Cleared"); + expect(screen.getByText("Cleared").closest(".Badge")).not.toBeNull(); + }); + + it("FLAGS: a single-bit setFlags decodes to its label", () => { + renderOps(decodeSetOptions({ setFlags: 1 })); + + expect(rowValue("Set Flags")).toBe("Authorization Required"); + }); + + it("FLAGS: a combined setFlags bitmask decodes every set bit, not a blank value", () => { + // REVOCABLE (2) | CLAWBACK (8) = 10. The SDK types setFlags as a single + // AuthFlag, but the wire format is a bitmask — cast to exercise that. + renderOps(decodeSetOptions({ setFlags: 10 as any })); + + expect(rowValue("Set Flags")).toBe( + "Authorization Revocable, Authorization Clawback Enabled", + ); + }); + + it("FLAGS: a known bit combined with an unrecognized bit surfaces both", () => { + // REQUIRED (1) | future-bit (16) = 17 — the unknown bit must not be hidden. + renderOps(decodeSetOptions({ setFlags: 17 as any })); + + // The mocked t() does not interpolate, so {{bits}} stays literal here. + expect(rowValue("Set Flags")).toBe( + "Authorization Required, Unknown ({{bits}})", + ); + }); + + it("FLAGS: a combined clearFlags bitmask decodes every set bit", () => { + // REQUIRED (1) | REVOCABLE (2) = 3 + renderOps(decodeSetOptions({ clearFlags: 3 as any })); + + expect(rowValue("Clear Flags")).toBe( + "Authorization Required, Authorization Revocable", + ); + }); +}); + +describe("Operations — manageData value visibility", () => { + it("renders a set value", () => { + renderOps( + decodeOperation(Operation.manageData({ name: "k", value: "hi" })), + ); + + expect(rowValue("Value")).toBe("hi"); + }); + + it("renders the Value row for an empty value rather than hiding it", () => { + renderOps(decodeOperation(Operation.manageData({ name: "k", value: "" }))); + + expect(screen.getByText("Value")).toBeInTheDocument(); + expect(rowValue("Value")).toBe(""); + }); + + it("surfaces a deletion as a status badge when value is absent ", () => { + renderOps( + decodeOperation(Operation.manageData({ name: "k", value: null })), + ); + + expect(rowValue("Value")).toBe("Deleted"); + expect(screen.getByText("Deleted").closest(".Badge")).not.toBeNull(); + }); +}); + +describe("Operations — setTrustLineFlags visibility", () => { + const TRUSTOR = StrKey.encodeEd25519PublicKey(Buffer.alloc(32, 3)); + const ASSET = new Asset( + "USDC", + StrKey.encodeEd25519PublicKey(Buffer.alloc(32, 9)), + ); + + const decodeSetTrustLineFlags = (flags: { + authorized?: boolean; + authorizedToMaintainLiabilities?: boolean; + clawbackEnabled?: boolean; + }) => + decodeOperation( + Operation.setTrustLineFlags({ trustor: TRUSTOR, asset: ASSET, flags }), + ); + + it("renders a flag being enabled", () => { + renderOps(decodeSetTrustLineFlags({ authorized: true })); + + expect(rowValue("Authorized")).toBe("Enabled"); + }); + + it("renders a flag being cleared (set to false), not hidden", () => { + renderOps(decodeSetTrustLineFlags({ authorized: false })); + + expect(rowValue("Authorized")).toBe("Disabled"); + }); + + it("does not render a flag that is left unchanged", () => { + renderOps(decodeSetTrustLineFlags({ clawbackEnabled: false })); + + expect( + screen + .getAllByTestId("OperationKeyVal__key") + .some((el) => el.textContent === "Authorized"), + ).toBe(false); + expect(rowValue("Clawback Enabled")).toBe("Disabled"); + }); +}); diff --git a/extension/src/popup/components/signTransaction/Operations/index.tsx b/extension/src/popup/components/signTransaction/Operations/index.tsx index 7cc43c19b9..09b2573809 100644 --- a/extension/src/popup/components/signTransaction/Operations/index.tsx +++ b/extension/src/popup/components/signTransaction/Operations/index.tsx @@ -1,5 +1,5 @@ import React, { useEffect } from "react"; -import { Icon, IconButton } from "@stellar/design-system"; +import { Badge, Icon, IconButton } from "@stellar/design-system"; import { useSelector } from "react-redux"; import { useTranslation } from "react-i18next"; import { OperationRecord, Signer, xdr } from "stellar-sdk"; @@ -16,6 +16,7 @@ import { settingsNetworkDetailsSelector } from "popup/ducks/settings"; import { truncateString, truncatedPoolId } from "helpers/stellar"; import { scanAsset } from "popup/helpers/blockaid"; import { addressToString, getCreateContractArgs } from "popup/helpers/soroban"; +import { CopyValue } from "popup/components/CopyValue"; import { KeyValueClaimants, KeyValueInvokeHostFn, @@ -52,6 +53,35 @@ const MemoRequiredWarning = ({ ) : null; }; +// A neutral gray pill used for "cleared"/"deleted" states in the technical +// operation view — easier to scan at a glance than a parenthetical string. +const StatusBadge = ({ children }: { children: string }) => ( + + {children} + +); + +const MasterKeyDisableWarning = () => { + const { t } = useTranslation(); + + // Rendered as a full-width banner (not a KeyValueList row) so the message + // wraps instead of being truncated in the right-aligned value column. + return ( +
+
+ ); +}; + const DestinationWarning = ({ destination, flaggedKeys, @@ -75,6 +105,24 @@ const DestinationWarning = ({ ); }; +// On Stellar a non-native asset is identified by (code, issuer), not by code +// alone, so the signing UI must show the issuer for any value-bearing asset to +// let the user distinguish a legitimate asset from a look-alike using the same +// code. Native XLM has no issuer, so nothing is rendered for it. Mirrors the +// issuer row already shown by changeTrust (KeyValueLine) and PathList. +const KeyValueAssetIssuer = ({ issuer }: { issuer?: string }) => { + const { t } = useTranslation(); + + return issuer ? ( + + } + /> + ) : null; +}; + export const Operations = ({ flaggedKeys, isMemoRequired, @@ -101,6 +149,27 @@ export const Operations = ({ "8": "Authorization Clawback Enabled", }; + // Account flags are a bitmask, so a combined value (e.g. REVOCABLE | + // CLAWBACK = 10) is not a key in AuthorizationMapToDisplay. Decode each known + // bit individually so combined flags are never rendered as a blank value, and + // surface any remaining (unrecognized) bits so a future protocol flag isn't + // silently hidden when combined with a known one. + const decodeAuthorizationFlags = (bits: number) => { + const labels: string[] = []; + let remaining = bits; + Object.entries(AuthorizationMapToDisplay).forEach(([bit, label]) => { + const value = Number(bit); + if ((bits & value) !== 0) { + labels.push(t(label)); + remaining &= ~value; + } + }); + if (remaining !== 0) { + labels.push(t("Unknown ({{bits}})", { bits: remaining })); + } + return labels.join(", "); + }; + const RenderOpByType = ({ op }: { op: OperationRecord }) => { const networkDetails = useSelector(settingsNetworkDetailsSelector); @@ -184,6 +253,7 @@ export const Operations = ({ operationKey={t("Token Code")} operationValue={asset.code} /> + + + + + + + ); @@ -288,10 +364,12 @@ export const Operations = ({ operationKey={t("Selling")} operationValue={selling.code} /> + + @@ -310,6 +388,7 @@ export const Operations = ({ operationKey={t("Buying")} operationValue={buying.code} /> + + ); @@ -348,48 +428,53 @@ export const Operations = ({ operationValue={inflationDest} /> )} - {homeDomain && ( + {homeDomain !== undefined && ( {t("Cleared")} + ) : ( + homeDomain + ) + } /> )} - {highThreshold && ( + {highThreshold !== undefined && ( )} - {medThreshold && ( + {medThreshold !== undefined && ( )} - {lowThreshold && ( + {lowThreshold !== undefined && ( )} - {masterWeight && ( + {masterWeight !== undefined && ( )} - {setFlags && ( + {masterWeight === 0 && } + {setFlags !== undefined && ( )} - {clearFlags && ( + {clearFlags !== undefined && ( )} @@ -446,15 +531,23 @@ export const Operations = ({ case "manageData": { const { name, value } = op; + // A null/undefined value means the data entry is being deleted; an + // empty value decodes to a zero-length buffer. Always render the row so + // a deletion is never silently hidden from the approval screen. + const isDeletingEntry = value === undefined || value === null; return ( <> - {value && ( - - )} + {t("Deleted")} + ) : ( + value.toString() + ) + } + /> ); } @@ -474,6 +567,7 @@ export const Operations = ({ operationKey={t("Asset Code")} operationValue={asset.code} /> + @@ -548,22 +642,34 @@ export const Operations = ({ operationKey={t("Asset Code")} operationValue={asset.code} /> - {flags.authorized && ( + {/* + A flag present in the decoded `flags` object is being changed: + `true` enables it, `false` *clears* it. Use a presence check so a + cleared flag is never hidden, and render the value explicitly — a + raw boolean is not rendered by React. + */} + {flags.authorized !== undefined && ( )} - {flags.authorizedToMaintainLiabilities && ( + {flags.authorizedToMaintainLiabilities !== undefined && ( )} - {flags.clawbackEnabled && ( + {flags.clawbackEnabled !== undefined && ( )} diff --git a/extension/src/popup/components/signTransaction/Operations/styles.scss b/extension/src/popup/components/signTransaction/Operations/styles.scss index b0a2b7954e..7f631d4c79 100644 --- a/extension/src/popup/components/signTransaction/Operations/styles.scss +++ b/extension/src/popup/components/signTransaction/Operations/styles.scss @@ -137,4 +137,30 @@ flex-direction: column; } } + + &__warning { + display: flex; + align-items: flex-start; + gap: pxToRem(8px); + margin-top: pxToRem(4px); + padding: pxToRem(12px); + border-radius: pxToRem(12px); + line-height: 1.5rem; + color: var(--sds-clr-red-11); + background-color: var(--sds-clr-red-03); + border: 1px solid var(--sds-clr-red-06); + + svg { + flex-shrink: 0; + width: pxToRem(16px); + height: pxToRem(16px); + margin-top: pxToRem(4px); + } + + span { + min-width: 0; + white-space: normal; + overflow-wrap: anywhere; + } + } } diff --git a/extension/src/popup/locales/en/translation.json b/extension/src/popup/locales/en/translation.json index 169c44fea1..8738bf6304 100644 --- a/extension/src/popup/locales/en/translation.json +++ b/extension/src/popup/locales/en/translation.json @@ -81,6 +81,10 @@ "Asset not found": "Asset not found", "asset options": "asset options", "At the end of this process, Freighter will only display accounts related to the new backup phrase.": "At the end of this process, Freighter will only display accounts related to the new backup phrase.", + "Authorization Clawback Enabled": "Authorization Clawback Enabled", + "Authorization Immutable": "Authorization Immutable", + "Authorization Required": "Authorization Required", + "Authorization Revocable": "Authorization Revocable", "Authorizations": "Authorizations", "Authorize": "Authorize", "Authorized address": "Authorized address", @@ -113,6 +117,7 @@ "Clear Flags": "Clear Flags", "Clear Override": "Clear Override", "Clear recents": "Clear recents", + "Cleared": "Cleared", "Close": "Close", "Coinbase Logo": "Coinbase Logo", "Collectible": "Collectible", @@ -172,6 +177,7 @@ "Debug": "Debug", "Debug menu is only available in development mode.": "Debug menu is only available in development mode.", "Default": "Default", + "Deleted": "Deleted", "Description": "Description", "Destination": "Destination", "Destination account does not accept this asset": "Destination account does not accept this asset", @@ -666,6 +672,7 @@ "This token was flagged as suspicious": "This token was flagged as suspicious", "This token was flagged as suspicious (override active)": "This token was flagged as suspicious (override active)", "This transaction could not be completed.": "This transaction could not be completed.", + "This transaction disables your account's master key. You may permanently lose access to this account unless another signer with sufficient weight is added.": "This transaction disables your account's master key. You may permanently lose access to this account unless another signer with sufficient weight is added.", "This transaction does not appear safe for the following reasons": "This transaction does not appear safe for the following reasons", "This transaction does not appear safe for the following reasons.": "This transaction does not appear safe for the following reasons.", "This transaction is expected to fail": "This transaction is expected to fail", @@ -729,6 +736,7 @@ "Unable to scan transaction": "Unable to scan transaction", "Unable to sign out": "Unable to sign out", "Unexpected Error": "Unexpected Error", + "Unknown ({{bits}})": "Unknown ({{bits}})", "Unknown error occured": "Unknown error occured", "Unlock": "Unlock", "Unsupported signing method": "Unsupported signing method", diff --git a/extension/src/popup/locales/pt/translation.json b/extension/src/popup/locales/pt/translation.json index b19ef57bc4..ec33df5f74 100644 --- a/extension/src/popup/locales/pt/translation.json +++ b/extension/src/popup/locales/pt/translation.json @@ -81,6 +81,10 @@ "Asset not found": "Ativo não encontrado", "asset options": "opções de ativo", "At the end of this process, Freighter will only display accounts related to the new backup phrase.": "No final deste processo, o Freighter exibirá apenas contas relacionadas à nova frase de backup.", + "Authorization Clawback Enabled": "Recuperação de Autorização Habilitada (Clawback)", + "Authorization Immutable": "Autorização Imutável", + "Authorization Required": "Autorização Obrigatória", + "Authorization Revocable": "Autorização Revogável", "Authorizations": "Autorizações", "Authorize": "Autorizar", "Authorized address": "Endereço autorizado", @@ -113,6 +117,7 @@ "Clear Flags": "Limpar Flags", "Clear Override": "Limpar Substituição", "Clear recents": "Limpar recentes", + "Cleared": "Limpo", "Close": "Fechar", "Coinbase Logo": "Logo Coinbase", "Collectible": "Colecionável", @@ -172,6 +177,7 @@ "Debug": "Depuração", "Debug menu is only available in development mode.": "O menu de depuração está disponível apenas no modo de desenvolvimento.", "Default": "Padrão", + "Deleted": "Excluído", "Description": "Descrição", "Destination": "Destino", "Destination account does not accept this asset": "A conta de destino não aceita este ativo", @@ -666,6 +672,7 @@ "This token was flagged as suspicious": "Este token foi marcado como suspeito", "This token was flagged as suspicious (override active)": "Este token foi sinalizado como suspeito (substituição ativa)", "This transaction could not be completed.": "Esta transação não pôde ser concluída.", + "This transaction disables your account's master key. You may permanently lose access to this account unless another signer with sufficient weight is added.": "Esta transação desativa a chave mestra da sua conta. Você pode perder permanentemente o acesso a esta conta, a menos que outro assinante com peso suficiente seja adicionado.", "This transaction does not appear safe for the following reasons": "Esta transação não parece segura pelos seguintes motivos", "This transaction does not appear safe for the following reasons.": "Esta transação não parece segura pelos seguintes motivos.", "This transaction is expected to fail": "Esta transação deve falhar", @@ -729,6 +736,7 @@ "Unable to scan transaction": "Não foi possível verificar a transação", "Unable to sign out": "Não foi possível sair", "Unexpected Error": "Erro Inesperado", + "Unknown ({{bits}})": "Desconhecido ({{bits}})", "Unknown error occured": "Erro desconhecido ocorreu", "Unlock": "Desbloquear", "Unsupported signing method": "Método de assinatura não suportado",