fix(windows): stop a winpty pty id from matching a ConPTY job

winpty.cc and conpty.cc each mint their 'pty' id from an independent
counter, and windowsPtyAgent stores both in the same _pty field. So a
winpty-backed terminal's id can collide with a live ConPTY baton -- and
closing that pane would have terminated an unrelated pane's entire
process tree.

Both job entry points now take the shell pid and the native side refuses
unless GetProcessId(hShell) matches, which makes the id unforgeable.

Two more from the same read-through:
- ResumeThread's failure was ignored. A shell left suspended is a pane
  that never prints and never exits, which is far harder to diagnose
  than a failed spawn; it now cleans up and throws.
- handle->hJob was assigned before LoadConptyDll, which can throw. A
  baton carrying a job but never reaching SetupExitCallback has nothing
  left to close it, so the assignment moved down beside hShell.

config/patches/node-pty@1.1.0.patch

@@ -430,7 +430,7 @@ index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..383df0c9c48355547c65e6c9bbba593d
  }
  #endif
 diff --git a/src/win/conpty.cc b/src/win/conpty.cc
-index 7b286d3d644c26141df516929703aa6e129df4b2..a1a63d6380ba5b0a669339435064b9f51e0706dc 100644
+index 7b286d3d644c26141df516929703aa6e129df4b2..e62eadb7b8959a687a546da5d6c1a7b99fde43c5 100644
 --- a/src/win/conpty.cc
 +++ b/src/win/conpty.cc
 @@ -46,6 +46,11 @@ struct pty_baton {
@@ -471,7 +471,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..a1a63d6380ba5b0a669339435064b9f5
        envArg,                       // lpEnvironment
        mutableCwd.get(),             // lpCurrentDirectory
        &siEx.StartupInfo,            // lpStartupInfo
-@@ -426,6 +441,31 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
+@@ -426,6 +441,39 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
      throw errorWithCode(info, "Cannot create process");
    }
  
@@ -495,18 +495,50 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..a1a63d6380ba5b0a669339435064b9f5
 +      hJob = nullptr;
 +    }
 +  }
-+  handle->hJob = hJob;
-+
 +  // Safe to run now: either it is in the job, or we accepted that it is not.
-+  ResumeThread(piClient.hThread);
++  if (ResumeThread(piClient.hThread) == static_cast<DWORD>(-1)) {
++    // Why fatal: a shell left suspended produces a pane that never prints and
++    // never exits, which is far harder to diagnose than a failed spawn.
++    if (hJob != nullptr) {
++      CloseHandle(hJob);
++    }
++    TerminateProcess(piClient.hProcess, 1);
++    CloseHandle(piClient.hProcess);
++    CloseHandle(piClient.hThread);
++    throw errorWithCode(info, "Cannot resume process");
++  }
 +
    HANDLE hLibrary = LoadConptyDll(info, useConptyDll);
    bool fLoadedDll = hLibrary != nullptr;
    if (useConptyDll && fLoadedDll)
-@@ -567,6 +607,72 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) {
+@@ -440,6 +488,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
+ 
+   // Update handle
+   handle->hShell = piClient.hProcess;
++  // Why here and not earlier: LoadConptyDll above can throw, and a baton that
++  // carries a job but never reaches SetupExitCallback has nothing left to
++  // close it.
++  handle->hJob = hJob;
+ 
+   // Close the thread handle to avoid resource leak
+   CloseHandle(piClient.hThread);
+@@ -567,6 +619,85 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) {
    return env.Undefined();
  }
  
