Repository: denoland/deno
Pull request: fix(ext/node): handle raw fd instead rid in isatty

Needs:
- [x] https://github.com/denoland/deno/pull/31892

Previously, isatty(fd) was calling core.isTerminal(fd) which expected a Deno resource ID, but Node.js APIs pass raw OS file descriptors. For fd 0, 1, 2 (stdin/stdout/stderr) this happened to work because rid === fd, but it would fail for higher fds.

Changed files:
filename: ext/node/lib.rs
patch: @@ -369,6 +369,7 @@ deno_core::extension!(deno_node,
     ops::require::op_require_package_imports_resolve<TInNpmPackageChecker, TNpmPackageFolderResolver, TSys>,
     ops::require::op_require_break_on_next_statement,
     ops::util::op_node_guess_handle_type,
+    ops::util::op_node_is_tty,
     ops::util::op_node_view_has_buffer,
     ops::util::op_node_get_own_non_index_properties,
     ops::util::op_node_call_is_from_dependency<TInNpmPackageChecker, TNpmPackageFolderResolver, TSys>,

filename: ext/node/ops/util.rs
patch: @@ -26,6 +26,49 @@ enum HandleType {
   Unknown,
 }
 
+/// Check if a raw file descriptor is a TTY.
+/// This is used by Node.js `tty.isatty(fd)`.
+#[op2(fast)]
+pub fn op_node_is_tty(fd: i32) -> bool {
+  if fd < 0 {
+    return false;
+  }
+  is_tty(fd)
+}
+
+#[cfg(unix)]
+fn is_tty(fd: i32) -> bool {
+  // SAFETY: We're checking if the fd is a terminal.
+  // The fd may or may not be valid, but libc::isatty handles that safely.
+  unsafe { libc::isatty(fd) == 1 }
+}
+
+#[cfg(windows)]
+fn is_tty(fd: i32) -> bool {
+  use winapi::um::consoleapi::GetConsoleMode;
+  use winapi::um::processenv::GetStdHandle;
+  use winapi::um::winbase::STD_ERROR_HANDLE;
+  use winapi::um::winbase::STD_INPUT_HANDLE;
+  use winapi::um::winbase::STD_OUTPUT_HANDLE;
+
+  // SAFETY: GetStdHandle returns a borrowed handle to stdin/stdout/stderr.
+  // For fd > 2, we try to use it as a raw handle directly.
+  let handle = match fd {
+    // SAFETY: These are valid standard handles.
+    0 => unsafe { GetStdHandle(STD_INPUT_HANDLE) },
+    // SAFETY: These are valid standard handles.
+    1 => unsafe { GetStdHandle(STD_OUTPUT_HANDLE) },
+    // SAFETY: These are valid standard handles.
+    2 => unsafe { GetStdHandle(STD_ERROR_HANDLE) },
+    _ => fd as winapi::um::winnt::HANDLE,
+  };
+
+  let mut mode = 0;
+  // SAFETY: handle is either a valid standard handle or a raw fd cast to HANDLE.
+  // GetConsoleMode will return 0 if the handle is invalid or not a console.
+  unsafe { GetConsoleMode(handle, &mut mode) != 0 }
+}
+
 #[op2(fast)]
 pub fn op_node_guess_handle_type(state: &mut OpState, rid: u32) -> u32 {
   let handle = match state.resource_table.get_handle(rid) {

filename: ext/node/polyfills/tty.js
patch: @@ -19,7 +19,8 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 // SOFTWARE.
 
-import { core, primordials } from "ext:core/mod.js";
+import { primordials } from "ext:core/mod.js";
+import { op_node_is_tty } from "ext:core/ops";
 const {
   ArrayPrototypeSome,
   Error,
@@ -32,9 +33,6 @@ const {
   StringPrototypeSplit,
   StringPrototypeToLowerCase,
 } = primordials;
-const {
-  isTerminal,
-} = core;
 
 import { ERR_INVALID_FD } from "ext:deno_node/internal/errors.ts";
 import { validateInteger } from "ext:deno_node/internal/validators.mjs";
@@ -267,16 +265,7 @@ function isatty(fd) {
   if (typeof fd !== "number" || fd >> 0 !== fd || fd < 0) {
     return false;
   }
-  try {
-    /**
-     * TODO: Treat `fd` as real file descriptors. Currently, `rid` 0, 1, 2
-     * correspond to `fd` 0, 1, 2 (stdin, stdout, stderr). This may change in
-     * the future.
-     */
-    return isTerminal(fd);
-  } catch (_) {
-    return false;
-  }
+  return op_node_is_tty(fd);
 }
 
 export class ReadStream extends Socket {

filename: tests/unit_node/tty_test.ts
patch: @@ -5,11 +5,13 @@ import { assert } from "@std/assert";
 import { isatty } from "node:tty";
 import tty from "node:tty";
 import process from "node:process";
+import fs from "node:fs";
 
 Deno.test("[node/tty isatty] returns true when fd is a tty, false otherwise", () => {
-  assert(Deno.stdin.isTerminal() === isatty((Deno as any).stdin.rid));
-  assert(Deno.stdout.isTerminal() === isatty((Deno as any).stdout.rid));
-  assert(Deno.stderr.isTerminal() === isatty((Deno as any).stderr.rid));
+  // Uses raw file descriptors: 0 = stdin, 1 = stdout, 2 = stderr
+  assert(Deno.stdin.isTerminal() === isatty(0));
+  assert(Deno.stdout.isTerminal() === isatty(1));
+  assert(Deno.stderr.isTerminal() === isatty(2));
 });
 
 Deno.test("[node/tty isatty] returns false for irrelevant values", () => {
@@ -51,3 +53,13 @@ Deno.test("[node/tty WriteStream.hasColors] returns true when colors are support
 Deno.test("[node/tty WriteStream.getColorDepth] returns current terminal color depth", () => {
   assert([1, 4, 8, 24].includes(tty.WriteStream.prototype.getColorDepth()));
 });
+
+Deno.test("[node/tty isatty] returns false for raw file fd", () => {
+  // Open a file and get its raw fd - files are never TTYs
+  const fd = fs.openSync("README.md", "r");
+  try {
+    assert(!isatty(fd), `fd ${fd} should not be a tty`);
+  } finally {
+    fs.closeSync(fd);
+  }
+});