Fix node interception issue with spaces in paths on Windows

Previously if the path to the overrides directory contained spaces (e.g.
because your username had a space) then we quoted the path in
NODE_OPTIONS, and some combination of Node + Git Bash behaviour ended up
treating all Windows backslashes as escape characters and breaking the
string entirely.

There's a few possible solutions, but the easiest one is to avoid using
backslashes entirely, and just stick to fully supported POSIX paths
instead.

Author: pimterry

src/interceptors/docker/docker-interception-services.ts

@@ -107,7 +107,8 @@ export async function startDockerInterceptionServices(
         // container connecting to a network):
         prepareDockerTunnel(),
         // Create a Docker volume, containing our cert and the override files:
-        ensureDockerInjectionVolumeExists(httpsConfig.certContent)]);
+        ensureDockerInjectionVolumeExists(httpsConfig.certContent)
+    ]);
 }
 
 export async function ensureDockerServicesRunning(proxyPort: number) {

src/interceptors/terminal/existing-terminal-interceptor.ts

@@ -83,7 +83,7 @@ export class ExistingTerminalInterceptor implements Interceptor {
 
         const serverState = { server, isActive: false };
 
-        const posixEnvVars = getTerminalEnvVars(proxyPort, this.config.https, 'posix-runtime-inherit', {});
+        const posixEnvVars = getTerminalEnvVars(proxyPort, this.config.https, 'posix-runtime-inherit');
 
         // Endpoints for each of the various setup scripts:
         await server.forGet('/setup').thenReply(200,
@@ -99,7 +99,7 @@ export class ExistingTerminalInterceptor implements Interceptor {
             { "content-type": "application/x-fish" }
         );
 
-        const powerShellEnvVars = getTerminalEnvVars(proxyPort, this.config.https, 'powershell-runtime-inherit', {});
+        const powerShellEnvVars = getTerminalEnvVars(proxyPort, this.config.https, 'powershell-runtime-inherit');
         await server.forGet('/ps-setup').thenReply(200,
             getPowerShellScript(server.urlFor('/success'), powerShellEnvVars),
             { "content-type": "text/plain" }

src/interceptors/terminal/fresh-terminal-interceptor.ts

@@ -228,7 +228,7 @@ export class FreshTerminalInterceptor implements Interceptor {
             _.assign(options || {}, {
                 env: {
                     ...currentEnv,
-                    ...getTerminalEnvVars(proxyPort, this.config.https, currentEnv, {}),
+                    ...getTerminalEnvVars(proxyPort, this.config.https, currentEnv),
                 },
                 cwd: currentEnv.HOME || currentEnv.USERPROFILE
             })

src/interceptors/terminal/terminal-env-overrides.ts

@@ -25,17 +25,19 @@ export function getTerminalEnvVars(
         | { [key: string]: string | undefined }
         | 'posix-runtime-inherit'
         | 'powershell-runtime-inherit',
-    targetEnvConfig: {
-        httpToolkitHost?: string,
-        overridePath?: string,
-        targetPlatform?: NodeJS.Platform
-    } = {}
+
+    // All 3 of the below must be overriden together, or not at all, to avoid
+    // mixing platforms & default (platform-specific) paths
+    targetEnvConfig?: {
+        httpToolkitHost: string,
+        overridePath: string,
+        targetPlatform: NodeJS.Platform
+    }
 ): { [key: string]: string } {
-    const { overridePath, targetPlatform, httpToolkitHost } = {
+    const { overridePath, targetPlatform, httpToolkitHost } = targetEnvConfig ?? {
         httpToolkitHost: '127.0.0.1',
         overridePath: OVERRIDES_DIR,
-        targetPlatform: process.platform,
-        ...targetEnvConfig
+        targetPlatform: process.platform
     };
 
     const runtimeInherit = currentEnv === 'posix-runtime-inherit'
@@ -63,7 +65,16 @@ export function getTerminalEnvVars(
     const rubyGemsPath = joinPath(overridePath, RUBY_OVERRIDE_DIR);
     const pythonPath = joinPath(overridePath, PYTHON_OVERRIDE_DIR);
     const phpPath = joinPath(overridePath, PHP_OVERRIDE_DIR);
-    const nodePrependScript = joinPath(overridePath, ...NODE_PREPEND_SCRIPT);
+
+    // Node supports POSIX paths everywhere, and using those solves some weird issues
+    // when combining backslashes with quotes in Git Bash on Windows:
+    const nodePrependScript = path.posix.join(
+        ...(targetPlatform === 'win32'
+            ? overridePath.split(path.win32.sep)
+            : [overridePath]
+        ),
+        ...NODE_PREPEND_SCRIPT
+    );
     const nodePrependOption = `--require ${
         // Avoid quoting except when necessary, because node 8 doesn't support quotes here
         nodePrependScript.includes(' ')

test/integration/interceptors/docker-terminal-interception.spec.ts

@@ -64,7 +64,7 @@ describe('Docker CLI interception', function () {
             const { server, httpsConfig} = await testSetup;
             const mainRule = await server.forGet('https://example.test').thenReply(200);
 
-            const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env, {});
+            const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env);
 
             childProc.spawn(
                 'docker', ['run', '--rm', imageId, 'https://example.test'],
@@ -95,7 +95,7 @@ describe('Docker CLI interception', function () {
 
         const mainRule = await server.forGet("https://example.test").thenReply(200);
 
-        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env, {});
+        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env);
 
         const { exitCode, stdout, stderr } = await spawnToResult(
             'docker', ['run', '--rm', 'node:14', '-e', `require("https").get("https://example.test")`],
@@ -119,7 +119,7 @@ describe('Docker CLI interception', function () {
 
         const mainRule = await server.forAnyRequest().thenReply(200, "Mock response\n");
 
-        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env, {});
+        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env);
 
         const { exitCode, stdout, stderr } = await spawnToResult('docker', ['build', '.'], {
             env: { ...process.env, ...terminalEnvOverrides },
@@ -185,7 +185,7 @@ Successfully built <hash>
     it("should support proxying 'docker exec'", async function () {
         const { server, httpsConfig } = await testSetup;
 
-        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env, {});
+        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env);
 
         // Launch a background Node.js container that stays running for 10 seconds
         const runResult = await spawnToResult(
@@ -232,7 +232,7 @@ Successfully built <hash>
             true
         );
 
-        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env, {});
+        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env);
 
         // "DC Up" the same project, but in an intercepted env. Should ignore the existing containers,
         // create new intercepted containers, and then up those as normal.
@@ -281,7 +281,7 @@ Successfully built <hash>
             true
         );
 
-        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env, {});
+        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env);
 
         // "DC Up" the same project, but in an intercepted env. Should ignore the existing containers,
         // create new intercepted containers, and then up those as normal.
@@ -316,7 +316,7 @@ Successfully built <hash>
     it("should clean up containers after shutdown", async () => {
         const { server, httpsConfig } = await testSetup;
 
-        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env, {});
+        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env);
 
         const uninterceptedResult = await spawnToResult('docker', ['create', 'node:14']);
 
@@ -358,7 +358,7 @@ Successfully built <hash>
         const { server, httpsConfig } = await testSetup;
         const docker = new Docker();
 
-        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env, {});
+        const terminalEnvOverrides = getTerminalEnvVars(server.port, httpsConfig, process.env);
 
         // Tunnel doesn't start preemptively:
         await delay(500);