Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 3 additions & 3 deletions crates/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@
},
"scripts": {
"build": "pnpm run build:platform && pnpm run build:wasm",
"build:platform": "napi build --platform --release",
"build:platform": "node ../../scripts/run-task.mjs build -- napi build --platform --release",
"postbuild:platform": "node ./scripts/move-artifacts.mjs",
"build:wasm": "napi build --release --target wasm32-wasip1-threads",
"build:wasm": "node ../../scripts/run-task.mjs build -- napi build --release --target wasm32-wasip1-threads",
"postbuild:wasm": "node ./scripts/move-artifacts.mjs",
"dev": "cargo watch --quiet --shell 'npm run build'",
"build:debug": "napi build --platform",
"build:debug": "node ../../scripts/run-task.mjs build -- napi build --platform",
"version": "napi version"
},
"optionalDependencies": {
Expand Down
15 changes: 8 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,20 @@
]
},
"scripts": {
"check:env": "node ./scripts/preflight.mjs doctor",
"format": "prettier --write .",
"lint": "prettier --check . && turbo lint",
"build": "turbo build --filter=!./playgrounds/*",
"build": "node ./scripts/run-task.mjs build",
"postbuild": "node ./scripts/pack-packages.mjs",
"dev": "turbo dev --filter=!./playgrounds/*",
"test": "cargo test && vitest run --hideSkippedTests",
"test:integrations": "vitest --root=./integrations",
"test:ui": "pnpm run --filter=tailwindcss test:ui && pnpm run --filter=@tailwindcss/browser test:ui",
"dev": "node ./scripts/run-task.mjs dev",
"test": "node ./scripts/run-task.mjs test",
"test:integrations": "node ./scripts/run-task.mjs test:integrations",
"test:ui": "node ./scripts/run-task.mjs test:ui",
"tdd": "vitest --hideSkippedTests",
"bench": "vitest bench",
"version-packages": "node ./scripts/version-packages.mjs",
"vite": "pnpm run --filter=vite-playground dev",
"nextjs": "pnpm run --filter=nextjs-playground dev"
"vite": "node ./scripts/run-task.mjs vite",
"nextjs": "node ./scripts/run-task.mjs nextjs"
},
"license": "MIT",
"devDependencies": {
Expand Down
179 changes: 179 additions & 0 deletions scripts/preflight.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import { spawnSync } from 'node:child_process'
import fs from 'node:fs'
import path from 'node:path'
import process from 'node:process'
import { fileURLToPath } from 'node:url'
import { commandExists, hasDefaultCargoBin, readInstalledRustTargets, rustEnv } from './rust-env.mjs'

const __dirname = path.dirname(fileURLToPath(import.meta.url))
const root = path.resolve(__dirname, '..')
const command = process.argv[2] ?? 'doctor'
const wasmTarget = 'wasm32-wasip1-threads'

const contexts = {
doctor: {
label: 'pnpm run check:env',
needsRust: true,
needsWasmTarget: true,
},
build: {
label: 'pnpm build',
needsRust: true,
needsWasmTarget: true,
},
dev: {
label: 'pnpm dev',
needsRust: true,
needsWasmTarget: true,
},
test: {
label: 'pnpm test',
needsRust: true,
needsWasmTarget: false,
},
'test:integrations': {
label: 'pnpm test:integrations',
needsRust: true,
needsWasmTarget: true,
needsBuildArtifacts: true,
},
'test:ui': {
label: 'pnpm test:ui',
needsRust: true,
needsWasmTarget: true,
needsBuildArtifacts: true,
},
vite: {
label: 'pnpm vite',
needsRust: true,
needsWasmTarget: true,
needsBuildArtifacts: true,
needsBun: true,
},
nextjs: {
label: 'pnpm nextjs',
needsRust: true,
needsWasmTarget: true,
needsBuildArtifacts: true,
},
}

const context = contexts[command] ?? contexts.doctor

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.

⚠️ Potential issue | 🟡 Minor

Unknown context silently falls back to doctor.

contexts[command] ?? contexts.doctor means an unrecognized task arg runs the doctor checks instead of erroring. In practice run-task.mjs already validates the task, so this is only reachable when preflight is invoked directly (e.g. node ./scripts/preflight.mjs typo), but the success message on line 178 still gates on command === 'doctor', so a typo'd context would run doctor-equivalent checks and print nothing on success, which is confusing. Consider erroring on unknown commands, or at least logging which context was selected.

Proposed fix
-const context = contexts[command] ?? contexts.doctor
+const context = contexts[command]
+if (!context) {
+  console.error(`Unknown preflight context: ${command}`)
+  console.error(`Known contexts: ${Object.keys(contexts).join(', ')}`)
+  process.exit(1)
+}
📝 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
const context = contexts[command] ?? contexts.doctor
const context = contexts[command]
if (!context) {
console.error(`Unknown preflight context: ${command}`)
console.error(`Known contexts: ${Object.keys(contexts).join(', ')}`)
process.exit(1)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/preflight.mjs` at line 67, The current assignment const context =
contexts[command] ?? contexts.doctor silently treats unknown command values as
doctor; instead, explicitly validate the command: check whether contexts
hasOwnProperty(command) (or command in contexts) and if not, log an error and
exit non‑zero (or throw) so typos like "typo" fail fast; if you prefer
permissive behavior, at minimum log which context was chosen (e.g., console.log
or processLogger) before running the task — update the code around the
contexts/command lookup (the const context = ... line and the success message
branch that checks command === 'doctor') to use this validation/logging
approach.


const requiredArtifacts = [
'crates/node/index.js',
'packages/tailwindcss/dist/lib.js',
]

function run(command, args) {
return spawnSync(command, args, {
cwd: root,
encoding: 'utf8',
stdio: ['ignore', 'pipe', 'pipe'],
env: rustEnv,
})
}

function formatList(items) {
return items.map((item) => `- ${item}`).join('\n')
}

function relativeArtifact(artifact) {
return artifact.replace(/\\/g, '/')
}

let errors = []
let warnings = []

if (context.needsRust) {
let missingRustTools = ['cargo', 'rustc'].filter((tool) => !commandExists(tool))

if (missingRustTools.length > 0) {
errors.push({
title: 'Missing required Rust tools',
body: [
formatList(missingRustTools),
'How to fix:',
'1. Install Rustup from https://rustup.rs/',
hasDefaultCargoBin()
? '2. Rust appears to be installed in the default cargo directory, so rerun the command through the repo scripts such as `pnpm build` or `pnpm run check:env`.'
: '2. If you installed Rustup to a custom location, make sure `CARGO_HOME`, `RUSTUP_HOME`, or your PATH points at the installation.',
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
'3. Run `rustup default stable`.',
`4. Run \`rustup target add ${wasmTarget}\`.`,
].join('\n'),
})
}

