Validate Pipfile markers against the PEP 508 grammar

packages/adapters/python/src/marker-syntax.test.ts

@@ -0,0 +1,43 @@
+import assert from "node:assert/strict";
+import { describe, it } from "node:test";
+import { isValidMarker } from "./marker-syntax.js";
+
+describe("isValidMarker", () => {
+  it("accepts PEP 508 markers", () => {
+    for (const text of [
+      "python_version >= '3.11'",
+      'sys_platform == "win32"',
+      "'x' in platform_version",
+      "os_name not in 'a b'",
+      "(os_name == 'nt' or os_name == 'posix') and python_version < '3.11'",
+      "extra == 'speed'",
+      "'3.10' <= python_version",
+    ])
+      assert.ok(isValidMarker(text), text);
+  });
+
+  it("rejects text that is not a marker", () => {
+    for (const text of [
+      "",
+      "python_version",
+      "python_version < 3.11",
+      "sys_platform == win32",
+      "python_version >=",
+      "(os_name == 'nt'",
+      "os_name == 'nt')",
+      "os_name == 'nt' and",
+      "bogus == 'x'",
+      "os_name = 'nt'",
+      "os_name == 'nt' python_version == '3'",
+      "os_name == 'a\nb'",
+    ])
+      assert.ok(!isValidMarker(text), text);
+  });
+
+  it("treats very deep nesting as invalid instead of overflowing the stack", () => {
+    const deep = `${"(".repeat(20000)}os_name == 'nt'${")".repeat(20000)}`;
+    assert.equal(isValidMarker(deep), false);
+    const ok = `${"(".repeat(10)}os_name == 'nt'${")".repeat(10)}`;
+    assert.equal(isValidMarker(ok), true);
+  });
+});

packages/adapters/python/src/marker-syntax.ts

@@ -0,0 +1,112 @@
+/**
+ * Whether a string is a syntactically valid PEP 508 environment marker.
+ * Syntax only: variables and strings are not evaluated. Pipfile shorthand
+ * values are joined into marker text, and pipenv discards a combined marker
+ * that does not parse, so the reader needs the same test.
+ */
+const VARIABLES = new Set([
+  "python_version",
+  "python_full_version",
+  "os_name",
+  "sys_platform",
+  "platform_release",
+  "platform_system",
+  "platform_version",
+  "platform_machine",
+  "platform_python_implementation",
+  "implementation_name",
+  "implementation_version",
+  "extra",
+  // Legacy dotted spellings that packaging still accepts.
+  "os.name",
+  "sys.platform",
+  "platform.version",
+  "platform.machine",
+  "platform.python_implementation",
+  "python_implementation",
+]);
+
+const TOKEN =
+  /\s*(?:('[^'\n\r]*'|"[^"\n\r]*")|(===|==|!=|<=|>=|~=|<|>)|(\()|(\))|([A-Za-z_][A-Za-z0-9_.]*))/y;
+
+/** Nesting beyond this is treated as invalid rather than recursing without bound. */
+const MAX_DEPTH = 64;
+
+export function isValidMarker(text: string): boolean {
+  const tokens: { kind: "str" | "op" | "open" | "close" | "word"; value: string }[] = [];
+  TOKEN.lastIndex = 0;
+  let position = 0;
+  while (position < text.length) {
+    if (/^\s*$/.test(text.slice(position))) break;
+    TOKEN.lastIndex = position;
+    const match = TOKEN.exec(text);
+    if (match === null) return false;
+    position = TOKEN.lastIndex;
+    if (match[1] !== undefined) tokens.push({ kind: "str", value: match[1] });
+    else if (match[2] !== undefined) tokens.push({ kind: "op", value: match[2] });
+    else if (match[3] !== undefined) tokens.push({ kind: "open", value: "(" });
+    else if (match[4] !== undefined) tokens.push({ kind: "close", value: ")" });
+    else tokens.push({ kind: "word", value: match[5] as string });
+  }
+
+  let index = 0;
+  let depth = 0;
+  const word = (value: string): boolean => {
+    const token = tokens[index];
+    if (token?.kind === "word" && token.value === value) {
+      index += 1;
+      return true;
+    }
+    return false;
+  };
+  const variable = (): boolean => {
+    const token = tokens[index];
+    if (token === undefined) return false;
+    if (token.kind === "str" || (token.kind === "word" && VARIABLES.has(token.value))) {
+      index += 1;
+      return true;
+    }
+    return false;
+  };
+  const operator = (): boolean => {
+    const token = tokens[index];
+    if (token?.kind === "op") {
+      index += 1;
+      return true;
+    }
+    if (word("in")) return true;
+    if (
+      tokens[index]?.kind === "word" &&
+      tokens[index]?.value === "not" &&
+      tokens[index + 1]?.kind === "word" &&
+      tokens[index + 1]?.value === "in"
+    ) {
+      index += 2;
+      return true;
+    }
+    return false;
+  };
+  const expression = (): boolean => {
+    if (tokens[index]?.kind === "open") {
+      index += 1;
+      depth += 1;
+      if (depth > MAX_DEPTH || !orExpression()) return false;
+      depth -= 1;
+      if (tokens[index]?.kind !== "close") return false;
+      index += 1;
+      return true;
+    }
+    return variable() && operator() && variable();
+  };
+  const andExpression = (): boolean => {
+    if (!expression()) return false;
+    while (word("and")) if (!expression()) return false;
+    return true;
+  };
+  const orExpression = (): boolean => {
+    if (!andExpression()) return false;
+    while (word("or")) if (!andExpression()) return false;
+    return true;
+  };
+  return tokens.length > 0 && orExpression() && index === tokens.length;
+}

packages/adapters/python/src/pipfile.test.ts

@@ -96,3 +96,39 @@ it("ignores a Pipfile shorthand value with no operator and the non-pipenv extra
   assert.equal(parsed.requirements[0]?.marker, undefined);
   assert.equal(parsed.requirements[1]?.marker, undefined);
 });
