diff --git a/packages/bitcore-cli/src/wallet.ts b/packages/bitcore-cli/src/wallet.ts index 5d67ff146f..80d5cbf5b6 100644 --- a/packages/bitcore-cli/src/wallet.ts +++ b/packages/bitcore-cli/src/wallet.ts @@ -239,7 +239,7 @@ export class Wallet implements IWallet { } const lockFilename = Utils.getWalletLockFileName(this.name, this.dir); try { - fs.writeFileSync(lockFilename, process.pid.toString(), { flag: 'wx', mode: 0o444 }); // wx flag ensures it fails if the file already exists + fs.writeFileSync(lockFilename, process.pid.toString() + '\n' + JSON.stringify(process.argv), { flag: 'wx', mode: 0o444 }); // wx flag ensures it fails if the file already exists _hasLockFile = true; process.on('exit', () => { try { @@ -252,16 +252,24 @@ export class Wallet implements IWallet { } catch (e) { if (e.code === 'EEXIST') { // Check if process is still running - const pid = fs.readFileSync(lockFilename, 'utf-8')?.trim(); + const lockFileData = fs.readFileSync(lockFilename, 'utf-8')?.trim(); + const [pid, lockedFullCommand] = lockFileData.split('\n'); + const lockedScriptPath: string = CWCUtils.tryParse(lockedFullCommand)?.[1]; if (os.platform() === 'win32') { // TODO } else { const stat = fs.statSync(lockFilename); if (stat.uid === process.getuid()) { // make sure the lock file belongs to the current user - const response = execSync(`ps -q ${pid} -o args || true`, { encoding: 'utf-8' }); - const command = response?.split('\n')[1] || ''; - if (!command.includes(`bitcore-cli ${this.name}`) && !command.includes(`build/src/cli.js ${this.name}`)) { + const runningPid = execSync(`ps -q ${pid} -o args || true`, { encoding: 'utf-8' }); + const runningScriptPath = runningPid?.split('\n')[1]?.split(' ')[1]; + // Ensure the PID hasn't been re-assigned to another process + // Resolve both paths (relative to this process' cwd) since argv[1] may be relative or absolute + // depending on how bitcore-cli was invoked. The lock file name includes the wallet name, so the + // lock should indeed be for this wallet. + const isSameProcess = !!runningScriptPath && !!lockedScriptPath && path.resolve(runningScriptPath) === path.resolve(lockedScriptPath); + if (!isSameProcess) { // Stale lock file, remove it and continue + prompt.log.warn('Stale wallet lock file detected. Removing it now, but please make sure you do not have another instance of bitcore-cli running for this wallet.'); fs.rmSync(lockFilename); return this.lockLoadedWallet(); } diff --git a/packages/bitcore-cli/test/wallet.test.ts b/packages/bitcore-cli/test/wallet.test.ts index 95dd7226b7..13724a99d3 100644 --- a/packages/bitcore-cli/test/wallet.test.ts +++ b/packages/bitcore-cli/test/wallet.test.ts @@ -192,7 +192,8 @@ describe('Wallet', function() { const lines = helpers.decolor(checkpointOutput).split(os.EOL); const mainmenuLine = lines.findIndex(l => l.match(`[ Main Menu - ${WALLETS.BTC.SINGLE_SIG} ]`)); assert(mainmenuLine > -1, 'Did not reach main menu. Got: ' + checkpointOutput); - assert(fs.readFileSync(lockFileName, 'utf-8') === child.pid.toString(), 'Lock file does not match child PID'); + const [lockedPid] = fs.readFileSync(lockFileName, 'utf-8').split('\n'); + assert.equal(lockedPid, child.pid.toString(), 'Lock file does not match child PID'); break; } @@ -231,6 +232,164 @@ describe('Wallet', function() { }); }); + it('should record the pid and full invocation command in the lock file', function(done) { + const lockFileName = Utils.getWalletLockFileName(WALLETS.BTC.SINGLE_SIG, DIR); + const stepInputs = [ + // Checkpoint1: Upon wallet load + [KEYSTROKES.ARROW_UP], // Proposals -> Exit + [KEYSTROKES.ENTER], // Exit + ]; + let step = 0; + const io = new Transform({ + encoding: 'utf-8', + transform: function (chunk, encoding, respond) { + try { + chunk = chunk.toString(); + + // Uncomment to see CLI output during test + // process.stdout.write(chunk); + + const isStep = chunk.endsWith(OUTPUT_END_SEQ); + if (isStep) { + switch (step) { + default: + break; // no-op for non-checkpoint steps + case 0: { + const [lockedPid, lockedArgvJson] = fs.readFileSync(lockFileName, 'utf-8').split('\n'); + assert.equal(lockedPid, child.pid.toString(), 'Lock file pid should match the running process'); + const lockedArgv = JSON.parse(lockedArgvJson); + assert.ok(Array.isArray(lockedArgv), 'Lock file should contain the full process.argv as JSON'); + assert.ok(lockedArgv[1]?.endsWith('cli.js'), 'Lock file argv should include the path to cli.js'); + break; + } + } + + for (const input of stepInputs[step]) { + this.push(input); + } + step++; + } else if (chunk.includes('Error:')) { + return respond(chunk); + } + if (chunk.includes('👋')) { + child.stdin.end(); // send EOF to child so it can exit cleanly + } + respond(); + } catch (e) { + return respond(e); + } + } + }); + const child = spawn('node', [CLI_EXEC, WALLETS.BTC.SINGLE_SIG, ...cmdOpts], CLI_OPTS); + child.stderr.pipe(process.stderr); + child.stdout.pipe(io).pipe(child.stdin); + io.on('error', (e) => { + done(e); + }); + child.on('error', (e) => { + done(e); + }); + child.on('close', (code) => { + try { + assert.equal(code, 0); + done(); + } catch (e) { + done(e); + } + }); + }); + + it('should treat a lock file as stale when its pid is reused by an unrelated process', function(done) { + // Simulates a crashed bitcore-cli process whose pid has since been reused by some other, unrelated + // process that also happens to be invoked via a `cli.js` script (regression: an exact script-path + // comparison must be used instead of a loose "looks like bitcore-cli" heuristic). + const fakeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'bitcore-cli-fake-')); + const fakeCliPath = path.join(fakeDir, 'cli.js'); + fs.writeFileSync(fakeCliPath, 'setInterval(() => {}, 100000);\n'); + const fakeProcess = spawn('node', [fakeCliPath], { detached: true, stdio: 'ignore' }); + + const lockFileName = Utils.getWalletLockFileName(WALLETS.BTC.SINGLE_SIG, DIR); + // Recorded lock claims a different cli.js path than the one the reused pid is now actually running + const originalArgv = ['/usr/bin/node', '/original/path/to/bitcore-cli/build/src/cli.js']; + fs.writeFileSync(lockFileName, `${fakeProcess.pid}\n${JSON.stringify(originalArgv)}`, { mode: 0o444 }); + + const stepInputs = [ + // Checkpoint1: Upon wallet load + [KEYSTROKES.ARROW_UP], // Proposals -> Exit + [KEYSTROKES.ENTER], // Exit + ]; + let step = 0; + let output = ''; + const checkpoints = new Set([0]); + let checkpointOutput = ''; + const io = new Transform({ + encoding: 'utf-8', + transform: function (chunk, encoding, respond) { + try { + chunk = chunk.toString(); + output += chunk; + if (checkpoints.has(step)) { + checkpointOutput += chunk; + } else { + checkpointOutput = ''; + } + + // Uncomment to see CLI output during test + // process.stdout.write(chunk); + + const isStep = chunk.endsWith(OUTPUT_END_SEQ); + if (isStep) { + switch (step) { + default: + break; // no-op for non-checkpoint steps + case Array.from(checkpoints)[0]: { + const lines = helpers.decolor(checkpointOutput).split(os.EOL); + const mainmenuLine = lines.findIndex(l => l.match(`[ Main Menu - ${WALLETS.BTC.SINGLE_SIG} ]`)); + assert(mainmenuLine > -1, 'Did not reach main menu. Got: ' + checkpointOutput); + break; + } + } + + for (const input of stepInputs[step]) { + this.push(input); + } + step++; + } else if (chunk.includes('Error:')) { + return respond(chunk); + } + if (chunk.includes('👋')) { + child.stdin.end(); // send EOF to child so it can exit cleanly + } + respond(); + } catch (e) { + return respond(e); + } + } + }); + const child = spawn('node', [CLI_EXEC, WALLETS.BTC.SINGLE_SIG, ...cmdOpts], CLI_OPTS); + child.stderr.pipe(process.stderr); + child.stdout.pipe(io).pipe(child.stdin); + io.on('error', (e) => { + done(e); + }); + child.on('error', (e) => { + done(e); + }); + child.on('close', (code) => { + try { + fakeProcess.kill(); + } catch { /* already dead */ } + fs.rmSync(fakeDir, { recursive: true, force: true }); + try { + assert.equal(code, 0); + assert.match(helpers.decolor(output), /Stale wallet lock file detected/); + done(); + } catch (e) { + done(e); + } + }); + }); + }); describe('save', function() { diff --git a/packages/crypto-wallet-core/src/utils/index.ts b/packages/crypto-wallet-core/src/utils/index.ts index ff0e3e2404..d6ea6b1d69 100644 --- a/packages/crypto-wallet-core/src/utils/index.ts +++ b/packages/crypto-wallet-core/src/utils/index.ts @@ -176,4 +176,13 @@ export function normalizeXrpFlag(flag: string | number, flagEnum?: typeof xrpl.A throw new Error(`Invalid XRP flag: ${flag}. Flag is not in enum ${enumName}`); } return normalizedFlag; +} + +export function tryParse(json: unknown, fallback?: any) { + try { + if (typeof json !== 'string') return json; // already parsed + return JSON.parse(json); + } catch { + return fallback; + } } \ No newline at end of file diff --git a/packages/crypto-wallet-core/test/utils.test.ts b/packages/crypto-wallet-core/test/utils.test.ts index 547a9f4a95..51f07633d2 100644 --- a/packages/crypto-wallet-core/test/utils.test.ts +++ b/packages/crypto-wallet-core/test/utils.test.ts @@ -404,4 +404,43 @@ describe('Utils', function() { expect(() => utils.normalizeXrpFlag('tfPartialPayment')).to.throw(Error).with.property('message', 'Invalid XRP flag: tfPartialPayment. Flag is not in enum AccountSetTfFlags'); }); }); + + describe('tryParse', function() { + it('should parse valid JSON', function() { + const json = '{"key":"value"}'; + const result = utils.tryParse(json); + expect(result).to.deep.equal({ key: 'value' }); + }); + + it('should parse JSON primitives and arrays', function() { + expect(utils.tryParse('0')).to.equal(0); + expect(utils.tryParse('false')).to.equal(false); + expect(utils.tryParse('null', 'fallback')).to.equal(null); + expect(utils.tryParse('[1,2]')).to.deep.equal([1, 2]); + }); + + it('should return fallback for invalid JSON', function() { + const json = '{"key": "value"'; + const fallback = { fallback: true }; + const result = utils.tryParse(json, fallback); + expect(result).to.deep.equal(fallback); + }); + + it('should return undefined for invalid JSON without a fallback', function() { + expect(utils.tryParse('{invalid')).to.equal(undefined); + }); + + it('should return the object itself if already parsed', function() { + const obj = { key: 'value' }; + const result = utils.tryParse(obj); + expect(result).to.equal(obj); + }); + + it('should return non-string inputs unchanged even when a fallback is provided', function() { + expect(utils.tryParse(0, 'fallback')).to.equal(0); + expect(utils.tryParse(false, 'fallback')).to.equal(false); + expect(utils.tryParse(null, 'fallback')).to.equal(null); + expect(utils.tryParse(undefined, 'fallback')).to.equal(undefined); + }); + }); }); \ No newline at end of file