Harden JavaScript expression parser against prototype pollution and traversal attacks

actions/setup/js/runtime_import.cjs

@@ -116,18 +116,52 @@ const ALLOWED_EXPRESSIONS = [
 function isSafeExpression(expr) {
   const trimmed = expr.trim();
 
+  // Block dangerous JavaScript built-in property names
+  const DANGEROUS_PROPS = [
+    "constructor",
+    "__proto__",
+    "prototype",
+    "__defineGetter__",
+    "__defineSetter__",
+    "__lookupGetter__",
+    "__lookupSetter__",
+    "hasOwnProperty",
+    "isPrototypeOf",
+    "propertyIsEnumerable",
+    "toString",
+    "valueOf",
+    "toLocaleString",
+  ];
+
+  // Split expression into parts and check each for dangerous properties
+  // Handle both dot notation (e.g., "github.event.issue") and bracket notation (e.g., "release.assets[0].id")
+  const parts = trimmed.split(/[.\[\]]+/).filter(p => p && !/^\d+$/.test(p));
+
+  for (const part of parts) {
+    if (DANGEROUS_PROPS.includes(part)) {
+      return false; // Block dangerous property
+    }
+  }
+
   // Check exact match in allowed list
   if (ALLOWED_EXPRESSIONS.includes(trimmed)) {
     return true;
   }
 
   // Check if it matches dynamic patterns:
-  // - needs.* and steps.* (job dependencies and step outputs)
+  // - needs.* and steps.* (job dependencies and step outputs) - max depth 5 levels
   // - github.event.inputs.* (workflow_dispatch inputs)
   // - github.aw.inputs.* (shared workflow inputs)
   // - inputs.* (workflow_call inputs)
   // - env.* (environment variables)
-  const dynamicPatterns = [/^(needs|steps)\.[a-zA-Z0-9_-]+(\.[a-zA-Z0-9_-]+)*$/, /^github\.event\.inputs\.[a-zA-Z0-9_-]+$/, /^github\.aw\.inputs\.[a-zA-Z0-9_-]+$/, /^inputs\.[a-zA-Z0-9_-]+$/, /^env\.[a-zA-Z0-9_-]+$/];
+  // Limit nesting depth to max 5 levels to prevent deep traversal attacks
+  const dynamicPatterns = [
+    /^(needs|steps)\.[a-zA-Z0-9_-]+\.[a-zA-Z0-9_-]+(\.[a-zA-Z0-9_-]+){0,2}$/, // Max depth: needs.job.outputs.foo.bar (5 levels)
+    /^github\.event\.inputs\.[a-zA-Z0-9_-]+$/,
+    /^github\.aw\.inputs\.[a-zA-Z0-9_-]+$/,
+    /^inputs\.[a-zA-Z0-9_-]+$/,
+    /^env\.[a-zA-Z0-9_-]+$/,
+  ];
 
   for (const pattern of dynamicPatterns) {
     if (pattern.test(trimmed)) {
@@ -245,8 +279,13 @@ function evaluateExpression(expr) {
         inputs: context.payload?.inputs || {},
       };
 
+      // Freeze the evaluation context to prevent modification
+      Object.freeze(evalContext);
+      Object.freeze(evalContext.github);
+
       // Parse property access (e.g., "github.actor" -> ["github", "actor"])
       const parts = trimmed.split(".");
+      /** @type {any} */
       let value = evalContext;
 
       for (const part of parts) {
@@ -255,9 +294,27 @@ function evaluateExpression(expr) {
         if (arrayMatch) {
           const key = arrayMatch[1];
           const index = parseInt(arrayMatch[2], 10);
-          value = value?.[key]?.[index];
+          // Use Object.prototype.hasOwnProperty.call() to prevent prototype chain access
+          if (value && typeof value === "object" && Object.prototype.hasOwnProperty.call(value, key)) {
+            const arrayValue = value[key];
+            if (Array.isArray(arrayValue) && index >= 0 && index < arrayValue.length) {
+              value = arrayValue[index];
+            } else {
+              value = undefined;
+              break;
+            }
+          } else {
+            value = undefined;
+            break;
+          }
         } else {
-          value = value?.[part];
+          // Use Object.prototype.hasOwnProperty.call() to prevent prototype chain access
+          if (value && typeof value === "object" && Object.prototype.hasOwnProperty.call(value, part)) {
+            value = value[part];
+          } else {
+            value = undefined;
+            break;
+          }
         }
 
         if (value === undefined || value === null) {

actions/setup/js/runtime_import.test.cjs

@@ -127,6 +127,86 @@ describe("runtime_import", () => {
         expect(isSafeExpression("inputs.value || secrets.TOKEN")).toBe(!1);
         expect(isSafeExpression("github.actor || vars.NAME")).toBe(!1);
       });
+      it("should block dangerous property names - constructor", () => {
+        expect(isSafeExpression("github.constructor")).toBe(!1);
+        expect(isSafeExpression("inputs.constructor")).toBe(!1);
+        expect(isSafeExpression("github.event.constructor")).toBe(!1);
+        expect(isSafeExpression("needs.job.constructor")).toBe(!1);
+      });
+      it("should block dangerous property names - __proto__", () => {
+        expect(isSafeExpression("github.__proto__")).toBe(!1);
+        expect(isSafeExpression("inputs.__proto__")).toBe(!1);
+        expect(isSafeExpression("github.event.__proto__")).toBe(!1);
+      });
+      it("should block dangerous property names - prototype", () => {
+        expect(isSafeExpression("github.prototype")).toBe(!1);
+        expect(isSafeExpression("inputs.prototype")).toBe(!1);
+        expect(isSafeExpression("github.event.prototype")).toBe(!1);
+      });
+      it("should block dangerous property names - __defineGetter__", () => {
+        expect(isSafeExpression("github.__defineGetter__")).toBe(!1);
+        expect(isSafeExpression("inputs.__defineGetter__")).toBe(!1);
+      });
+      it("should block dangerous property names - __defineSetter__", () => {
+        expect(isSafeExpression("github.__defineSetter__")).toBe(!1);
+        expect(isSafeExpression("inputs.__defineSetter__")).toBe(!1);
+      });
+      it("should block dangerous property names - __lookupGetter__", () => {
+        expect(isSafeExpression("github.__lookupGetter__")).toBe(!1);
+        expect(isSafeExpression("inputs.__lookupGetter__")).toBe(!1);
+      });
+      it("should block dangerous property names - __lookupSetter__", () => {
+        expect(isSafeExpression("github.__lookupSetter__")).toBe(!1);
+        expect(isSafeExpression("inputs.__lookupSetter__")).toBe(!1);
+      });
+      it("should block dangerous property names - hasOwnProperty", () => {
+        expect(isSafeExpression("github.hasOwnProperty")).toBe(!1);
+        expect(isSafeExpression("inputs.hasOwnProperty")).toBe(!1);
+      });
+      it("should block dangerous property names - isPrototypeOf", () => {
+        expect(isSafeExpression("github.isPrototypeOf")).toBe(!1);
+        expect(isSafeExpression("inputs.isPrototypeOf")).toBe(!1);
+      });
+      it("should block dangerous property names - propertyIsEnumerable", () => {
+        expect(isSafeExpression("github.propertyIsEnumerable")).toBe(!1);
+        expect(isSafeExpression("inputs.propertyIsEnumerable")).toBe(!1);
+      });
+      it("should block dangerous property names - toString", () => {
+        expect(isSafeExpression("github.toString")).toBe(!1);
+        expect(isSafeExpression("inputs.toString")).toBe(!1);
+      });
+      it("should block dangerous property names - valueOf", () => {
+        expect(isSafeExpression("github.valueOf")).toBe(!1);
+        expect(isSafeExpression("inputs.valueOf")).toBe(!1);
+      });
+      it("should block dangerous property names - toLocaleString", () => {
+        expect(isSafeExpression("github.toLocaleString")).toBe(!1);
+        expect(isSafeExpression("inputs.toLocaleString")).toBe(!1);
+      });
+      it("should block dangerous properties in array access", () => {
+        expect(isSafeExpression("github.event.release.assets[0].constructor")).toBe(!1);
+        expect(isSafeExpression("github.event.release.assets[0].__proto__")).toBe(!1);
+      });
+      it("should reject excessive nesting depth (more than 5 levels)", () => {
+        // Valid: max 5 levels (needs.job.outputs.foo.bar)
+        expect(isSafeExpression("needs.job.outputs.foo.bar")).toBe(!0);
+        expect(isSafeExpression("steps.step.outputs.foo.bar")).toBe(!0);
+        // Invalid: 6 levels
+        expect(isSafeExpression("needs.job.outputs.foo.bar.baz")).toBe(!1);
+        expect(isSafeExpression("steps.step.outputs.foo.bar.baz")).toBe(!1);
+        // Invalid: 7+ levels
+        expect(isSafeExpression("needs.job.outputs.foo.bar.baz.qux")).toBe(!1);
+      });
+      it("should allow valid nested expressions within depth limit", () => {
+        // 2 levels
+        expect(isSafeExpression("needs.job.outputs")).toBe(!0);
+        // 3 levels
+        expect(isSafeExpression("needs.job.outputs.result")).toBe(!0);
+        // 4 levels
+        expect(isSafeExpression("needs.job.outputs.foo.value")).toBe(!0);
+        // 5 levels (max)
+        expect(isSafeExpression("needs.job.outputs.foo.bar")).toBe(!0);
+      });
     }),
     describe("evaluateExpression", () => {
       beforeEach(() => {
@@ -172,6 +252,43 @@ describe("runtime_import", () => {
         expect(evaluateExpression("inputs.missing")).toContain("${{");
         expect(evaluateExpression("inputs.missing")).toContain("inputs.missing");
       });
+      it("should not access prototype chain properties", () => {
+        // These should return undefined (wrapped expression) instead of accessing prototype
+        const result = evaluateExpression("github.constructor");
+        expect(result).toContain("${{");
+        expect(result).toContain("github.constructor");
+      });
+      it("should not access __proto__ property", () => {
+        const result = evaluateExpression("github.__proto__");
+        expect(result).toContain("${{");
+        expect(result).toContain("github.__proto__");
+      });
+      it("should safely handle missing properties without prototype pollution", () => {
+        // These properties don't exist in the context object and should be undefined
+        expect(evaluateExpression("github.nonexistent")).toContain("${{");
+        expect(evaluateExpression("inputs.toString")).toContain("${{");
+      });
+      it("should handle array access safely with bounds checking", () => {
+        // Test with actual array in context
+        global.context = {
+          actor: "testuser",
+          job: "test-job",
+          repo: { owner: "testorg", repo: "testrepo" },
+          runId: 12345,
+          runNumber: 67,
+          workflow: "test-workflow",
+          payload: {
+            inputs: { repository: "testorg/testrepo", name: "test-name" },
+            release: { assets: [{ id: 123 }, { id: 456 }] },
+          },
+        };
+        // Valid array access
+        expect(evaluateExpression("github.event.release.assets[0].id")).toBe("123");
+        expect(evaluateExpression("github.event.release.assets[1].id")).toBe("456");
+        // Out of bounds - should return undefined
+        const outOfBounds = evaluateExpression("github.event.release.assets[999].id");
+        expect(outOfBounds).toContain("${{");
+      });
     }),
     describe("processRuntimeImport", () => {
       (it("should read and return file content", async () => {