fix(signer): reject a non-integer or negative VECTOR_SIGNER_ACCOUNT_INDEX

Number(env.VECTOR_SIGNER_ACCOUNT_INDEX ?? '0') was never validated. Verified
against the real walletFromSeed: a NaN-producing value, a negative integer,
and a fractional value each derived a DIFFERENT, syntactically valid
addr1... wallet with no error anywhere in the chain, and describe() would
echo the nonsense value back as if nothing were wrong ("mnemonic (account
index NaN)"). Spend limits three lines away already fail loud on the same
shape of mistake (BigInt(...) throws on garbage) -- this applies that same
instinct here: parseAccountIndex() rejects anything that is not a
non-negative integer, naming the variable and the offending value.

Only called from the two MnemonicKeySource-constructing branches, not
unconditionally -- PrivateKeyKeySource never reads an account index, so a
leftover garbage VECTOR_SIGNER_ACCOUNT_INDEX must not block a
private-key-configured signer that never uses it. Pinned by its own test.

config.test.ts covers: NaN-producing input, a negative integer, a fractional
value, a valid value (cross-checked against a directly-constructed
MnemonicKeySource for the same wallet), the unset default, and the
PrivateKeyKeySource non-interference case. Verified by mutation (temporarily
disabling the guard) that exactly the three invalid-input tests fail and the
other three are unaffected -- see the task report.

Final whole-branch review, Fix 2 of 4.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

packages/signer/src/config.ts

@@ -35,13 +35,11 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): SignerConfig {
 }
 
 function resolveKeySource(env: NodeJS.ProcessEnv): KeySource {
-  const accountIndex = Number(env.VECTOR_SIGNER_ACCOUNT_INDEX ?? '0');
-
   if (env.VECTOR_SIGNER_PRIVATE_KEY && env.VECTOR_SIGNER_ADDRESS) {
     return new PrivateKeyKeySource(env.VECTOR_SIGNER_PRIVATE_KEY, env.VECTOR_SIGNER_ADDRESS);
   }
   if (env.VECTOR_SIGNER_MNEMONIC) {
-    return new MnemonicKeySource(env.VECTOR_SIGNER_MNEMONIC, accountIndex);
+    return new MnemonicKeySource(env.VECTOR_SIGNER_MNEMONIC, parseAccountIndex(env.VECTOR_SIGNER_ACCOUNT_INDEX));
   }
   if (env.VECTOR_SIGNER_MNEMONIC_FILE) {
     let contents: string;
@@ -53,7 +51,7 @@ function resolveKeySource(env: NodeJS.ProcessEnv): KeySource {
         `Could not read VECTOR_SIGNER_MNEMONIC_FILE (${env.VECTOR_SIGNER_MNEMONIC_FILE}): ${err instanceof Error ? err.message : String(err)}`
       );
     }
-    return new MnemonicKeySource(contents, accountIndex);
+    return new MnemonicKeySource(contents, parseAccountIndex(env.VECTOR_SIGNER_ACCOUNT_INDEX));
   }
 
   throw new Error(
@@ -61,3 +59,32 @@ function resolveKeySource(env: NodeJS.ProcessEnv): KeySource {
       'VECTOR_SIGNER_MNEMONIC, or VECTOR_SIGNER_PRIVATE_KEY together with VECTOR_SIGNER_ADDRESS.'
   );
 }
+
+// Number(x) returns NaN for garbage input rather than throwing, and nothing
+// downstream re-validates it: MnemonicKeySource's constructor takes
+// accountIndex on faith (its own validation is about the mnemonic, not this
+// argument), and walletFromSeed's HD derivation does not throw on a NaN,
+// negative, or fractional index either -- it just walks a different (but
+// still syntactically valid-looking) branch of the tree. A single env-var
+// typo would otherwise silently start the signer against a different
+// wallet, with no error anywhere in the chain and describe() printing the
+// nonsense value back out as if nothing were wrong (e.g. "mnemonic (account
+// index NaN)"). This file's own spend limits, a few lines up, already fail
+// loud on the same shape of mistake (BigInt(...) throws on non-numeric
+// input) -- this applies that same instinct here.
+//
+// Only called from the two MnemonicKeySource-constructing branches above,
+// not unconditionally: PrivateKeyKeySource never uses an account index (see
+// its own doc comment in keysource.ts), so a leftover/garbage
+// VECTOR_SIGNER_ACCOUNT_INDEX must not block startup for a config that never
+// reads it.
+function parseAccountIndex(raw: string | undefined): number {
+  const source = raw ?? '0';
+  const accountIndex = Number(source);
+  if (!Number.isInteger(accountIndex) || accountIndex < 0) {
+    throw new Error(
+      `Invalid VECTOR_SIGNER_ACCOUNT_INDEX: expected a non-negative integer, got "${source}".`
+    );
+  }
+  return accountIndex;
+}