++/**
++ * Orca: confirm a baton really is the pty the caller means.
++ *
++ * The winpty backend mints its own `pty` ids from a separate counter, and the
++ * JS layer stores both in the same field -- so a winpty terminal's id can
++ * collide with a live ConPTY baton here and terminate an unrelated pane's whole
++ * process tree. Matching the shell pid makes the id unforgeable.
++ */
++static bool ownsShell(const pty_baton* handle, DWORD expectedShellPid) {
++  return handle != nullptr && handle->hJob != nullptr && handle->hShell != nullptr &&
++         expectedShellPid != 0 && GetProcessId(handle->hShell) == expectedShellPid;
++}
++
 +/**
 + * Orca: kill this pty's entire tree in one syscall.
 + *
@@ -518,12 +550,12 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..a1a63d6380ba5b0a669339435064b9f5
 +  Napi::Env env(info.Env());
 +  Napi::HandleScope scope(env);
 +
-+  if (info.Length() != 1 || !info[0].IsNumber()) {
-+    throw Napi::Error::New(env, "Usage: pty.terminateJob(id)");
++  if (info.Length() != 2 || !info[0].IsNumber() || !info[1].IsNumber()) {
++    throw Napi::Error::New(env, "Usage: pty.terminateJob(id, shellPid)");
 +  }
 +
 +  const pty_baton* handle = get_pty_baton(info[0].As<Napi::Number>().Int32Value());
-+  if (handle == nullptr || handle->hJob == nullptr) {
++  if (!ownsShell(handle, info[1].As<Napi::Number>().Uint32Value())) {
 +    return Napi::Boolean::New(env, false);
 +  }
 +  return Napi::Boolean::New(env, !!TerminateJobObject(handle->hJob, 1));
@@ -541,12 +573,12 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..a1a63d6380ba5b0a669339435064b9f5
 +  Napi::Env env(info.Env());
 +  Napi::HandleScope scope(env);
 +
-+  if (info.Length() != 1 || !info[0].IsNumber()) {
-+    throw Napi::Error::New(env, "Usage: pty.listJobProcessIds(id)");
++  if (info.Length() != 2 || !info[0].IsNumber() || !info[1].IsNumber()) {
++    throw Napi::Error::New(env, "Usage: pty.listJobProcessIds(id, shellPid)");
 +  }
 +
 +  const pty_baton* handle = get_pty_baton(info[0].As<Napi::Number>().Int32Value());
-+  if (handle == nullptr || handle->hJob == nullptr) {
++  if (!ownsShell(handle, info[1].As<Napi::Number>().Uint32Value())) {
 +    return env.Null();
 +  }
 +
@@ -576,7 +608,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..a1a63d6380ba5b0a669339435064b9f5
  /**
  * Init
  */
