Repository navigation
Conversation
16a073a to
81cd7d7
Compare
Ark-kun
changed the title
| /** | ||
| * Installs `specs` into `<installRoot>/node_modules`, throwing with npm's | ||
| * output tail. npm runs in a fresh prefix so a bundle-shipped `package.json`, | ||
| * lockfile, or `.npmrc` can't add packages or change the registry. |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
Registry / .npmrc isolation may not actually hold. This comment states that a fresh --prefix means a bundle-shipped package.json/lockfile/.npmrc "can't add packages or change the registry" — but --prefix only controls where packages are written, not npm's config resolution. npm derives the project .npmrc from localPrefix, which it finds by walking up from cwd (here stage, inside the bundle-controlled .tangent/). A bundle shipping .tangent/.npmrc (plus a package.json so .tangent becomes the localPrefix) could set registry=… and redirect the install; a host .npmrc/package.json above the session root could leak in too.
Since manifest validation already treats specs as semi-untrusted (rejecting file:/git/URL/alias), this is worth closing:
- Pass an explicit
--registry=<trusted>(CLI flags beat the project.npmrc), and/or - Neutralize config via
npm_config_userconfig/npm_config_globalconfigenv + acwdoutside the bundle tree (mindEXDEVon the laterrename).
Related: a published package's lifecycle scripts still run as the server user during install. That's within the existing bundle trust model (triggers/extensions already run server-side), but consider --ignore-scripts for entries that don't need a native build.
| await execFileAsync( | ||
| npmBin, | ||
| ["install", "--prefix", stage, "--no-audit", "--no-fund", ...specs], | ||
| { cwd: stage, timeout: INSTALL_TIMEOUT_MS, maxBuffer: 16 * 1024 * 1024 }, |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
Nit: maxBuffer: 16 * 1024 * 1024 means an unusually chatty install whose combined stdout/stderr exceeds 16 MB rejects with ERR_CHILD_PROCESS_STDIO_MAXBUFFER, which then surfaces as a misleading "npm install failed" rather than the real cause. Unlikely with --no-audit --no-fund, but if you want to be safe, raise the buffer or stream the output instead of capturing it.
| const abs = path.join(dir, entry.name); | ||
| if (entry.isDirectory()) { | ||
| walk(abs); | ||
| if (entry.name !== "node_modules") walk(abs); |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
readInstalledEntries skips only a dir named exactly node_modules. installNpmPackages cleans its .npm-* staging dir in a finally, but if the process is killed mid-install that finally never runs, leaving .tangent/.npm-*/… on disk. On the next revive, those files are read back as bogus bundle entries (keyed .npm-xxxx/…) and fed into resolveSessionConfig. Consider also skipping entries whose name starts with .npm- (or any dot-prefixed staging dir) here.
|
Closes #125
Description
Screenshots
How to test
Notes