Merge pull request #8253 from utkarsh-bruno/fix/BRU-3531

packages/bruno-electron/src/ipc/collection.js

@@ -61,6 +61,7 @@ const {
 const { getCollectionConfigFile, openCollectionDialog, openCollectionsByPathname, registerScratchCollectionPath } = require('../app/collections');
 const { generateUidBasedOnHash, stringifyJson, safeStringifyJSON, safeParseJSON } = require('../utils/common');
 const { isValidNpmPackageName, runNpmInstall } = require('../utils/install-packages');
+const { waitForShellEnv } = require('../store/shell-env-state');
 const { moveRequestUid, deleteRequestUid, syncExampleUidsCache } = require('../cache/requestUids');
 const { deleteCookiesForDomain, getDomainsWithCookies, addCookieForDomain, modifyCookieForDomain, parseCookieString, createCookieString, deleteCookie } = require('../utils/cookies');
 const EnvironmentSecretsStore = require('../store/env-secrets');
@@ -2157,6 +2158,7 @@ const registerRendererEventHandlers = (mainWindow, watcher) => {
       throw new Error(`Invalid package name(s): ${invalid.join(', ')}`);
     }
 
+    await waitForShellEnv();
     return runNpmInstall({ collectionPath: collectionPathname, packages });
   });
 

packages/bruno-electron/src/utils/install-packages.js

@@ -1,11 +1,19 @@
 const { spawn } = require('child_process');
+const path = require('path');
+const fs = require('fs');
 
-// npm package name grammar (scoped + unscoped). Conservative enough to prevent
-// shell-metachar smuggling even though spawn() runs without a shell.
 const NPM_NAME_REGEX = /^(?:@[a-z0-9][\w.-]*\/)?[a-z0-9][\w.-]*$/i;
 
+const shouldUseShellForNpmSpawn = (npmCommand, platform = process.platform) => {
+  return platform === 'win32' && /\.(cmd|bat)$/i.test(npmCommand);
+};
+
 const DEFAULT_TIMEOUT_MS = 5 * 60 * 1000; // npm installs can legitimately take minutes
 const DEFAULT_MAX_OUTPUT_BYTES = 1024 * 1024; // bound captured stdout/stderr
+const NODE_SHIM_ENV_KEYS = ['NVM_BIN', 'FNM_MULTISHELL_PATH'];
+const NPM_NOT_FOUND_MESSAGE = 'npm was not found on your PATH. Install Node.js/npm, then try again or run the command manually.';
+
+let cachedNpmInvocation = null;
 
 const isValidNpmPackageName = (name) => typeof name === 'string' && NPM_NAME_REGEX.test(name);
 
@@ -16,13 +24,82 @@ const appendCapped = (buffer, chunk, cap) => {
   return next.length > cap ? next.slice(next.length - cap) : next;
 };
 
