fix(platform): pass Windows browser URLs as validated data

Preserve contributor history and current-main behavior while resolving the exact reviewed follow-up. Source-PR: https://github.com/affaan-m/ECC/pull/3244 Source-Parent: 5ffe239ee5 Review-Manifest-SHA256: c18c5d1d4b29c0b78fc1638be8f5d72dc6141235c6ec26cb0dd20c91911cdbfe
2026-09-28 20:45:11 +02:00 · 2026-09-28 00:57:22 -04:00
parent 5ffe239ee5
commit 490585fbcb
2 changed files with 225 additions and 11 deletions
@@ -8,7 +8,7 @@
 * tri-platform branch) and scripts/control-pane.js (which had a darwin-only
 * branch that silently no-op'd on Windows/Linux). This helper:
 *
- *   1. Dispatches `open` / `cmd /c start` / `xdg-open` based on process.platform
+ *   1. Dispatches `open` / a fixed PowerShell launcher / `xdg-open` by platform
 *   2. Handles the child's 'error' event so a missing launcher does not cause
 *      an unhandled error after a detached spawn
 *   3. Returns a structured { opened, reason } result for the launch request.
@@ -23,6 +23,44 @@

 const { spawn } = require('child_process');

+const WINDOWS_BROWSER_URL = 'ECC_BROWSER_URL';
+// Only this constant is encoded as PowerShell source. The validated URL is
+// process-environment data, never command text or an interpolated argument.
+const WINDOWS_BROWSER_SCRIPT = `$ErrorActionPreference = 'Stop'
+try {
+  $value = [System.Environment]::GetEnvironmentVariable('ECC_BROWSER_URL', 'Process')
+  [System.Environment]::SetEnvironmentVariable('ECC_BROWSER_URL', $null, 'Process')
+  $uri = $null
+  if (-not [System.Uri]::TryCreate($value, [System.UriKind]::Absolute, [ref]$uri) -or @('http', 'https') -notcontains $uri.Scheme -or $uri.UserInfo) { exit 1 }
+  $info = New-Object System.Diagnostics.ProcessStartInfo
+  $info.FileName = $value
+  $info.UseShellExecute = $true
+  [void][System.Diagnostics.Process]::Start($info)
+} catch { exit 1 }
+`;
+const WINDOWS_BROWSER_COMMAND = Buffer.from(WINDOWS_BROWSER_SCRIPT, 'utf16le').toString('base64');
+
+function normalizeBrowserUrl(value) {
+  if (typeof value !== 'string' || !value || value !== value.trim()
+    || value.includes('\\') || !/^https?:\/\//i.test(value)) return null;
+  for (const character of value) {
+    const code = character.charCodeAt(0);
+    if (code < 32 || code === 127) return null;
+  }
+  try {
+    const url = new URL(value);
+    if (!['http:', 'https:'].includes(url.protocol) || !url.hostname || url.username || url.password) return null;
+    return url.href;
+  } catch {
+    return null;
+  }
+}
+
+function windowsBrowserEnvironment(url, environment) {
+  const entries = Object.entries(environment).filter(([key]) => key.toUpperCase() !== WINDOWS_BROWSER_URL);
+  return { ...Object.fromEntries(entries), [WINDOWS_BROWSER_URL]: url };
+}
+
 /**
 * Pick the platform-appropriate opener command + args.
 * Returns [cmd, args] suitable for child_process.spawn.
@@ -33,12 +71,14 @@ const { spawn } = require('child_process');
 */
 function openerCommandFor(platform, url) {
  if (platform === 'darwin') return ['open', [url]];
-  if (platform === 'win32') return ['cmd', ['/c', 'start', '', url]];
+  if (platform === 'win32') {
+    return ['powershell.exe', ['-NoLogo', '-NoProfile', '-NonInteractive', '-EncodedCommand', WINDOWS_BROWSER_COMMAND]];
+  }
  return ['xdg-open', [url]];
 }

 /**
- * Open a URL in the user's default browser, dispatching per-platform.
+ * Open an absolute HTTP/S URL in the default browser, dispatching per-platform.
 *
 * Returns the synchronous launch-request result. Asynchronous child errors
 * are handled, but are not an acknowledgment that a browser opened.
@@ -46,19 +86,26 @@ function openerCommandFor(platform, url) {
 * @param {string} url
 * @param {NodeJS.Platform} [platform] - injectable for tests; defaults to process.platform
 * @param {typeof spawn} [spawnProcess] - injectable process launcher for tests
+ * @param {NodeJS.ProcessEnv} [environment] - optional Windows child environment for tests
 * @returns {{ opened: boolean, reason: string }}
 */
-function openBrowser(url, platform = process.platform, spawnProcess = spawn) {
-  if (typeof url !== 'string' || url.length === 0) {
+function openBrowser(url, platform = process.platform, spawnProcess = spawn, environment) {
+  const normalizedUrl = normalizeBrowserUrl(url);
+  if (!normalizedUrl) {
    return { opened: false, reason: 'invalid-url' };
  }

-  const [cmd, args] = openerCommandFor(platform, url);
+  const [cmd, args] = openerCommandFor(platform, normalizedUrl);
  let child;
  try {
    child = spawnProcess(cmd, args, {
      detached: true,
      stdio: 'ignore',
+      shell: false,
+      ...(platform === 'win32' ? {
+        windowsHide: true,
+        env: windowsBrowserEnvironment(normalizedUrl, environment === undefined ? process.env : environment),
+      } : {}),
    });
  } catch (err) {
    return {
@@ -68,7 +115,7 @@ function openBrowser(url, platform = process.platform, spawnProcess = spawn) {
  }

  // Listen for ENOENT/EACCES/etc that would otherwise be silently swallowed
-  // when the user has no `open` / `xdg-open` / `start` available.
+  // when the user has no `open` / `xdg-open` / `powershell.exe` available.
  let capturedError = null;
  child.on('error', (err) => {
    capturedError = err && err.code ? err.code : 'spawn-error';
@@ -4,12 +4,47 @@ const test = require('node:test');
 const assert = require('node:assert/strict');
 const { openBrowser, openerCommandFor } = require('../../scripts/lib/platform-launch');

+// These values are private fixtures; no test reads or changes process.env.
+const privateEnvironment = Object.freeze({
+  Path: '/synthetic/bin', ECC_BROWSER_URL: 'old-upper',
+  ecc_browser_url: 'old-lower', EcC_BrOwSeR_Url: 'old-mixed', FIXTURE_VALUE: 'keep',
+});
+const expectedWindowsScript = `$ErrorActionPreference = 'Stop'
+try {
+  $value = [System.Environment]::GetEnvironmentVariable('ECC_BROWSER_URL', 'Process')
+  [System.Environment]::SetEnvironmentVariable('ECC_BROWSER_URL', $null, 'Process')
+  $uri = $null
+  if (-not [System.Uri]::TryCreate($value, [System.UriKind]::Absolute, [ref]$uri) -or @('http', 'https') -notcontains $uri.Scheme -or $uri.UserInfo) { exit 1 }
+  $info = New-Object System.Diagnostics.ProcessStartInfo
+  $info.FileName = $value
+  $info.UseShellExecute = $true
+  [void][System.Diagnostics.Process]::Start($info)
+} catch { exit 1 }
+`;
+
+function fakeChild() {
+  return { on() {}, unref() {} };
+}
+
+function captureLaunch(url, platform) {
+  const calls = [];
+  const result = openBrowser(url, platform, (command, args, options) => {
+    calls.push({ command, args, options });
+    return fakeChild();
+  }, privateEnvironment);
+  return { result, calls };
+}
+
 test('openerCommandFor: darwin returns open', () => {
  assert.deepEqual(openerCommandFor('darwin', 'http://x'), ['open', ['http://x']]);
 });

-test('openerCommandFor: win32 returns cmd /c start', () => {
-  assert.deepEqual(openerCommandFor('win32', 'http://x'), ['cmd', ['/c', 'start', '', 'http://x']]);
+test('openerCommandFor: win32 returns a static encoded PowerShell command', () => {
+  const [command, args] = openerCommandFor('win32', 'http://x');
+  assert.equal(command, 'powershell.exe');
+  assert.deepEqual(args.slice(0, 4), ['-NoLogo', '-NoProfile', '-NonInteractive', '-EncodedCommand']);
+  assert.equal(args.length, 5);
+  assert.equal(Buffer.from(args[4], 'base64').toString('utf16le'), expectedWindowsScript);
 });

 test('openerCommandFor: linux returns xdg-open', () => {
@@ -56,8 +91,8 @@ test('openBrowser: installs an error listener and detaches the launcher', () =>
  const result = openBrowser('http://localhost:0', 'linux', (command, args, options) => {
    launchCalls += 1;
    assert.equal(command, 'xdg-open');
-    assert.deepEqual(args, ['http://localhost:0']);
-    assert.deepEqual(options, { detached: true, stdio: 'ignore' });
+    assert.deepEqual(args, ['http://localhost:0/']);
+    assert.deepEqual(options, { detached: true, stdio: 'ignore', shell: false });
    return child;
  });

@@ -83,3 +118,135 @@ test('openBrowser: a detach failure does not escape after the listener is instal
  assert.deepEqual(result, { opened: true, reason: 'spawned' });
  assert.doesNotThrow(() => listener({ code: 'EACCES' }));
 });
+
+
+for (const [name, value] of [
+  ['empty', ''], ['null', null], ['undefined', undefined], ['number', 42],
+  ['boolean', true], ['array', ['https://example.test']],
+  ['boxed string', Object('https://example.test')],
+  ['object', { toString() { throw new Error('must not coerce'); } }],
+  ['leading whitespace', ' https://example.test'],
+  ['trailing whitespace', 'https://example.test '],
+  ['tab', 'https://example.test/\tdata'],
+  ['newline', 'https://example.test/\ndata'],
+  ['carriage return', 'https://example.test/\rdata'],
+  ['NUL', 'https://example.test/\u0000data'],
+  ['DEL', 'https://example.test/\u007fdata'],
+  ['backslash', 'https://example.test/\\data'],
+  ['relative', '/example'], ['protocol relative', '//example.test'],
+  ['javascript', 'javascript:alert(1)'], ['data', 'data:text/plain,hello'],
+  ['file', 'file:///tmp/example'], ['custom protocol', 'app://example'],
+  ['missing host', 'https://'], ['missing double slash', 'https:example.test'],
+  ['credentials', 'https://user:password@example.test'],
+  ['username', 'https://user@example.test'], ['password only', 'https://:password@example.test'],
+  ['invalid port', 'https://example.test:65536/'],
+  ['invalid IPv6', 'http://::1:3000/'], ['invalid host', 'https://exa mple.test/'],
+]) {
+  test(`openBrowser: rejects ${name} before every platform dispatch`, () => {
+    for (const platform of ['win32', 'darwin', 'linux']) {
+      const { result, calls } = captureLaunch(value, platform);
+      assert.deepEqual(result, { opened: false, reason: 'invalid-url' });
+      assert.equal(calls.length, 0);
+    }
+  });
+}
+
+test('openBrowser: rejects every raw ASCII control byte without coercion', () => {
+  for (let code = 0; code < 32; code += 1) {
+    const { result, calls } = captureLaunch(`https://example.test/a${String.fromCharCode(code)}b`, 'win32');
+    assert.deepEqual(result, { opened: false, reason: 'invalid-url' });
+    assert.equal(calls.length, 0);
+  }
+});
+
+for (const [input, normalized] of [
+  ['http://localhost:0', 'http://localhost:0/'],
+  ['HTTP://127.0.0.1:3000/test?q=1#frag', 'http://127.0.0.1:3000/test?q=1#frag'],
+  ['https://[::1]:443', 'https://[::1]/'],
+  ['HTTPS://EXAMPLE.TEST:443', 'https://example.test/'],
+  ['https://example.test/a%20b?q=a&x=b#fragment', 'https://example.test/a%20b?q=a&x=b#fragment'],
+  ['https://b\u00fccher.example/caf\u00e9', 'https://xn--bcher-kva.example/caf%C3%A9'],
+]) {
+  test(`openBrowser: normalizes an accepted HTTP/S URL on every platform (${input})`, () => {
+    for (const platform of ['darwin', 'linux', 'freebsd', 'win32']) {
+      const { result, calls } = captureLaunch(input, platform);
+      assert.deepEqual(result, { opened: true, reason: 'spawned' });
+      assert.equal(calls.length, 1);
+      const { command, args, options } = calls[0];
+      assert.equal(options.shell, false);
+      assert.equal(options.detached, true);
+      assert.equal(options.stdio, 'ignore');
+      assert.notEqual(options.windowsVerbatimArguments, true);
+      if (platform === 'win32') {
+        assert.equal(command, 'powershell.exe');
+        assert.equal(options.env.ECC_BROWSER_URL, normalized);
+        assert.equal(options.windowsHide, true);
+      } else {
+        assert.equal(command, platform === 'darwin' ? 'open' : 'xdg-open');
+        assert.deepEqual(args, [normalized]);
+        assert.equal(Object.hasOwn(options, 'env'), false);
+      }
+    }
+  });
+}
+
+test('openBrowser: different untrusted URL data leaves Windows code and argv identical', () => {
+  const inputs = [
+    'https://example.test/?left=1&right=2',
+    `https://example.test/?q="';$(Get-Process);&next=%COMSPEC%&name=caf\u00e9`,
+    'https://example.test/?q=%22%26%0d%0a&next=https%3A%2F%2Fexample.test',
+  ];
+  let firstArgs;
+  for (const input of inputs) {
+    const { result, calls } = captureLaunch(input, 'win32');
+    assert.deepEqual(result, { opened: true, reason: 'spawned' });
+    assert.equal(calls.length, 1);
+    const { command, args, options } = calls[0];
+    assert.equal(command, 'powershell.exe');
+    assert.deepEqual(args.slice(0, 4), ['-NoLogo', '-NoProfile', '-NonInteractive', '-EncodedCommand']);
+    assert.equal(args.length, 5);
+    assert.equal(Buffer.from(args[4], 'base64').toString('utf16le'), expectedWindowsScript);
+    assert.ok(!args.some(arg => arg.includes(input)));
+    if (firstArgs) assert.deepEqual(args, firstArgs);
+    else firstArgs = args;
+    assert.equal(options.env.ECC_BROWSER_URL, new URL(input).href);
+    assert.equal(options.shell, false);
+  }
+  assert.doesNotMatch(expectedWindowsScript, /Invoke-Expression|Start-Process|\.Arguments|cmd|ExecutionPolicy/);
+  assert.ok(expectedWindowsScript.indexOf('::SetEnvironmentVariable') < expectedWindowsScript.indexOf('::Start($info)'));
+});
+
+test('openBrowser: Windows child environment removes case-insensitive aliases without mutation', () => {
+  const before = { ...privateEnvironment };
+  const { calls } = captureLaunch('https://example.test/', 'win32');
+  assert.equal(calls.length, 1);
+  const childEnvironment = calls[0].options.env;
+  assert.notEqual(childEnvironment, privateEnvironment);
+  assert.deepEqual(Object.keys(childEnvironment).filter(key => key.toLowerCase() === 'ecc_browser_url'), ['ECC_BROWSER_URL']);
+  assert.equal(childEnvironment.ECC_BROWSER_URL, 'https://example.test/');
+  assert.equal(childEnvironment.Path, before.Path);
+  assert.equal(childEnvironment.FIXTURE_VALUE, before.FIXTURE_VALUE);
+  assert.deepEqual(privateEnvironment, before);
+});
+
+test('openBrowser: default platform still uses the injected launcher and private environment', () => {
+  const { result, calls } = captureLaunch('https://example.test', undefined);
+  assert.deepEqual(result, { opened: true, reason: 'spawned' });
+  assert.equal(calls.length, 1);
+  assert.deepEqual([calls[0].command, calls[0].args], openerCommandFor(process.platform, 'https://example.test/'));
+});
+
+for (const [error, reason] of [
+  [{ code: 'EACCES' }, 'child-error:EACCES'],
+  [new Error('no code'), 'child-error:spawn-error'],
+]) {
+  test(`openBrowser: captures an already-signalled child failure (${reason})`, () => {
+    let unrefCalls = 0;
+    const result = openBrowser('https://example.test/', 'win32', () => ({
+      on(event, listener) { assert.equal(event, 'error'); listener(error); },
+      unref() { unrefCalls += 1; },
+    }), privateEnvironment);
+    assert.deepEqual(result, { opened: false, reason });
+    assert.equal(unrefCalls, 1);
+  });
+}