if (!commandExists('rustup')) {
warnings.push(
'rustup was not found, so the script could not verify the installed WASM targets.',
)
} else if (context.needsWasmTarget) {
let installedTargets = readInstalledRustTargets()

if (installedTargets === null) {
warnings.push('Could not read the installed Rust targets from rustup.')
} else if (!installedTargets.has(wasmTarget)) {
errors.push({
title: 'Missing required Rust target',
body: [`- ${wasmTarget}`, `Run \`rustup target add ${wasmTarget}\`.`].join('\n'),
})
}
}
}

if (context.needsBun && !commandExists('bun')) {
errors.push({
title: 'Missing required tool for the Vite playground',
body: [
'- bun',
'The Vite playground uses `bun --bun vite`.',
'Install Bun, or validate Vite changes with `pnpm build && pnpm test:integrations -- --run integrations/vite`.',
].join('\n'),
})
}

if (context.needsBuildArtifacts) {
let missingArtifacts = requiredArtifacts.filter((artifact) => {
return !fs.existsSync(path.join(root, artifact))
})

if (missingArtifacts.length > 0) {
errors.push({
title: 'Missing build artifacts required by this command',
body: [
formatList(missingArtifacts.map(relativeArtifact)),
'Run `pnpm build` first, then retry this command.',
].join('\n'),
})
}
}

