fix(ssh): drop the per-session cumulative request counters

Both loops counted every request a session had ever received and aborted
the channel past 4096. That was the right shape while the queue behind
them was unbounded; now that Channel bounds the backlog at its source, a
lifetime ceiling guards nothing and costs something. window-change is
counted, and a client sends one per terminal resize, so a long-lived
interactive shell could be killed for behaving normally.

Removing them leaves the peer bounded by the backlog cap only if the loop
is uniformly serial, so windowChange and signal are now awaited. They were
typed `=> void` and called bare, and void-return assignability let an
async handler through to run unawaited.

Removes SessionLimits.maxRequests, which is a breaking change.

Closes #231.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

packages/ssh/src/session.test.ts

@@ -693,17 +693,51 @@ suite("ssh session rejection and failure", () => {
     }
   })
 
-  test("a flood of channel requests aborts only that session", async () => {
-    const { client, close } = await connected({ exec: echoHandler }, { maxRequests: 8 })
+  test("a session serviced past the old lifetime ceiling keeps working", async () => {
+    let seen = 0
+    const { client, close } = await connected({
+      exec: echoHandler,
+      signal: () => {
+        seen++
+      },
+    })
     try {
-      const channel = await client.openSession()
-      for (let i = 0; i < 8; i++) await channel.request("signal", new Uint8Array(4), false)
-      await assert.rejects(async () => {
-        for (let i = 0; i < 8; i++) await channel.request("signal", new Uint8Array(4), false)
-        assert.equal(await channel.read(), null)
-        throw new Error("channel was not aborted")
-      })
-      const session = await client.exec("still-here")
+      const session = await client.exec("echo")
+      // The removed counter aborted the channel at 4096 requests for its whole
+      // life. A long-lived interactive session reaches that by resizing.
+      for (let i = 0; i < 4999; i++) {
+        await session.channel.request("signal", encodeStrings("WINCH"), false)
+      }
+      // The loop handles requests in order, so a round trip on the last one
+      // proves every request before it has already been serviced.
+      await session.channel.request("signal", encodeStrings("WINCH"), true).catch(() => {})
+      assert.equal(seen, 5000)
+      await session.closeSend()
+      await drain(() => session.read())
+      assert.deepEqual(await session.wait(), { type: "exit", code: 0 })
+    } finally {
+      await close()
+    }
+  })
+
+  test("an async signal handler is awaited rather than left running", async () => {
+    let running = 0
+    let peak = 0
+    const { client, close } = await connected({
+      exec: echoHandler,
+      signal: async () => {
+        peak = Math.max(peak, ++running)
+        await new Promise((r) => setTimeout(r, 1))
+        running--
+      },
+    })
+    try {
+      const session = await client.exec("echo")
+      for (let i = 0; i < 20; i++) {
+        await session.channel.request("signal", encodeStrings("WINCH"), false)
+      }
+      await delay(100)
+      assert.equal(peak, 1, "handlers should not overlap")
       await session.closeSend()
       await drain(() => session.read())
       assert.deepEqual(await session.wait(), { type: "exit", code: 0 })

packages/ssh/src/session.ts

@@ -3,7 +3,6 @@ import type { Channel } from "./channel.js"
 
 const MAX_PTY_CHARS = 10000
 const MAX_PTY_PIXELS = 1 << 20
-const MAX_PEER_REQUESTS = 4096
 
 export const PTY_MODE = {
   TTY_OP_END: 0,
@@ -141,7 +140,6 @@ export class SSHSession {
   }
 
   async #requestLoop(): Promise<void> {
-    let requests = 0
     for (;;) {
       let req
       try {
@@ -150,11 +148,6 @@ export class SSHSession {
         this.#finish()
         return
       }
-      if (++requests > MAX_PEER_REQUESTS) {
-        await this.#channel.abort(new Error("too many channel requests"))
-        this.#finish()
-        return
-      }
       // Nothing awaits this loop, so a malformed payload from the peer must
       // never escape it as an unhandled rejection.
       try {
@@ -316,8 +309,8 @@ export type SessionHandlers = {
   env?: (name: string, value: string) => boolean | Promise<boolean>
   pty?: (pty: PtyRequest) => boolean | Promise<boolean>
   authorize?: (start: SessionStart) => boolean | Promise<boolean>
-  windowChange?: (size: WindowSize, session: ServerSession) => void
-  signal?: (name: string, session: ServerSession) => void
+  windowChange?: (size: WindowSize, session: ServerSession) => void | Promise<void>
+  signal?: (name: string, session: ServerSession) => void | Promise<void>
   exec?: (command: string, session: ServerSession) => void | Promise<void>
   shell?: (session: ServerSession) => void | Promise<void>
   subsystem?: (name: string, session: ServerSession) => void | Promise<void>
@@ -327,7 +320,6 @@ export type SessionHandlers = {
 export type SessionLimits = {
   maxEnv?: number
   maxEnvBytes?: number
-  maxRequests?: number
 }
 
 function startHandler(
@@ -345,7 +337,6 @@ function startHandler(
 const DEFAULT_LIMITS: Required<SessionLimits> = {
   maxEnv: 32,
   maxEnvBytes: 8192,
-  maxRequests: 4096,
 }
 
 class ServerSessionState implements ServerSession {
@@ -401,10 +392,9 @@ export async function serveSession(
   handlers: SessionHandlers,
   limits: SessionLimits = {},
 ): Promise<void> {
-  const { maxEnv, maxEnvBytes, maxRequests } = { ...DEFAULT_LIMITS, ...limits }
+  const { maxEnv, maxEnvBytes } = { ...DEFAULT_LIMITS, ...limits }
   const state = new ServerSessionState(channel)
   let running: Promise<void> | null = null
-  let requests = 0
   let markStarted = (): void => {}
   const started = new Promise<void>((resolve) => (markStarted = resolve))
 
@@ -426,10 +416,6 @@ export async function serveSession(
       } catch {
         return
       }
-      if (++requests > maxRequests) {
-        await channel.abort(new Error("too many channel requests"))
-        return
-      }
       // Callers run this loop fire-and-forget alongside the command, so a
       // malformed payload must fail only this request, never escape as an
       // unhandled rejection that takes the whole process down.
@@ -464,7 +450,7 @@ export async function serveSession(
             const size = decodeWindowSize(new Reader(req.payload))
             if (state.pty) {
               state.pty = { ...state.pty, ...size }
-              handlers.windowChange?.(size, state)
+              await handlers.windowChange?.(size, state)
             }
             // RFC 4254 §6.7 never wants a reply here, but answer a non-conforming
             // peer rather than leaving it waiting.
@@ -472,7 +458,7 @@ export async function serveSession(
             break
           }
           case "signal": {
-            if (running) handlers.signal?.(new Reader(req.payload).utf8(), state)
+            if (running) await handlers.signal?.(new Reader(req.payload).utf8(), state)
             if (req.wantReply) await channel.reply(running !== null)
             break
           }