fix: Validate port number before final lsof kill

[odaysec]

Add port validation before executing lsof kill command. fix this type of problem you need to prevent untrusted input from being interpreted by the shell. Either (a) avoid sh -c entirely and pass arguments as an array to spawn/spawnSync/execFile, or (b) if you must use the shell for features like pipes and redirections, safely escape or sanitize the untrusted data (for example using a library like shell-quote, or by enforcing that it is a simple numeric port).

The command lsof -ti:${this.options.port} | xargs kill -9 2>/dev/null uses a pipe and redirection, so the shell is convenient. However, the only dynamic part is the port, which is supposed to be a TCP port number. The simplest robust fix without changing behavior is to ensure that this.options.port is a safe, numeric string before building the command. We can do this by converting it to an integer with Number/parseInt, checking that it’s a finite integer in the valid port range (1–65535), and then interpolating that validated number instead of the original this.options.port. If the port turns out to be invalid, we can skip the lsof kill attempt (and log a debug message), which fails closed rather than risking command injection.

Concretely, in handleShutdown (lines 2097–2338 region), before constructing the lsof command we will compute:

const portNum = Number(this.options.port)
if (!Number.isInteger(portNum) || portNum <= 0 || portNum > 65535) { ... } else {
  const result = spawnSync("sh", ["-c", `lsof -ti:${portNum} | xargs kill -9 2>/dev/null`], { ... })
}

[vercel]

@odaysec is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

[odaysec]

Change the logging calls in www/lib/workflow-storage.ts that interpolate prefix, blob.url, fetchUrl, or other potentially tainted values directly into the first argument. Use a constant format string (for example including %s placeholders) as the first argument and provide the dynamic values as additional arguments. This preserves all existing information in the logs while removing the format-string dependency on user-controlled data.

Concretely, in listWorkflowRuns:

• On the catch block around list({ prefix }), change:

  console.error(`[Workflow Storage] Failed to list blobs for ${prefix}:`, error)

  to something like:

  console.error("[Workflow Storage] Failed to list blobs for prefix %s:", prefix, error)

  so prefix is no longer inside the format string itself.

• For consistency and to avoid any similar issues elsewhere, update other console.error calls that currently embed blob URLs or other dynamic values in template literals, e.g.:

  • console.error([Workflow Storage] Blob not found: ${blob.url})
  • console.error([Workflow Storage] Unexpected content type ${blobInfo.contentType} for ${blob.url})
  • console.error(\[Workflow Storage] HTTP ${response.status} fetching ${fetchUrl} (content-type: ${response.headers.get("content-type")})`)`
  • console.error(\[Workflow Storage] Response is not JSON: ${contentType} for ${fetchUrl} (likely Vercel Security Checkpoint)`)`
  • console.error(\[Workflow Storage] Failed to fetch ${blob.url}:, error)

  into calls that use a static message plus arguments, e.g.:

  console.error("[Workflow Storage] Blob not found for URL %s", blob.url)

[elsigh]

@odaysec Can you do a merge? I made some changes today to account for argument sanitizing

[elsigh]

Thanks! This looks mostly benign/positive, but I think there’s a Windows regression and a tooling availability risk:\n\n- cdp-monitor.ts removed the PowerShell listProcesses() path and now relies on sh + pgrep and ps aux + sleep. That won’t work on Windows out of the box, and pgrep can be missing in minimal container images too.\n- If Windows support is still expected, we should keep a Windows-specific path (PowerShell) or guard the Unix-specific logic by platform and fall back to no-op.\n\nEverything else looks safe (log formatting, port validation), but the process discovery/kill changes are the risky part.