node:fs: validate options.signal in readFile, writeFile and watch like node

src/jsc/JSGlobalObject.rs

@@ -541,6 +541,7 @@ impl JSGlobalObject {
     }
 
     /// "The {argname} argument must be of type {typename}. Received {value}"
+    /// (a dotted `argname` is a "property", see `InvalidArgTypeName`).
     ///
     /// Accepts `&str`, `&[u8]`, or `b"..."` for `argname`/`typename`.
     pub fn throw_invalid_argument_type_value(
@@ -556,15 +557,17 @@ impl JSGlobalObject {
         self.err(
             JscError::INVALID_ARG_TYPE,
             format_args!(
-                "The \"{}\" argument must be of type {}. Received {}",
-                bstr::BStr::new(argname.as_ref()),
+                "The {} must be of type {}. Received {}",
+                InvalidArgTypeName(argname.as_ref()),
                 bstr::BStr::new(typename.as_ref()),
                 actual_string_value
             ),
         )
         .throw()
     }
 
+    /// Like [`Self::throw_invalid_argument_type_value`], but `typename` is the whole
+    /// phrase after "must be" (e.g. `"an instance of Array"`).
     pub fn throw_invalid_argument_type_value2(
         &self,
         argname: impl AsRef<[u8]>,
@@ -578,8 +581,8 @@ impl JSGlobalObject {
         self.err(
             JscError::INVALID_ARG_TYPE,
             format_args!(
-                "The \"{}\" argument must be {}. Received {}",
-                bstr::BStr::new(argname.as_ref()),
+                "The {} must be {}. Received {}",
+                InvalidArgTypeName(argname.as_ref()),
                 bstr::BStr::new(typename.as_ref()),
                 actual_string_value
             ),
@@ -627,8 +630,8 @@ impl JSGlobalObject {
         self.err(
             JscError::INVALID_ARG_TYPE,
             format_args!(
-                "The \"{}\" argument must be one of type {}. Received {}",
-                bstr::BStr::new(argname.as_ref()),
+                "The {} must be one of type {}. Received {}",
+                InvalidArgTypeName(argname.as_ref()),
                 bstr::BStr::new(typename.as_ref()),
                 actual_string_value
             ),
@@ -1380,6 +1383,26 @@ pub struct SysErrOptions {
     pub name: Option<&'static [u8]>,
 }
 
+/// The subject of Node's `ERR_INVALID_ARG_TYPE` message: `"name" argument`,
+/// `"options.name" property` for a dotted name, or the name verbatim when it
+/// already ends in ` argument` ("first argument"). Same rule as
+/// `Bun::Message::addParameter` in ErrorCode.cpp.
+/// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/errors.js#L1407-L1414
+struct InvalidArgTypeName<'a>(&'a [u8]);
+
+impl core::fmt::Display for InvalidArgTypeName<'_> {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        let name = bstr::BStr::new(self.0);
+        if self.0.ends_with(b" argument") {
+            write!(f, "{name}")
+        } else if strings::contains_char(self.0, b'.') {
+            write!(f, "\"{name}\" property")
+        } else {
+            write!(f, "\"{name}\" argument")
+        }
+    }
+}
+
 // Unified with the crate-root definitions (lib.rs) — re-exported here so
 // `bun_jsc::js_global_object::{IntegerRange, ValidateObjectOpts}` keep
 // resolving for any caller that named them via this path. The previous local

src/runtime/node/node_fs.rs

@@ -4074,17 +4074,11 @@ pub mod args {
                     if let Some(flag_) = arg.get_truthy(ctx, "flag")? {
                         flag = FileSystemFlags::from_js(ctx, flag_)?.unwrap_or(flag);
                     }
-                    if let Some(value) = arg.get_truthy(ctx, "signal")? {
-                        if let Some(signal) = AbortSignal::ref_from_js(value) {
-                            signal.pending_activity_ref();
-                            *abort_signal = Some(signal);
-                        } else {
-                            return Err(ctx.throw_invalid_argument_type_value(
-                                b"signal",
-                                b"AbortSignal",
-                                value,
-                            ));
-                        }
+                    if let Some(value) = arg.get(ctx, "signal")? {
+                        let signal =
+                            validators::validate_abort_signal(ctx, value, "options.signal")?;
+                        signal.pending_activity_ref();
+                        *abort_signal = Some(signal);
                     }
                 }
             }
@@ -4183,17 +4177,11 @@ pub mod args {
                     if let Some(mode_) = arg.get_truthy(ctx, "mode")? {
                         mode = node::mode_from_js(ctx, mode_)?.unwrap_or(mode);
                     }
-                    if let Some(value) = arg.get_truthy(ctx, "signal")? {
-                        if let Some(signal) = AbortSignal::ref_from_js(value) {
-                            signal.pending_activity_ref();
-                            *abort_signal = Some(signal);
-                        } else {
-                            return Err(ctx.throw_invalid_argument_type_value(
-                                b"signal",
-                                b"AbortSignal",
-                                value,
-                            ));
-                        }
+                    if let Some(value) = arg.get(ctx, "signal")? {
+                        let signal =
+                            validators::validate_abort_signal(ctx, value, "options.signal")?;
+                        signal.pending_activity_ref();
+                        *abort_signal = Some(signal);
                     }
                     if let Some(flush_) = arg.get(ctx, "flush")? {
                         if flush_.is_boolean() || flush_.is_undefined_or_null() {

src/runtime/node/node_fs_watcher.rs

@@ -15,7 +15,7 @@ use bun_jsc::JsCell;
 use bun_jsc::abort_signal::AbortListener;
 use bun_jsc::node::PathLike;
 use bun_jsc::{
-    self as jsc, AbortSignal, AbortSignalRef, ArgumentsSlice, CallFrame, CommonAbortReason,
+    self as jsc, AbortSignalRef, ArgumentsSlice, CallFrame, CommonAbortReason,
     CommonAbortReasonExt as _, GlobalRef, JSGlobalObject, JSValue, JsRef, JsResult, SysErrorJsc,
     VirtualMachineRef as VirtualMachine, ZigStringJsc as _,
 };
@@ -24,6 +24,7 @@ use bun_sys::{self, SystemErrno};
 use bun_threading::Mutex;
 
 use crate::node::types::{Encoding, PathLikeExt};
+use crate::node::util::validators;
 use crate::webcore::encoding as Encoder;
 
 bun_output::declare_scope!(fs_watch, hidden);
@@ -640,7 +641,7 @@ pub struct Arguments<'a> {
     pub path: PathLike,
     pub(crate) listener: JSValue,
     pub global_this: &'a JSGlobalObject,
-    pub(crate) signal: Option<&'a AbortSignal>,
+    pub(crate) signal: Option<AbortSignalRef>,
     pub(crate) persistent: bool,
     pub(crate) recursive: bool,
     pub(crate) encoding: Encoding,
@@ -660,7 +661,7 @@ impl<'a> Arguments<'a> {
         // drops `path` automatically.
 
         let mut listener: JSValue = JSValue::ZERO;
-        let mut signal: Option<&AbortSignal> = None;
+        let mut signal: Option<AbortSignalRef> = None;
         let mut persistent: bool = true;
         let mut recursive: bool = false;
         let mut encoding: Encoding = Encoding::Utf8;
@@ -700,22 +701,12 @@ impl<'a> Arguments<'a> {
                     recursive = recursive_.to_boolean();
                 }
 
-                // abort signal
-                if let Some(signal_) = options_or_callable.get_truthy(ctx, "signal")? {
-                    if let Some(signal_obj) = AbortSignal::from_js(signal_) {
-                        // Keep it alive
-                        signal_.ensure_still_alive();
-                        // `signal_obj` is the live C++ AbortSignal owned by
-                        // `signal_` (kept reachable for the duration of the call
-                        // by `ensure_still_alive`). `AbortSignal` is an
-                        // `opaque_ffi!` ZST handle; `opaque_ref` is the
-                        // centralised deref proof.
-                        signal = Some(AbortSignal::opaque_ref(signal_obj));
-                    } else {
-                        return Err(ctx.throw_invalid_arguments(format_args!(
-                            "signal is not of type AbortSignal"
-                        )));
-                    }
+                if let Some(signal_) = options_or_callable.get(ctx, "signal")? {
+                    signal = Some(validators::validate_abort_signal(
+                        ctx,
+                        signal_,
+                        "options.signal",
+                    )?);
                 }
 
                 // listener
@@ -1180,13 +1171,7 @@ impl FSWatcher {
                 ..Default::default()
             }),
             mutex: Mutex::default(),
-            // SAFETY: `args.signal` is a live borrow of the JS AbortSignal (kept
-            // reachable by the caller's frame); `ref_()` bumps the C++ intrusive
-            // refcount and `adopt` takes ownership of that +1.
-            signal: JsCell::new(
-                args.signal
-                    .map(|s| unsafe { AbortSignalRef::adopt(s.ref_()) }),
-            ),
+            signal: JsCell::new(args.signal.clone()),
             persistent: Cell::new(args.persistent),
             path_watcher: Cell::new(None),
             global_this: GlobalRef::from(args.global_this),
@@ -1206,7 +1191,7 @@ impl FSWatcher {
 
         ctx_ref
             .path_watcher
-            .set(if args.signal.is_none_or(|s| !s.aborted()) {
+            .set(if args.signal.as_deref().is_none_or(|s| !s.aborted()) {
                 // The two backends take different arities (the Windows
                 // backend dropped the callback parameters — only one valid
                 // value each), so the call is cfg-split.

src/runtime/node/util/validators.rs

@@ -1,7 +1,9 @@
 use core::fmt;
 
 use bun_core::ZigString;
-use bun_jsc::{self as jsc, JSGlobalObject, JSValue, JsError, JsResult};
+use bun_jsc::{
+    self as jsc, AbortSignal, AbortSignalRef, JSGlobalObject, JSValue, JsError, JsResult,
+};
 
 fn get_type_name(global_object: &JSGlobalObject, value: JSValue) -> ZigString {
     let js_type = value.js_type();
@@ -470,6 +472,19 @@ pub(crate) fn validate_function(
     Ok(value)
 }
 
+/// Node's `validateAbortSignal`, except that an `aborted`-shaped object is rejected
+/// too: the native abort paths need a real `AbortSignal`.
+/// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/validators.js#L444-L451
+pub(crate) fn validate_abort_signal(
+    global_this: &JSGlobalObject,
+    value: JSValue,
+    name: &str,
+) -> JsResult<AbortSignalRef> {
+    AbortSignal::ref_from_js(value).ok_or_else(|| {
+        global_this.throw_invalid_argument_type_value2(name, "an instance of AbortSignal", value)
+    })
+}
+
 /// Rust has no field reflection; enums opt in via this trait.
 /// Implementors should typically `#[derive(strum::EnumString, strum::VariantNames)]`
 /// and provide `VALUES_INFO` as the `|`-joined variant names.

test/js/node/fs/promises.test.js

@@ -418,6 +418,99 @@ it("teardown waits for every concurrent in-flight write", async () => {
   expect(fh.fd).toBe(-1);
 });
 
+// node's getOptions() runs validateAbortSignal(options.signal, "options.signal")
+// for readFile, writeFile and appendFile: anything other than undefined that is
+// not a signal is an ERR_INVALID_ARG_TYPE, including null and "".
+// Every expected string below is node v26.3.0's output for the same call.
+describe("options.signal validation matches node", () => {
+  const invalid = received =>
+    `TypeError ERR_INVALID_ARG_TYPE: The "options.signal" property must be an instance of AbortSignal. Received ${received}`;
+
+  function outcome(fn) {
+    try {
+      const result = fn();
+      if (typeof result?.then === "function") {
+        return result.then(
+          () => "resolved",
+          e => `${e.constructor.name} ${e.code}: ${e.message}`,
+        );
+      }
+      return "resolved";
+    } catch (e) {
+      return `${e.constructor.name} ${e.code}: ${e.message}`;
+    }
+  }
+
+  test("readFileSync reports every non-signal value like node", async () => {
+    await using dir = tempDir("fs-signal-values", { "f.txt": "hello" });
+    const file = join(dir, "f.txt");
+    const values = [1, 0, null, "", "abc", true, false, {}, [], new AbortController()];
+    expect(values.map(signal => outcome(() => fs.readFileSync(file, { signal })))).toEqual([
+      invalid("type number (1)"),
+      invalid("type number (0)"),
+      invalid("null"),
+      invalid("type string ('')"),
+      invalid("type string ('abc')"),
+      invalid("type boolean (true)"),
+      invalid("type boolean (false)"),
+      invalid("an instance of Object"),
+      invalid("an instance of Array"),
+      invalid("an instance of AbortController"),
+    ]);
+    // Only undefined means "no signal".
+    expect(fs.readFileSync(file, { signal: undefined, encoding: "utf8" })).toBe("hello");
+    expect(fs.readFileSync(file, { signal: new AbortController().signal, encoding: "utf8" })).toBe("hello");
+  });
+
+  test("every readFile, writeFile and appendFile entry point rejects an invalid signal the same way", async () => {
+    await using dir = tempDir("fs-signal-entry-points", { "in.txt": "hello" });
+    const input = join(dir, "in.txt");
+    const output = join(dir, "out.txt");
+    const bad = { signal: null };
+    const noop = () => {};
+    const fh = await fsPromises.open(input, "r+");
+    let results;
+    try {
+      results = {
+        readFileSync: outcome(() => fs.readFileSync(input, bad)),
+        readFile: outcome(() => fs.readFile(input, bad, noop)),
+        "promises.readFile": await outcome(() => fsPromises.readFile(input, bad)),
+        "promises.readFile(handle)": await outcome(() => fsPromises.readFile(fh, bad)),
+        "handle.readFile": await outcome(() => fh.readFile(bad)),
+        writeFileSync: outcome(() => fs.writeFileSync(output, "x", bad)),
+        writeFile: outcome(() => fs.writeFile(output, "x", bad, noop)),
+        "promises.writeFile": await outcome(() => fsPromises.writeFile(output, "x", bad)),
+        "promises.writeFile(handle)": await outcome(() => fsPromises.writeFile(fh, "x", bad)),
+        appendFileSync: outcome(() => fs.appendFileSync(output, "x", bad)),
+        appendFile: outcome(() => fs.appendFile(output, "x", bad, noop)),
+        "promises.appendFile": await outcome(() => fsPromises.appendFile(output, "x", bad)),
+        "promises.appendFile(handle)": await outcome(() => fsPromises.appendFile(fh, "x", bad)),
+      };
+    } finally {
+      await fh.close();
+    }
+    const rejectedNull = invalid("null");
+    expect(results).toEqual({
+      readFileSync: rejectedNull,
+      readFile: rejectedNull,
+      "promises.readFile": rejectedNull,
+      "promises.readFile(handle)": rejectedNull,
+      "handle.readFile": rejectedNull,
+      writeFileSync: rejectedNull,
+      writeFile: rejectedNull,
+      "promises.writeFile": rejectedNull,
+      "promises.writeFile(handle)": rejectedNull,
+      appendFileSync: rejectedNull,
+      appendFile: rejectedNull,
+      "promises.appendFile": rejectedNull,
+      "promises.appendFile(handle)": rejectedNull,
+    });
+    // The option is validated before any I/O happens.
+    expect(fs.existsSync(output)).toBe(false);
+    expect(fs.readFileSync(input, "utf8")).toBe("hello");
+  });
+});
+
 // node rejects abortable fs APIs with an AbortError (an Error whose code is the
 // string "ABORT_ERR" and whose cause is signal.reason), never with the raw
 // DOMException held in signal.reason (whose .code is the number 20).

test/js/node/watch/fs.watch.test.ts

@@ -462,6 +462,32 @@ describe("fs.watch", () => {
     });
   });
 
+  // node validates options.signal in getOptions() before creating the watcher;
+  // the strings below are node v26.3.0's output for the same calls (null
+  // included: only undefined means "no signal").
+  test("a signal option that is not an AbortSignal throws node's ERR_INVALID_ARG_TYPE", () => {
+    const filepath = path.join(testDir, "abort.txt");
+    const invalid = (received: string) =>
+      `TypeError ERR_INVALID_ARG_TYPE: The "options.signal" property must be an instance of AbortSignal. Received ${received}`;
+    const outcome = (signal: unknown) => {
+      try {
+        fs.watch(filepath, { signal } as any).close();
+        return "watcher created";
+      } catch (e: any) {
+        return `${e.constructor.name} ${e.code}: ${e.message}`;
+      }
+    };
+    expect([1, null, "", {}, new AbortController()].map(outcome)).toEqual([
+      invalid("type number (1)"),
+      invalid("null"),
+      invalid("type string ('')"),
+      invalid("an instance of Object"),
+      invalid("an instance of AbortController"),
+    ]);
+    expect(outcome(undefined)).toBe("watcher created");
+    expect(outcome(new AbortController().signal)).toBe("watcher created");
+  });
+
   test("an abort with no 'error' listener only closes; an 'error' listener that throws is an uncaught exception", async () => {
     await using proc = Bun.spawn({
       cmd: [