fix: add shell option to npm spawn on Windows

[veerababu1729]

Description

Fixes Windows-specific EINVAL error when spawning npm install commands.

Problem

On Windows, the run_npm_install function in Kernel.js was failing with spawn EINVAL error because it was missing the shell: true option when spawning npm.cmd.

Solution

• Added shell: true to spawn options in run_npm_install function
• Makes the spawn call consistent with other spawn calls in the codebase (ProcessService.js and LocalTerminalService.js)
• Cross-platform compatible (works on Windows, Linux, macOS)

Changes

• File: src/backend/src/Kernel.js
• Line: 581
• Change: Added shell: true to spawn options

Fixes

• Closes Windows Error #1748
• Closes Invalid Host header. & workers error #1797

Why Windows Needs shell: true:

• On Windows, npm.cmd is a batch file, not an executable
• Node.js spawn() cannot execute .cmd files directly without shell: true
• Without it, Windows throws EINVAL (Invalid argument) error

Proof from Codebase:
Other spawn calls in the same codebase already use shell: true:

• ProcessService.js line 76: spawn(command, args, { shell: true, ... })
• LocalTerminalService.js line 93: spawn(profile.shell[0], args, { shell: true, ... })

Testing

• Verified fix follows existing code patterns in the codebase
• Minimal change with low risk
• No functional changes to other parts of the system

Type of Change

• Bug fix (non-breaking change which fixes an issue)

[CLAassistant]

All committers have signed the CLA.

[veerababu1729]

I almost worked one day to find this, please review as soon as possible and feel free to ask anything...

[jelveh]

Hey @veerababu1729. thank you for you PR. I've assigned @ProgrammerIn-wonderland to review this. Please stay tuned.

[ProgrammerIn-wonderland]

I think @KernelDeimos mentioned on call that we shouldn't do (shell: true) since it says a deprecation warning in shell but I'll check if that's still the case

[ProgrammerIn-wonderland]

yeah this gives me the deprecation warning on startup. Instead of using shell: true, maybe we should stop using npm.cmd on windows? regular npm works fine for me

[KernelDeimos]

I suspect shell: true is necessary in some cases. The solution might be to try everything and see what sticks.

Yesterday I wrote a good explanation of the situation but GitHub didn't save the comment I apparently didn't send so we can't have everything; I'll try my best to recreate that: apparently the Node.js maintainers decided that passing args with shell:true should be deprecated which means it might be removed in a future version of node (i.e. instead of getting a warning here, this would be an error and Puter won't be able to continue running). The rationale here makes sense: the fact that spawn accepts this array of input args kind of implies that it will properly process and escape them for you, but it doesn't. (and in hindsight, how could it with shell: true? There are so many shells!)

In our case the security concern isn't applicable - we're passing a static string. It turns out (and I didn't realize this) that you can just pass the argument in the string in the first parameter. This is only available with shell: true so the difference here is really executing a process given its path vs executing a "whatever this string does in the shell" using the shell.

Here's my "try everything and see what sticks" solution:

const executables = ['npm', 'npm.cmd'];
const spawners = [
  (exe) => spawn(exe, 'install', { cwd: path, stdio: "pipe" }),
  (exe) => spawn(`${exe} install`, { cwd: path, stdio: "pipe" }),
];

for ( const exe of executables ) {
  for ( const spawner of spawners ) {
    try {
      spawner(exe);
    } catch (e) {
      // ...
      continue;
    }
    break;
  }
}
const proc = spawn(npmCmd + ' install', { shell: true, cwd: path, stdio: "pipe" });

[veerababu1729]

Let me test your proposal code and let you know.......

[ProgrammerIn-wonderland]

any updates