if (errors.length > 0) {
console.error(`Environment preflight failed for ${context.label}.`)
console.error('')

for (let error of errors) {
console.error(`${error.title}:`)
console.error(error.body)
console.error('')
}

if (warnings.length > 0) {
console.error('Warnings:')
console.error(formatList(warnings))
console.error('')
}

process.exit(1)
}

if (command === 'doctor') {
console.log(`Environment preflight passed for ${context.label}.`)

if (warnings.length > 0) {
console.log('')
console.log('Warnings:')
console.log(formatList(warnings))
}
}
67 changes: 67 additions & 0 deletions scripts/run-task.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { spawnSync } from 'node:child_process'
import path from 'node:path'
import process from 'node:process'
import { fileURLToPath } from 'node:url'
import { isWindows, rustEnv } from './rust-env.mjs'

const __dirname = path.dirname(fileURLToPath(import.meta.url))
const root = path.resolve(__dirname, '..')
const cwd = process.cwd()

const task = process.argv[2]
const passthrough = process.argv.slice(3)

const predefinedCommands = {
build: [['turbo', ['build', '--filter=!./playgrounds/*']]],
dev: [['turbo', ['dev', '--filter=!./playgrounds/*']]],
test: [
['cargo', ['test']],
['vitest', ['run', '--hideSkippedTests']],
],
'test:integrations': [['vitest', ['--root=./integrations']]],
'test:ui': [
['pnpm', ['run', '--filter=tailwindcss', 'test:ui']],
['pnpm', ['run', '--filter=@tailwindcss/browser', 'test:ui']],
],
vite: [['pnpm', ['run', '--filter=vite-playground', 'dev']]],
nextjs: [['pnpm', ['run', '--filter=nextjs-playground', 'dev']]],
}

let commands = predefinedCommands[task]

if (passthrough[0] === '--') {
commands = [[passthrough[1], passthrough.slice(2)]]
} else if (commands && passthrough.length > 0) {
commands = commands.map((entry, index) => {
if (index !== commands.length - 1) return entry
return [entry[0], [...entry[1], ...passthrough]]
})
}

