diff --git a/src/bun_core/fmt.rs b/src/bun_core/fmt.rs index 2dc33cbfe4fc..f02091dae246 100644 --- a/src/bun_core/fmt.rs +++ b/src/bun_core/fmt.rs @@ -3325,6 +3325,87 @@ 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`, `\r`, `\t`, `\x1b`, `\x7f`, `\u009b`) instead of +/// written raw. For text somebody else authored (a registry manifest, a +/// dependency's `package.json`): printed raw, an ESC/CR/C1 sequence can erase +/// or repaint the line it is shown on and a newline can forge further lines +/// of our output. Everything else passes through unchanged. +pub struct EscapeControlChars(pub T); + +/// [`EscapeControlChars`] for a value that is legitimately multi-line and is +/// printed on its own (`bun pm view readme`): `\t`, `\n` and `\r\n` +/// pass through. A `\r` not followed by `\n` is still escaped; on a terminal +/// it only overwrites the line printed so far. +pub struct EscapeControlCharsMultiline(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)) +} + +/// [`EscapeControlCharsMultiline`] over raw bytes; invalid UTF-8 renders as U+FFFD. +pub fn escape_control_chars_multiline(text: &[u8]) -> EscapeControlCharsMultiline<&bstr::BStr> { + EscapeControlCharsMultiline(bstr::BStr::new(text)) +} + +impl Display for EscapeControlChars { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let mut writer = EscapeControlCharsWriter { + f, + keep_line_breaks: false, + }; + write!(writer, "{}", self.0) + } +} + +impl Display for EscapeControlCharsMultiline { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let mut writer = EscapeControlCharsWriter { + f, + keep_line_breaks: true, + }; + write!(writer, "{}", self.0) + } +} + +struct EscapeControlCharsWriter<'a, 'f> { + f: &'a mut Formatter<'f>, + keep_line_breaks: bool, +} + +impl fmt::Write for EscapeControlCharsWriter<'_, '_> { + fn write_str(&mut self, s: &str) -> fmt::Result { + let mut start = 0; + let mut chars = s.char_indices().peekable(); + while let Some((i, c)) = chars.next() { + let pass_through = match c { + '\t' | '\n' => self.keep_line_breaks, + '\r' => self.keep_line_breaks && matches!(chars.peek(), Some((_, '\n'))), + '\0'..='\x1f' | '\x7f' | '\u{80}'..='\u{9f}' => false, + _ => true, + }; + if pass_through { + continue; + } + self.f.write_str(&s[start..i])?; + match c { + '\n' => self.f.write_str("\\n")?, + '\r' => self.f.write_str("\\r")?, + '\t' => self.f.write_str("\\t")?, + c if c.is_ascii() => write!(self.f, "\\x{:02x}", c as u32)?, + c => write!(self.f, "\\u{:04x}", c as u32)?, + } + start = i + c.len_utf8(); + } + self.f.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/runtime/cli/pm_view_command.rs b/src/runtime/cli/pm_view_command.rs index ebeca184aac6..205293552cce 100644 --- a/src/runtime/cli/pm_view_command.rs +++ b/src/runtime/cli/pm_view_command.rs @@ -316,7 +316,10 @@ pub(crate) fn view( bun_fmt::format_json_string_utf8(slice, Default::default()) )); } else { - Output::print(format_args!("{}\n", BStr::new(slice))); + Output::print(format_args!( + "{}\n", + bun_fmt::escape_control_chars_multiline(slice) + )); } Output::flush(); return Ok(()); @@ -416,21 +419,23 @@ pub(crate) fn view( .len_u32() as usize; } + // Every string printed below is registry-controlled: always go through + // `escape_control_chars`. prettyln!( "{}@{} | {} | deps: {} | versions: {}", - BStr::new(pkg_name), - BStr::new(pkg_version), - BStr::new(license), + bun_fmt::escape_control_chars(pkg_name), + bun_fmt::escape_control_chars(pkg_version), + bun_fmt::escape_control_chars(license), dep_count, versions_len, ); // Get description and homepage from the top-level package manifest, not the version-specific one if let Some(desc) = json.get_string_cloned(&bump, b"description").ok().flatten() { - prettyln!("{}", BStr::new(desc)); + prettyln!("{}", bun_fmt::escape_control_chars(desc)); } if let Some(hp) = json.get_string_cloned(&bump, b"homepage").ok().flatten() { - prettyln!("{}", BStr::new(hp)); + prettyln!("{}", bun_fmt::escape_control_chars(hp)); } if let Some(mut iter) = json.get_array(b"keywords") { @@ -447,7 +452,10 @@ pub(crate) fn view( } } if !keywords.list.is_empty() { - prettyln!("keywords: {}", BStr::new(keywords.list.as_slice())); + prettyln!( + "keywords: {}", + bun_fmt::escape_control_chars(keywords.list.as_slice()) + ); } } @@ -481,8 +489,8 @@ pub(crate) fn view( }; prettyln!( "- {}: {}", - BStr::new(dep_name), - BStr::new(dep_version), + bun_fmt::escape_control_chars(dep_name), + bun_fmt::escape_control_chars(dep_version), ); } } @@ -490,13 +498,22 @@ pub(crate) fn view( if let Some(dist) = manifest.get_object(b"dist") { prettyln!("\ndist"); if let Some(t) = dist.get_string_cloned(&bump, b"tarball").ok().flatten() { - prettyln!(" .tarball: {}", BStr::new(t)); + prettyln!( + " .tarball: {}", + bun_fmt::escape_control_chars(t) + ); } if let Some(s) = dist.get_string_cloned(&bump, b"shasum").ok().flatten() { - prettyln!(" .shasum: {}", BStr::new(s)); + prettyln!( + " .shasum: {}", + bun_fmt::escape_control_chars(s) + ); } if let Some(i) = dist.get_string_cloned(&bump, b"integrity").ok().flatten() { - prettyln!(" .integrity: {}", BStr::new(i)); + prettyln!( + " .integrity: {}", + bun_fmt::escape_control_chars(i) + ); } if let Some(u) = dist.get_number(b"unpackedSize") { prettyln!( @@ -522,12 +539,14 @@ pub(crate) fn view( let val_expr = prop.value.as_ref().expect("infallible: prop has value"); if let Some(tag) = tagname_expr.as_string(&bump) { if let Some(val) = val_expr.as_string(&bump) { + let tag_fmt = bun_fmt::escape_control_chars(tag); + let val_fmt = bun_fmt::escape_control_chars(val); if tag == b"latest" { - prettyln!("{}: {}", BStr::new(tag), BStr::new(val)); + prettyln!("{}: {}", tag_fmt, val_fmt); } else if tag == b"beta" { - prettyln!("{}: {}", BStr::new(tag), BStr::new(val)); + prettyln!("{}: {}", tag_fmt, val_fmt); } else { - prettyln!("{}: {}", BStr::new(tag), BStr::new(val)); + prettyln!("{}: {}", tag_fmt, val_fmt); } } } @@ -548,9 +567,13 @@ pub(crate) fn view( .flatten() .unwrap_or(b""); if !em.is_empty() { - prettyln!("- {} \\<{}\\>", BStr::new(nm), BStr::new(em)); + prettyln!( + "- {} \\<{}\\>", + bun_fmt::escape_control_chars(nm), + bun_fmt::escape_control_chars(em) + ); } else if !nm.is_empty() { - prettyln!("- {}", BStr::new(nm)); + prettyln!("- {}", bun_fmt::escape_control_chars(nm)); } } } @@ -563,13 +586,19 @@ pub(crate) fn view( .ok() .flatten() { - prettyln!("\nPublished: {}", BStr::new(published_time)); + prettyln!( + "\nPublished: {}", + bun_fmt::escape_control_chars(published_time) + ); } else if let Some(modified_time) = time_obj .get_string_cloned(&bump, b"modified") .ok() .flatten() { - prettyln!("\nPublished: {}", BStr::new(modified_time)); + prettyln!( + "\nPublished: {}", + bun_fmt::escape_control_chars(modified_time) + ); } } diff --git a/test/cli/install/bun-info.test.ts b/test/cli/install/bun-info.test.ts index 04b1d9fe80b7..17715a244f57 100644 --- a/test/cli/install/bun-info.test.ts +++ b/test/cli/install/bun-info.test.ts @@ -1,6 +1,6 @@ import { spawn } from "bun"; import { describe, expect, it, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, tempDirWithFiles } from "harness"; +import { bunEnv, bunExe, isASAN, tempDir, tempDirWithFiles } from "harness"; import { join } from "node:path"; describe.concurrent("bun info", () => { @@ -372,6 +372,101 @@ describe.concurrent("bun info", () => { }); }); +describe.concurrent("bun info with control characters in the packument", () => { + // One of each kind of byte the registry must not be able to write to the + // terminal: ESC (clear screen, then an OSC title change), BEL, a bare CR + // (overwrites the line), CRLF, tab, DEL, and the C1 controls NEL (U+0085) + // and CSI (U+009B). + const hostile = "\x1b[2J\x1b]0;PWNED\x07\rX\r\nY\tZ\x7f\u0085\u009b[31m"; + // How the summary view renders it: every control becomes a visible escape. + const escaped = String.raw`\x1b[2J\x1b]0;PWNED\x07\rX\r\nY\tZ\x7f\u0085\u009b[31m`; + // How `bun pm view ` renders it: CRLF and tab are kept since + // the field itself may be multi-line (readme), everything else is escaped. + const escapedMultiline = String.raw`\x1b[2J\x1b]0;PWNED\x07\rX` + "\r\nY\tZ" + String.raw`\x7f\u0085\u009b[31m`; + + const packument = { + name: "hostile", + "dist-tags": { latest: "1.0.0", ["tag" + hostile]: "1.0.0" }, + description: "D" + hostile, + homepage: "H" + hostile, + keywords: ["K" + hostile, "plain"], + maintainers: [{ name: "M" + hostile, email: "E" + hostile }, { name: "N" + hostile }], + time: { "1.0.0": "P" + hostile }, + versions: { + "1.0.0": { + name: "n" + hostile, + version: "1.0.0", + license: "L" + hostile, + description: "D" + hostile, + dependencies: { ["dep" + hostile]: "^1" + hostile }, + dist: { tarball: "T" + hostile, shasum: "S" + hostile, integrity: "I" + hostile }, + }, + }, + }; + + async function view(...args: string[]) { + await using server = Bun.serve({ + port: 0, + fetch: () => Response.json(packument), + }); + using dir = tempDir("bun-info-control-chars", { + "package.json": JSON.stringify({ name: "app", version: "1.0.0" }), + }); + await using proc = spawn({ + cmd: [bunExe(), ...args], + cwd: String(dir), + env: { ...bunEnv, NPM_CONFIG_REGISTRY: server.url.href }, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + it("escapes every field of the summary view", async () => { + const { stdout, stderr, exitCode } = await view("info", "hostile"); + + // Only the newlines bun itself prints between lines may remain. + expect(stdout).not.toMatch(/[\x00-\x09\x0b-\x1f\x7f\u0080-\u009f]/); + expect(stdout.replaceAll(escaped, "")).toMatchInlineSnapshot(` + "n@1.0.0 | L | deps: 1 | versions: 1 + D + H + keywords: K, plain + + dependencies (1): + - dep: ^1 + + dist + .tarball: T + .shasum: S + .integrity: I + + dist-tags: + latest: 1.0.0 + tag: 1.0.0 + + maintainers: + - M > + - N + + Published: P + " + `); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); + + it("keeps line structure but escapes the rest when printing a single string field", async () => { + const { stdout, stderr, exitCode } = await view("pm", "view", "hostile", "description"); + + expect(stdout).toBe(`D${escapedMultiline}\n`); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); +}); + // LSan's default conservative scan only flags the `send_sync` response-metadata // leak when no idle thread parks with a stale pointer in a callee-saved // register; excluding registers as roots makes the check deterministic.