Add timeout guards to setup JS child processes (#53507)

Author: Copilot

actions/setup/js/apply_samples.cjs

@@ -38,10 +38,12 @@ const os = require("os");
 const { getErrorMessage } = require("./error_helpers.cjs");
 const { ERR_VALIDATION, ERR_PARSE, ERR_SYSTEM, ERR_API, ERR_CONFIG } = require("./error_codes.cjs");
 const { findRepoCheckout } = require("./find_repo_checkout.cjs");
+const { getSetupTimeoutMs } = require("./child_process_timeouts.cjs");
 
 const DEFAULT_BASE_BRANCH = process.env.GH_AW_CUSTOM_BASE_BRANCH || process.env.GITHUB_BASE_REF || process.env.GITHUB_REF_NAME || "main";
 const PATCH_SIDECAR_TOOLS = new Set(["create_pull_request", "push_to_pull_request_branch"]);
-const FETCH_TIMEOUT_MS = 120_000;
+const FETCH_TIMEOUT_MS = getSetupTimeoutMs("applySamplesFetch");
+const GIT_COMMAND_TIMEOUT_MS = getSetupTimeoutMs("applySamplesGit");
 
 /**
  * @typedef {Object} SampleEntry
@@ -95,7 +97,7 @@ function loadSamples() {
  */
 function runGit(args, cwd) {
   const { spawnSync } = require("child_process");
-  const result = spawnSync("git", args, { cwd, encoding: "utf8" });
+  const result = spawnSync("git", args, { cwd, encoding: "utf8", timeout: GIT_COMMAND_TIMEOUT_MS });
   if (result.error) {
     throw result.error;
   }
@@ -628,6 +630,9 @@ async function main() {
     stdio: ["pipe", "pipe", "inherit"],
     env: process.env,
   });
+  child.on("error", err => {
+    core.error(`apply_samples: failed to launch MCP server: ${getErrorMessage(err)}`);
+  });
 
   const stdoutIter = lineIterator(child.stdout);
   let nextId = 1;

actions/setup/js/artifact_client.cjs

@@ -16,15 +16,18 @@ const { pipeline } = require("stream/promises");
 const { spawnSync } = require("child_process");
 
 const { getErrorMessage } = require("./error_helpers.cjs");
+const { getSetupTimeoutMs } = require("./child_process_timeouts.cjs");
 
 const DEFAULT_RETRY_ATTEMPTS = 5;
 const RETRY_DELAY_MS = 5000;
 const RESULTS_SCOPE_PREFIX = "Actions.Results:";
 const TWIRP_ARTIFACT_SERVICE = "github.actions.results.api.v1.ArtifactService";
 const MAX_ARTIFACTS = 1000;
 const PAGE_SIZE = 100;
-const FETCH_TIMEOUT_MS = 120_000;
-const FETCH_TRANSFER_TIMEOUT_MS = 300_000;
+const FETCH_TIMEOUT_MS = getSetupTimeoutMs("artifactFetch");
+const FETCH_TRANSFER_TIMEOUT_MS = getSetupTimeoutMs("artifactTransfer");
+const ARCHIVE_COMMAND_TIMEOUT_MS = getSetupTimeoutMs("artifactArchive");
+const ARCHIVE_PROBE_TIMEOUT_MS = getSetupTimeoutMs("artifactArchiveProbe");
 
 function sleep(ms) {
   return new Promise(resolve => setTimeout(resolve, ms));
@@ -172,7 +175,7 @@ async function streamToFile(response, filePath) {
 }
 
 function ensureZipAvailable() {
-  const result = spawnSync("zip", ["-v"], { stdio: "ignore" });
+  const result = spawnSync("zip", ["-v"], { stdio: "ignore", timeout: ARCHIVE_PROBE_TIMEOUT_MS });
   if (result.error) {
     throw result.error;
   }
@@ -182,7 +185,7 @@ function ensureZipAvailable() {
 }
 
 function ensureUnzipAvailable() {
-  const result = spawnSync("unzip", ["-v"], { stdio: "ignore" });
+  const result = spawnSync("unzip", ["-v"], { stdio: "ignore", timeout: ARCHIVE_PROBE_TIMEOUT_MS });
   if (result.error) {
     throw result.error;
   }
@@ -201,6 +204,7 @@ function createZipFromFiles(files, rootDirectory, outputPath) {
   const result = spawnSync("zip", ["-q", "-r", outputPath, ...relativeFiles], {
     cwd: rootDirectory,
     encoding: "utf8",
+    timeout: ARCHIVE_COMMAND_TIMEOUT_MS,
   });
   if (result.error) {
     throw result.error;
@@ -372,7 +376,7 @@ class DefaultArtifactClient {
       const tempZip = path.join(tempDownloadDir, "artifact.zip");
       try {
         digest = await streamToFile(blobResponse, tempZip);
-        const unzipResult = spawnSync("unzip", ["-q", tempZip, "-d", destination], { encoding: "utf8" });
+        const unzipResult = spawnSync("unzip", ["-q", tempZip, "-d", destination], { encoding: "utf8", timeout: ARCHIVE_COMMAND_TIMEOUT_MS });
         if (unzipResult.error) {
           throw unzipResult.error;
         }

actions/setup/js/artifact_client.test.cjs

@@ -343,6 +343,11 @@ describe("DefaultArtifactClient.uploadArtifact", () => {
       output: [],
       signal: null,
     });
+    vi.stubEnv("GH_AW_ARTIFACT_ARCHIVE_PROBE_TIMEOUT_MS", "12345");
+    vi.stubEnv("GH_AW_ARTIFACT_ARCHIVE_TIMEOUT_MS", "67890");
+    const artifactClientPath = req.resolve("./artifact_client.cjs");
+    delete req.cache[artifactClientPath];
+    const { DefaultArtifactClient: FreshArtifactClient } = req("./artifact_client.cjs");
 
     let capturedName;
     const mockFetch = vi.fn().mockImplementation(async (url, opts) => {
@@ -360,7 +365,7 @@ describe("DefaultArtifactClient.uploadArtifact", () => {
     vi.stubEnv("ACTIONS_RUNTIME_TOKEN", buildFakeToken("runId:jobId"));
     vi.stubEnv("ACTIONS_RESULTS_URL", "https://results.example.com");
 
-    const client = new DefaultArtifactClient();
+    const client = new FreshArtifactClient();
     // Will fail at zip creation (no real zip output), but we still capture the name from CreateArtifact
     try {
       await client.uploadArtifact("archive-name", [filePath], tmpDir);
@@ -369,6 +374,9 @@ describe("DefaultArtifactClient.uploadArtifact", () => {
     }
 
     expect(capturedName).toBe("archive-name");
+    expect(spawnSyncSpy).toHaveBeenNthCalledWith(1, "zip", ["-v"], expect.objectContaining({ timeout: 12_345 }));
+    expect(spawnSyncSpy).toHaveBeenNthCalledWith(2, "zip", expect.any(Array), expect.objectContaining({ timeout: 67_890 }));
     spawnSyncSpy.mockRestore();
+    delete req.cache[artifactClientPath];
   });
 });

actions/setup/js/child_process_timeouts.cjs

@@ -0,0 +1,72 @@
+// @ts-check
+"use strict";
+
+/**
+ * Largest timeout Node.js can represent as a timer delay (2^31 - 1 ms, ~24.8 days).
+ * Larger delays overflow and are silently converted to 1 ms, so overrides above
+ * this bound are clamped to it.
+ */
+const MAX_SETUP_TIMEOUT_MS = 2_147_483_647;
+
+/**
+ * Setup/runtime command timeout defaults, in milliseconds. Each timeout can be
+ * overridden with its environment variable by setting a positive integer value.
+ */
+const SETUP_TIMEOUTS = Object.freeze({
+  applySamplesFetch: { env: "GH_AW_APPLY_SAMPLES_FETCH_TIMEOUT_MS", defaultMs: 120_000 },
+  applySamplesGit: { env: "GH_AW_APPLY_SAMPLES_GIT_TIMEOUT_MS", defaultMs: 120_000 },
+  artifactArchive: { env: "GH_AW_ARTIFACT_ARCHIVE_TIMEOUT_MS", defaultMs: 300_000 },
+  artifactArchiveProbe: { env: "GH_AW_ARTIFACT_ARCHIVE_PROBE_TIMEOUT_MS", defaultMs: 15_000 },
+  artifactFetch: { env: "GH_AW_ARTIFACT_FETCH_TIMEOUT_MS", defaultMs: 120_000 },
+  artifactTransfer: { env: "GH_AW_ARTIFACT_TRANSFER_TIMEOUT_MS", defaultMs: 300_000 },
+  gitBranch: { env: "GH_AW_GIT_BRANCH_TIMEOUT_MS", defaultMs: 15_000 },
+  importGit: { env: "GH_AW_IMPORT_GIT_TIMEOUT_MS", defaultMs: 300_000 },
+  mcpConfigConverter: { env: "GH_AW_MCP_CONFIG_CONVERTER_TIMEOUT_MS", defaultMs: 120_000 },
+  mcpContainerStatus: { env: "GH_AW_MCP_CONTAINER_STATUS_TIMEOUT_MS", defaultMs: 15_000 },
+  mcpDockerCleanup: { env: "GH_AW_MCP_DOCKER_CLEANUP_TIMEOUT_MS", defaultMs: 30_000 },
+  mcpServerCheck: { env: "GH_AW_MCP_SERVER_CHECK_TIMEOUT_MS", defaultMs: 120_000 },
+  outcomeGh: { env: "GH_AW_OUTCOME_GH_TIMEOUT_MS", defaultMs: 300_000 },
+  safeoutputsCli: { env: "GH_AW_SAFEOUTPUTS_CLI_TIMEOUT_MS", defaultMs: 120_000 },
+});
+
+/**
+ * @param {string} envName
+ * @param {number} defaultMs
+ * @param {NodeJS.ProcessEnv} [env]
+ * @returns {number}
+ */
+function getPositiveEnvIntOrDefault(envName, defaultMs, env = process.env) {
+  const raw = env[envName];
+  if (!raw || !raw.trim()) {
+    return defaultMs;
+  }
+  const trimmed = raw.trim();
+  if (!/^\d+$/.test(trimmed)) {
+    return defaultMs;
+  }
+  const parsed = Number(trimmed);
+  if (!Number.isSafeInteger(parsed) || parsed <= 0) {
+    return defaultMs;
+  }
+  return Math.min(parsed, MAX_SETUP_TIMEOUT_MS);
+}
+
+/**
+ * @param {keyof typeof SETUP_TIMEOUTS} name
+ * @param {NodeJS.ProcessEnv} [env]
+ * @returns {number}
+ */
+function getSetupTimeoutMs(name, env = process.env) {
+  const timeout = SETUP_TIMEOUTS[name];
+  if (!timeout) {
+    throw new Error(`Unknown setup timeout: ${String(name)}`);
+  }
+  return getPositiveEnvIntOrDefault(timeout.env, timeout.defaultMs, env);
+}
+
+module.exports = {
+  MAX_SETUP_TIMEOUT_MS,
+  SETUP_TIMEOUTS,
+  getPositiveEnvIntOrDefault,
+  getSetupTimeoutMs,
+};

actions/setup/js/child_process_timeouts.test.cjs

@@ -0,0 +1,36 @@
+import { describe, expect, it } from "vitest";
+import { createRequire } from "module";
+
+const req = createRequire(import.meta.url);
+const { MAX_SETUP_TIMEOUT_MS, SETUP_TIMEOUTS, getPositiveEnvIntOrDefault, getSetupTimeoutMs } = req("./child_process_timeouts.cjs");
+
+describe("child process timeout helpers", () => {
+  it("uses defaults when overrides are unset or invalid", () => {
+    expect(getPositiveEnvIntOrDefault("MISSING_TIMEOUT", 123, {})).toBe(123);
+    expect(getPositiveEnvIntOrDefault("BAD_TIMEOUT", 123, { BAD_TIMEOUT: "0" })).toBe(123);
+    expect(getPositiveEnvIntOrDefault("BAD_TIMEOUT", 123, { BAD_TIMEOUT: "-1" })).toBe(123);
+    expect(getPositiveEnvIntOrDefault("BAD_TIMEOUT", 123, { BAD_TIMEOUT: "10s" })).toBe(123);
+  });
+
+  it("accepts positive integer millisecond overrides", () => {
+    expect(getPositiveEnvIntOrDefault("CUSTOM_TIMEOUT", 123, { CUSTOM_TIMEOUT: " 456 " })).toBe(456);
+  });
+
+  it("clamps overrides above Node's maximum timer delay", () => {
+    expect(MAX_SETUP_TIMEOUT_MS).toBe(2_147_483_647);
+    expect(getPositiveEnvIntOrDefault("BIG_TIMEOUT", 123, { BIG_TIMEOUT: String(MAX_SETUP_TIMEOUT_MS) })).toBe(MAX_SETUP_TIMEOUT_MS);
+    expect(getPositiveEnvIntOrDefault("BIG_TIMEOUT", 123, { BIG_TIMEOUT: String(MAX_SETUP_TIMEOUT_MS + 1) })).toBe(MAX_SETUP_TIMEOUT_MS);
+    expect(getPositiveEnvIntOrDefault("BIG_TIMEOUT", 123, { BIG_TIMEOUT: "99999999999999999999" })).toBe(123);
+  });
+
+  it("documents an environment override for every setup timeout", () => {
+    for (const [name, timeout] of Object.entries(SETUP_TIMEOUTS)) {
+      expect(name).toBeTruthy();
+      expect(timeout.env).toMatch(/^GH_AW_[A-Z0-9_]+_TIMEOUT_MS$/);
+      expect(timeout.defaultMs).toBeGreaterThan(0);
+      expect(timeout.defaultMs).toBeLessThanOrEqual(MAX_SETUP_TIMEOUT_MS);
+      expect(getSetupTimeoutMs(name, { [timeout.env]: "9876" })).toBe(9876);
+      expect(getSetupTimeoutMs(name, { [timeout.env]: "9999999999" })).toBe(MAX_SETUP_TIMEOUT_MS);
+    }
+  });
+});

actions/setup/js/evaluate_outcomes.cjs

@@ -29,6 +29,7 @@ const fs = require("fs");
 const path = require("path");
 const crypto = require("crypto");
 const { execFileSync } = require("child_process");
+const { getSetupTimeoutMs } = require("./child_process_timeouts.cjs");
 
 // ---------------------------------------------------------------------------
 // Paths
@@ -48,6 +49,7 @@ const CLOSING_COMMENT_KEYWORDS = ["not planned", "won't fix", "wontfix", "duplic
 
 const DEFAULT_ISSUE_IMMEDIATE_CLOSE_WINDOW_SEC = 60 * 60;
 const DEFAULT_LABEL_RETENTION_WINDOW_SEC = 24 * 60 * 60;
+const GH_COMMAND_TIMEOUT_MS = getSetupTimeoutMs("outcomeGh");
 
 const POSITIVE_REACTIONS = ["+1", "heart", "hooray", "rocket"];
 const NEGATIVE_REACTIONS = ["-1", "confused"];
@@ -79,7 +81,7 @@ const LABEL_RETENTION_WINDOW_SEC = getEnvPositiveIntOrDefault("OUTCOME_LABEL_RET
  */
 function gh(args) {
   try {
-    return execFileSync("gh", args, { encoding: "utf8", stdio: ["pipe", "pipe", "pipe"] }).trim();
+    return execFileSync("gh", args, { encoding: "utf8", stdio: ["pipe", "pipe", "pipe"], timeout: GH_COMMAND_TIMEOUT_MS }).trim();
   } catch {
     return null;
   }

actions/setup/js/get_current_branch.cjs

@@ -3,6 +3,9 @@
 
 const { execSync } = require("child_process");
 const { ERR_CONFIG } = require("./error_codes.cjs");
+const { getSetupTimeoutMs } = require("./child_process_timeouts.cjs");
+
+const GIT_BRANCH_TIMEOUT_MS = getSetupTimeoutMs("gitBranch");
 
 /**
  * Get the current git branch name
@@ -19,6 +22,7 @@ function getCurrentBranch(customCwd) {
       encoding: "utf8",
       cwd: cwd,
       stdio: ["pipe", "pipe", "pipe"],
+      timeout: GIT_BRANCH_TIMEOUT_MS,
     }).trim();
     // "HEAD" means the repo is in a detached-HEAD state (common with the
     // default actions/checkout behaviour).  It is not a valid branch name;

actions/setup/js/merge_remote_agent_github_folder.cjs

@@ -27,6 +27,9 @@ require("./shim.cjs");
 
 const { getErrorMessage } = require("./error_helpers.cjs");
 const { ERR_CONFIG, ERR_PARSE, ERR_SYSTEM, ERR_VALIDATION } = require("./error_codes.cjs");
+const { getSetupTimeoutMs } = require("./child_process_timeouts.cjs");
+
+const GIT_COMMAND_TIMEOUT_MS = getSetupTimeoutMs("importGit");
 
 /**
  * Parse the agent import specification to extract repository details
@@ -188,11 +191,11 @@ function sparseCheckoutGithubFolder(owner, repo, ref, tempDir) {
 
   try {
     // Initialize git repository
-    execFileSync("git", ["init"], { cwd: tempDir, stdio: "pipe" });
+    execFileSync("git", ["init"], { cwd: tempDir, stdio: "pipe", timeout: GIT_COMMAND_TIMEOUT_MS });
     core.info("Initialized temporary git repository");
 
     // Configure sparse checkout
-    execFileSync("git", ["config", "core.sparseCheckout", "true"], { cwd: tempDir, stdio: "pipe" });
+    execFileSync("git", ["config", "core.sparseCheckout", "true"], { cwd: tempDir, stdio: "pipe", timeout: GIT_COMMAND_TIMEOUT_MS });
     core.info("Enabled sparse checkout");
 
     // Set sparse checkout pattern to only include .github folder
@@ -201,15 +204,15 @@ function sparseCheckoutGithubFolder(owner, repo, ref, tempDir) {
     core.info("Configured sparse checkout pattern: .github/");
 
     // Add remote - using execFileSync prevents shell injection
-    execFileSync("git", ["remote", "add", "origin", repoUrl], { cwd: tempDir, stdio: "pipe" });
+    execFileSync("git", ["remote", "add", "origin", repoUrl], { cwd: tempDir, stdio: "pipe", timeout: GIT_COMMAND_TIMEOUT_MS });
     core.info(`Added remote: ${repoUrl}`);
 
     // Fetch and checkout - using execFileSync with validated ref
     core.info(`Fetching ref: ${ref}`);
-    execFileSync("git", ["fetch", "--depth", "1", "origin", ref], { cwd: tempDir, stdio: "pipe" });
+    execFileSync("git", ["fetch", "--depth", "1", "origin", ref], { cwd: tempDir, stdio: "pipe", timeout: GIT_COMMAND_TIMEOUT_MS });
 
     core.info("Checking out .github folder");
-    execFileSync("git", ["checkout", "FETCH_HEAD"], { cwd: tempDir, stdio: "pipe" });
+    execFileSync("git", ["checkout", "FETCH_HEAD"], { cwd: tempDir, stdio: "pipe", timeout: GIT_COMMAND_TIMEOUT_MS });
 
     core.info("Sparse checkout completed successfully");
   } catch (error) {

actions/setup/js/safeoutputs_cli.cjs

@@ -15,6 +15,9 @@
 const childProcess = require("child_process");
 const fs = require("fs");
 const { ERR_VALIDATION, ERR_SYSTEM } = require("./error_codes.cjs");
+const { getSetupTimeoutMs } = require("./child_process_timeouts.cjs");
+
+const SAFEOUTPUTS_CLI_TIMEOUT_MS = getSetupTimeoutMs("safeoutputsCli");
 
 /**
  * @typedef {(toolName: string, args: Record<string, string>) => void} RunSafeOutputsCLILike
@@ -38,7 +41,7 @@ function runSafeOutputsCLI(toolName, args) {
     commandArgs.push(value);
   }
   try {
-    childProcess.execFileSync(command, commandArgs, { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] });
+    childProcess.execFileSync(command, commandArgs, { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], timeout: SAFEOUTPUTS_CLI_TIMEOUT_MS });
   } catch (error) {
     const err = /** @type {{message?: string, stderr?: string | Buffer}} */ error ?? {};
     const stderr = typeof err.stderr === "string" ? err.stderr.trim() : Buffer.isBuffer(err.stderr) ? err.stderr.toString("utf8").trim() : "";