-@@ -577,6 +683,8 @@ Napi::Object init(Napi::Env env, Napi::Object exports) {
+@@ -577,6 +708,8 @@ Napi::Object init(Napi::Env env, Napi::Object exports) {
    exports.Set("resize", Napi::Function::New(env, PtyResize));
    exports.Set("clear", Napi::Function::New(env, PtyClear));
    exports.Set("kill", Napi::Function::New(env, PtyKill));

pnpm-lock.yaml

@@ -27,7 +27,7 @@ patchedDependencies:
     hash: 7333b3837f80a7fbd045964db6d76ba4fc118e49134bdbabb00585b6b7b60673
     path: config/patches/lint-staged@16.4.0.patch
   node-pty@1.1.0:
-    hash: 65d30811c4f422be2637e01f1ad25c385ddc6b59d5c90309e5656964dda192b0
+    hash: 76cf66a7ad9b69ebf9a717023ce41886aeaf1ee944d3fb3787cc06766899f9b6
     path: config/patches/node-pty@1.1.0.patch
 
 importers:
@@ -69,7 +69,7 @@ importers:
         version: 3.3.1
       node-pty:
         specifier: ^1.1.0
-        version: 1.1.0(patch_hash=65d30811c4f422be2637e01f1ad25c385ddc6b59d5c90309e5656964dda192b0)
+        version: 1.1.0(patch_hash=76cf66a7ad9b69ebf9a717023ce41886aeaf1ee944d3fb3787cc06766899f9b6)
       posthog-node:
         specifier: ^5.33.3
         version: 5.33.3
@@ -12046,7 +12046,7 @@ snapshots:
 
   node-int64@0.4.0: {}
 
-  node-pty@1.1.0(patch_hash=65d30811c4f422be2637e01f1ad25c385ddc6b59d5c90309e5656964dda192b0):
+  node-pty@1.1.0(patch_hash=76cf66a7ad9b69ebf9a717023ce41886aeaf1ee944d3fb3787cc06766899f9b6):
     dependencies:
       node-addon-api: 7.1.1
 

src/main/windows/windows-pty-job.test.ts

@@ -7,7 +7,7 @@ import {
   terminatePtyJob
 } from './windows-pty-job'
 
-const ptyWithHandle = (id: unknown): IPty => ({ _pty: id }) as unknown as IPty
+const ptyWithHandle = (id: unknown, pid = 4242): IPty => ({ _pty: id, pid }) as unknown as IPty
 
 afterEach(() => {
   __setConptyJobNativeForTests()
@@ -19,7 +19,7 @@ describe('terminatePtyJob', () => {
     __setConptyJobNativeForTests(() => ({ terminateJob, listJobProcessIds: vi.fn() }))
 
     expect(terminatePtyJob(ptyWithHandle(7))).toBe('terminated')
-    expect(terminateJob).toHaveBeenCalledWith(7)
+    expect(terminateJob).toHaveBeenCalledWith(7, 4242)
   })
 
   it.each([
@@ -47,6 +47,25 @@ describe('terminatePtyJob', () => {
     expect(terminatePtyJob(ptyWithHandle(id))).toBe('unavailable')
   })
 
+  it('passes the shell pid so an id from another backend cannot match', () => {
+    // winpty mints `pty` ids from its own counter and the JS layer stores both
+    // in the same field, so an id alone can name a live ConPTY pane. The native
+    // side refuses on a pid mismatch; this pins that we always send the pid.
+    const terminateJob = vi.fn().mockReturnValue(true)
+    __setConptyJobNativeForTests(() => ({ terminateJob, listJobProcessIds: vi.fn() }))
+
+    terminatePtyJob(ptyWithHandle(3, 9001))
+    expect(terminateJob).toHaveBeenCalledWith(3, 9001)
+  })
+
+  it('reports unavailable when the shell pid is unusable', () => {
+    __setConptyJobNativeForTests(() => ({
+      terminateJob: vi.fn().mockReturnValue(true),
+      listJobProcessIds: vi.fn()
+    }))
+    expect(terminatePtyJob(ptyWithHandle(3, 0))).toBe('unavailable')
+  })
+
   it('reports unavailable when this build has no job support', () => {
     // A node-pty rebuilt from unpatched sources exports neither symbol.
     __setConptyJobNativeForTests(() => null)

src/main/windows/windows-pty-job.ts

@@ -27,8 +27,8 @@ import { createRequire } from 'node:module'
 const requireFromMain = createRequire(__filename)
 
 type ConptyNative = {
-  terminateJob: (id: number) => boolean
-  listJobProcessIds: (id: number) => number[] | null
+  terminateJob: (id: number, shellPid: number) => boolean
+  listJobProcessIds: (id: number, shellPid: number) => number[] | null
 }
 
 let cachedNative: ConptyNative | null | undefined
@@ -60,14 +60,20 @@ function loadConptyNative(): ConptyNative | null {
 }
 
 /**
- * node-pty's per-terminal handle id.
+ * node-pty's per-terminal handle id, paired with the shell pid that proves it.
  *
- * Not on the public `IPty` surface, so read defensively: a winpty fallback
- * terminal and every POSIX terminal have none.
+ * Neither is on the public `IPty` surface, so read defensively. The pid is not
+ * belt-and-braces: the winpty backend mints `pty` ids from its own counter and
+ * the JS layer stores both in the same field, so an id alone can name a
+ * different, live ConPTY pane. The native side refuses on a pid mismatch.
  */
-function ptyHandleId(proc: IPty): number | null {
+function ptyJobTarget(proc: IPty): { id: number; shellPid: number } | null {
   const id = (proc as unknown as { _pty?: unknown })._pty
-  return typeof id === 'number' && Number.isInteger(id) ? id : null
+  const shellPid = proc.pid
+  if (!Number.isInteger(id) || !Number.isInteger(shellPid) || (shellPid as number) <= 0) {
+    return null
+  }
+  return { id: id as number, shellPid: shellPid as number }
 }
 
 export type JobTerminationOutcome = 'terminated' | 'unavailable'
@@ -81,13 +87,13 @@ export type JobTerminationOutcome = 'terminated' | 'unavailable'
  * to be misread as "nothing to kill".
  */
 export function terminatePtyJob(proc: IPty): JobTerminationOutcome {
-  const id = ptyHandleId(proc)
+  const target = ptyJobTarget(proc)
   const native = nativeLoader()
-  if (id === null || !native) {
+  if (!target || !native) {
     return 'unavailable'
   }
   try {
-    return native.terminateJob(id) ? 'terminated' : 'unavailable'
+    return native.terminateJob(target.id, target.shellPid) ? 'terminated' : 'unavailable'
   } catch {
     return 'unavailable'
   }
@@ -108,13 +114,13 @@ export function terminatePtyJob(proc: IPty): JobTerminationOutcome {
  * including children that detached from the console.
  */
 export function listPtyJobProcessIds(proc: IPty): readonly number[] | null {
-  const id = ptyHandleId(proc)
+  const target = ptyJobTarget(proc)
   const native = nativeLoader()
-  if (id === null || !native) {
+  if (!target || !native) {
     return null
   }
   try {
-    return native.listJobProcessIds(id)
+    return native.listJobProcessIds(target.id, target.shellPid)
   } catch {
     return null
   }