+
+it("drops a Pipfile marker that does not parse, including unquoted shorthand values", () => {
+  const parsed = parsePipfileText(
+    [
+      "[packages]",
+      'unquoted = {version = "*", python_version = "< 3.11"}',
+      'platform = {version = "*", sys_platform = "== win32"}',
+      'badfull = {version = "*", markers = "python_version >="}',
+      'mixed = {version = "*", markers = "os_name == \'nt\'", python_version = "< 3.11"}',
+      'good = {version = "*", sys_platform = "== \'win32\'", platform_machine = "== \'x86_64\'"}',
+      "",
+    ].join("\n"),
+    project,
+    "Pipfile",
+  );
+  assert.equal(parsed.complete, true);
+  const marker = (name: string) =>
+    parsed.requirements.find((r) => r.dependency.name === name)?.marker;
+  assert.equal(marker("unquoted"), undefined);
+  assert.equal(marker("platform"), undefined);
+  assert.equal(marker("badfull"), undefined);
+  assert.equal(marker("mixed"), undefined);
+  assert.equal(marker("good"), "(sys_platform == 'win32') and (platform_machine == 'x86_64')");
+});
+
+it("keeps parsing when a Pipfile marker is nested absurdly deep", () => {
+  const deep = `${"(".repeat(20000)}os_name == 'nt'${")".repeat(20000)}`;
+  const parsed = parsePipfileText(
+    `[packages]\ndeep = {version = "*", markers = "${deep}"}\nplain = "*"\n`,
+    project,
+    "Pipfile",
+  );
+  assert.equal(parsed.complete, true);
+  assert.equal(parsed.requirements.length, 2);
+  assert.equal(parsed.requirements[0]?.marker, undefined);
+});

packages/adapters/python/src/pipfile.ts

@@ -2,6 +2,7 @@
 import { parse as parseToml } from "smol-toml";
 import type { Dependency, Evidence, ProjectRef } from "@ghostdeps/core";
 import { PyprojectLines } from "./declared-line.js";
+import { isValidMarker } from "./marker-syntax.js";
 import { normaliseName } from "./pep508.js";
 import { classifyUrl, type PythonRequirement } from "./pyproject.js";
 
@@ -167,7 +168,6 @@ export function parsePipfileText(
         groups: [],
       };
       const markerParts: string[] = [];
-      let shorthandInvalid = false;
       if (typeof options?.markers === "string" && options.markers.length > 0)
         markerParts.push(options.markers);
       for (const key of [
@@ -185,20 +185,17 @@ export function parsePipfileText(
       ]) {
         const value = options?.[key];
         if (typeof value !== "string" || value.length === 0) continue;
-        // pipenv builds "<key> <value>" and discards the whole combined marker
-        // when that is not valid marker syntax, so a value with no operator
-        // leaves the requirement unconditional.
-        if (!/^\s*(?:===|==|!=|<=|>=|~=|<|>|in\b|not\s+in\b)\s*\S/.test(value)) {
-          shorthandInvalid = true;
-          continue;
-        }
         markerParts.push(`${key} ${value}`);
       }
-      if (markerParts.length > 0 && !shorthandInvalid)
-        requirement.marker =
+      // pipenv joins "<key> <value>" parts and discards the whole combined
+      // marker when it does not parse, leaving the requirement unconditional.
+      if (markerParts.length > 0) {
+        const combined =
           markerParts.length === 1
             ? markerParts[0]!
             : markerParts.map((part) => `(${part})`).join(" and ");
+        if (isValidMarker(combined)) requirement.marker = combined;
+      }
       byKey.set(key, requirement);
       requirements.push(requirement);
     }