Merge pull request #47 from szhygulin/refactor/reduce-size

refactor + audit hardening: boilerplate dedup + 6 security fixes

Author: szhygulin

Dockerfile

@@ -12,4 +12,10 @@ ENV NODE_ENV=production
 COPY package.json package-lock.json ./
 RUN npm ci --omit=dev
 COPY --from=builder /app/dist ./dist
+# The node:alpine image ships a pre-created unprivileged `node` user/group.
+# Running as root gives a compromise inside the process write access to the
+# whole container filesystem; dropping to `node` keeps the blast radius
+# confined to /app and /tmp. No network/USB privileges are needed — TRON
+# signing runs on the host, this image is for EVM-only read surfaces.
+USER node
 ENTRYPOINT ["node", "dist/index.js"]

src/modules/execution/index.ts

@@ -1,4 +1,4 @@
-import { encodeFunctionData, formatUnits, parseEther, parseUnits } from "viem";
+import { encodeFunctionData, formatUnits, isAddress, parseEther, parseUnits } from "viem";
 import qrcodeTerminal from "qrcode-terminal";
 import {
   initiatePairing,
@@ -266,10 +266,29 @@ export async function prepareEigenLayerDeposit(args: PrepareEigenLayerDepositArg
 
 // ----- Native + ERC-20 transfers -----
 
+/**
+ * Accept recipient addresses that are either all-lowercase hex (no checksum
+ * intent) or valid EIP-55 checksummed. Reject mixed-case with a wrong
+ * checksum — that is the class of error where a user pasted an address with
+ * a single-character case typo; viem's bare `as 0x${string}` cast would
+ * otherwise pass it through silently. viem's `isAddress(x, { strict: true })`
+ * encodes exactly this policy.
+ */
+function assertRecipient(addr: string): `0x${string}` {
+  if (!isAddress(addr, { strict: true })) {
+    throw new Error(
+      `Invalid recipient address ${addr}: failed EIP-55 checksum or malformed hex. ` +
+        `If you pasted a mixed-case address, a single-character case typo is the most ` +
+        `likely cause — re-check the source.`,
+    );
+  }
+  return addr as `0x${string}`;
+}
+
 export async function prepareNativeSend(args: PrepareNativeSendArgs): Promise<UnsignedTx> {
   const wallet = args.wallet as `0x${string}`;
   const chain = args.chain as SupportedChain;
-  const to = args.to as `0x${string}`;
+  const to = assertRecipient(args.to);
   const value = parseEther(args.amount);
   return enrichTx({
     chain,
@@ -286,7 +305,7 @@ export async function prepareTokenSend(args: PrepareTokenSendArgs): Promise<Unsi
   const wallet = args.wallet as `0x${string}`;
   const chain = args.chain as SupportedChain;
   const token = args.token as `0x${string}`;
-  const to = args.to as `0x${string}`;
+  const to = assertRecipient(args.to);
   const meta = await resolveTokenMeta(chain, token);
 
   let amountWei: bigint;
@@ -523,9 +542,13 @@ async function runEvmPreSignGuards(tx: UnsignedTx): Promise<void> {
  * appears. `send_transaction` then reads the stashed pin verbatim and
  * forwards it through WalletConnect, so the on-device hash is deterministic.
  *
- * Re-entrant: calling `previewSend` twice on the same handle overwrites the
- * prior pin. This is intentional — if the user pauses for minutes, gas
- * conditions drift and a fresh pin (with a fresh hash) is the right fix.
+ * Re-entrant with an explicit opt-in: calling `previewSend` a second time on
+ * the same handle returns the existing pin verbatim. Pass `refresh: true` to
+ * re-pin (e.g. if the user paused for minutes and wants fresh fees). Without
+ * this guard, a buggy or adversarial agent could silently swap the pre-sign
+ * hash between the moment the user reads it in chat and the moment Ledger
+ * displays it — the hash-match UX would still catch the change, but the
+ * guard makes the default deterministic.
  */
 export async function previewSend(args: PreviewSendArgs): Promise<{
   handle: string;
@@ -539,6 +562,7 @@ export async function previewSend(args: PreviewSendArgs): Promise<{
     maxPriorityFeePerGas: string;
     gas: string;
   };
+  refreshed?: boolean;
 }> {
   if (hasTronHandle(args.handle)) {
     throw new Error(
@@ -548,6 +572,22 @@ export async function previewSend(args: PreviewSendArgs): Promise<{
     );
   }
   const tx = consumeHandle(args.handle);
+  const existing = getPinnedGas(args.handle);
+  if (existing && !args.refresh) {
+    return {
+      handle: args.handle,
+      chain: tx.chain,
+      to: tx.to,
+      valueWei: tx.value,
+      preSignHash: existing.preSignHash,
+      pinned: {
+        nonce: existing.nonce,
+        maxFeePerGas: existing.maxFeePerGas.toString(),
+        maxPriorityFeePerGas: existing.maxPriorityFeePerGas.toString(),
+        gas: existing.gas.toString(),
+      },
+    };
+  }
   await runEvmPreSignGuards(tx);
   const from =
     tx.from ?? ((await getConnectedAccounts())[0] as `0x${string}` | undefined);
@@ -588,6 +628,7 @@ export async function previewSend(args: PreviewSendArgs): Promise<{
       maxPriorityFeePerGas: pinned.maxPriorityFeePerGas.toString(),
       gas: pinned.gas.toString(),
     },
+    ...(existing ? { refreshed: true } : {}),
   };
 }
 

src/modules/execution/schemas.ts

@@ -132,7 +132,17 @@ export const previewSendInput = z.object({
         "returns the LEDGER BLIND-SIGN HASH block so the user can see and confirm the " +
         "hash BEFORE the Ledger device prompt appears. A follow-up send_transaction " +
         "call forwards the pinned fields verbatim. Handles expire 15 minutes after " +
-        "prepare; a fresh preview_send call can be made to refresh the pin if fees drift."
+        "prepare. Once a pin exists, re-calling preview_send on the same handle returns " +
+        "the existing pin unchanged unless `refresh: true` is passed."
+    ),
+  refresh: z
+    .boolean()
+    .optional()
+    .describe(
+      "Set to true to re-pin nonce/fees/gas (e.g. after the user paused for minutes and " +
+        "wants fresh fees). Default is false: the existing pin and its pre-sign hash are " +
+        "returned verbatim, so the hash the user matched in chat cannot silently drift " +
+        "between preview and send."
     ),
 });
 

src/modules/swap/index.ts

@@ -334,6 +334,26 @@ export async function prepareSwap(args: PrepareSwapArgs): Promise<UnsignedTx> {
     );
   }
 
+  // Exact-in invariant: the approval amount and the swap tx's transferFrom target
+  // are both derived from `quote.action.fromAmount`, while the preview text
+  // (description, decoded.args) echoes the user's `args.amount`. If LiFi returns
+  // a `fromAmount` different from what we asked for, those two values drift and
+  // the user signs bytes that pull more (or less) than the MCP preview shows.
+  // The existing `toUsd / fromUsd > 10` gate is asymmetric and does not catch
+  // proportional inflation. Refuse on any drift — we literally passed
+  // `fromAmount` in, any different value in the response is hostile or buggy.
+  if (!isExactOut) {
+    const quotedFromWei = BigInt(quote.action.fromAmount);
+    if (quotedFromWei !== BigInt(amountWei)) {
+      throw new Error(
+        `LiFi returned fromAmount=${quotedFromWei} for an exact-in quote of ${amountWei} ` +
+          `(${args.amount} ${quote.action.fromToken.symbol}). The approval and swap bytes ` +
+          `would pull a different amount than the MCP preview displays — refusing to return ` +
+          `calldata. Re-run get_swap_quote.`
+      );
+    }
+  }
+
   // Sanity-check the quote before returning signable calldata. LiFi has been observed
   // returning toAmount scaled wrong on certain aggregator integrations (e.g. 10 USDC →
   // ~4500 ETH). The calldata embeds the bogus minOut and won't execute, but we refuse

src/setup.ts

@@ -240,7 +240,11 @@ async function pairLedgerLiveFlow(p: Prompt): Promise<void> {
       resolve();
     })
   );
-  console.log(`URI: ${pairing.uri}\n`);
+  // The URI carries the WalletConnect topic + a one-shot symmetric key. It
+  // isn't long-lived key material, but anyone who pairs with it first wins
+  // the session, so keep it out of long-lived logs and terminal scrollback
+  // shared with others.
+  console.log(`URI (sensitive — don't share; one-time pairing secret): ${pairing.uri}\n`);
   console.log("Waiting for you to approve the session in Ledger Live (Ctrl-C to cancel)...");
 
   try {

src/signing/tron-usb-signer.ts

@@ -153,6 +153,30 @@ async function openTronApp() {
   return { app, transport, appVersion };
 }
 
+/**
+ * HID handles are exclusive — two concurrent attempts to open the Ledger USB
+ * transport race and the loser sees "cannot open device". That's only a DoS
+ * today (never a wrong-tx-signed), but it's a noisy failure the moment two
+ * MCP tools run in parallel (e.g. `get_ledger_status` refreshes while a sign
+ * is in flight). Serialize all transport-using calls through a module-local
+ * queue so a second caller waits for the first to finish closing instead of
+ * failing.
+ */
+let usbLock: Promise<void> = Promise.resolve();
+async function withUsbLock<T>(fn: () => Promise<T>): Promise<T> {
+  const prev = usbLock;
+  let release!: () => void;
+  usbLock = new Promise<void>((resolve) => {
+    release = resolve;
+  });
+  try {
+    await prev;
+    return await fn();
+  } finally {
+    release();
+  }
+}
+
 /**
  * Query the device for its TRON address at `path`. Used by `pair_ledger_tron`
  * to cache the address for subsequent sign calls, and as the identity check
@@ -161,21 +185,23 @@ async function openTronApp() {
 export async function getTronLedgerAddress(
   path: string = DEFAULT_TRON_PATH
 ): Promise<{ address: string; publicKey: string; path: string; appVersion: string }> {
-  const { app, transport, appVersion } = await openTronApp();
-  try {
-    const { address, publicKey } = await app.getAddress(path, false);
-    if (!isTronAddress(address)) {
-      throw new Error(
-        `Ledger returned an address that doesn't look like a TRON mainnet address: "${address}". ` +
-          `Is the TRON (not Tron-classic / testnet) app open on the device?`
-      );
+  return withUsbLock(async () => {
+    const { app, transport, appVersion } = await openTronApp();
+    try {
+      const { address, publicKey } = await app.getAddress(path, false);
+      if (!isTronAddress(address)) {
+        throw new Error(
+          `Ledger returned an address that doesn't look like a TRON mainnet address: "${address}". ` +
+            `Is the TRON (not Tron-classic / testnet) app open on the device?`
+        );
+      }
+      return { address, publicKey, path, appVersion };
+    } catch (e) {
+      throw mapLedgerError(e, "getAddress");
+    } finally {
+      await transport.close().catch(() => {});
     }
-    return { address, publicKey, path, appVersion };
-  } catch (e) {
-    throw mapLedgerError(e, "getAddress");
-  } finally {
-    await transport.close().catch(() => {});
-  }
+  });
 }
 
 export interface TronSignRequest {
@@ -206,31 +232,33 @@ export async function signTronTxOnLedger(
   req: TronSignRequest
 ): Promise<{ signature: string; signerAddress: string }> {
   const path = req.path ?? DEFAULT_TRON_PATH;
-  const { app, transport } = await openTronApp();
-  try {
-    const { address } = await app.getAddress(path, false);
-    if (address !== req.expectedFrom) {
-      throw new Error(
-        `Ledger device address (${address}) does not match the prepared tx's \`from\` ` +
-          `(${req.expectedFrom}). Either connect the Ledger that holds keys for \`from\`, ` +
-          `or re-prepare the tx for the Ledger-derived address (\`pair_ledger_tron\`).`
-      );
-    }
-    const signature = await app.signTransaction(
-      path,
-      req.rawDataHex,
-      req.tokenSignatures ?? []
-    );
-    // Ledger returns the signature as a hex string (65 bytes: r || s || v).
-    if (!/^[0-9a-fA-F]{130}$/.test(signature)) {
-      throw new Error(
-        `Ledger returned an unexpected signature shape (length ${signature.length}). Expected 130 hex chars.`
+  return withUsbLock(async () => {
+    const { app, transport } = await openTronApp();
+    try {
+      const { address } = await app.getAddress(path, false);
+      if (address !== req.expectedFrom) {
+        throw new Error(
+          `Ledger device address (${address}) does not match the prepared tx's \`from\` ` +
+            `(${req.expectedFrom}). Either connect the Ledger that holds keys for \`from\`, ` +
+            `or re-prepare the tx for the Ledger-derived address (\`pair_ledger_tron\`).`
+        );
+      }
+      const signature = await app.signTransaction(
+        path,
+        req.rawDataHex,
+        req.tokenSignatures ?? []
       );
+      // Ledger returns the signature as a hex string (65 bytes: r || s || v).
+      if (!/^[0-9a-fA-F]{130}$/.test(signature)) {
+        throw new Error(
+          `Ledger returned an unexpected signature shape (length ${signature.length}). Expected 130 hex chars.`
+        );
+      }
+      return { signature, signerAddress: address };
+    } catch (e) {
+      throw mapLedgerError(e, "signTransaction");
+    } finally {
+      await transport.close().catch(() => {});
     }
-    return { signature, signerAddress: address };
-  } catch (e) {
-    throw mapLedgerError(e, "signTransaction");
-  } finally {
-    await transport.close().catch(() => {});
-  }
+  });
 }