+const resolveNodeExecutable = () => {
+  const nodeName = process.platform === 'win32' ? 'node.exe' : 'node';
+
+  for (const key of NODE_SHIM_ENV_KEYS) {
+    const dir = process.env[key];
+    if (!dir) continue;
+    const candidate = path.join(dir, nodeName);
+    if (fs.existsSync(candidate)) return candidate;
+  }
+
+  for (const dir of (process.env.PATH || '').split(path.delimiter).filter(Boolean)) {
+    const candidate = path.join(dir, nodeName);
+    if (fs.existsSync(candidate)) return candidate;
+  }
+
+  return null;
+};
+
+const resolveNpmCli = (nodePath) => {
+  const nodeDir = path.dirname(nodePath);
+  const candidates = [
+    path.join(nodeDir, 'npm'),
+    path.join(nodeDir, 'node_modules', 'npm', 'bin', 'npm-cli.js'),
+    path.join(nodeDir, '..', 'lib', 'node_modules', 'npm', 'bin', 'npm-cli.js')
+  ];
+  return candidates.find((candidate) => fs.existsSync(candidate)) || null;
+};
+
+const resolveNpmInvocation = () => {
+  if (cachedNpmInvocation) return cachedNpmInvocation;
+
+  const nodePath = resolveNodeExecutable();
+  if (!nodePath) return null;
+
+  const npmCliPath = resolveNpmCli(nodePath);
+  if (!npmCliPath) return null;
+
+  cachedNpmInvocation = { nodePath, npmCliPath };
+  return cachedNpmInvocation;
+};
+
+const clearNpmInvocationCache = () => {
+  cachedNpmInvocation = null;
+};
+
+const buildSafeEnv = (nodeBinDir) => {
+  const existingPath = process.env.PATH || '';
+  const newPath = [nodeBinDir, existingPath].filter(Boolean).join(path.delimiter);
+  return { ...process.env, PATH: newPath };
+};
+
+// CVE-2024-27980: Node.js rejects spawn/spawnSync of .cmd/.bat with shell:false on
+// Windows (EINVAL). npm.cmd is affected; node.exe + npm-cli.js is not.
+const isWindowsBatchFile = (filePath) => {
+  if (process.platform !== 'win32') return false;
+  const ext = path.extname(filePath).toLowerCase();
+  return ext === '.cmd' || ext === '.bat';
+};
+
+const buildSpawnOptions = ({ nodePath, collectionPath }) => ({
+  cwd: collectionPath,
+  env: buildSafeEnv(path.dirname(nodePath)),
+  shell: isWindowsBatchFile(nodePath),
+  windowsHide: true
+});
+
 /**
  * Runs `npm install --save <packages>` in a collection directory and resolves
  * with a structured result. Never rejects - runtime failures (non-zero exit,
  * npm-not-found, timeout) come back as `{ success: false, ... }` so callers
  * can surface a useful message.
  *
- * `spawnFn` and `timeoutMs` are injectable for testing.
+ * npm is invoked as `node <npm-cli.js> install --save ...` — not npm.cmd — so
+ * shell:false is safe on Windows (CVE-2024-27980 only blocks .cmd/.bat without shell).
+ *
+ * `spawnFn`, `timeoutMs`, and `resolveNpmInvocationFn` are injectable for testing.
  *
  * @returns {Promise<{ success: boolean, exitCode: number, stdout: string,
  *   stderr: string, installed: string[], errorCode?: string }>}
@@ -33,10 +110,26 @@ const runNpmInstall = ({
   spawnFn = spawn,
   timeoutMs = DEFAULT_TIMEOUT_MS,
   maxOutputBytes = DEFAULT_MAX_OUTPUT_BYTES,
-  npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm'
+  resolveNpmInvocationFn = resolveNpmInvocation
 }) => {
   const installed = Array.from(new Set(packages));
-  const args = ['install', '--save', ...installed];
+  const npmArgs = ['install', '--save', ...installed];
+
+  const invocation = resolveNpmInvocationFn();
+  if (!invocation) {
+    return Promise.resolve({
+      success: false,
+      exitCode: -1,
+      stdout: '',
+      stderr: NPM_NOT_FOUND_MESSAGE,
+      installed,
+      errorCode: 'NPM_NOT_FOUND'
+    });
+  }
+
+  const { nodePath, npmCliPath } = invocation;
+  const spawnArgs = [npmCliPath, ...npmArgs];
+  const spawnOptions = buildSpawnOptions({ nodePath, collectionPath });
 
   return new Promise((resolve) => {
     let stdout = '';
@@ -53,7 +146,7 @@ const runNpmInstall = ({
 
     let child;
     try {
-      child = spawnFn(npmCommand, args, { cwd: collectionPath, env: process.env, shell: false });
+      child = spawnFn(nodePath, spawnArgs, spawnOptions);
     } catch (err) {
       finish({ success: false, exitCode: -1, stderr: err.message, errorCode: 'SPAWN_FAILED' });
       return;
@@ -86,9 +179,7 @@ const runNpmInstall = ({
         success: false,
         exitCode: -1,
         errorCode: isMissingNpm ? 'NPM_NOT_FOUND' : 'SPAWN_ERROR',
-        stderr: isMissingNpm
-          ? 'npm was not found on your PATH. Install Node.js/npm, then try again or run the command manually.'
-          : `${stderr}\n${err.message}`
+        stderr: isMissingNpm ? NPM_NOT_FOUND_MESSAGE : `${stderr}\n${err.message}`
       });
     });
 
@@ -100,8 +191,17 @@ const runNpmInstall = ({
 
 module.exports = {
   isValidNpmPackageName,
+  shouldUseShellForNpmSpawn,
   runNpmInstall,
+  resolveNodeExecutable,
+  resolveNpmCli,
+  resolveNpmInvocation,
+  clearNpmInvocationCache,
+  buildSafeEnv,
+  buildSpawnOptions,
+  isWindowsBatchFile,
   NPM_NAME_REGEX,
   DEFAULT_TIMEOUT_MS,
-  DEFAULT_MAX_OUTPUT_BYTES
+  DEFAULT_MAX_OUTPUT_BYTES,
+  NPM_NOT_FOUND_MESSAGE
 };

packages/bruno-electron/tests/utils/install-packages.spec.js

@@ -1,5 +1,37 @@
+const fs = require('fs');
+const path = require('path');
 const { EventEmitter } = require('events');
-const { isValidNpmPackageName, runNpmInstall } = require('../../src/utils/install-packages');
+const {
+  isValidNpmPackageName,
+  runNpmInstall,
+  resolveNodeExecutable,
+  resolveNpmCli,
+  resolveNpmInvocation,
+  clearNpmInvocationCache,
+  buildSafeEnv,
+  buildSpawnOptions,
+  isWindowsBatchFile
+} = require('../../src/utils/install-packages');
+
+const nodeExecutableName = () => (process.platform === 'win32' ? 'node.exe' : 'node');
+const fixturePath = (...segments) => path.join('fixtures', 'install-packages', ...segments);
+
+const NODE_BIN = fixturePath('node', 'bin');
+const NODE_EXECUTABLE = path.join(NODE_BIN, nodeExecutableName());
+const NPM_BIN = path.join(NODE_BIN, 'npm');
+const NPM_CLI_LIB_LAYOUT = path.join(NODE_BIN, '..', 'lib', 'node_modules', 'npm', 'bin', 'npm-cli.js');
+const NODE_DIR_BESIDE = fixturePath('nodejs');
+const NODE_EXECUTABLE_BESIDE = path.join(NODE_DIR_BESIDE, nodeExecutableName());
+const NPM_CLI_BESIDE_LAYOUT = path.join(NODE_DIR_BESIDE, 'node_modules', 'npm', 'bin', 'npm-cli.js');
+const NVM_BIN_DIR = fixturePath('nvm', 'v20', 'bin');
+const NVM_NODE_EXECUTABLE = path.join(NVM_BIN_DIR, nodeExecutableName());
+const SYSTEM_BIN_DIR = fixturePath('system', 'bin');
+const COLLECTION_DIR = fixturePath('collection');
+
+const mockNpmInvocation = () => ({
+  nodePath: NODE_EXECUTABLE,
+  npmCliPath: NPM_CLI_LIB_LAYOUT
+});
 
 // Minimal stand-in for a child_process handle: stdout/stderr are emitters and
 // the child itself emits 'close' / 'error'. Lets us drive npm outcomes
@@ -39,12 +71,152 @@ describe('isValidNpmPackageName', () => {
   });
 });
 
+describe('resolveNodeExecutable', () => {
+  let existsSyncSpy;
+
+  beforeEach(() => {
+    existsSyncSpy = jest.spyOn(fs, 'existsSync').mockReturnValue(false);
+    delete process.env.NVM_BIN;
+    delete process.env.FNM_MULTISHELL_PATH;
+    delete process.env.PATH;
+  });
+
+  afterEach(() => {
+    existsSyncSpy.mockRestore();
+  });
+
+  test('prefers NVM_BIN over PATH', () => {
+    process.env.NVM_BIN = NVM_BIN_DIR;
+    process.env.PATH = SYSTEM_BIN_DIR;
+    existsSyncSpy.mockImplementation((candidate) => candidate === NVM_NODE_EXECUTABLE);
+
+    expect(resolveNodeExecutable()).toBe(NVM_NODE_EXECUTABLE);
+  });
+
+  test('walks PATH when shim env vars are unset', () => {
+    process.env.PATH = [NODE_BIN, SYSTEM_BIN_DIR].join(path.delimiter);
+    existsSyncSpy.mockImplementation((candidate) => candidate === NODE_EXECUTABLE);
+
+    expect(resolveNodeExecutable()).toBe(NODE_EXECUTABLE);
+  });
+
+  test('returns null when node is not found', () => {
+    process.env.PATH = SYSTEM_BIN_DIR;
+    expect(resolveNodeExecutable()).toBeNull();
+  });
+});
+
+describe('resolveNpmCli', () => {
+  let existsSyncSpy;
+
+  beforeEach(() => {
+    existsSyncSpy = jest.spyOn(fs, 'existsSync').mockReturnValue(false);
+  });
+
+  afterEach(() => {
+    existsSyncSpy.mockRestore();
+  });
+
+  test('prefers bin/npm when present', () => {
+    existsSyncSpy.mockImplementation((candidate) => candidate === NPM_BIN);
+
+    expect(resolveNpmCli(NODE_EXECUTABLE)).toBe(NPM_BIN);
+  });
+
+  test('finds npm-cli via lib layout when bin/npm is absent', () => {
+    existsSyncSpy.mockImplementation((candidate) => candidate === NPM_CLI_LIB_LAYOUT);
+
+    expect(resolveNpmCli(NODE_EXECUTABLE)).toBe(NPM_CLI_LIB_LAYOUT);
+  });
+
+  test('finds npm-cli via node_modules layout beside node', () => {
+    existsSyncSpy.mockImplementation((candidate) => candidate === NPM_CLI_BESIDE_LAYOUT);
+
+    expect(resolveNpmCli(NODE_EXECUTABLE_BESIDE)).toBe(NPM_CLI_BESIDE_LAYOUT);
+  });
+});
+
+describe('resolveNpmInvocation', () => {
+  beforeEach(() => {
+    clearNpmInvocationCache();
+  });
+
+  test('caches the resolved invocation', () => {
+    const existsSyncSpy = jest.spyOn(fs, 'existsSync').mockImplementation((candidate) => {
+      return candidate === NODE_EXECUTABLE || candidate === NPM_CLI_LIB_LAYOUT;
+    });
+    process.env.PATH = NODE_BIN;
+
+    const first = resolveNpmInvocation();
+    existsSyncSpy.mockRestore();
+
+    expect(first).toEqual({ nodePath: NODE_EXECUTABLE, npmCliPath: NPM_CLI_LIB_LAYOUT });
+    expect(resolveNpmInvocation()).toBe(first);
+  });
+});
+
+describe('buildSafeEnv', () => {
+  test('prepends the node bin directory to PATH', () => {
+    process.env.PATH = SYSTEM_BIN_DIR;
+    process.env.HOME = fixturePath('home');
+
+    const env = buildSafeEnv(NODE_BIN);
+
+    expect(env.PATH).toBe([NODE_BIN, SYSTEM_BIN_DIR].join(path.delimiter));
+    expect(env.HOME).toBe(fixturePath('home'));
+  });
+});
+
+describe('buildSpawnOptions', () => {
+  const originalPlatform = process.platform;
+
+  afterEach(() => {
+    Object.defineProperty(process, 'platform', { value: originalPlatform });
+  });
+
+  test('uses shell:false when spawning node.exe with npm-cli.js (CVE-2024-27980 safe)', () => {
+    Object.defineProperty(process, 'platform', { value: 'win32' });
+
+    const nodePath = path.join('fixtures', 'install-packages', 'node', 'bin', 'node.exe');
+    const npmCliPath = path.join('fixtures', 'install-packages', 'node', 'lib', 'node_modules', 'npm', 'bin', 'npm-cli.js');
+    const options = buildSpawnOptions({ nodePath, collectionPath: COLLECTION_DIR });
+
+    expect(isWindowsBatchFile(nodePath)).toBe(false);
+    expect(isWindowsBatchFile(npmCliPath)).toBe(false);
+    expect(options.shell).toBe(false);
+    expect(options.windowsHide).toBe(true);
+    expect(options.cwd).toBe(COLLECTION_DIR);
+  });
+
+  test('uses shell:true only when the executable is a Windows batch file', () => {
+    Object.defineProperty(process, 'platform', { value: 'win32' });
+
+    const npmCmd = path.join('fixtures', 'install-packages', 'node', 'bin', 'npm.cmd');
+    expect(isWindowsBatchFile(npmCmd)).toBe(true);
+
+    const options = buildSpawnOptions({ nodePath: npmCmd, collectionPath: COLLECTION_DIR });
+    expect(options.shell).toBe(true);
+  });
+
+  test('does not treat batch extensions as special on non-Windows platforms', () => {
+    Object.defineProperty(process, 'platform', { value: 'linux' });
+
+    const npmCmd = path.join('fixtures', 'install-packages', 'node', 'bin', 'npm.cmd');
+    expect(isWindowsBatchFile(npmCmd)).toBe(false);
+  });
+});
+
 describe('runNpmInstall', () => {
   test('resolves success on exit code 0 and captures stdout', async () => {
     const child = makeFakeChild();
     const spawnFn = jest.fn(() => child);
 
-    const promise = runNpmInstall({ collectionPath: '/coll', packages: ['dayjs'], spawnFn });
+    const promise = runNpmInstall({
+      collectionPath: COLLECTION_DIR,
+      packages: ['dayjs'],
+      spawnFn,
+      resolveNpmInvocationFn: mockNpmInvocation
+    });
     child.stdout.emit('data', Buffer.from('added 1 package'));
     child.emit('close', 0);
 
@@ -55,29 +227,71 @@ describe('runNpmInstall', () => {
     expect(result.installed).toEqual(['dayjs']);
   });
 
-  test('passes the correct npm args, cwd, and runs without a shell', async () => {
+  test('spawns node with npm-cli.js, correct args, cwd, and no shell', async () => {
     const child = makeFakeChild();
     const spawnFn = jest.fn(() => child);
+    const systemPath = fixturePath('system', 'path');
+    process.env.PATH = systemPath;
 
     const promise = runNpmInstall({
-      collectionPath: '/my/coll',
+      collectionPath: COLLECTION_DIR,
       packages: ['dayjs', 'dayjs', 'zod'],
       spawnFn,
-      npmCommand: 'npm'
+      resolveNpmInvocationFn: mockNpmInvocation
     });
     child.emit('close', 0);
     await promise;
 
     expect(spawnFn).toHaveBeenCalledWith(
-      'npm',
-      ['install', '--save', 'dayjs', 'zod'],
-      expect.objectContaining({ cwd: '/my/coll', shell: false })
+      NODE_EXECUTABLE,
+      [NPM_CLI_LIB_LAYOUT, 'install', '--save', 'dayjs', 'zod'],
+      expect.objectContaining({
+        cwd: COLLECTION_DIR,
+        shell: false,
+        windowsHide: true,
+        env: expect.objectContaining({
+          PATH: [NODE_BIN, systemPath].join(path.delimiter)
+        })
+      })
+    );
+  });
+
+  test('spawns node with bin/npm when that entry is resolved', async () => {
+    const child = makeFakeChild();
+    const spawnFn = jest.fn(() => child);
+    const systemPath = fixturePath('system', 'path');
+    process.env.PATH = systemPath;
+
+    const promise = runNpmInstall({
+      collectionPath: COLLECTION_DIR,
+      packages: ['dayjs'],
+      spawnFn,
+      resolveNpmInvocationFn: () => ({ nodePath: NODE_EXECUTABLE, npmCliPath: NPM_BIN })
+    });
+    child.emit('close', 0);
+    await promise;
+
+    expect(spawnFn).toHaveBeenCalledWith(
+      NODE_EXECUTABLE,
+      [NPM_BIN, 'install', '--save', 'dayjs'],
+      expect.objectContaining({
+        cwd: COLLECTION_DIR,
+        shell: false,
+        env: expect.objectContaining({
+          PATH: [NODE_BIN, systemPath].join(path.delimiter)
+        })
+      })
     );
   });
 
   test('dedupes packages in the result', async () => {
     const child = makeFakeChild();
-    const promise = runNpmInstall({ collectionPath: '/c', packages: ['a', 'a', 'b'], spawnFn: () => child });
+    const promise = runNpmInstall({
+      collectionPath: COLLECTION_DIR,
+      packages: ['a', 'a', 'b'],
+      spawnFn: () => child,
+      resolveNpmInvocationFn: mockNpmInvocation
+    });
     child.emit('close', 0);
     const result = await promise;
     expect(result.installed).toEqual(['a', 'b']);
@@ -85,7 +299,12 @@ describe('runNpmInstall', () => {
 
   test('resolves failure on a non-zero exit and surfaces stderr', async () => {
     const child = makeFakeChild();
-    const promise = runNpmInstall({ collectionPath: '/c', packages: ['bad-pkg'], spawnFn: () => child });
+    const promise = runNpmInstall({
+      collectionPath: COLLECTION_DIR,
+      packages: ['bad-pkg'],
+      spawnFn: () => child,
+      resolveNpmInvocationFn: mockNpmInvocation
+    });
     child.stderr.emit('data', Buffer.from('npm ERR! 404 Not Found'));
     child.emit('close', 1);
 
@@ -95,10 +314,27 @@ describe('runNpmInstall', () => {
     expect(result.stderr).toContain('404 Not Found');
   });
 
-  test('reports NPM_NOT_FOUND when npm is missing from PATH (ENOENT)', async () => {
+  test('reports NPM_NOT_FOUND when npm cannot be resolved', async () => {
+    const result = await runNpmInstall({
+      collectionPath: COLLECTION_DIR,
+      packages: ['a'],
+      resolveNpmInvocationFn: () => null
+    });
+
+    expect(result.success).toBe(false);
+    expect(result.errorCode).toBe('NPM_NOT_FOUND');
+    expect(result.stderr).toMatch(/not found on your PATH/i);
+  });
+
+  test('reports NPM_NOT_FOUND when spawn fails with ENOENT', async () => {
     const child = makeFakeChild();
-    const promise = runNpmInstall({ collectionPath: '/c', packages: ['a'], spawnFn: () => child });
-    const err = new Error('spawn npm ENOENT');
+    const promise = runNpmInstall({
+      collectionPath: COLLECTION_DIR,
+      packages: ['a'],
+      spawnFn: () => child,
+      resolveNpmInvocationFn: mockNpmInvocation
+    });
+    const err = new Error('spawn node ENOENT');
     err.code = 'ENOENT';
     child.emit('error', err);
 
@@ -110,7 +346,12 @@ describe('runNpmInstall', () => {
 
   test('reports SPAWN_ERROR for non-ENOENT spawn errors', async () => {
     const child = makeFakeChild();
-    const promise = runNpmInstall({ collectionPath: '/c', packages: ['a'], spawnFn: () => child });
+    const promise = runNpmInstall({
+      collectionPath: COLLECTION_DIR,
+      packages: ['a'],
+      spawnFn: () => child,
+      resolveNpmInvocationFn: mockNpmInvocation
+    });
     const err = new Error('EACCES permission denied');
     err.code = 'EACCES';
     child.emit('error', err);
@@ -124,7 +365,12 @@ describe('runNpmInstall', () => {
     const spawnFn = jest.fn(() => {
       throw new Error('boom');
     });
-    const result = await runNpmInstall({ collectionPath: '/c', packages: ['a'], spawnFn });
+    const result = await runNpmInstall({
+      collectionPath: COLLECTION_DIR,
+      packages: ['a'],
+      spawnFn,
+      resolveNpmInvocationFn: mockNpmInvocation
+    });
     expect(result.success).toBe(false);
     expect(result.errorCode).toBe('SPAWN_FAILED');
     expect(result.stderr).toContain('boom');
@@ -134,9 +380,10 @@ describe('runNpmInstall', () => {
     jest.useFakeTimers();
     const child = makeFakeChild();
     const promise = runNpmInstall({
-      collectionPath: '/c',
+      collectionPath: COLLECTION_DIR,
       packages: ['a'],
       spawnFn: () => child,
+      resolveNpmInvocationFn: mockNpmInvocation,
       timeoutMs: 1000
     });
 
@@ -152,9 +399,10 @@ describe('runNpmInstall', () => {
   test('caps captured output to the trailing maxOutputBytes', async () => {
     const child = makeFakeChild();
     const promise = runNpmInstall({
-      collectionPath: '/c',
+      collectionPath: COLLECTION_DIR,
       packages: ['a'],
       spawnFn: () => child,
+      resolveNpmInvocationFn: mockNpmInvocation,
       maxOutputBytes: 10
     });
     child.stdout.emit('data', 'abcdefghijklmnop'); // 16 chars
@@ -167,8 +415,13 @@ describe('runNpmInstall', () => {
 
   test('only settles once even if close fires after error', async () => {
     const child = makeFakeChild();
-    const promise = runNpmInstall({ collectionPath: '/c', packages: ['a'], spawnFn: () => child });
-    const err = new Error('spawn npm ENOENT');
+    const promise = runNpmInstall({
+      collectionPath: COLLECTION_DIR,
+      packages: ['a'],
+      spawnFn: () => child,
+      resolveNpmInvocationFn: mockNpmInvocation
+    });
+    const err = new Error('spawn node ENOENT');
     err.code = 'ENOENT';
     child.emit('error', err);
     child.emit('close', 1); // should be ignored

packages/bruno-requests/src/utils/shell-env.spec.ts

@@ -1,3 +1,4 @@
+import path from 'path';
 import { initializeShellEnv } from './shell-env';
 
 let mockShellEnvResult: Record<string, string> = {};
@@ -33,6 +34,32 @@ describe('initializeShellEnv', () => {
     delete process.env.http_proxy;
   });
 
+  test('should prepend shell PATH to existing process.env.PATH', async () => {
+    const shellNodeBin = path.join('fixtures', 'shell-env', 'node-bin');
+    const systemBin = path.join('fixtures', 'shell-env', 'system-bin');
+    const otherBin = path.join('fixtures', 'shell-env', 'other-bin');
+    process.env.PATH = [systemBin, otherBin].join(path.delimiter);
+    mockShellEnvResult = { PATH: shellNodeBin };
+
+    await initializeShellEnv();
+
+    expect(process.env.PATH).toBe(
+      [shellNodeBin, systemBin, otherBin].join(path.delimiter)
+    );
+    delete process.env.PATH;
+  });
+
+  test('should set PATH from shell when not in process.env', async () => {
+    const shellBin = path.join('fixtures', 'shell-env', 'shell-bin');
+    delete process.env.PATH;
+    mockShellEnvResult = { PATH: shellBin };
+
+    await initializeShellEnv();
+
+    expect(process.env.PATH).toBe(shellBin);
+    delete process.env.PATH;
+  });
+
   test('should preserve multiple existing env vars while adding new ones', async () => {
     process.env.EXISTING_VAR = 'existing';
     delete process.env.NEW_VAR;

packages/bruno-requests/src/utils/shell-env.ts

@@ -4,6 +4,8 @@
  * Fetches environment variables from the user's shell configuration files (e.g., .zshenv, .bashrc)
  */
 
+import path from 'path';
+
 const fetchShellEnv = async (): Promise<Record<string, string>> => {
   // Windows handles environment variables differently - skip
   if (process.platform === 'win32') {
@@ -29,7 +31,9 @@ const fetchShellEnv = async (): Promise<Record<string, string>> => {
 export const initializeShellEnv = async (): Promise<Record<string, string>> => {
   const shellEnvVars = await fetchShellEnv();
   for (const [key, value] of Object.entries(shellEnvVars)) {
-    if (!(key in process.env)) {
+    if (key === 'PATH' && process.env.PATH) {
+      process.env.PATH = `${value}${path.delimiter}${process.env.PATH}`;
+    } else if (!(key in process.env)) {
       process.env[key] = value;
     }
   }