packages/signer/test/config.test.ts

@@ -0,0 +1,77 @@
+import { describe, test } from 'node:test';
+import assert from 'node:assert/strict';
+import { loadConfig } from '../src/config.ts';
+import { MnemonicKeySource } from '../src/keysource.ts';
+
+// Throwaway test mnemonic, shared with keysource.test.ts and sign.test.ts.
+// Controls no funds. Never use outside tests.
+const TEST_MNEMONIC =
+  'test walk nut penalty hip pave soap entry language right filter choice';
+
+function envWithMnemonic(accountIndex?: string): NodeJS.ProcessEnv {
+  const env: NodeJS.ProcessEnv = { VECTOR_SIGNER_MNEMONIC: TEST_MNEMONIC };
+  if (accountIndex !== undefined) env.VECTOR_SIGNER_ACCOUNT_INDEX = accountIndex;
+  return env;
+}
+
+// VECTOR_SIGNER_ACCOUNT_INDEX used to go straight into `Number(...)` with no
+// validation anywhere downstream. Number() returns NaN for garbage input
+// rather than throwing; MnemonicKeySource's constructor takes accountIndex
+// on faith (its own validation is about the mnemonic, not this argument);
+// and CML's HD derivation (walletFromSeed) does not throw on a NaN,
+// negative, or fractional index either — it just walks a different branch
+// of the tree. Verified directly against the real walletFromSeed before
+// this fix (see the task report): a NaN-producing string, a negative
+// integer, and a fractional value each derived a DIFFERENT, syntactically
+// valid addr1... address, with no error anywhere in the chain — the
+// "confident wrong answer" shape, not a crash. describe() would even echo
+// the nonsense value back as if nothing were wrong (e.g. "mnemonic (account
+// index NaN)"). These tests pin the fix: anything that is not a
+// non-negative integer must throw, loudly, naming both the variable and the
+// offending value.
+describe('loadConfig — VECTOR_SIGNER_ACCOUNT_INDEX validation', () => {
+  test('a non-numeric value (Number(...) produces NaN) is rejected, not silently treated as account 0', () => {
+    assert.throws(
+      () => loadConfig(envWithMnemonic('not-a-number')),
+      /VECTOR_SIGNER_ACCOUNT_INDEX.*not-a-number/
+    );
+  });
+
+  test('a negative integer is rejected', () => {
+    assert.throws(() => loadConfig(envWithMnemonic('-1')), /VECTOR_SIGNER_ACCOUNT_INDEX.*-1/);
+  });
+
+  test('a fractional value is rejected', () => {
+    assert.throws(() => loadConfig(envWithMnemonic('1.5')), /VECTOR_SIGNER_ACCOUNT_INDEX.*1\.5/);
+  });
+
+  test('a valid non-negative integer is accepted and derives the SAME wallet as constructing MnemonicKeySource directly', () => {
+    // Not just "does not throw" — confirms the validated value actually
+    // reaches derivation unchanged, by cross-checking against an
+    // independently constructed MnemonicKeySource for the same index.
+    const viaConfig = loadConfig(envWithMnemonic('2')).keySource.load();
+    const direct = new MnemonicKeySource(TEST_MNEMONIC, 2).load();
+    assert.equal(viaConfig.address, direct.address);
+    assert.equal(viaConfig.privateKeyBech32, direct.privateKeyBech32);
+  });
+
+  test('an unset VECTOR_SIGNER_ACCOUNT_INDEX still defaults to account 0, unaffected by the new validation', () => {
+    const viaConfig = loadConfig(envWithMnemonic()).keySource.load();
+    const direct = new MnemonicKeySource(TEST_MNEMONIC, 0).load();
+    assert.equal(viaConfig.address, direct.address);
+  });
+
+  test('a garbage VECTOR_SIGNER_ACCOUNT_INDEX does not block a PRIVATE_KEY-configured signer, since that path never reads it', () => {
+    // parseAccountIndex() is only called from the two MnemonicKeySource
+    // branches, not unconditionally at the top of resolveKeySource — a
+    // leftover/mistyped VECTOR_SIGNER_ACCOUNT_INDEX in the environment must
+    // not newly block startup for a config that never uses it.
+    const derived = new MnemonicKeySource(TEST_MNEMONIC).load();
+    const env: NodeJS.ProcessEnv = {
+      VECTOR_SIGNER_PRIVATE_KEY: derived.privateKeyBech32,
+      VECTOR_SIGNER_ADDRESS: derived.address,
+      VECTOR_SIGNER_ACCOUNT_INDEX: 'not-a-number',
+    };
+    assert.doesNotThrow(() => loadConfig(env));
+  });
+});