From c27aa5f63451c17de1075b49db11258a1c39f917 Mon Sep 17 00:00:00 2001 From: Quoc Le Date: Fri, 23 Oct 2026 10:02:21 +0700 Subject: [PATCH 1/2] verify the passphrase on every unlock and bound the ttl --- src/commands/wallet.js | 10 ++-- src/config/constants.js | 1 + src/daemon/server.js | 39 +++++++++++--- src/ui/parsers.js | 18 +++++++ tests/integration/wallet.test.js | 14 ++++- tests/unit/daemon/server.test.js | 90 +++++++++++++++++++++++++++++++- 6 files changed, 155 insertions(+), 17 deletions(-) diff --git a/src/commands/wallet.js b/src/commands/wallet.js index 68e476f..dd33b61 100644 --- a/src/commands/wallet.js +++ b/src/commands/wallet.js @@ -19,13 +19,13 @@ import { KeyService } from '../services/key-service.js' import { WalletKeyring } from '../security/keyring.js' import { daemonClient } from '../daemon/client.js' import { configService } from '../services/config-service.js' -import { SESSION_TTL_MINUTES, getWalletDir } from '../config/constants.js' +import { SESSION_TTL_MINUTES, MAX_SESSION_TTL_MINUTES, getWalletDir } from '../config/constants.js' import { WdkCliError, ErrorCode, handleError } from '../errors/index.js' import { promptPassphrase, promptSeedPhrase, readLineFromStdin } from '../ui/prompts.js' import { requirePassphraseConfirmation } from '../ui/auth.js' import { configureHelp } from '../ui/help.js' import { createTable } from '../ui/tables.js' -import { nonNegativeInt } from '../ui/parsers.js' +import { sessionTtlMinutes } from '../ui/parsers.js' /** @typedef {import('commander').Command} Command */ @@ -383,8 +383,8 @@ export function registerWalletCommand (program) { .requiredOption('--name ', 'Wallet name') .option( '--ttl ', - 'Session duration in minutes (0 = unlimited)', - nonNegativeInt, + `Session duration in minutes, up to ${MAX_SESSION_TTL_MINUTES} (0 = unlimited)`, + sessionTtlMinutes, SESSION_TTL_MINUTES ) @@ -393,7 +393,7 @@ export function registerWalletCommand (program) { options: [ { flags: '--ttl ', - description: `Session duration in minutes, 0 = unlimited (default: ${SESSION_TTL_MINUTES})` + description: `Session duration in minutes, up to ${MAX_SESSION_TTL_MINUTES}, 0 = unlimited (default: ${SESSION_TTL_MINUTES})` } ] }) diff --git a/src/config/constants.js b/src/config/constants.js index 5eb6b4b..60bba52 100644 --- a/src/config/constants.js +++ b/src/config/constants.js @@ -49,6 +49,7 @@ export function getConfigDir () { } export const SESSION_TTL_MINUTES = 5 +export const MAX_SESSION_TTL_MINUTES = 7 * 24 * 60 export const DAEMON_SOCKET = 'daemon.sock' export const DAEMON_PID = 'daemon.pid' export const DAEMON_MAX_REQUEST_BYTES = 64 * 1024 diff --git a/src/daemon/server.js b/src/daemon/server.js index 45de790..7587ec9 100644 --- a/src/daemon/server.js +++ b/src/daemon/server.js @@ -22,6 +22,7 @@ import { getDaemonPidPath, getWalletPath, SESSION_TTL_MINUTES, + MAX_SESSION_TTL_MINUTES, DAEMON_MAX_REQUEST_BYTES } from '../config/constants.js' import { configService } from '../services/config-service.js' @@ -62,6 +63,20 @@ import { formatAmount } from '../ui/formatters.js' /** Max length of a summarized third-party error message before truncation. */ const MAX_ERROR_LEN = 200 +/** + * Reports whether a requested session TTL is one the daemon will honour: whole + * minutes, from 0 (no expiry) to `MAX_SESSION_TTL_MINUTES`. + * + * @param {unknown} ttl - The TTL the caller asked for, in minutes. + * @returns {boolean} True when the TTL is in range. + */ +function isValidTtl (ttl) { + return typeof ttl === 'number' && + Number.isInteger(ttl) && + ttl >= 0 && + ttl <= MAX_SESSION_TTL_MINUTES +} + /** * Builds a failure DaemonResponse that preserves the error code across IPC. * Picks up `code` and `suggestion` from WdkCliError, plus `code` from @@ -152,21 +167,17 @@ export class WalletDaemon { } /** - * Synchronously decrypts and loads a wallet into the in-memory session map. + * Synchronously decrypts and loads a wallet into the in-memory session map, + * or extends the session when that wallet is already unlocked. The + * passphrase is verified either way. * * @param {string} name - The wallet name. * @param {string} passphrase - The wallet passphrase. * @param {number} ttlMinutes - The session TTL in minutes (0 = no expiry). * @returns {void} + * @throws {WdkCliError} WRONG_PASSPHRASE when the passphrase does not decrypt the seed. */ #unlockWalletSync (name, passphrase, ttlMinutes) { - // If already unlocked, just reset the timer - const existing = this.#wallets.get(name) - if (existing) { - this.#resetTimer(name, ttlMinutes) - return - } - const walletPath = getWalletPath(name) const data = readFileSync(walletPath, 'utf8') const payload = JSON.parse(data) @@ -179,6 +190,12 @@ export class WalletDaemon { } catch { throw new WdkCliError('Incorrect passphrase.', ErrorCode.WRONG_PASSPHRASE) } + + if (this.#wallets.has(name)) { + this.#resetTimer(name, ttlMinutes) + return + } + // Buffer (not the immutable mnemonic string) so the seed can be zeroed on lock. const seedBuffer = mnemonicToSeedSync(seedPhrase) const wdk = new WdkService() @@ -341,6 +358,12 @@ export class WalletDaemon { if (req.passphrase == null) { return { ok: false, error: 'Missing passphrase' } } + if (req.ttl != null && !isValidTtl(req.ttl)) { + return { + ok: false, + error: `Invalid ttl: ${req.ttl}. Give whole minutes from 0 to ${MAX_SESSION_TTL_MINUTES} (0 = unlimited).` + } + } try { const ttl = req.ttl ?? SESSION_TTL_MINUTES this.#unlockWalletSync(wallet, req.passphrase, ttl) diff --git a/src/ui/parsers.js b/src/ui/parsers.js index f7a91ef..7876e12 100644 --- a/src/ui/parsers.js +++ b/src/ui/parsers.js @@ -15,6 +15,7 @@ import { readFileSync } from 'node:fs' import { InvalidArgumentError } from 'commander' import BigNumber from 'bignumber.js' +import { MAX_SESSION_TTL_MINUTES } from '../config/constants.js' import { WdkCliError, ErrorCode } from '../errors/index.js' /** @@ -45,6 +46,23 @@ export function nonNegativeInt (value) { return n } +/** + * Commander argParser for an unlock session TTL in minutes, bounded by + * `MAX_SESSION_TTL_MINUTES`. 0 means the session never expires. + * + * @param {string} value - The raw CLI argument. + * @returns {number} The validated TTL in minutes. + */ +export function sessionTtlMinutes (value) { + const n = Number(value) + if (!Number.isInteger(n) || n < 0 || n > MAX_SESSION_TTL_MINUTES) { + throw new InvalidArgumentError( + `Must be a whole number of minutes from 0 to ${MAX_SESSION_TTL_MINUTES} (0 = unlimited).` + ) + } + return n +} + /** * Commander argParser for a transaction finality target. * diff --git a/tests/integration/wallet.test.js b/tests/integration/wallet.test.js index 1741703..474f1f3 100644 --- a/tests/integration/wallet.test.js +++ b/tests/integration/wallet.test.js @@ -180,7 +180,7 @@ describe('wallet unlock and the session it opens', () => { // The ttl decides how long a decrypted seed stays in memory, so a value that // cannot be honoured has to be rejected rather than rounded into something. - it.each(['-5', 'abc'])('refuses a ttl of %p', async (ttl) => { + it.each(['-5', 'abc', '10081'])('refuses a ttl of %p', async (ttl) => { await cli.json(['wallet', 'create', '--name', 'w1'], { unlocked: true }) const result = await cli.run( @@ -189,10 +189,20 @@ describe('wallet unlock and the session it opens', () => { expect(result.code).toBe(1) expect(result.output).toContain( - `option '--ttl ' argument '${ttl}' is invalid. Must be a non-negative integer.` + `option '--ttl ' argument '${ttl}' is invalid. ` + + 'Must be a whole number of minutes from 0 to 10080 (0 = unlimited).' ) }) + it('opens a session of the longest length it accepts', async () => { + await cli.json(['wallet', 'create', '--name', 'w1'], { unlocked: true }) + + await cli.json(['wallet', 'unlock', '--name', 'w1', '--ttl', '10080'], { unlocked: true }) + const { wallets } = await cli.json(['wallet', 'list']) + + expect(wallets[0]).toMatchObject({ unlocked: true, ttlMs: 10080 * 60 * 1000 }) + }) + it('reports a wallet that was already unlocked, rather than failing', async () => { await importAndUnlock() diff --git a/tests/unit/daemon/server.test.js b/tests/unit/daemon/server.test.js index 75508a2..c381177 100644 --- a/tests/unit/daemon/server.test.js +++ b/tests/unit/daemon/server.test.js @@ -14,22 +14,48 @@ import { jest } from '@jest/globals' import { connect } from 'node:net' -import { mkdtempSync, rmSync, statSync } from 'node:fs' +import { mkdtempSync, rmSync, statSync, writeFileSync, mkdirSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' +import { encrypt } from '@tetherto/wdk-utils' const tempDir = mkdtempSync(join(tmpdir(), 'wdk-daemon-test-')) const SOCKET_PATH = join(tempDir, 'daemon.sock') const PID_PATH = join(tempDir, 'daemon.pid') +const PASSPHRASE = 'correct-horse-battery-staple' +const SEED = 'test test test test test test test test test test test junk' + +/** + * Writes a real encrypted seed file, so the daemon's own `deriveKey` and + * `decryptWithKey` decide whether a passphrase is accepted. + * + * @param {string} name - The wallet name. + * @returns {void} + */ +function writeWallet (name) { + const dir = join(tempDir, 'wallets', name) + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, 'seed.enc'), JSON.stringify(encrypt(SEED, PASSPHRASE)), 'utf8') +} + const constants = await import('../../../src/config/constants.js') jest.unstable_mockModule('../../../src/config/constants.js', () => ({ ...constants, getDaemonSocketPath: () => SOCKET_PATH, - getDaemonPidPath: () => PID_PATH + getDaemonPidPath: () => PID_PATH, + getWalletPath: (name) => join(tempDir, 'wallets', name, 'seed.enc') +})) + +jest.unstable_mockModule('../../../src/services/wdk-service.js', () => ({ + WdkService: class { + createInstance () {} + dispose () {} + } })) const { WalletDaemon } = await import('../../../src/daemon/server.js') +const { MAX_SESSION_TTL_MINUTES } = constants /** @type {WalletDaemon} */ let daemon @@ -120,6 +146,66 @@ describe('the daemon IPC endpoint', () => { expect(reply).toEqual({ ok: false, error: 'Missing required field: hash' }) }) + it('refuses an unlock_wallet whose ttl it could not honour', async () => { + writeWallet('ttl') + + const replies = await Promise.all( + [-1, 1.5, MAX_SESSION_TTL_MINUTES + 1, '5'].map((ttl) => + send({ action: 'unlock_wallet', wallet: 'ttl', passphrase: PASSPHRASE, ttl }) + ) + ) + + for (const reply of replies) { + expect(reply.ok).toBe(false) + expect(reply.error).toContain(`from 0 to ${MAX_SESSION_TTL_MINUTES}`) + } + }) + + it('opens a session for the right passphrase', async () => { + writeWallet('cold') + + const reply = await send({ action: 'unlock_wallet', wallet: 'cold', passphrase: PASSPHRASE, ttl: 1 }) + + expect(reply.ok).toBe(true) + }) + + it('refuses the wrong passphrase on a locked wallet', async () => { + writeWallet('locked') + + const reply = await send({ action: 'unlock_wallet', wallet: 'locked', passphrase: 'nope', ttl: 1 }) + + expect(reply).toMatchObject({ ok: false, code: 'WRONG_PASSPHRASE' }) + }) + + it('refuses the wrong passphrase on a wallet that is already unlocked', async () => { + writeWallet('live') + await send({ action: 'unlock_wallet', wallet: 'live', passphrase: PASSPHRASE, ttl: 1 }) + + const reply = await send({ action: 'unlock_wallet', wallet: 'live', passphrase: 'nope', ttl: 2 }) + + expect(reply).toMatchObject({ ok: false, code: 'WRONG_PASSPHRASE' }) + }) + + it('leaves the session length alone when it refuses the passphrase', async () => { + writeWallet('keep') + await send({ action: 'unlock_wallet', wallet: 'keep', passphrase: PASSPHRASE, ttl: 1 }) + + await send({ action: 'unlock_wallet', wallet: 'keep', passphrase: 'nope', ttl: 0 }) + const { wallets } = (await send({ action: 'status' })).data + + expect(wallets.find((w) => w.name === 'keep').ttlMs).toBe(60_000) + }) + + it('extends the session for the right passphrase', async () => { + writeWallet('extend') + await send({ action: 'unlock_wallet', wallet: 'extend', passphrase: PASSPHRASE, ttl: 1 }) + + await send({ action: 'unlock_wallet', wallet: 'extend', passphrase: PASSPHRASE, ttl: 2 }) + const { wallets } = (await send({ action: 'status' })).data + + expect(wallets.find((w) => w.name === 'extend').ttlMs).toBe(120_000) + }) + it('rejects a line that is not JSON instead of closing the connection', async () => { const reply = await new Promise((resolve, reject) => { const socket = connect(SOCKET_PATH) From dceefff78c943618d3eac602b1c19400983d55e4 Mon Sep 17 00:00:00 2001 From: Quoc Le Date: Fri, 23 Oct 2026 13:50:15 +0700 Subject: [PATCH 2/2] parse the ttl strictly, so an empty value is not unlimited --- src/daemon/client.js | 2 +- src/daemon/protocol.js | 2 +- src/daemon/server.js | 2 ++ src/ui/parsers.js | 7 ++++--- tests/integration/wallet.test.js | 10 +--------- tests/unit/daemon/server.test.js | 21 +++++++++++++++++++-- 6 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/daemon/client.js b/src/daemon/client.js index edff2b5..3d6daaf 100644 --- a/src/daemon/client.js +++ b/src/daemon/client.js @@ -452,7 +452,7 @@ export class DaemonClient { * * @param {string} name - The wallet name. * @param {string} passphrase - The wallet passphrase. - * @param {number} [ttlMinutes] - Session TTL in minutes (0 = no expiry). + * @param {number} [ttlMinutes] - Session TTL in minutes, from 0 to `MAX_SESSION_TTL_MINUTES` (0 = no expiry). * @returns {Promise} */ async unlockWallet (name, passphrase, ttlMinutes = 5) { diff --git a/src/daemon/protocol.js b/src/daemon/protocol.js index 8d4a9bc..9565e84 100644 --- a/src/daemon/protocol.js +++ b/src/daemon/protocol.js @@ -50,7 +50,7 @@ * @property {DaemonAction} action - The action to perform. * @property {string} [wallet] - The wallet name. * @property {string} [passphrase] - The wallet passphrase (only for unlock). - * @property {number} [ttl] - The unlock TTL in minutes (only for unlock). + * @property {number} [ttl] - The unlock TTL in minutes, from 0 to `MAX_SESSION_TTL_MINUTES` (only for unlock; 0 = no expiry). * @property {string} [network] - The blockchain network name. * @property {number} [index] - The BIP-44 account index. * @property {string} [token] - The token symbol. diff --git a/src/daemon/server.js b/src/daemon/server.js index 7587ec9..b5aadaf 100644 --- a/src/daemon/server.js +++ b/src/daemon/server.js @@ -176,6 +176,8 @@ export class WalletDaemon { * @param {number} ttlMinutes - The session TTL in minutes (0 = no expiry). * @returns {void} * @throws {WdkCliError} WRONG_PASSPHRASE when the passphrase does not decrypt the seed. + * @throws {WdkCliError} INVALID_ARGUMENT when the wallet name is not a valid one. + * @throws {Error} When the wallet's seed file cannot be read or parsed. */ #unlockWalletSync (name, passphrase, ttlMinutes) { const walletPath = getWalletPath(name) diff --git a/src/ui/parsers.js b/src/ui/parsers.js index 7876e12..6629f80 100644 --- a/src/ui/parsers.js +++ b/src/ui/parsers.js @@ -54,13 +54,14 @@ export function nonNegativeInt (value) { * @returns {number} The validated TTL in minutes. */ export function sessionTtlMinutes (value) { - const n = Number(value) - if (!Number.isInteger(n) || n < 0 || n > MAX_SESSION_TTL_MINUTES) { + // Digits only, before any coercion: Number('') and Number(' ') are 0, which + // is the one value that means "never expires". + if (!/^\d+$/.test(value) || Number(value) > MAX_SESSION_TTL_MINUTES) { throw new InvalidArgumentError( `Must be a whole number of minutes from 0 to ${MAX_SESSION_TTL_MINUTES} (0 = unlimited).` ) } - return n + return Number(value) } /** diff --git a/tests/integration/wallet.test.js b/tests/integration/wallet.test.js index 474f1f3..8734198 100644 --- a/tests/integration/wallet.test.js +++ b/tests/integration/wallet.test.js @@ -180,7 +180,7 @@ describe('wallet unlock and the session it opens', () => { // The ttl decides how long a decrypted seed stays in memory, so a value that // cannot be honoured has to be rejected rather than rounded into something. - it.each(['-5', 'abc', '10081'])('refuses a ttl of %p', async (ttl) => { + it.each(['-5', 'abc', '10081', '', ' ', '0x10', '1e3', '5.0'])('refuses a ttl of %p', async (ttl) => { await cli.json(['wallet', 'create', '--name', 'w1'], { unlocked: true }) const result = await cli.run( @@ -194,14 +194,6 @@ describe('wallet unlock and the session it opens', () => { ) }) - it('opens a session of the longest length it accepts', async () => { - await cli.json(['wallet', 'create', '--name', 'w1'], { unlocked: true }) - - await cli.json(['wallet', 'unlock', '--name', 'w1', '--ttl', '10080'], { unlocked: true }) - const { wallets } = await cli.json(['wallet', 'list']) - - expect(wallets[0]).toMatchObject({ unlocked: true, ttlMs: 10080 * 60 * 1000 }) - }) it('reports a wallet that was already unlocked, rather than failing', async () => { await importAndUnlock() diff --git a/tests/unit/daemon/server.test.js b/tests/unit/daemon/server.test.js index c381177..e650db5 100644 --- a/tests/unit/daemon/server.test.js +++ b/tests/unit/daemon/server.test.js @@ -24,7 +24,7 @@ const SOCKET_PATH = join(tempDir, 'daemon.sock') const PID_PATH = join(tempDir, 'daemon.pid') const PASSPHRASE = 'correct-horse-battery-staple' -const SEED = 'test test test test test test test test test test test junk' +const SEED = 'cook voyage document eight skate token alien guide drink uncle term abuse' /** * Writes a real encrypted seed file, so the daemon's own `deriveKey` and @@ -166,7 +166,24 @@ describe('the daemon IPC endpoint', () => { const reply = await send({ action: 'unlock_wallet', wallet: 'cold', passphrase: PASSPHRASE, ttl: 1 }) - expect(reply.ok).toBe(true) + expect(reply).toEqual({ + ok: true, + data: { message: "Wallet 'cold' unlocked", wallet: 'cold' } + }) + }) + + // A ttl past setTimeout's 32-bit range fires after 1ms, locking the wallet + // at once. Raising MAX_SESSION_TTL_MINUTES beyond that must fail here. + it('keeps a session open at the longest ttl it accepts', async () => { + writeWallet('max') + await send({ + action: 'unlock_wallet', wallet: 'max', passphrase: PASSPHRASE, ttl: MAX_SESSION_TTL_MINUTES + }) + + await new Promise((resolve) => setTimeout(resolve, 50)) + const { wallets } = (await send({ action: 'status' })).data + + expect(wallets.find((w) => w.name === 'max').ttlMs).toBe(MAX_SESSION_TTL_MINUTES * 60_000) }) it('refuses the wrong passphrase on a locked wallet', async () => {