error: do not read a frame's source URL from disk unless the loader loaded it

src/jsc/SavedSourceMap.rs

@@ -3,7 +3,7 @@
 use core::ffi::c_void;
 use std::sync::Arc;
 
-use bun_collections::{HashMap, IdentityContext, TaggedPtrUnion};
+use bun_collections::{HashMap, IdentityContext, StringArrayHashMap, TaggedPtrUnion};
 use bun_core::MutableString;
 use bun_core::Ordinal;
 use bun_ptr::tagged_pointer::TagType;
@@ -16,6 +16,8 @@ use bun_wyhash::hash;
 pub struct SavedSourceMap {
     /// Only accessed between [`Self::lock`] and [`Self::unlock`].
     map: HashTable,
+    /// Every path inserted into `map`, by bytes (`map` keys are only hashes).
+    paths: StringArrayHashMap<()>,
     mutex: Mutex,
 }
 
@@ -141,6 +143,7 @@ impl SavedSourceMap {
         };
         if refers_to_provider {
             self.map.remove(&key);
+            self.paths.swap_remove(path);
             // SAFETY: `old_value` was stored by us; the table's ownership of
             // it ends here.
             unsafe { Self::release_value(old_value) };
@@ -248,10 +251,30 @@ impl SavedSourceMap {
                 v.insert(value.ptr());
             }
         }
+        if !self.paths.contains(path) {
+            self.paths.insert(path, ());
+        }
         self.unlock();
         Ok(())
     }
 
