From 53e773d70e4eed552928e604e38cecf2d7a9cbba Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Thu, 27 Aug 2026 02:06:59 -0400
Subject: [PATCH] chore(scripts): stop the assignment capture at the closing
 quote (#2146)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Copilot, eighth review: the bare-value pattern ran to whitespace, so a QUOTED
assignment swallowed the closing quote — `echo "SMOKE_BROWSER=chromium" >>
$GITHUB_ENV` captured `chromium"`, which is not `chromium`, and the guard
rejected a PERMITTED assignment. A false positive on the allowed value is the
worse direction to be wrong in: it fails a workflow that is doing the right
thing, and there is no way to satisfy the guard except to stop using the
supported spelling.

The value is now a quoted string or a bare run stopping at a quote or a `;`
(PowerShell's separator), so all three dialects unwrap the same way.
Regression cases for the permitted quoted `chromium` in $GITHUB_ENV, in a
cmd-style `set`, and single-quoted in PowerShell — each paired with the
firefox spelling that must still fail.

Signed-off-by: Cliff Hall <cliff@futurescale.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
---
 scripts/lib/workflow-gate.mjs      | 31 +++++++++++++++++++++++++-----
 scripts/lib/workflow-gate.test.mjs | 24 +++++++++++++++++++++++
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/scripts/lib/workflow-gate.mjs b/scripts/lib/workflow-gate.mjs
index 66d18c0d..0a17dd78 100644
--- a/scripts/lib/workflow-gate.mjs
+++ b/scripts/lib/workflow-gate.mjs
@@ -145,20 +145,41 @@ const DYNAMIC_ENGINE_RE = new RegExp(
  * More than one dialect is needed because `shell:` and the runner's OS decide
  * which one a step is written in, and **PowerShell is the default on a Windows
  * runner** — so a POSIX-only pattern is silent on exactly the workflow least
- * likely to be reviewed closely (Copilot). `\S*` rather than `\S+` throughout,
- * because assigning nothing sets the empty string, which `resolveBrowserName`
- * rejects as present-but-empty rather than treating as unset.
+ * likely to be reviewed closely (Copilot). Every one of them may match an EMPTY
+ * value, because assigning nothing sets the empty string, which
+ * `resolveBrowserName` rejects as present-but-empty rather than treating as
+ * unset.
  */
+/**
+ * The value half of an assignment: a quoted string, or a bare run of
+ * non-delimiter characters.
+ *
+ * `\S*` was wrong in the direction that matters least often and hurts most:
+ * for `echo "SMOKE_BROWSER=chromium" >> $GITHUB_ENV` it captured `chromium"`,
+ * which is not `chromium`, so the guard rejected a **permitted** assignment
+ * (Copilot). A false positive on the allowed value is worse than one on a
+ * forbidden one — it fails a workflow that is doing the right thing.
+ *
+ * The bare arm therefore stops at a quote or a `;` (PowerShell's separator)
+ * rather than running to whitespace, and the quoted arms keep a value that
+ * really is quoted intact for `stripQuotes` to unwrap. Empty stays matchable —
+ * `SMOKE_BROWSER=` sets the empty string, which is its own finding.
+ */
+const ASSIGNED_VALUE = String.raw`("[^"]*"|'[^']*'|[^\s"';]*)`;
+
 const BROWSER_ASSIGNMENTS = [
   // POSIX `SMOKE_BROWSER=firefox`. Also covers cmd's `set SMOKE_BROWSER=…` and
   // the GitHub-native `echo "SMOKE_BROWSER=…" >> $GITHUB_ENV`, since both carry
   // the same `NAME=value` substring.
   // Case-insensitive for the same Windows reason as the `env:` key above.
-  { re: new RegExp(String.raw`\b${BROWSER_ENV_VAR}=(\S*)`, "gi"), value: 1 },
+  {
+    re: new RegExp(String.raw`\b${BROWSER_ENV_VAR}=${ASSIGNED_VALUE}`, "gi"),
+    value: 1,
+  },
   // PowerShell `$env:SMOKE_BROWSER = "firefox"`, and its `${env:NAME}` form.
   {
     re: new RegExp(
-      String.raw`\$\{?env:${BROWSER_ENV_VAR}\}?\s*=\s*(\S*)`,
+      String.raw`\$\{?env:${BROWSER_ENV_VAR}\}?\s*=\s*${ASSIGNED_VALUE}`,
       "gi",
     ),
     value: 1,
diff --git a/scripts/lib/workflow-gate.test.mjs b/scripts/lib/workflow-gate.test.mjs
index 6b1e655f..4d3263e9 100644
--- a/scripts/lib/workflow-gate.test.mjs
+++ b/scripts/lib/workflow-gate.test.mjs
@@ -237,6 +237,30 @@ describe("findWorkflowViolations", () => {
       ),
       rules: [VIOLATION.BROWSER_ENV],
     },
+    {
+      // Copilot, eighth review: the bare-value pattern used to run to
+      // whitespace, so a QUOTED assignment captured the closing quote —
+      // rejecting a permitted `chromium`. A false positive on the allowed value
+      // fails a workflow that is doing the right thing, which is the worse
+      // direction to be wrong in.
+      name: "allows a quoted chromium appended to $GITHUB_ENV",
+      text: workflow(
+        '      - run: echo "SMOKE_BROWSER=chromium" >> $GITHUB_ENV',
+      ),
+      rules: [],
+    },
+    {
+      name: "allows a quoted chromium in a cmd-style set",
+      text: workflow('      - run: set "SMOKE_BROWSER=chromium"'),
+      rules: [],
+    },
+    {
+      name: "allows a single-quoted chromium in PowerShell",
+      text: workflow(
+        "      - run: $env:SMOKE_BROWSER = 'chromium'; npm run smoke",
+      ),
+      rules: [],
+    },
     {
       // cmd's `set NAME=value` and the GitHub-native `>> $GITHUB_ENV` append
       // both carry the POSIX `NAME=value` substring, so one pattern covers all