fix: manually address PR feedback

Author: mathieuartu

packages/keyring-eth-trezor/jest.config.js

@@ -23,10 +23,10 @@ module.exports = merge(baseConfig, {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 61.18,
-      functions: 93.24,
-      lines: 93.59,
-      statements: 93.68,
+      branches: 62.65,
+      functions: 93.15,
+      lines: 93.57,
+      statements: 93.66,
     },
   },
 });

packages/keyring-eth-trezor/src/trezor-keyring-v2.test.ts

@@ -519,15 +519,25 @@ describe('TrezorKeyringV2', () => {
       ).rejects.toThrow(/Invalid derivation path/u);
     });
 
-    it('throws error for unsupported HD path', async () => {
+    it('throws error for unsupported HD path prefix', async () => {
       const { wrapper } = createEmptyWrapper();
 
-      // Valid format but unsupported base path for Trezor
+      // Valid format but unsupported base path (coin type 100 instead of 60 or 1)
       await expect(
         wrapper.createAccounts(derivePathOptions(`m/44'/100'/0'/0/0`)),
       ).rejects.toThrow(/Invalid derivation path/u);
     });
 
+    it('throws error for valid format but disallowed HD path', async () => {
+      const { wrapper } = createEmptyWrapper();
+
+      // Path matches the regex (starts with m/44'/60') but the base path
+      // m/44'/60'/1'/0 is not in ALLOWED_HD_PATHS
+      await expect(
+        wrapper.createAccounts(derivePathOptions(`m/44'/60'/1'/0/0`)),
+      ).rejects.toThrow(/Invalid derivation path/u);
+    });
+
     it('throws error for entropy source mismatch with derive-path', async () => {
       const { wrapper } = createEmptyWrapper();
 

packages/keyring-eth-trezor/src/trezor-keyring-v2.ts

@@ -43,23 +43,40 @@ const trezorKeyringV2Capabilities: KeyringCapabilities = {
  */
 const BIP44_HD_PATH_PREFIX = `m/44'/60'/0'/0`;
 
+/**
+ * SLIP-0044 testnet HD path prefix constant.
+ */
+const SLIP0044_TESTNET_PATH = `m/44'/1'/0'/0`;
+
+/**
+ * Legacy MEW (MyEtherWallet) HD path prefix constant.
+ */
+const LEGACY_MEW_PATH = `m/44'/60'/0'`;
+
 /**
  * Allowed HD paths for Trezor keyring.
  * These must match the keys in ALLOWED_HD_PATHS from trezor-keyring.ts.
  */
-type AllowedHdPath = `m/44'/60'/0'/0` | `m/44'/60'/0'` | `m/44'/1'/0'/0`;
+const ALLOWED_HD_PATHS = [
+  BIP44_HD_PATH_PREFIX,
+  SLIP0044_TESTNET_PATH,
+  LEGACY_MEW_PATH,
+] as const;
+
+/**
+ * Type representing one of the allowed Trezor HD paths.
+ * Used by inner.setHdPath which expects one of these specific paths.
+ */
+type AllowedHdPath = (typeof ALLOWED_HD_PATHS)[number];
 
 /**
  * Regex pattern for validating and parsing Trezor derivation paths.
- * Only matches the allowed Trezor HD paths from the legacy keyring:
- * - m/44'/60'/0'/0/{index} (BIP44 standard)
- * - m/44'/60'/0'/{index} (legacy MEW)
- * - m/44'/1'/0'/0/{index} (SLIP0044 testnet)
- *
+ * Matches BIP-44 style paths: m/44'/{coin}'/{segments}/{index}
+ * where coin is 60' (Ethereum) or 1' (testnet).
  * Captures: [1] = base path prefix, [2] = index
+ * The prefix is then validated against ALLOWED_HD_PATHS.
  */
-const DERIVATION_PATH_PATTERN =
-  /^(m\/44'\/60'\/0'\/0|m\/44'\/60'\/0'|m\/44'\/1'\/0'\/0)\/(\d+)$/u;
+const DERIVATION_PATH_PATTERN = /^(m\/44'\/(?:60'|1')(?:\/\d+'?)*)\/(\d+)$/u;
 
 /**
  * Concrete {@link KeyringV2} adapter for {@link TrezorKeyring}.
@@ -144,18 +161,29 @@ export class TrezorKeyringV2
     index: number;
   } {
     const match = derivationPath.match(DERIVATION_PATH_PATTERN);
-    if (!match) {
+    if (!match?.[1] || !match[2]) {
+      throw new Error(
+        `Invalid derivation path: ${derivationPath}. ` +
+          `Expected format: {base}/{index} where base is one of: ` +
+          `${ALLOWED_HD_PATHS.join(', ')}.`,
+      );
+    }
+
+    const basePath = match[1];
+    const index = parseInt(match[2], 10);
+
+    // Validate the base path is one of the allowed paths
+    if (!ALLOWED_HD_PATHS.includes(basePath as AllowedHdPath)) {
       throw new Error(
         `Invalid derivation path: ${derivationPath}. ` +
           `Expected format: {base}/{index} where base is one of: ` +
-          `m/44'/60'/0'/0, m/44'/60'/0', m/44'/1'/0'/0.`,
+          `${ALLOWED_HD_PATHS.join(', ')}.`,
       );
     }
 
-    // The regex guarantees match[1] is one of the allowed HD paths and match[2] is an index
     return {
-      basePath: match[1] as AllowedHdPath,
-      index: parseInt(match[2] as string, 10),
+      basePath: basePath as AllowedHdPath,
+      index,
     };
   }