From 6607112d0b87e1ddc05ebdda96db212d3dd1a65e Mon Sep 17 00:00:00 2001
From: Colin McDonnell <colinmcd94@gmail.com>
Date: Sun, 3 May 2026 17:33:13 +0000
Subject: [PATCH] Exclude GITHUB_WORKSPACE and relative entries from PATH walk
 (#558)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Exclude GITHUB_WORKSPACE and relative entries from PATH walk

resolveExecutable previously walked any directory listed in process.env.PATH,
which trusts that nothing earlier in the workflow prepended an
attacker-controlled location. A malicious PR could land bin/npx in the repo
and add `echo "$GITHUB_WORKSPACE/bin" >> $GITHUB_PATH` to a prior step,
causing pullfrog to exec the attacker's binary with our scoped tokens in env.

Filter out (a) any non-absolute PATH entry (., bin, .., etc., which resolve
against cwd) and (b) any entry equal to or under GITHUB_WORKSPACE. The walk
then continues to the next legitimate system tooling dir.

* Address PR #558 review: comment typo + Windows case bypass

- Drop double space in the threat-model comment.
- Lowercase paths on Windows before comparing against GITHUB_WORKSPACE.
  Without this, an attacker can bypass the filter by varying case in their
  injected PATH entry (`d:\a\repo\bin` vs `D:\a\repo`) — string compare
  misses but NTFS still resolves the executable inside the workspace.
---
 runCli.ts | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/runCli.ts b/runCli.ts
index b57907b..d19d26f 100644
--- a/runCli.ts
+++ b/runCli.ts
@@ -1,6 +1,6 @@
 import { execFileSync } from "node:child_process";
 import { accessSync, constants, existsSync } from "node:fs";
-import { delimiter, dirname, join } from "node:path";
+import { delimiter, dirname, isAbsolute, join, resolve, sep } from "node:path";
 import { fileURLToPath } from "node:url";
 import actionPackageJson from "./package.json" with { type: "json" };
 
@@ -42,9 +42,45 @@ function canAccessExecutable(path: string): boolean {
   }
 }
 
+// reject PATH entries that an attacker can plausibly write to before pullfrog
+// runs. specifically: relative entries (., bin, etc., which resolve against
+// cwd), and anything inside the customer's checkout. an attacker who can land
+// a malicious `npx` in the repo and prepend `$GITHUB_WORKSPACE/bin` to
+// `GITHUB_PATH` from a prior workflow step would otherwise get full code
+// execution under our action token.
+//
+// on Windows the filesystem is case-insensitive but `resolve()` preserves
+// input case, so we lowercase both sides before comparing — otherwise an
+// attacker can bypass the filter by varying the case of GITHUB_WORKSPACE in
+// their injected PATH entry (`d:\a\repo` vs `D:\a\repo`).
+function normalizePathForCompare(path: string): string {
+  return process.platform === "win32" ? resolve(path).toLowerCase() : resolve(path);
+}
+
+function isUntrustedPathEntry(entry: string, untrustedRoots: string[]): boolean {
+  if (!isAbsolute(entry)) return true;
+  const normalized = normalizePathForCompare(entry);
+  for (const root of untrustedRoots) {
+    if (normalized === root) return true;
+    if (normalized.startsWith(root + sep)) return true;
+  }
+  return false;
+}
+
+function getUntrustedPathRoots(env: NodeJS.ProcessEnv): string[] {
+  const roots: string[] = [];
+  const workspace = env.GITHUB_WORKSPACE;
+  if (workspace && isAbsolute(workspace)) roots.push(normalizePathForCompare(workspace));
+  return roots;
+}
+
 function resolveExecutable(params: { command: string; env: NodeJS.ProcessEnv }): string | null {
   const pathValue = params.env.PATH ?? "";
-  const pathEntries = pathValue.split(delimiter).filter(Boolean);
+  const untrustedRoots = getUntrustedPathRoots(params.env);
+  const pathEntries = pathValue
+    .split(delimiter)
+    .filter(Boolean)
+    .filter((entry) => !isUntrustedPathEntry(entry, untrustedRoots));
   const extensions =
     process.platform === "win32"
       ? (params.env.PATHEXT ?? ".COM;.EXE;.BAT;.CMD").split(";").filter(Boolean)