Make bitcore-cli lockfile logic more robust

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();
             }

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() {

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;
+  }
 }

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);
+    });
+  });
 });