Why are we swapping to spawnSync over execSync here? This is more complex maintainability-wise and shouldn't be needed.

I'm not totally opposed to spawnSync actually, some stuff does work out better this way.

Sorry, had a lengthy meeting couldn't look into this.

I converted to spawnSync to avoid converting all parameters to string first, spawnSync let us pass each parameter on the array so it is automatically escaped.
With execSync I had some problems passing on "Security Hotspot" tests because it allowed the user to pass arbitrary commands to execSync, like appending in the last parameter: "&& rm -rf *" this would delete everything on the current folder. Using the spawnSync command prevents the user to do this.

So with nx-plugins and other dev tooling in general, the hotspots that pop up from sonarqube can be mostly ignored. The reason being that those commands are being run by the developer, on their own machine. It's not really an attack point, as someone would already have to be able to run arbitrary commands to exercise it.

The solution for most things that come up like that, are just for me to hit the "not a problem" button while reviewing the PR.

That would be a lot easier because I had to change the method a lot, I'll take it into account next time, for this one I was kinda in a rush to have a deployable version because the previous one was broken.

Adding 'inherit' here causes the returned buffers to be empty

I have a new solution for this have to discuss with you, I didn't merge it yet. This should resolve the problem:

  private execute(params: string[], stdio: StdioOptions = 'pipe'): Buffer {
    const proc = spawnSync(this.cliCommand.command, params, {
      stdio,
      cwd: this.cwd || process.cwd(),
    });

    let result = Buffer.alloc(0);
    if (proc?.stdout) {
      result = Buffer.concat([result, proc.stdout]);
    }
    if (proc?.stderr) {
      result = Buffer.concat([result, proc.stderr]);
    }
    return result;
  }

This would work, but its fine to duplicate the spawnSync call for this. The idea behind execute is that we are doing some kind of processing to the Buffer on our side, and we need it. If we are displaying the outputs of the command to the user, its the wrong method to use.

I agree with different calls for spawnSync with different behavior, it's easier to not mess it up.

This method needs the 'stdio: inherit' so that things like nx build show terminal output.