if (!task || !commands) {
console.error(`Unknown task: ${task ?? '(missing)'}`)
process.exit(1)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

let preflight = spawnSync(process.execPath, [path.join(root, 'scripts', 'preflight.mjs'), task], {
cwd,
env: rustEnv,
stdio: 'inherit',
})

if (preflight.status !== 0) {
process.exit(preflight.status ?? 1)
}

for (let [command, args] of commands) {
let result = spawnSync(command, args, {
cwd,
env: rustEnv,
stdio: 'inherit',
shell: isWindows,
})
Comment on lines +61 to +67

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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Does Node.js spawnSync with shell:true on Windows properly quote args containing spaces and special shell characters?

💡 Result:

No, Node.js spawnSync with shell: true on Windows does not properly quote individual arguments containing spaces or special shell characters like '&'. Instead, it simply joins the command and all args array elements with spaces into a single string, which is then passed to cmd.exe /d /s /c. This raw concatenation means spaces split arguments unless the user has already included shell-quoting in the arg strings themselves, and special characters like '&' are interpreted by cmd.exe, potentially truncating or altering command execution. The official documentation warns against passing unsanitized input with shell: true due to this risk of command injection via shell metacharacters. When shell: true and the shell is CMD, windowsVerbatimArguments is automatically true, meaning no additional quoting/escaping is done by Node.js before passing to the shell.

Citations:


🏁 Script executed:

# First, find the file and inspect the relevant code sections
cd /tmp && find . -name "run-task.mjs" -type f 2>/dev/null | head -5

Repository: tailwindlabs/tailwindcss

Length of output: 50


🏁 Script executed:

# Check if file exists at the expected path
if [ -f scripts/run-task.mjs ]; then
  echo "File found at scripts/run-task.mjs"
  wc -l scripts/run-task.mjs
fi

Repository: tailwindlabs/tailwindcss

Length of output: 126


🏁 Script executed:

# Read the relevant sections of the file
if [ -f scripts/run-task.mjs ]; then
  echo "=== Lines 40-70 (including context around the issue) ==="
  sed -n '40,70p' scripts/run-task.mjs | cat -n
fi

Repository: tailwindlabs/tailwindcss

Length of output: 1070


🏁 Script executed:

# Also check lines around line 43 to see the passthrough path
if [ -f scripts/run-task.mjs ]; then
  echo "=== Lines 1-50 (to understand setup and line 43) ==="
  sed -n '1,50p' scripts/run-task.mjs | cat -n
fi

Repository: tailwindlabs/tailwindcss

Length of output: 2012


Args may not be shell-escaped correctly when shell: true on Windows.

With shell: isWindows, spawnSync passes command and joined args through cmd.exe. Node.js does minimal quoting for args when shell is truthy, so passthrough values that contain spaces, &, |, ^, ", etc. can be reinterpreted by the shell. For the hard-coded turbo/cargo/vitest/pnpm invocations this is fine, but the -- passthrough path (line 43) forwards arbitrary user-supplied tokens into this call, e.g. from crates/node napi build … --target x or any future ad-hoc use. A single arg containing a space (e.g. a path) will be split into two args on Windows.

Low impact for the current hard-coded call sites, but worth flagging since the override path is explicitly designed to accept arbitrary commands.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/run-task.mjs` around lines 61 - 67, The loop using spawnSync over the
commands array is vulnerable to cmd.exe re-parsing when shell: isWindows is set;
update the call to avoid using a shell on Windows (i.e. set shell: false) so
spawnSync(command, args, ...) passes args directly to the executable, or if a
shell is absolutely required for some commands, build a single command string
and properly escape each arg before passing it to spawnSync as a string; change
the code around spawnSync, commands and the shell: isWindows option (and the
passthrough path that produces those args) so arbitrary user-supplied tokens are
not handed unescaped into cmd.exe.


if (result.status !== 0) {
process.exit(result.status ?? 1)
}
}
60 changes: 60 additions & 0 deletions scripts/rust-env.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import fs from 'node:fs'
import path from 'node:path'
import process from 'node:process'
import { spawnSync } from 'node:child_process'

export const isWindows = process.platform === 'win32'

const home = process.env.USERPROFILE ?? process.env.HOME ?? process.env.HOMEPATH ?? ''

export const cargoHome = process.env.CARGO_HOME ?? (home ? path.join(home, '.cargo') : '')
export const rustupHome = process.env.RUSTUP_HOME ?? (home ? path.join(home, '.rustup') : '')
export const cargoBin = cargoHome ? path.join(cargoHome, 'bin') : ''

export const mergedPath = cargoBin
? [cargoBin, process.env.PATH ?? ''].filter(Boolean).join(path.delimiter)
: process.env.PATH ?? ''

export const rustEnv = {
...process.env,
...(cargoHome ? { CARGO_HOME: cargoHome } : {}),
...(rustupHome ? { RUSTUP_HOME: rustupHome } : {}),
PATH: mergedPath,
...(isWindows
? {
RUSTUP_TOOLCHAIN:
process.env.RUSTUP_TOOLCHAIN ?? 'stable-x86_64-pc-windows-msvc',
}
: {}),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

export function commandExists(command, { cwd = process.cwd() } = {}) {
let result = spawnSync(command, ['--version'], {
cwd,
env: rustEnv,
stdio: 'ignore',
shell: isWindows,
})

return !result.error && result.status === 0
}

export function readInstalledRustTargets({ cwd = process.cwd() } = {}) {
let result = spawnSync('rustup', ['target', 'list', '--installed'], {
cwd,
env: rustEnv,
encoding: 'utf8',
stdio: ['ignore', 'pipe', 'pipe'],
shell: isWindows,
})

if (result.error || result.status !== 0) {
return null
}

return new Set(result.stdout.split(/\r?\n/).filter(Boolean))
}

export function hasDefaultCargoBin() {
return Boolean(cargoBin) && fs.existsSync(cargoBin)
}