+    /// Records a path that a loaded module's own source map names as an original source.
+    pub(crate) fn trust_path(&mut self, path: &[u8]) {
+        self.lock();
+        if !self.paths.contains(path) {
+            self.paths.insert(path, ());
+        }
+        self.unlock();
+    }
+
+    /// Whether `path` is exactly a loaded module, or an original source one of them maps to.
+    pub(crate) fn is_loaded_path(&mut self, path: &[u8]) -> bool {
+        self.lock();
+        let found = self.paths.contains(path);
+        self.unlock();
+        found
+    }
+
     /// You must call `sourcemap.map.deref()` or you will leak memory
     fn get_with_content(
         &mut self,

src/jsc/VirtualMachine.rs

@@ -5632,10 +5632,8 @@ impl VirtualMachine {
                     bun_sourcemap::SourceContentHandling::NoSourceContents,
                 )
                 .map(|lookup| {
-                    (
-                        lookup.display_source_url_if_needed(source_url.slice()),
-                        lookup,
-                    )
+                    let display_url = self.remapped_source_url(&lookup, source_url.slice());
+                    (display_url, lookup)
                 })
             };
             if let Some((display_url, lookup)) = resolved {
@@ -5833,6 +5831,13 @@ impl VirtualMachine {
         }
 
         let already_remapped = frames[top].remapped;
+        // A frame parsed from `error.stack` names whatever the thrown code chose.
+        let allow_source_from_disk = if already_remapped {
+            let url = frames[top].source_url.to_utf8();
+            self.source_mappings.is_loaded_path(url.slice()) || self.is_embedded_module(url.slice())
+        } else {
+            true
+        };
         let resolved = {
             let top_source_url = frames[top].source_url.to_utf8();
             let maybe_lookup: Option<bun_sourcemap::mapping::Lookup> = if already_remapped {
@@ -5865,7 +5870,7 @@ impl VirtualMachine {
             maybe_lookup.map(|lookup| {
                 let mapping = lookup.mapping;
                 let display_url = if !already_remapped {
-                    lookup.display_source_url_if_needed(top_source_url.slice())
+                    self.remapped_source_url(&lookup, top_source_url.slice())
                 } else {
                     None
                 };
@@ -5900,6 +5905,9 @@ impl VirtualMachine {
                     // Avoid printing "export default 'native'"
                     break 'code bun_core::Utf8Bytes::EMPTY;
                 }
+                if !allow_source_from_disk {
+                    break 'code bun_core::Utf8Bytes::EMPTY;
+                }
                 let mut log = bun_ast::Log::default();
                 let Ok(original_source) = Self::fetch_without_on_load_plugins(
                     self,
@@ -5979,10 +5987,8 @@ impl VirtualMachine {
                         bun_sourcemap::SourceContentHandling::NoSourceContents,
                     )
                     .map(|lookup| {
-                        (
-                            lookup.display_source_url_if_needed(source_url.slice()),
-                            lookup,
-                        )
+                        let display_url = self.remapped_source_url(&lookup, source_url.slice());
+                        (display_url, lookup)
                     })
                 };
                 if let Some((display_url, lookup)) = resolved {
@@ -6845,6 +6851,26 @@ impl VirtualMachine {
         let _ = writer.flush();
     }
 
+    /// Whether `path` is a file embedded in this `bun build --compile` executable.
+    fn is_embedded_module(&self, path: &[u8]) -> bool {
+        bun_options_types::standalone_path::is_bun_standalone_file_path(path)
+            && self
+                .standalone_module_graph
+                .is_some_and(|graph| graph.find_assume_standalone_path(path).is_some())
+    }
+
+    /// The URL `lookup` remaps `source_url` to, recorded as a path the printer may read.
+    fn remapped_source_url(
+        &mut self,
+        lookup: &bun_sourcemap::mapping::Lookup,
+        source_url: &[u8],
+    ) -> Option<bun_core::String> {
+        let display_url = lookup.display_source_url_if_needed(source_url)?;
+        self.source_mappings
+            .trust_path(display_url.to_utf8().slice());
+        Some(display_url)
+    }
+
     /// Looks up the source-map mapping for `path` at `line:column`.
     pub(crate) fn resolve_source_mapping(
         &mut self,

test/js/node/vm/__snapshots__/vm-sourceUrl.test.ts.snap

@@ -8,7 +8,7 @@ throw new Error("hello");
 Error: hello
     at hellohello.js:2:16
     at runInNewContext (unknown)
-    at <anonymous> (<this-url>:6:5)"
+    at <anonymous> (<this-url>:8:5)"
 `;
 
 exports[`can get sourceURL inside node:vm 1`] = `
@@ -17,7 +17,7 @@ exports[`can get sourceURL inside node:vm 1`] = `
 error: hello
       at hello (hellohello.js:4:24)
       at hellohello.js:7:6
-      at <anonymous> (<this-url>:21:15)
+      at <anonymous> (<this-url>:23:15)
 "
 `;
 
@@ -27,6 +27,6 @@ exports[`eval sourceURL is correct 1`] = `
 error: hello
       at hello (hellohello.js:4:24)
       at eval (hellohello.js:7:6)
-      at <anonymous> (<this-url>:39:15)
+      at <anonymous> (<this-url>:41:15)
 "
 `;

test/js/node/vm/vm-sourceUrl.test.ts

@@ -1,4 +1,6 @@
-import { expect, test } from "bun:test";
+import { describe, expect, test } from "bun:test";
+import { bunEnv, bunExe, tempDir } from "harness";
+import path from "node:path";
 import { runInNewContext } from "node:vm";
 
 test("can get sourceURL from eval inside node:vm", () => {
@@ -50,3 +52,109 @@ hello();
   );
   expect(err.replaceAll(import.meta.path, "<this-url>")).toMatchSnapshot();
 });
+
+const CANARY = "SECRET_CANARY_DO_NOT_LEAK_8f2a";
+
+// The error printer reads a frame's source file from disk for a code-frame
+// excerpt. A `//# sourceURL` directive, or a node:vm `filename`, lets the code
+// that throws choose that source URL. The printer must not open a file the
+// module loader never loaded.
+describe.concurrent("error printer does not read attacker-named source files", () => {
+  for (const via of ["sourceURL", "filename"] as const) {
+    for (const caught of [true, false]) {
+      for (const nul of [false, true]) {
+        const label = `${via}, ${caught ? "caught" : "uncaught"}${nul ? ", interior NUL in the path" : ""}`;
+        test(`vm code does not leak a named file's contents (${label})`, async () => {
+          using dir = tempDir("vm-sourceurl-leak", {
+            "secret.txt": CANARY + "\n",
+            "run.mjs": `
+              import * as vm from "node:vm";
+              const target = process.env.CANARY_PATH + ${JSON.stringify(nul ? "\0.js" : "")};
+              const code = 'function f(){ throw new Error("boom") }; f()' ${
+                via === "sourceURL" ? `+ '\\n//# sourceURL=' + target` : ""
+              };
+              const options = { filename: ${via === "filename" ? "target" : '"sandbox.js"'} };
+              ${
+                caught
+                  ? `try { vm.runInNewContext(code, {}, options); } catch (e) { console.error(e); }`
+                  : `vm.runInNewContext(code, {}, options);`
+              }
+            `,
+          });
+
+          await using proc = Bun.spawn({
+            cmd: [bunExe(), path.join(String(dir), "run.mjs")],
+            env: { ...bunEnv, CANARY_PATH: path.join(String(dir), "secret.txt") },
+            stdout: "pipe",
+            stderr: "pipe",
+          });
+          const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+          const output = stdout + stderr;
+
+          // The error is still reported.
+          expect(output).toContain("boom");
+          // The file's contents are never shown.
+          expect(output).not.toContain(CANARY);
+          // An interior NUL in the name must not crash the printer. An
+          // uncaught error exits 1, a caught one exits 0.
+          expect(exitCode).toBe(caught ? 0 : 1);
+        });
+      }
+    }
+  }
+});
+
+// Frames parsed back out of an already-materialized `error.stack` are the
+// gated path. A loaded module, an original source named by a loaded module's
+// source map, and a file embedded in a compiled executable must keep their
+// code frame there.
+describe.concurrent("a loaded module still shows its source code frame", () => {
+  const app = `function doWork(): void {
+  throw new Error("real module error");
+}
+try {
+  doWork();
+} catch (e) {
+  void (e as Error).stack;
+  console.error(e);
+}
+`;
+
+  async function run(cmd: string[], cwd: string) {
+    await using proc = Bun.spawn({ cmd, env: bunEnv, cwd, stdout: "pipe", stderr: "pipe" });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    return { output: stdout + stderr, exitCode };
+  }
+
+  test("run from source", async () => {
+    using dir = tempDir("vm-sourceurl-real", { "app.ts": app });
+    const { output, exitCode } = await run([bunExe(), "app.ts"], String(dir));
+    expect(output).toContain(`throw new Error("real module error");`);
+    expect(output).toContain("app.ts:2:");
+    expect(exitCode).toBe(0);
+  });
+
+  test("bundled with an external source map", async () => {
+    using dir = tempDir("vm-sourceurl-bundled", { "src/app.ts": app });
+    const build = await run(
+      [bunExe(), "build", "--target=bun", "--sourcemap=external", "--outdir=dist", "src/app.ts"],
+      String(dir),
+    );
+    expect(build.exitCode).toBe(0);
+    const { output, exitCode } = await run([bunExe(), "dist/app.js"], String(dir));
+    // The frame names the original `src/app.ts`, which the map points at.
+    expect(output).toContain(`throw new Error("real module error");`);
+    expect(output).toContain(`${path.join("src", "app.ts")}:2:`);
+    expect(exitCode).toBe(0);
+  });
+
+  test("compiled executable", async () => {
+    using dir = tempDir("vm-sourceurl-compiled", { "app.ts": app });
+    const exe = path.join(String(dir), process.platform === "win32" ? "app.exe" : "app");
+    const build = await run([bunExe(), "build", "--compile", "app.ts", "--outfile", exe], String(dir));
+    expect(build.exitCode).toBe(0);
+    const { output, exitCode } = await run([exe], String(dir));
+    expect(output).toContain(`throw new Error("real module error");`);
+    expect(exitCode).toBe(0);
+  });
+});