feat(standalone): migrate standalone binaries to Node 26 Single Executable Applications (SEAs) - #10899
feat(standalone): migrate standalone binaries to Node 26 Single Executable Applications (SEAs)#10899joehan wants to merge 3 commits into
Conversation
…table Applications (SEAs) - Replace legacy @yao-pkg/pkg with native Node 26 --build-sea and esbuild - Intercept child process execution via in-memory script routing with createRequire to eliminate duplicate Node binary - Replace shelljs runtime dependency with native fs/child_process shell polyfill - Add multi-target build script supporting Linux x64, macOS Universal 2 (arm64 + x64 with lipo), and Windows x64 - Update firepit-builder pipeline to produce SEA artifacts and SHA256 checksums - Document architecture and build workflows in standalone/README.md
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try the Wiz Code extension for VS Code, JetBrains, or Visual Studio. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the Firepit standalone executable builder from @yao-pkg/pkg to Node.js 26's native Single Executable Application (--build-sea) capabilities and esbuild, removing external dependencies like shelljs in favor of a native inline shell polyfill. Feedback on these changes highlights several critical cross-platform and reliability issues: Unix-specific commands (cp -R and find) are used in build-sea.js which will fail on Windows; unconsumed response streams during HTTP redirects or errors in downloadFile can cause socket leaks; and reading directories without verifying they are files in the pipeline script poses an EISDIR crash risk. Additionally, in firepit.js, executing child scripts via createRequire fails to set up require.main properly, and ImitateNode() does not account for leading Node.js options.
| execSync( | ||
| `cp -R "${path.join(vendorDir, "node_modules")}"/* "${targetNodeModules}/"` | ||
| ); |
There was a problem hiding this comment.
Using the Unix-specific cp -R command via execSync will fail on Windows environments. Since this script is intended to support Windows (win.exe target), we should use Node's native, cross-platform fs.cpSync API instead.
fs.cpSync(path.join(vendorDir, "node_modules"), targetNodeModules, { recursive: true });| if (fs.existsSync(prodNodeModules)) { | ||
| execSync( | ||
| `cp -R "${prodNodeModules}"/* "${targetNodeModules}/" 2>/dev/null || true` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Using cp -R via execSync is not cross-platform and will fail on Windows. Use Node's native fs.cpSync instead.
| if (fs.existsSync(prodNodeModules)) { | |
| execSync( | |
| `cp -R "${prodNodeModules}"/* "${targetNodeModules}/" 2>/dev/null || true` | |
| ); | |
| } | |
| if (fs.existsSync(prodNodeModules)) { | |
| try { | |
| fs.cpSync(prodNodeModules, targetNodeModules, { recursive: true }); | |
| } catch (e) {} | |
| } |
| if (fs.existsSync(rootNodeModules)) { | ||
| execSync( | ||
| `cp -R "${rootNodeModules}"/* "${targetNodeModules}/" 2>/dev/null || true` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Using cp -R via execSync is not cross-platform and will fail on Windows. Use Node's native fs.cpSync instead.
| if (fs.existsSync(rootNodeModules)) { | |
| execSync( | |
| `cp -R "${rootNodeModules}"/* "${targetNodeModules}/" 2>/dev/null || true` | |
| ); | |
| } | |
| if (fs.existsSync(rootNodeModules)) { | |
| try { | |
| fs.cpSync(rootNodeModules, targetNodeModules, { recursive: true }); | |
| } catch (e) {} | |
| } |
| try { | ||
| execSync(`find "${assetsLibDir}" -name "*.node" -delete 2>/dev/null || true`, { stdio: "ignore" }); | ||
| } catch (e) {} |
There was a problem hiding this comment.
The find command is Unix-specific and will fail or execute the wrong utility on Windows. Since this build script is designed to run cross-platform, we should use a native Node.js recursive file search and deletion instead.
| try { | |
| execSync(`find "${assetsLibDir}" -name "*.node" -delete 2>/dev/null || true`, { stdio: "ignore" }); | |
| } catch (e) {} | |
| try { | |
| const removeNativeAddons = (dir) => { | |
| if (!fs.existsSync(dir)) return; | |
| const list = fs.readdirSync(dir); | |
| for (const file of list) { | |
| const fullPath = path.join(dir, file); | |
| const stat = fs.statSync(fullPath); | |
| if (stat.isDirectory()) { | |
| removeNativeAddons(fullPath); | |
| } else if (file.endsWith(".node")) { | |
| fs.unlinkSync(fullPath); | |
| } | |
| } | |
| }; | |
| removeNativeAddons(assetsLibDir); | |
| } catch (e) {} |
| try { | ||
| const scriptRequire = createRequire(resolvedScriptPath); | ||
| scriptRequire(resolvedScriptPath); | ||
| } catch (err) { | ||
| console.error(err); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
Executing the child script via createRequire and requiring it directly does not set up the script as the main module (require.main). Many Node.js scripts rely on require.main === module checks to execute their entrypoint logic, which will fail under this implementation.
Using Node's native Module.runMain() solves this by properly setting up the module context and executing it as the main entrypoint.
| try { | |
| const scriptRequire = createRequire(resolvedScriptPath); | |
| scriptRequire(resolvedScriptPath); | |
| } catch (err) { | |
| console.error(err); | |
| process.exit(1); | |
| } | |
| try { | |
| process.argv[1] = resolvedScriptPath; | |
| const Module = require("module"); | |
| Module.runMain(); | |
| } catch (err) { | |
| console.error(err); | |
| process.exit(1); | |
| } |
| return new Promise(resolve => { | ||
| const cmd = fork(nodeArgs[0], nodeArgs.slice(1), { | ||
| const target = path.resolve(nodeArgs[0]); | ||
| const cmd = fork(target, nodeArgs.slice(1), { | ||
| stdio: "inherit", | ||
| env: process.env | ||
| }); |
There was a problem hiding this comment.
In ImitateNode(), passing nodeArgs[0] directly to path.resolve assumes that the first argument is always the script path. However, Node.js is often invoked with options (e.g., --require, --max-old-space-size, --inspect), which will cause path.resolve to resolve the option as a file path and fail to execute.
We should parse and separate the leading Node.js options into execArgv and pass them to fork accordingly.
return new Promise(resolve => {
const execArgv = [];
let scriptIndex = 0;
while (scriptIndex < nodeArgs.length) {
const arg = nodeArgs[scriptIndex];
if (arg.startsWith("-")) {
execArgv.push(arg);
if ((arg === "-r" || arg === "--require" || arg === "--import") && scriptIndex + 1 < nodeArgs.length) {
execArgv.push(nodeArgs[scriptIndex + 1]);
scriptIndex += 2;
} else {
scriptIndex += 1;
}
} else {
break;
}
}
const scriptPath = nodeArgs[scriptIndex];
const scriptArgs = nodeArgs.slice(scriptIndex + 1);
const target = scriptPath ? path.resolve(scriptPath) : "";
const cmd = fork(target, scriptArgs, {
stdio: "inherit",
env: process.env,
execArgv
});| if (response.statusCode === 301 || response.statusCode === 302) { | ||
| get(response.headers.location); | ||
| return; | ||
| } | ||
| if (response.statusCode !== 200) { | ||
| reject(new Error(`Failed to download ${currentUrl}: HTTP ${response.statusCode}`)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
In downloadFile, when a redirect (301 or 302) or a non-200 status code is encountered, the response stream is not consumed or resumed. In Node.js, failing to consume the response stream can cause memory leaks and keep sockets open, potentially hanging the build process.
Please call response.resume() before returning or rejecting to ensure the stream is properly closed and resources are freed.
| if (response.statusCode === 301 || response.statusCode === 302) { | |
| get(response.headers.location); | |
| return; | |
| } | |
| if (response.statusCode !== 200) { | |
| reject(new Error(`Failed to download ${currentUrl}: HTTP ${response.statusCode}`)); | |
| return; | |
| } | |
| if (response.statusCode === 301 || response.statusCode === 302) { | |
| response.resume(); | |
| get(response.headers.location); | |
| return; | |
| } | |
| if (response.statusCode !== 200) { | |
| response.resume(); | |
| reject(new Error(`Failed to download ${currentUrl}: HTTP ${response.statusCode}`)); | |
| return; | |
| } |
| ls("firebase-tools*").forEach((file) => { | ||
| if (file.endsWith(".json") || file.endsWith(".txt") || file.endsWith(".js") || file.endsWith(".tar.gz")) return; | ||
| const data = fs.readFileSync(file); | ||
| const hash = crypto.createHash("sha256").update(data).digest("hex"); | ||
| sha256Lines.push(`${hash} ${file}`); | ||
| }); |
There was a problem hiding this comment.
If there are any directories matching the firebase-tools* pattern in the workspace, calling fs.readFileSync(file) on them will throw an EISDIR error and crash the pipeline.
We should add a defensive check using fs.statSync(file).isFile() to ensure we only read files.
ls("firebase-tools*").forEach((file) => {
if (file.endsWith(".json") || file.endsWith(".txt") || file.endsWith(".js") || file.endsWith(".tar.gz")) return;
if (!fs.statSync(file).isFile()) return;
const data = fs.readFileSync(file);
const hash = crypto.createHash("sha256").update(data).digest("hex");
sha256Lines.push(`${hash} ${file}`);
});![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Production release pipeline mode | ||
| console.log("[build-sea] Using vendor/node_modules from pipeline..."); | ||
| execSync( | ||
| `cp -R "${path.join(vendorDir, "node_modules")}"/* "${targetNodeModules}/"` |
| const prodNodeModules = path.join(tmpPackDir, "node_modules"); | ||
| if (fs.existsSync(prodNodeModules)) { | ||
| execSync( | ||
| `cp -R "${prodNodeModules}"/* "${targetNodeModules}/" 2>/dev/null || true` |
| const rootNodeModules = path.join(standaloneDir, "node_modules"); | ||
| if (fs.existsSync(rootNodeModules)) { | ||
| execSync( | ||
| `cp -R "${rootNodeModules}"/* "${targetNodeModules}/" 2>/dev/null || true` |
…EA build for automatic cache invalidation
…al binary creation via rcodesign in Linux builds and Dockerfile
3 participants
Description
This PR migrates the
firebase-toolsstandalone binaries to use Node.js 26 native Single Executable Applications (--build-sea) andesbuild, completely replacing the deprecated@yao-pkg/pkgcompiler and eliminating the need for external binary injection tools likepostject.Architectural Highlights
Native Node 26 SEA Packaging (
--build-sea):firepit.jsandwelcome.jswithesbuild(--bundle --platform=node --target=node26).In-Memory Subprocess Routing (No Duplicate Node Binary):
child_process.fork()orfirebase is:node,firepit.jsintercepts script arguments at the entrypoint and resolves them viacreateRequire.Native Inline Shell Polyfill:
shelljsanduser-homeruntime dependencies with a native cross-platform polyfill (mkdir,rm,cp,chmod,ln,ls,cat,exec) backed by Nodefs,os.homedir(), andchild_process.spawnSync.Multi-Target Build Script & macOS Universal 2 Binaries:
standalone/build-sea.jsdownloads official Node 26 binaries and builds executables for:firebase-tools-linux)firebase-tools-macos-arm64)firebase-tools-macos-x64)lipoand ad-hoc signed withcodesign(firebase-tools-macos)firebase-tools-win.exe)--current-onlyfor fast local development iteration.Release Pipeline Integration:
scripts/firepit-builder/pipeline.jsto build SEA artifacts, package all macOS targets, and generate SHA256 checksums inSHA256SUMS.txt.Documentation:
standalone/README.md.Verification & Testing
15.26.0)firebase is:node -e)firebase is:node -p)firebase is:npm --version)fork()pipeline.js