node compat: fix tty prototype pollution, MessagePort listener aliases, fs.read position validation

src/js/node/fs.ts

@@ -372,6 +372,12 @@ var access = function access(path, mode, callback) {
     if (!callback) {
       throw $ERR_INVALID_ARG_TYPE("callback", "function", callback);
     }
+    // Validate `position`: must be null/undefined, a number, or a bigint.
+    // Mirrors Node.js `validatePosition` (lib/internal/fs/utils.js), which
+    // rejects objects, strings, booleans, etc. with TypeError.
+    if (position != null && typeof position !== "number" && typeof position !== "bigint") {
+      throw $ERR_INVALID_ARG_TYPE("position", ["integer", "bigint"], position);
+    }
     fs.read(fd, buffer, offset, length, position).then(
       bytesRead => void callback(null, bytesRead, buffer),
       err => callback(err),
@@ -574,6 +580,13 @@ var access = function access(path, mode, callback) {
       ({ offset = 0, length = buffer.byteLength - offset, position = null } = offsetOrOptions ?? {});
     }
 
+    // Validate `position`: must be null/undefined, a number, or a bigint.
+    // Mirrors Node.js `validatePosition` (lib/internal/fs/utils.js), which
+    // rejects objects, strings, booleans, etc. with TypeError.
+    if (position != null && typeof position !== "number" && typeof position !== "bigint") {
+      throw $ERR_INVALID_ARG_TYPE("position", ["integer", "bigint"], position);
+    }
+
     return fs.readSync(fd, buffer, offset, length, position);
   },
   writeSync = fs.writeSync.bind(fs),

src/js/node/tty.ts

@@ -27,6 +27,11 @@ Object.defineProperty(ReadStream, "prototype", {
   get() {
     const Prototype = Object.create(fs.ReadStream.prototype);
 
+    // tty.ReadStream.prototype.isTTY must be `true` by convention, matching
+    // Node.js (lib/tty.js). Instance values set by the constructor still
+    // override this for fds that are not actually a TTY.
+    Prototype.isTTY = true;
+
     // Add ref/unref methods to make tty.ReadStream behave like Node.js
     // where TTY streams have socket-like behavior
     Prototype.ref = function () {
@@ -124,10 +129,18 @@ function WriteStream(fd): void {
 
 Object.defineProperty(WriteStream, "prototype", {
   get() {
-    const Real = fs.WriteStream.prototype;
-    Object.defineProperty(WriteStream, "prototype", { value: Real });
-
-    WriteStream.prototype._refreshSize = function () {
+    // Use Object.create so the tty.WriteStream methods below do not leak onto
+    // `fs.WriteStream.prototype`. This also gives tty.WriteStream a distinct
+    // prototype that can own its own `isTTY` default without polluting fs.
+    const Prototype = Object.create(fs.WriteStream.prototype);
+    Object.defineProperty(WriteStream, "prototype", { value: Prototype });
+
+    // tty.WriteStream.prototype.isTTY must be `true` by convention, matching
+    // Node.js (lib/tty.js). Instance values set by the constructor still
+    // override this for fds that are not actually a TTY.
+    Prototype.isTTY = true;
+
+    Prototype._refreshSize = function () {
       const oldCols = this.columns;
       const oldRows = this.rows;
       const windowSizeArray = [0, 0];
@@ -140,30 +153,30 @@ Object.defineProperty(WriteStream, "prototype", {
       }
     };
 
-    WriteStream.prototype.clearLine = function (dir, cb) {
+    Prototype.clearLine = function (dir, cb) {
       return require("node:readline").clearLine(this, dir, cb);
     };
 
-    WriteStream.prototype.clearScreenDown = function (cb) {
+    Prototype.clearScreenDown = function (cb) {
       return require("node:readline").clearScreenDown(this, cb);
     };
 
-    WriteStream.prototype.cursorTo = function (x, y, cb) {
+    Prototype.cursorTo = function (x, y, cb) {
       return require("node:readline").cursorTo(this, x, y, cb);
     };
 
     // The `getColorDepth` API got inspired by multiple sources such as
     // https://github.com/chalk/supports-color,
     // https://github.com/isaacs/color-support.
-    WriteStream.prototype.getColorDepth = function (env = process.env) {
+    Prototype.getColorDepth = function (env = process.env) {
       return require("internal/tty").getColorDepth(env);
     };
 
-    WriteStream.prototype.getWindowSize = function () {
+    Prototype.getWindowSize = function () {
       return [this.columns, this.rows];
     };
 
-    WriteStream.prototype.hasColors = function (count, env) {
+    Prototype.hasColors = function (count, env) {
       if (env === undefined && (count === undefined || (typeof count === "object" && count !== null))) {
         env = count;
         count = 16;
@@ -174,21 +187,21 @@ Object.defineProperty(WriteStream, "prototype", {
       return count <= 2 ** this.getColorDepth(env);
     };
 
-    WriteStream.prototype.moveCursor = function (dx, dy, cb) {
+    Prototype.moveCursor = function (dx, dy, cb) {
       return require("node:readline").moveCursor(this, dx, dy, cb);
     };
 
     // Add Symbol.asyncIterator to make tty.WriteStream compatible with code
     // that expects stdout/stderr to be async iterable (like in Node.js where they're Duplex)
-    WriteStream.prototype[Symbol.asyncIterator] = function () {
+    Prototype[Symbol.asyncIterator] = function () {
       // Since WriteStream is write-only, we return an empty async iterator
       // This matches the behavior of Node.js Duplex streams used for stdout/stderr
       return (async function* () {
         // stdout/stderr don't produce readable data, so yield nothing
       })();
     };
 
-    return Real;
+    return Prototype;
   },
   enumerable: true,
   configurable: true,

src/js/node/worker_threads.ts

@@ -108,6 +108,12 @@ function injectFakeEmitter(Class) {
 
   Class.prototype.prependListener = Class.prototype.on;
   Class.prototype.prependOnceListener = Class.prototype.once;
+
+  // Node.js EventEmitter exposes `addListener`/`removeListener` as aliases of
+  // `on`/`off`. MessagePort in node:worker_threads is expected to implement
+  // the same surface, so code that checks `"removeListener" in port` works.
+  Class.prototype.addListener = Class.prototype.on;
+  Class.prototype.removeListener = Class.prototype.off;
 }
 
 const _MessagePort = globalThis.MessagePort;

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

@@ -1234,6 +1234,41 @@ describe("readSync", () => {
     expect(readSync(2147483640, Buffer.alloc(0))).toBe(0);
     expect(readSync(2147483640, Buffer.alloc(10), 0, 0, 0)).toBe(0);
   });
+
+  it("throws TypeError when position is an object", () => {
+    // Matches Node.js `validatePosition` (lib/internal/fs/utils.js):
+    // non-null `position` must be a number or bigint, never a plain object.
+    const dest = join(tmpdir(), "readSync-invalid-position.txt");
+    rmSync(dest, { force: true });
+    writeFileSync(dest, "");
+    const fd = openSync(dest, "r+");
+    try {
+      const buf = new Uint8Array(0);
+      expect(() => readSync(fd, buf, 0, buf.length, { not: "a number" } as any)).toThrow(TypeError);
+      expect(() => readSync(fd, buf, 0, buf.length, "0" as any)).toThrow(TypeError);
+      expect(() => readSync(fd, buf, 0, buf.length, true as any)).toThrow(TypeError);
+    } finally {
+      closeSync(fd);
+      rmSync(dest, { force: true });
+    }
+  });
+
+  it("accepts null, undefined, number, and bigint for position", () => {
+    const dest = join(tmpdir(), "readSync-valid-position.txt");
+    rmSync(dest, { force: true });
+    writeFileSync(dest, "Hello");
+    const fd = openSync(dest, "r");
+    try {
+      const buf = Buffer.alloc(5);
+      expect(() => readSync(fd, buf, 0, 5, null)).not.toThrow();
+      expect(() => readSync(fd, buf, 0, 5, undefined)).not.toThrow();
+      expect(() => readSync(fd, buf, 0, 5, 0)).not.toThrow();
+      expect(() => readSync(fd, buf, 0, 5, 0n as any)).not.toThrow();
+    } finally {
+      closeSync(fd);
+      rmSync(dest, { force: true });
+    }
+  });
 });
 
 it("writevSync", () => {

test/js/node/tty.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, it } from "bun:test";
 import { isWindows } from "harness";
-import { WriteStream } from "node:tty";
+import fs from "node:fs";
+import { ReadStream, WriteStream } from "node:tty";
 
 describe("WriteStream.prototype.getColorDepth", () => {
   it("iTerm ancient", () => {
@@ -24,3 +25,38 @@ describe("WriteStream.prototype.getColorDepth", () => {
     expect(WriteStream.prototype.getColorDepth.call(undefined, {})).toBe(isWindows ? 24 : 1);
   });
 });
+
+describe("tty.{ReadStream,WriteStream}.prototype.isTTY", () => {
+  it("WriteStream.prototype.isTTY is true", () => {
+    // Matches Node.js: `tty.WriteStream.prototype.isTTY === true`.
+    expect(WriteStream.prototype.isTTY).toBe(true);
+  });
+
+  it("ReadStream.prototype.isTTY is true", () => {
+    // Matches Node.js: `tty.ReadStream.prototype.isTTY === true`.
+    expect(ReadStream.prototype.isTTY).toBe(true);
+  });
+
+  it("does not pollute fs.WriteStream.prototype", () => {
+    // tty.WriteStream inherits from fs.WriteStream but must not add tty-only
+    // members to fs.WriteStream.prototype. `isTTY` and the cursor helpers
+    // should only exist on the tty subclass prototype.
+    expect(Object.hasOwn(fs.WriteStream.prototype, "isTTY")).toBe(false);
+    expect(Object.hasOwn(fs.WriteStream.prototype, "clearLine")).toBe(false);
+    expect(Object.hasOwn(fs.WriteStream.prototype, "clearScreenDown")).toBe(false);
+    expect(Object.hasOwn(fs.WriteStream.prototype, "cursorTo")).toBe(false);
+    expect(Object.hasOwn(fs.WriteStream.prototype, "getColorDepth")).toBe(false);
+    expect(Object.hasOwn(fs.WriteStream.prototype, "hasColors")).toBe(false);
+    expect(Object.hasOwn(fs.WriteStream.prototype, "moveCursor")).toBe(false);
+
+    // fs.WriteStream instances must not inherit isTTY from anywhere either.
+    expect((fs.WriteStream.prototype as any).isTTY).toBeUndefined();
+  });
+
+  it("tty.WriteStream.prototype is distinct from fs.WriteStream.prototype", () => {
+    expect(WriteStream.prototype).not.toBe(fs.WriteStream.prototype);
+    // ...but still inherits from it, so `instanceof fs.WriteStream` holds for
+    // tty.WriteStream instances.
+    expect(Object.getPrototypeOf(WriteStream.prototype)).toBe(fs.WriteStream.prototype);
+  });
+});

test/js/node/worker_threads/worker_threads.test.ts

@@ -132,6 +132,57 @@ test("all worker_threads worker instance properties are present", async () => {
   await worker.terminate();
 });
 
+test("MessagePort exposes full EventEmitter alias surface", () => {
+  // Regression test for oven-sh/bun#29022. MessagePort.prototype was missing
+  // `addListener`/`removeListener` aliases, so code that does
+  // `if ("removeListener" in port)` (e.g. undici, node-fetch) would
+  // incorrectly skip the cleanup path.
+  const { port1, port2 } = new MessageChannel();
+  try {
+    for (const name of [
+      "on",
+      "off",
+      "once",
+      "emit",
+      "addListener",
+      "removeListener",
+      "prependListener",
+      "prependOnceListener",
+    ] as const) {
+      expect(name in port1).toBe(true);
+      expect(typeof port1[name]).toBe("function");
+    }
+
+    // `addListener`/`removeListener` must be live aliases of `on`/`off`, as in
+    // Node.js EventEmitter, so a listener attached via one name can be removed
+    // via the other.
+    let received = 0;
+    const handler = () => {
+      received++;
+    };
+    port1.addListener("message", handler);
+    port2.postMessage("hello");
+    // Give the event loop a tick so the message is delivered.
+    return new Promise<void>(resolve => {
+      setTimeout(() => {
+        expect(received).toBe(1);
+        port1.removeListener("message", handler);
+        port2.postMessage("world");
+        setTimeout(() => {
+          expect(received).toBe(1);
+          port1.close();
+          port2.close();
+          resolve();
+        }, 10);
+      }, 10);
+    });
+  } catch (err) {
+    port1.close();
+    port2.close();
+    throw err;
+  }
+});
+
 test("threadId module and worker property is consistent", async () => {
   const worker1 = new Worker(new URL("./worker-thread-id.ts", import.meta.url).href);
   expect(threadId).toBe(0);