From a9f7d7bbcf04e7c0dc91d8272230d37fab9b7e9e Mon Sep 17 00:00:00 2001
From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com>
Date: Sat, 31 Oct 2026 12:38:28 +0000
Subject: [PATCH] Refuse a `..` step behind a backslash in a browser binary
 path

#1449 split a POSIX-form candidate on `/` alone, so the `..` check stopped
seeing `\..\` in it. A Windows host reads `/opt\..\x/agent-browser.exe` as
rooted on the current drive with `\` as a separator, so the gate accepted a
parent step S168 says it refuses. The `..` check now splits on both
separators; the file-name check keeps the per-form split.

Re-seeds the unc-binary-path canary over the moved lines.
---
 .github/audit/canaries/unc-binary-path.patch | 22 ++++++++++----------
 dor-lib-common/src/browser-providers.ts      |  8 ++++---
 lib/src/lib/agent-browser-binary.test.ts     |  4 +++-
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/.github/audit/canaries/unc-binary-path.patch b/.github/audit/canaries/unc-binary-path.patch
index 93d3ccfde..26c5c982b 100644
--- a/.github/audit/canaries/unc-binary-path.patch
+++ b/.github/audit/canaries/unc-binary-path.patch
@@ -4,7 +4,7 @@ Expect: dor-lib-common/src/browser-providers.ts
 Source: reverses #1040 (a UNC binaryPath passed the executable gate)
 
 diff --git a/dor-lib-common/src/browser-providers.ts b/dor-lib-common/src/browser-providers.ts
-index 378c8a6bd..2d6e9bd35 100644
+index 46e980958..ca8385e1e 100644
 --- a/dor-lib-common/src/browser-providers.ts
 +++ b/dor-lib-common/src/browser-providers.ts
 @@ -67,10 +67,10 @@ export const DEFAULT_PLAYWRIGHT_BIN = 'playwright-cli';
@@ -28,17 +28,17 @@ index 378c8a6bd..2d6e9bd35 100644
    if (candidate === name) return true;
 -  if (isUncOrDevicePath(candidate) || !ABSOLUTE_RE.test(candidate)) return false;
 +  if (!ABSOLUTE_RE.test(candidate)) return false;
-   // Each path form's own separators: a POSIX file name may contain `\`, so
-   // only a drive path splits on it.
-   const segments = candidate.split(candidate.startsWith('/') ? '/' : /[\\/]/);
+   // A `..` step on either separator: a Windows host reads `/opt\..\x` as
+   // rooted on the current drive, with `\` separating there too.
+   if (candidate.split(/[\\/]/).includes('..')) return false;
 diff --git a/lib/src/lib/agent-browser-binary.test.ts b/lib/src/lib/agent-browser-binary.test.ts
-index 18d5d9f5e..91a2f7485 100644
+index 22e62c34c..5eafa34b8 100644
 --- a/lib/src/lib/agent-browser-binary.test.ts
 +++ b/lib/src/lib/agent-browser-binary.test.ts
-@@ -39,14 +39,6 @@ describe('isAllowedAgentBrowserBinary', () => {
-       // A POSIX path splits on `/` alone: `\` is a file-name character there.
-       '/r/x\\agent-browser',
+@@ -41,14 +41,6 @@ describe('isAllowedAgentBrowserBinary', () => {
        'C:\\x\\..\\agent-browser.exe',
+       // Windows reads a leading-`/` path as drive-rooted, `\` a separator in it.
+       '/opt\\..\\x/agent-browser.exe',
 -      // UNC and device paths: Windows would run a file off another machine.
 -      '\\\\host\\share\\agent-browser.exe',
 -      '//host/share/agent-browser.exe',
@@ -50,11 +50,11 @@ index 18d5d9f5e..91a2f7485 100644
        // Control characters are how one argument becomes two.
        '/usr/local/bin/agent-browser\n/bin/sh',
        '/usr/local/bin/agent-browser\u0000',
-@@ -65,6 +57,6 @@ describe('isAllowedAgentBrowserBinary', () => {
+@@ -67,6 +59,6 @@ describe('isAllowedAgentBrowserBinary', () => {
  
   it('validates Playwright executable hints independently of agent-browser', () => {
    for (const candidate of ['playwright-cli', '/opt/bin/playwright-cli', 'C:\\tools\\playwright-cli.cmd']) expect(isAllowedPlaywrightBinary(candidate)).toBe(true);
--  for (const candidate of ['agent-browser', '/bin/sh', './playwright-cli', '/opt/../playwright-cli', '/r/x\\playwright-cli', '/opt/playwright-cli\n', '/opt/a\u0085b/playwright-cli', '\\\\host\\share\\playwright-cli.cmd', '//host/share/playwright-cli']) expect(isAllowedPlaywrightBinary(candidate)).toBe(false);
-+  for (const candidate of ['agent-browser', '/bin/sh', './playwright-cli', '/opt/../playwright-cli', '/r/x\\playwright-cli', '/opt/playwright-cli\n', '/opt/a\u0085b/playwright-cli']) expect(isAllowedPlaywrightBinary(candidate)).toBe(false);
+-  for (const candidate of ['agent-browser', '/bin/sh', './playwright-cli', '/opt/../playwright-cli', '/opt\\..\\x/playwright-cli', '/r/x\\playwright-cli', '/opt/playwright-cli\n', '/opt/a\u0085b/playwright-cli', '\\\\host\\share\\playwright-cli.cmd', '//host/share/playwright-cli']) expect(isAllowedPlaywrightBinary(candidate)).toBe(false);
++  for (const candidate of ['agent-browser', '/bin/sh', './playwright-cli', '/opt/../playwright-cli', '/opt\\..\\x/playwright-cli', '/r/x\\playwright-cli', '/opt/playwright-cli\n', '/opt/a\u0085b/playwright-cli']) expect(isAllowedPlaywrightBinary(candidate)).toBe(false);
    expect(isAllowedPlaywrightBinary('/opt/custom-wrapper', '/opt/custom-wrapper')).toBe(true);
  });
diff --git a/dor-lib-common/src/browser-providers.ts b/dor-lib-common/src/browser-providers.ts
index 378c8a6bd..46e980958 100644
--- a/dor-lib-common/src/browser-providers.ts
+++ b/dor-lib-common/src/browser-providers.ts
@@ -110,10 +110,12 @@ function isAllowedBrowserBinary(candidate: unknown, configuredPath: string | und
   if (configuredPath && candidate === configuredPath) return true;
   if (candidate === name) return true;
   if (isUncOrDevicePath(candidate) || !ABSOLUTE_RE.test(candidate)) return false;
-  // Each path form's own separators: a POSIX file name may contain `\`, so
-  // only a drive path splits on it.
+  // A `..` step on either separator: a Windows host reads `/opt\..\x` as
+  // rooted on the current drive, with `\` separating there too.
+  if (candidate.split(/[\\/]/).includes('..')) return false;
+  // The file name per each path form's own separators: a POSIX file name may
+  // contain `\`, so only a drive path splits on it.
   const segments = candidate.split(candidate.startsWith('/') ? '/' : /[\\/]/);
-  if (segments.includes('..')) return false;
   return filename.test(segments[segments.length - 1] ?? '');
 }
 
diff --git a/lib/src/lib/agent-browser-binary.test.ts b/lib/src/lib/agent-browser-binary.test.ts
index 18d5d9f5e..22e62c34c 100644
--- a/lib/src/lib/agent-browser-binary.test.ts
+++ b/lib/src/lib/agent-browser-binary.test.ts
@@ -39,6 +39,8 @@ describe('isAllowedAgentBrowserBinary', () => {
       // A POSIX path splits on `/` alone: `\` is a file-name character there.
       '/r/x\\agent-browser',
       'C:\\x\\..\\agent-browser.exe',
+      // Windows reads a leading-`/` path as drive-rooted, `\` a separator in it.
+      '/opt\\..\\x/agent-browser.exe',
       // UNC and device paths: Windows would run a file off another machine.
       '\\\\host\\share\\agent-browser.exe',
       '//host/share/agent-browser.exe',
@@ -65,6 +67,6 @@ describe('isAllowedAgentBrowserBinary', () => {
 
  it('validates Playwright executable hints independently of agent-browser', () => {
   for (const candidate of ['playwright-cli', '/opt/bin/playwright-cli', 'C:\\tools\\playwright-cli.cmd']) expect(isAllowedPlaywrightBinary(candidate)).toBe(true);
-  for (const candidate of ['agent-browser', '/bin/sh', './playwright-cli', '/opt/../playwright-cli', '/r/x\\playwright-cli', '/opt/playwright-cli\n', '/opt/a\u0085b/playwright-cli', '\\\\host\\share\\playwright-cli.cmd', '//host/share/playwright-cli']) expect(isAllowedPlaywrightBinary(candidate)).toBe(false);
+  for (const candidate of ['agent-browser', '/bin/sh', './playwright-cli', '/opt/../playwright-cli', '/opt\\..\\x/playwright-cli', '/r/x\\playwright-cli', '/opt/playwright-cli\n', '/opt/a\u0085b/playwright-cli', '\\\\host\\share\\playwright-cli.cmd', '//host/share/playwright-cli']) expect(isAllowedPlaywrightBinary(candidate)).toBe(false);
   expect(isAllowedPlaywrightBinary('/opt/custom-wrapper', '/opt/custom-wrapper')).toBe(true);
 });