diff --git a/src/bun_core/fmt.rs b/src/bun_core/fmt.rs index 2dc33cbfe4fc..528fd29ae191 100644 --- a/src/bun_core/fmt.rs +++ b/src/bun_core/fmt.rs @@ -3325,6 +3325,50 @@ fn escape_powershell_impl(str: &[u8], writer: &mut impl fmt::Write) -> fmt::Resu write_bytes(writer, remain) } +// ─────────────────────────────────────────────────────────────────────────── +// escapeControlChars +// ─────────────────────────────────────────────────────────────────────────── + +/// Renders the wrapped `Display` with C0 controls, DEL and C1 controls spelled +/// out (`\n`, `\x1b`, `\x7f`, `\u009b`) instead of written raw, for text a +/// registry or a package authored; everything else passes through unchanged. +pub struct EscapeControlChars(pub T); + +/// [`EscapeControlChars`] over raw bytes; invalid UTF-8 renders as U+FFFD. +pub fn escape_control_chars(text: &[u8]) -> EscapeControlChars<&bstr::BStr> { + EscapeControlChars(bstr::BStr::new(text)) +} + +impl Display for EscapeControlChars { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let mut writer = EscapeControlCharsWriter(f); + write!(writer, "{}", self.0) + } +} + +struct EscapeControlCharsWriter<'a, 'f>(&'a mut Formatter<'f>); + +impl fmt::Write for EscapeControlCharsWriter<'_, '_> { + fn write_str(&mut self, s: &str) -> fmt::Result { + let mut start = 0; + for (i, c) in s.char_indices() { + if !matches!(c, '\0'..='\x1f' | '\x7f' | '\u{80}'..='\u{9f}') { + continue; + } + self.0.write_str(&s[start..i])?; + match c { + '\n' => self.0.write_str("\\n")?, + '\r' => self.0.write_str("\\r")?, + '\t' => self.0.write_str("\\t")?, + c if c.is_ascii() => write!(self.0, "\\x{:02x}", c as u32)?, + c => write!(self.0, "\\u{:04x}", c as u32)?, + } + start = i + c.len_utf8(); + } + self.0.write_str(&s[start..]) + } +} + // js_bindings (fmtString for highlighter.test.ts) lives in src/jsc/fmt_jsc.rs // alongside fmt_jsc.bind.ts; bun_core/ stays JSC-free. diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 6c96867f593b..ac8b18d445c0 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -403,24 +403,15 @@ impl<'a> LazyPackageDestinationDir<'a> { } } -/// A dependency alias becomes the install destination inside `node_modules` -/// (the existing entry is renamed aside, deleted, and re-created). Reject -/// anything that could escape `node_modules`: empty names, `.`/`..` -/// components, absolute paths, drive letters, backslashes, NUL bytes, and any -/// separator other than the single `/` in a scoped name (`@scope/name`). +/// The alias is the install destination inside `node_modules` (renamed aside, +/// deleted and re-created), so on top of `is_safe_install_folder_name` it must +/// be a single path component, or two for a scoped name. pub(crate) fn alias_is_safe_install_target(alias: &[u8]) -> bool { - if alias.is_empty() || alias.len() >= MAX_PATH_BYTES || strings::contains_any(alias, b"\\:\0") { + if alias.len() >= MAX_PATH_BYTES || !crate::dependency::is_safe_install_folder_name(alias) { return false; } - let mut component_count = 0usize; - for component in strings::split(alias, b"/") { - component_count += 1; - if component.is_empty() || component == b"." || component == b".." { - return false; - } - } - + let component_count = strings::split(alias, b"/").count(); component_count == 1 || (component_count == 2 && alias[0] == b'@') } @@ -1295,7 +1286,7 @@ impl<'a> PackageInstaller<'a> { if log_level != Options::LogLevel::Silent { bun_core::pretty_errorln!( "error: refusing to install dependency with unsafe name {}", - bstr::BStr::new(alias.slice(string_buf!())), + bun_core::fmt::escape_control_chars(alias.slice(string_buf!())), ); } self.summary.fail += 1; diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index ca23f272fd55..f3bc05ae29ed 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -770,6 +770,55 @@ pub fn enqueue_dependency_with_main_and_success_fn( version_was_replaced = false; break 'version dependency.version.clone(); }; + + // Refuse an unsafe alias (the future `node_modules/` folder) or registry + // name (the request and the package name) here, before either is fetched + // or printed. Empty names are tolerated, as in the tree builder. + let invalid_name = { + let alias = this.lockfile.str(&dependency.name); + let alias_is_safe = if alias == this.lockfile.str(&dependency.version.literal) { + // `bun add ` uses the specifier as the alias until + // `assign_resolution` names it: never a folder, but still printed. + !dependency::contains_control_character(alias) + } else { + alias.is_empty() || dependency::is_safe_install_folder_name(alias) + }; + if !alias_is_safe { + Some(alias) + } else { + match version.tag { + dependency::version::Tag::Npm | dependency::version::Tag::DistTag => { + let registry_name = this.lockfile.str(&name); + (!registry_name.is_empty() + && !dependency::is_safe_install_folder_name(registry_name)) + .then_some(registry_name) + } + _ => None, + } + } + }; + if let Some(invalid_name) = invalid_name { + if let Some(fail) = fail_fn { + fail(this, dependency, id, crate::Error::InvalidDependencyName); + return Ok(()); + } + let name = bun_fmt::escape_control_chars(invalid_name); + if dependency.behavior.is_required() { + this.log_mut().add_error_fmt( + None, + bun_ast::Loc::EMPTY, + format_args!("Invalid dependency name \"{name}\""), + ); + } else { + this.log_mut().add_warning_fmt( + None, + bun_ast::Loc::EMPTY, + format_args!("Invalid dependency name \"{name}\""), + ); + } + return Ok(()); + } + let mut loaded_manifest: Option = None; match version.tag { diff --git a/src/install/PackageManager/PackageManagerResolution.rs b/src/install/PackageManager/PackageManagerResolution.rs index 04093280799e..f2ec57c7bad9 100644 --- a/src/install/PackageManager/PackageManagerResolution.rs +++ b/src/install/PackageManager/PackageManagerResolution.rs @@ -352,14 +352,20 @@ impl PackageManager { { Output::err_generic( "{} failed to resolve", - (failed_dep.version.literal.fmt(string_buf),), + (bun_core::fmt::escape_control_chars( + failed_dep.version.literal.slice(string_buf), + ),), ); } else { Output::err_generic( "{}@{} failed to resolve", ( - bstr::BStr::new(failed_dep.name.slice(string_buf)), - failed_dep.version.literal.fmt(string_buf), + bun_core::fmt::escape_control_chars( + failed_dep.name.slice(string_buf), + ), + bun_core::fmt::escape_control_chars( + failed_dep.version.literal.slice(string_buf), + ), ), ); } diff --git a/src/install/TarballStream.rs b/src/install/TarballStream.rs index 7ae9f698469b..77e9bd906cb7 100644 --- a/src/install/TarballStream.rs +++ b/src/install/TarballStream.rs @@ -1089,7 +1089,7 @@ impl TarballStream { bun_ast::Loc::EMPTY, format_args!( "Refusing to install package with invalid name \"{}\"", - bun_fmt::s(tarball.name_and_basename().0), + bun_fmt::escape_control_chars(tarball.name_and_basename().0), ), ); } else { diff --git a/src/install/dependency.rs b/src/install/dependency.rs index 3a2661f63ab6..8e3732297153 100644 --- a/src/install/dependency.rs +++ b/src/install/dependency.rs @@ -566,11 +566,11 @@ pub fn is_scoped_package_name(name: &[u8]) -> Result { Err(PackageNameError::InvalidPackageName) } -/// A dependency name/alias becomes a directory under `node_modules/`. Names -/// come from untrusted `package.json` / manifest keys, so reject anything that -/// could resolve outside that directory. `@scope/name` stays valid. +/// Names come from untrusted `package.json` / manifest keys and end up as +/// `node_modules/` directories and in progress and error output, so reject path +/// escapes and terminal control characters. `@scope/name` stays valid. pub(crate) fn is_safe_install_folder_name(name: &[u8]) -> bool { - if name.is_empty() { + if name.is_empty() || contains_control_character(name) { return false; } @@ -578,7 +578,7 @@ pub(crate) fn is_safe_install_folder_name(name: &[u8]) -> bool { if component.is_empty() || component == b"." || component == b".." { return false; } - if strings::contains_any(component, b"\\:\0") { + if strings::contains_any(component, b"\\:") { return false; } } @@ -586,6 +586,18 @@ pub(crate) fn is_safe_install_folder_name(name: &[u8]) -> bool { true } +/// C0 controls and DEL, plus UTF-8 encoded C1 controls (`C2 80`..`C2 9F`, +/// U+0080..=U+009F), which terminals interpret too. +pub(crate) fn contains_control_character(name: &[u8]) -> bool { + name.iter().enumerate().any(|(i, &byte)| { + byte.is_ascii_control() + || (byte == 0xC2 + && name + .get(i + 1) + .is_some_and(|next| (0x80..=0x9F).contains(next))) + }) +} + /// assumes version is valid pub fn without_build_tag(version: &[u8]) -> &[u8] { if let Some(plus) = strings::index_of_char(version, b'+') { diff --git a/src/install/error.rs b/src/install/error.rs index ab1c59a3a839..104a2952cc56 100644 --- a/src/install/error.rs +++ b/src/install/error.rs @@ -70,6 +70,8 @@ pub enum Error { Failed, #[error("UnrecognizedDependencyFormat")] UnrecognizedDependencyFormat, + #[error("InvalidDependencyName")] + InvalidDependencyName, #[error("No global directory found")] NoGlobalDirectoryFound, #[error("InvalidPackageID")] @@ -292,6 +294,7 @@ impl Error { Self::HTTPError => "HTTPError", Self::Failed => "Failed", Self::UnrecognizedDependencyFormat => "UnrecognizedDependencyFormat", + Self::InvalidDependencyName => "InvalidDependencyName", Self::NoGlobalDirectoryFound => "No global directory found", Self::InvalidPackageID => "InvalidPackageID", Self::PartialInstallFailed => "PartialInstallFailed", diff --git a/src/install/extract_tarball.rs b/src/install/extract_tarball.rs index b8d3a8b091b5..cdb719ae36ed 100644 --- a/src/install/extract_tarball.rs +++ b/src/install/extract_tarball.rs @@ -247,7 +247,7 @@ impl ExtractTarball { bun_ast::Loc::EMPTY, format_args!( "Refusing to install package with invalid name \"{}\"", - bun_fmt::s(name), + bun_fmt::escape_control_chars(name), ), ); return Err(crate::Error::InstallFailed); @@ -466,7 +466,7 @@ impl ExtractTarball { bun_ast::Loc::EMPTY, format_args!( "Refusing to install package with invalid name \"{}\"", - bun_fmt::s(name), + bun_fmt::escape_control_chars(name), ), ); return Err(crate::Error::InstallFailed); diff --git a/src/install/isolated_install.rs b/src/install/isolated_install.rs index 9b94cdda2d30..af8b2a9d153e 100644 --- a/src/install/isolated_install.rs +++ b/src/install/isolated_install.rs @@ -2123,7 +2123,7 @@ pub(crate) fn install_isolated_packages( if let Some(name) = unsafe_folder_name { Output::err_generic( "\"{}\" is not a valid install folder name", - (BStr::new(name),), + (bun_core::fmt::escape_control_chars(name),), ); Output::flush(); Global::exit(1); diff --git a/src/install/lockfile/Tree.rs b/src/install/lockfile/Tree.rs index 66d7c3c067ce..840c3df70994 100644 --- a/src/install/lockfile/Tree.rs +++ b/src/install/lockfile/Tree.rs @@ -372,7 +372,7 @@ pub(crate) fn relative_path_and_depth<'b, const PATH_STYLE: IteratorPathStyle>( if !folder_name_is_safe(name) { Output::err_generic( "Lockfile is malformed (dependency name \"{}\" is not a valid folder name)", - (bstr::BStr::new(name),), + (bun_core::fmt::escape_control_chars(name),), ); bun_core::Global::crash(); } @@ -791,17 +791,18 @@ impl Tree { // don't treat it as unsafe — match the lockfile parser and isolated // installer (`bun.lock.rs`, `isolated_install.rs`) which guard // `!name.is_empty()` here rather than failing the whole install. + // Neither does an unresolved dependency, and if its name is why it + // did not resolve, enqueue already reported it. let dependency_name = dependency .name .slice(lockfile.buffers.string_bytes.as_slice()); - if !dependency_name.is_empty() + if pkg_id != invalid_package_id + && !dependency_name.is_empty() && !crate::dependency::is_safe_install_folder_name(dependency_name) { builder.maybe_report_error(format_args!( "Invalid dependency name \"{}\"", - dependency - .name - .fmt(lockfile.buffers.string_bytes.as_slice()), + bun_core::fmt::escape_control_chars(dependency_name), )); continue 'dep; } diff --git a/src/install/lockfile/bun.lock.rs b/src/install/lockfile/bun.lock.rs index 379de751af9b..425c2aa77480 100644 --- a/src/install/lockfile/bun.lock.rs +++ b/src/install/lockfile/bun.lock.rs @@ -3228,8 +3228,8 @@ fn dependency_resolution_failure( format_args!( "Failed to resolve {} dependency '{}' for package '{}'", behavior_str, - bstr::BStr::new(dep.name.slice(buf)), - bstr::BStr::new(path), + bun_core::fmt::escape_control_chars(dep.name.slice(buf)), + bun_core::fmt::escape_control_chars(path), ), ); } else { @@ -3239,7 +3239,7 @@ fn dependency_resolution_failure( format_args!( "Failed to resolve root {} dependency '{}'", behavior_str, - bstr::BStr::new(dep.name.slice(buf)), + bun_core::fmt::escape_control_chars(dep.name.slice(buf)), ), ); } diff --git a/test/cli/install/bun-install-registry.test.ts b/test/cli/install/bun-install-registry.test.ts index e40b14dcaf7a..2f755ac01d18 100644 --- a/test/cli/install/bun-install-registry.test.ts +++ b/test/cli/install/bun-install-registry.test.ts @@ -9133,8 +9133,10 @@ test("rejects npm aliases whose manifest URL resolves to a different host than t const err = await stderr.text(); await stdout.text(); - // The manifest request must be refused with a clear error... - expect(err).toContain("is not on registry"); + // The manifest request must be refused with a clear error (today the name is + // already refused while resolving, before a URL is built; the URL check stays + // behind it as a second line of defense)... + expect(err).toMatch(/Invalid dependency name|is not on registry/); // ...and no request (carrying the registry Authorization header) may reach // a host other than the configured registry. expect(received).toEqual([]); diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index 0602dec36d5c..e54b4edb41c8 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -9637,13 +9637,231 @@ it("does not extract a tarball for a dependency alias containing '..' path segme expect(await readdirSorted(zone)).toEqual(["a"]); expect(await readdirSorted(join(zone, "a"))).toEqual(["b"]); expect(await readdirSorted(join(zone, "a", "b"))).toEqual(["c"]); - // The unsafe alias is reported as an error and nothing is installed. - expect(err).toContain("Refusing to install package with invalid name"); + // The unsafe alias is reported as an error before the tarball is even + // requested, and nothing is installed. + expect(err).toContain('Invalid dependency name "x/../../../.."'); + expect(urls).toEqual([]); expect(out).not.toContain("1 package installed"); expect(exitCode).not.toBe(0); }); }); +describe("dependency names containing terminal control characters", () => { + // OSC 52 (write to the clipboard) followed by CSI 2J (clear the screen). A + // registry serves it as an ordinary JSON string (`"ev\u001b]52;..."`), so it + // reaches bun like any other dependency name. + const controlName = "ev\x1b]52;c;aGkK\x07\x1b[2Jil"; + // How bun reports it: with the control characters spelled out. + const escapedName = String.raw`ev\x1b]52;c;aGkK\x07\x1b[2Jil`; + // Forces the progress line, which echoes the name of every manifest being + // fetched, even though stderr is a pipe here. + const progressEnv = { ...env, BUN_INSTALL_PROGRESS: "1" }; + + /** + * The dummy registry, which has a manifest for every name asked of it, plus: + * the control-character package really is installable (its tarball is + * bar's) and `bar@0.0.2` declares `barDeps`. + */ + function registry(ctx: TestContext, urls: string[], barDeps: object = {}) { + const dummy = dummyRegistryForContext(ctx, urls); + return async (req: Request) => { + const name = decodeURIComponent(new URL(req.url).pathname.slice(`/${ctx.id}/`.length)); + if (name.endsWith(".tgz") && name.includes(controlName)) { + urls.push(req.url); + return new Response(file(join(import.meta.dir, "bar-0.0.2.tgz"))); + } + const res = await dummy(req); + if (name !== "bar") return res; + const manifest = await res.json(); + Object.assign(manifest.versions["0.0.2"], barDeps); + return Response.json(manifest); + }; + } + + function assertNameNeverLeaked(out: string, err: string, urls: string[]) { + expect(out).not.toContain(controlName); + expect(err).not.toContain(controlName); + expect(err).not.toContain("\x1b]52;"); + expect(urls.filter(url => url.includes("%1B") || url.includes("\x1b"))).toEqual([]); + } + + /** Entries of node_modules other than the cache this registry setup (`cache = false`) puts there. */ + async function installed(ctx: TestContext) { + const entries = await readdirSorted(join(ctx.package_dir, "node_modules")).catch(() => []); + return entries.filter(entry => entry !== ".cache"); + } + + // A dependency whose key repeats its specifier is how `bun add ` + // records a dependency it has not named yet, so such a key is exempt from the + // folder name rules, but not from this one: git tasks put it on the progress + // line as-is. + const gitSpecifier = `git+https://127.0.0.1:9/${controlName}.git`; + const escapedGitSpecifier = `git+https://127.0.0.1:9/${escapedName}.git`; + const manifestCases: [string, object, string][] = [ + ["a dependency name", { dependencies: { [controlName]: "0.0.2" } }, escapedName], + ["an unnamed git dependency", { dependencies: { [gitSpecifier]: gitSpecifier } }, escapedGitSpecifier], + ]; + for (const [description, barDeps, expectedName] of manifestCases) { + it(`rejects ${description} declared by a package manifest before requesting or printing it`, async () => { + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + setContextHandler(ctx, registry(ctx, urls, barDeps)); + await writeFile( + join(ctx.package_dir, "package.json"), + JSON.stringify({ name: "foo", version: "0.0.1", dependencies: { bar: "0.0.2" } }), + ); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: ctx.package_dir, + stdout: "pipe", + stderr: "pipe", + env: progressEnv, + }); + const [out, err, exitCode] = await Promise.all([stdout.text(), stderr.text(), exited]); + + expect(err).toContain(`error: Invalid dependency name "${expectedName}"`); + assertNameNeverLeaked(out, err, urls); + expect(urls).toContain(`${ctx.registry_url}bar`); + expect(await installed(ctx)).toEqual([]); + expect(exitCode).toBe(1); + }); + }); + } + + it("keeps the unnamed form for specifiers without control characters", async () => { + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + setContextHandler(ctx, registry(ctx, urls)); + await writeFile(join(ctx.package_dir, "package.json"), JSON.stringify({ name: "foo", version: "0.0.1" })); + + // The alias of this dependency is the specifier until the tarball has + // been resolved: full of characters a folder name may not contain. + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "add", `${ctx.registry_url}bar-0.0.2.tgz`], + cwd: ctx.package_dir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, err, exitCode] = await Promise.all([stdout.text(), stderr.text(), exited]); + + expect(err).not.toContain("Invalid dependency name"); + expect(out).toContain("installed bar@"); + expect(await installed(ctx)).toEqual(["bar"]); + expect(exitCode).toBe(0); + }); + }); + + it("only warns when the name is declared as an optional dependency", async () => { + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + setContextHandler(ctx, registry(ctx, urls, { optionalDependencies: { [controlName]: "0.0.2" } })); + await writeFile( + join(ctx.package_dir, "package.json"), + JSON.stringify({ name: "foo", version: "0.0.1", dependencies: { bar: "0.0.2" } }), + ); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: ctx.package_dir, + stdout: "pipe", + stderr: "pipe", + env: progressEnv, + }); + const [out, err, exitCode] = await Promise.all([stdout.text(), stderr.text(), exited]); + + expect(err).toContain(`warn: Invalid dependency name "${escapedName}"`); + assertNameNeverLeaked(out, err, urls); + expect(await installed(ctx)).toEqual(["bar"]); + expect(out).toContain("1 package installed"); + expect(exitCode).toBe(0); + }); + }); + + // [description, root dependencies, how the unresolved dependency is summarized] + const aliasCases: [string, Record, string][] = [ + ["as the alias of a dependency", { [controlName]: "npm:bar@0.0.2" }, `${escapedName}@npm:bar@0.0.2`], + [ + "as the target of an npm: alias", + { "safe-alias": `npm:${controlName}@0.0.2` }, + `safe-alias@npm:${escapedName}@0.0.2`, + ], + ]; + for (const [description, dependencies, summary] of aliasCases) { + it(`rejects a name used ${description}`, async () => { + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + setContextHandler(ctx, registry(ctx, urls)); + await writeFile( + join(ctx.package_dir, "package.json"), + JSON.stringify({ name: "foo", version: "0.0.1", dependencies }), + ); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: ctx.package_dir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, err, exitCode] = await Promise.all([stdout.text(), stderr.text(), exited]); + + expect(err).toContain(`error: Invalid dependency name "${escapedName}"`); + expect(err).toContain(`error: ${summary} failed to resolve`); + assertNameNeverLeaked(out, err, urls); + expect(await installed(ctx)).toEqual([]); + expect(exitCode).toBe(1); + }); + }); + } + + it("rejects a package name found in bun.lock instead of installing or listing it", async () => { + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + setContextHandler(ctx, registry(ctx, urls)); + await Promise.all([ + writeFile( + join(ctx.package_dir, "package.json"), + JSON.stringify({ name: "foo", version: "0.0.1", dependencies: { [controlName]: "0.0.2" } }), + ), + writeFile( + join(ctx.package_dir, "bun.lock"), + textLockfile(1, { + workspaces: { "": { name: "foo", dependencies: { [controlName]: "0.0.2" } } }, + packages: { [controlName]: [`${controlName}@0.0.2`, `${ctx.registry_url}bar-0.0.2.tgz`, {}, ""] }, + }), + ), + ]); + + // `bun pm ls` prints straight from the lockfile, so it runs first, before + // `bun install` gets a chance to touch it. + const commands: [string[], string][] = [ + [["pm", "ls", "--all"], "Error loading lockfile: InvalidLockfile"], + [["install"], "Invalid package name"], + ]; + for (const [args, expectedError] of commands) { + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), ...args], + cwd: ctx.package_dir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, err, exitCode] = await Promise.all([stdout.text(), stderr.text(), exited]); + + expect(err).toContain(expectedError); + assertNameNeverLeaked(out, err, urls); + expect(exitCode).toBe(1); + } + // Ignoring the rejected lockfile must not make `bun install` fall back to + // resolving the same name from package.json either. + expect(urls).toEqual([]); + expect(await installed(ctx)).toEqual([]); + }); + }); +}); + it("does not install transitive file: dependencies that point outside their package", async () => { // A dependency declared by a non-workspace package (here: a folder dependency // of the project) uses a file: specifier pointing at an absolute path outside diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index a4ff2a70903f..e99cbbccff35 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -2768,7 +2768,8 @@ test("rejects dependency aliases that traverse outside node_modules", async () = // A (transitively) malicious package.json can use an arbitrary string as a // dependency alias. The alias becomes a `node_modules/` path // component in the isolated store layout, so a `..` segment lets it plant - // symlinks outside of node_modules. + // symlinks outside of node_modules. Such an alias is refused while resolving + // (the installer has its own check as well, see the next test). await write( packageJson, JSON.stringify({ @@ -2788,7 +2789,7 @@ test("rejects dependency aliases that traverse outside node_modules", async () = }); const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); - expect(stderr).toContain("is not a valid install folder name"); + expect(stderr).toContain('Invalid dependency name "../pwned-by-alias"'); // Nothing may be created outside of node_modules. `lstatSync` instead of // `existsSync` because the escaped artifact would be a dangling symlink. expect(() => lstatSync(join(packageDir, "pwned-by-alias"))).toThrow();