From 7e2bb6225f7c113634097008db5be669bfbba540 Mon Sep 17 00:00:00 2001
From: nntrivi2001 <vinnt@smartgift.vn>
Date: Thu, 13 Aug 2026 11:14:18 +0700
Subject: [PATCH] fix(run): forward text-frame keystrokes to pty and fix ws-pty
 upgrade dispatch
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

xterm.js's onData hands the browser a plain string, and WebSocket.send(string)
always emits a TEXT frame — pty-attach.js only forwarded BINARY frames to
pty.write(), so every keystroke was silently dropped as an unparseable JSON
control message. Now any non-control text frame reaches the pty.

Deeper root cause of the terminal never accepting input at all: the /ws
WebSocketServer used the {server, path} shorthand, whose own internal
upgrade listener calls handleUpgrade() for every upgrade on the shared
http.Server and aborts with 400 on a path mismatch — killing /ws-pty/*
upgrades before the PTY server's own listener ever ran. Switched /ws to
noServer + a manual path-checked dispatch, matching /ws-pty's pattern.
---
 server/__tests__/pty-attach.test.js | 11 +++++++++++
 server/lib/pty-attach.js            | 30 ++++++++++++++++++++---------
 server/websocket.js                 | 20 +++++++++++++++----
 3 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/server/__tests__/pty-attach.test.js b/server/__tests__/pty-attach.test.js
index 8482438..8c65654 100644
--- a/server/__tests__/pty-attach.test.js
+++ b/server/__tests__/pty-attach.test.js
@@ -78,6 +78,17 @@ describe("pty-attach", () => {
     assert.deepEqual(fakePty.__writes[0], { resize: [100, 40] });
   });
 
+  it("forwards a plain text WS frame (keystrokes) to pty.write", () => {
+    // xterm.js's onData hands the browser a plain string, and
+    // WebSocket.send(string) always emits a TEXT frame — so every keystroke
+    // arrives here as non-binary. This must reach the pty, not be dropped as
+    // an unparseable control message.
+    const ws = makeFakeWs();
+    ptyAttach.attach(ws, "ccam-lane-1", { cols: 80, rows: 24 });
+    ws.__emitter.emit("message", Buffer.from("ls -la\r"), { binary: false });
+    assert.deepEqual(fakePty.__writes[0], "ls -la\r");
+  });
+
   it("sends an exit control message and closes on PTY exit", () => {
     const ws = makeFakeWs();
     let closed = false;
diff --git a/server/lib/pty-attach.js b/server/lib/pty-attach.js
index 96527d3..1d8fe34 100644
--- a/server/lib/pty-attach.js
+++ b/server/lib/pty-attach.js
@@ -71,19 +71,31 @@ function attach(ws, runId, { cols, rows }) {
     // callback arg (newer) or via `data.binary` on some transports — this
     // helper's own tests exercise the `{binary}` option shape used above.
     const binary = typeof isBinary === "boolean" ? isBinary : !!(isBinary && isBinary.binary);
+    const text = data.toString("utf8");
     if (binary) {
-      pty.write(data.toString("utf8"));
+      pty.write(text);
       return;
     }
-    let msg;
-    try {
-      msg = JSON.parse(data.toString("utf8"));
-    } catch {
-      return;
-    }
-    if (msg && msg.type === "resize" && Number.isFinite(msg.cols) && Number.isFinite(msg.rows)) {
-      pty.resize(msg.cols, msg.rows);
+    // The browser's WebSocket API sends a JS string as a text frame, and
+    // xterm.js's onData callback hands over plain strings — so every
+    // keystroke arrives here as text, not binary. Only a JSON control frame
+    // (matched by this same prefix check the client uses for output) is
+    // NOT keystroke input; everything else must reach the pty or typing
+    // does nothing.
+    if (text.startsWith('{"type"')) {
+      let msg;
+      try {
+        msg = JSON.parse(text);
+      } catch {
+        pty.write(text);
+        return;
+      }
+      if (msg && msg.type === "resize" && Number.isFinite(msg.cols) && Number.isFinite(msg.rows)) {
+        pty.resize(msg.cols, msg.rows);
+        return;
+      }
     }
+    pty.write(text);
   });
 
   ws.on("close", () => {
diff --git a/server/websocket.js b/server/websocket.js
index 6ca4423..b4cffd9 100644
--- a/server/websocket.js
+++ b/server/websocket.js
@@ -9,11 +9,15 @@ const { isHostAllowed, isWebSocketAuthorized } = require("./lib/security");
 let wss = null;
 
 function initWebSocket(server) {
-  // Express middleware doesn't run on WS upgrades, so enforce the same Host
-  // allowlist (anti DNS-rebinding) and optional token here (GHSA-gr74-4xfh-6jw9).
+  // `noServer: true` + a manual, path-checked `server.on("upgrade", ...)`
+  // rather than the `{server, path}` shorthand: that shorthand's own
+  // internal upgrade listener calls `handleUpgrade` for EVERY upgrade on the
+  // shared http.Server (path filtering happens inside `handleUpgrade`,
+  // which `abortHandshake`s with 400 on a mismatch) — so it was answering,
+  // and killing, `/ws-pty/*` upgrades before the PTY server's own listener
+  // (registered below by `initPtyWebSocket`) ever got a chance to run.
   wss = new WebSocketServer({
-    server,
-    path: "/ws",
+    noServer: true,
     maxPayload: 64 * 1024,
     verifyClient(info, done) {
       if (!isHostAllowed(info.req.headers.host)) return done(false, 403, "host not allowed");
@@ -22,6 +26,14 @@ function initWebSocket(server) {
     },
   });
 
+  server.on("upgrade", (req, socket, head) => {
+    const url = new URL(req.url, "http://localhost");
+    if (url.pathname !== "/ws") return; // not ours — `/ws-pty/*` handles its own path.
+    wss.handleUpgrade(req, socket, head, (ws) => {
+      wss.emit("connection", ws, req);
+    });
+  });
+
   wss.on("connection", (ws) => {
     ws.isAlive = true;
     ws.on("pong", () => {