Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ jobs:
- name: 📥 Download deps
run: pnpm install --frozen-lockfile --filter trigger.dev...

# Prisma clients generate concurrently and share one engine binary in the
# pnpm store; the generate script retries the transient Windows EPERM race.
- name: 📀 Generate Prisma Client
run: pnpm run generate

Expand Down
2 changes: 1 addition & 1 deletion internal-packages/database/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
},
"scripts": {
"clean": "rimraf dist",
"generate": "prisma generate",
"generate": "node ../../scripts/retry-prisma-generate.mjs",
"db:migrate:dev:create": "prisma migrate dev --create-only",
"db:migrate:deploy": "prisma migrate deploy",
"db:push": "prisma db push",
Expand Down
2 changes: 1 addition & 1 deletion internal-packages/run-ops-database/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
},
"scripts": {
"clean": "rimraf dist",
"generate": "prisma generate",
"generate": "node ../../scripts/retry-prisma-generate.mjs",
"db:migrate:dev:create": "prisma migrate dev --create-only",
"db:migrate:deploy": "node scripts/migrate.mjs deploy",
"db:migrate:status": "node scripts/migrate.mjs status",
Expand Down
53 changes: 53 additions & 0 deletions scripts/retry-prisma-generate.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Retry wrapper around `prisma generate`. Our two prisma clients pin the same
// prisma version and so share one package instance in the pnpm store; when
// `turbo run generate` runs them concurrently both race to write the shared
// query-engine binary, and on Windows the loser fails with `EPERM ... rename`.
// Retrying lets it succeed once the engine file is present and unlocked. On
// non-Windows the first attempt succeeds, so this is a zero-cost no-op.
import { spawnSync } from "node:child_process";

const MAX_ATTEMPTS = 5;
const BASE_DELAY_MS = 500;

// Transient, retryable filesystem contention on the shared engine binary.
const TRANSIENT =
/\b(EPERM|EBUSY|EACCES)\b|operation not permitted|resource busy or locked|being used by another process/i;

const passthroughArgs = process.argv.slice(2);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does Node.js require shell:true to spawn .cmd binaries like prisma.cmd on Windows without EINVAL?

💡 Result:

Yes, Node.js now requires { shell: true } to spawn.bat or.cmd files on Windows [1][2]. Following a security update (CVE-2024-27980) implemented in newer versions of Node.js, attempting to use child_process.spawn or child_process.spawnSync to execute.bat or.cmd files (like prisma.cmd) directly without the shell option will result in an EINVAL error [1][3][2]. Because these file types are not directly executable on Windows and require a terminal environment to interpret them, Node.js previously allowed them to be spawned indirectly, but this behavior was restricted to mitigate security vulnerabilities [4][1][2]. When you set { shell: true }, Node.js spawns a shell (using the environment's ComSpec, typically cmd.exe) to handle the execution of the file [4][5][6]. Alternative approaches include using child_process.exec, which internally uses a shell, or manually spawning cmd.exe and passing the target file as an argument [4][5][6]. Keep in mind that using shell: true can introduce security risks if you pass unsanitized user input to the command [4][7].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== scripts/retry-prisma-generate.mjs ==\n'
cat -n scripts/retry-prisma-generate.mjs

printf '\n== surrounding lines in e2e.yml ==\n'
if [ -f .github/workflows/e2e.yml ]; then
  rg -n "node|prisma|retry-prisma-generate|shell: true" .github/workflows/e2e.yml
  cat -n .github/workflows/e2e.yml | sed -n '1,220p'
else
  echo "e2e.yml not found"
fi

printf '\n== find prisma references ==\n'
rg -n "retry-prisma-generate|spawnSync\\(|shell: true|prisma generate|prisma\\.cmd|process\\.argv\\.slice\\(2\\)" -S .

Repository: triggerdotdev/trigger.dev

Length of output: 10369


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

for path in [
    Path("scripts/retry-prisma-generate.mjs"),
    Path(".github/workflows/e2e.yml"),
    Path("package.json"),
]:
    if path.exists():
        print(f"\n== {path} ==")
        text = path.read_text()
        lines = text.splitlines()
        for i, line in enumerate(lines, 1):
            if path.name == "retry-prisma-generate.mjs" or "prisma" in line.lower() or "node" in line.lower() or "shell: true" in line:
                print(f"{i}: {line}")
    else:
        print(f"\n== {path} not found ==")
PY

Repository: triggerdotdev/trigger.dev

Length of output: 3431


🌐 Web query:

Node.js child_process.spawnSync shell:true args array are arguments escaped or concatenated; DEP0190 behavior

💡 Result:

When using child_process.spawnSync (or other child_process methods like spawn and execFile) with shell: true, the provided args array is concatenated with spaces rather than being escaped [1][2][3]. The behavior is as follows: 1. Concatenation: Node.js joins the elements of the args array into a single string, separated by spaces [1][2]. 2. Lack of Escaping: The individual arguments are not quoted or escaped [2][3]. This lack of automatic escaping creates a security vulnerability known as shell injection, as a malicious argument could contain characters that alter the command's execution in the shell [2][3]. DEP0190: DEP0190 is a runtime DeprecationWarning introduced in recent Node.js versions (e.g., v23.11.0, v22.15.0) [4] specifically to warn developers about the security risks associated with passing an args array when the shell option is enabled [2][3]. Because the arguments are concatenated without escaping, this pattern provides a false impression of safety that does not exist [3]. To address the deprecation, the recommended approach is to stop passing an args array when using shell: true [3]. Instead, developers should construct the full command string themselves, ensuring that all arguments are properly escaped according to the requirements of the target shell [3]. If necessary, the warning can be temporarily suppressed using the --disable-warning=DEP0190 Node.js flag [3].

Citations:


🌐 Web query:

Node.js spawnSync .cmd Windows shell:true required EINVAL docs child_process

💡 Result:

If you encounter an EINVAL error when using child_process.spawn or child_process.spawnSync on Windows to execute.bat or.cmd files, it is due to a security update (CVE-2024-27980) introduced in April 2024 [1][2]. This update mandates that spawning batch or command files now requires the shell: true option to be explicitly set [1][3]. To resolve this issue, include { shell: true } in the options argument of your spawn or spawnSync function call [1][3]. Example: const { spawnSync } = require('child_process'); spawnSync('npm.cmd', ['--version'], { shell: true, stdio: 'inherit' }); Background: Prior to this change, Node.js allowed these files to be spawned directly. However, the security patch addresses vulnerabilities related to how these files were executed on Windows [1][2]. Because tools like npm, pnpm, and yarn often resolve to.cmd shims on Windows, this breaking change frequently impacts scripts or build tools that invoke these package managers [2][3]. It is strongly advised to use the shell: true option rather than reverting the security fix using --security-revert=CVE-2024-27980, as the latter leaves your application vulnerable [3].

Citations:


🌐 Web query:

site:nodejs.org child_process shell true args concatenated not escaped DEP0190

💡 Result:

DEP0190 refers to the deprecation of passing arguments (the args array) to Node.js child_process functions (such as spawn or execFile) when the shell option is set to true [1][2]. When shell is enabled (shell: true), Node.js runs the command inside a shell [3]. Historically, if an args array was provided alongside this option, the arguments were concatenated without proper escaping or quoting before being passed to the shell [1][3]. This behavior creates a significant security vulnerability: because the arguments are not escaped, any unsanitized user input containing shell metacharacters can be manipulated to trigger arbitrary command execution (shell injection) [3][4]. Due to these security risks, Node.js has deprecated this usage [1][3]. Developers are advised to avoid passing the args array when shell: true [3][4]. If shell execution is necessary, the command and its arguments should be constructed carefully as a single string (if using exec) or, preferably, the shell option should be avoided entirely in favor of spawning commands directly without a shell, which is more secure [3][4]. If shell: true is absolutely required, you must ensure that no unsanitized user input is included in the command string [3].

Citations:


Scope shell: true to Windows only. Node concatenates the args array when shell: true is set, so passthroughArgs is shell-parsed instead of passed through as argv. If the shell is only needed for .cmd resolution on Windows, gate it on process.platform === "win32".


function sleepSync(ms) {
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
}

let lastStatus = 1;

for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) {
const result = spawnSync("prisma", ["generate", ...passthroughArgs], {
shell: true,
encoding: "utf8",
});

process.stdout.write(result.stdout ?? "");
process.stderr.write(result.stderr ?? "");

if (result.status === 0) {
process.exit(0);
}

lastStatus = result.status ?? 1;

const output = `${result.stdout ?? ""}${result.stderr ?? ""}`;
const isRetryable = TRANSIENT.test(output);

if (!isRetryable || attempt === MAX_ATTEMPTS) {
break;
}

const delay = BASE_DELAY_MS * attempt;
console.error(
`prisma generate hit a transient filesystem error (attempt ${attempt}/${MAX_ATTEMPTS}); retrying in ${delay}ms...`
);
sleepSync(delay);
}
Comment on lines +22 to +51

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Spawn failures are silently swallowed — surface result.error.

If spawnSync fails to launch the process at all (e.g. prisma not resolvable on PATH, ENOENT), result.status is null, result.stdout/result.stderr are null, and the TRANSIENT regex never matches empty output. The loop then breaks after the very first attempt and exits with lastStatus (falls back to 1) but never prints result.error.message, leaving no diagnostic in CI logs for what is otherwise a straightforward, fully-diagnosable failure.

🐛 Proposed fix to surface spawn errors
   process.stdout.write(result.stdout ?? "");
   process.stderr.write(result.stderr ?? "");
 
+  if (result.error) {
+    console.error(`prisma generate failed to start: ${result.error.message}`);
+  }
+
   if (result.status === 0) {
     process.exit(0);
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let lastStatus = 1;
for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) {
const result = spawnSync("prisma", ["generate", ...passthroughArgs], {
shell: true,
encoding: "utf8",
});
process.stdout.write(result.stdout ?? "");
process.stderr.write(result.stderr ?? "");
if (result.status === 0) {
process.exit(0);
}
lastStatus = result.status ?? 1;
const output = `${result.stdout ?? ""}${result.stderr ?? ""}`;
const isRetryable = TRANSIENT.test(output);
if (!isRetryable || attempt === MAX_ATTEMPTS) {
break;
}
const delay = BASE_DELAY_MS * attempt;
console.error(
`prisma generate hit a transient filesystem error (attempt ${attempt}/${MAX_ATTEMPTS}); retrying in ${delay}ms...`
);
sleepSync(delay);
}
let lastStatus = 1;
for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) {
const result = spawnSync("prisma", ["generate", ...passthroughArgs], {
shell: true,
encoding: "utf8",
});
process.stdout.write(result.stdout ?? "");
process.stderr.write(result.stderr ?? "");
if (result.error) {
console.error(`prisma generate failed to start: ${result.error.message}`);
}
if (result.status === 0) {
process.exit(0);
}
lastStatus = result.status ?? 1;
const output = `${result.stdout ?? ""}${result.stderr ?? ""}`;
const isRetryable = TRANSIENT.test(output);
if (!isRetryable || attempt === MAX_ATTEMPTS) {
break;
}
const delay = BASE_DELAY_MS * attempt;
console.error(
`prisma generate hit a transient filesystem error (attempt ${attempt}/${MAX_ATTEMPTS}); retrying in ${delay}ms...`
);
sleepSync(delay);
}


process.exit(lastStatus);
Loading