diff --git a/src/interfaces.ts b/src/interfaces.ts index 461b713ef..44b4c49b2 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -40,6 +40,12 @@ export interface ITerminal { */ clear(): void; + /** + * Sets the top-level window (HWND) that hosts this terminal. Returns whether the request was sent + * to ConPTY, which only happens on Windows with `useConptyDll`. + */ + setParentWindow(handle: Buffer): boolean; + /** * Close, kill and destroy the socket. */ diff --git a/src/native.d.ts b/src/native.d.ts index 03bfbebae..9bd646376 100644 --- a/src/native.d.ts +++ b/src/native.d.ts @@ -7,6 +7,7 @@ interface IConptyNative { connect(ptyId: number, commandLine: string, cwd: string, env: string[], useConptyDll: boolean, onExitCallback: (exitCode: number) => void): { pid: number }; resize(ptyId: number, cols: number, rows: number, useConptyDll: boolean): void; clear(ptyId: number, useConptyDll: boolean): void; + reparent(ptyId: number, hwnd: Buffer, useConptyDll: boolean): boolean; kill(ptyId: number, useConptyDll: boolean): void; } diff --git a/src/terminal.test.ts b/src/terminal.test.ts index c9e55177e..9d6fe704d 100644 --- a/src/terminal.test.ts +++ b/src/terminal.test.ts @@ -32,6 +32,9 @@ class TestTerminal extends Terminal { public clear(): void { throw new Error('Method not implemented.'); } + public setParentWindow(handle: Buffer): boolean { + throw new Error('Method not implemented.'); + } public destroy(): void { throw new Error('Method not implemented.'); } diff --git a/src/terminal.ts b/src/terminal.ts index 487e37565..7e268521a 100644 --- a/src/terminal.ts +++ b/src/terminal.ts @@ -180,6 +180,7 @@ export abstract class Terminal implements ITerminal { public abstract resize(cols: number, rows: number, pixelSize?: { width: number, height: number }): void; public abstract clear(): void; + public abstract setParentWindow(handle: Buffer): boolean; public abstract destroy(): void; public abstract kill(signal?: string): void; diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index 2776d501e..1670d8e29 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -283,6 +283,11 @@ export class UnixTerminal extends Terminal { } + public setParentWindow(handle: Buffer): boolean { + // Windows only + return false; + } + private _sanitizeEnv(env: IProcessEnv): void { // Make sure we didn't start our server from inside tmux. delete env['TMUX']; diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 931c5362d..7447f8f47 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -15,6 +15,7 @@ #include #include // PathCombine, PathIsRelative #include +#include #include #include #include @@ -40,6 +41,9 @@ typedef void (__stdcall *PFNRELEASEPSEUDOCONSOLE)(HPCON hpc); #endif +// Only exported by the bundled conpty.dll; kernel32 has no equivalent. +typedef HRESULT (__stdcall *PFNREPARENTPSEUDOCONSOLE)(HPCON hpc, HWND newParent); + struct pty_baton { int id; HANDLE hIn; @@ -495,7 +499,7 @@ static Napi::Value PtyResize(const Napi::CallbackInfo& info) { std::lock_guard lock(g_ptyHandlesMutex); const pty_baton* handle = get_pty_baton(lock, id); - if (handle != nullptr) { + if (handle != nullptr && handle->hpc != nullptr) { HANDLE hLibrary = LoadConptyDll(info, useConptyDll); bool fLoadedDll = hLibrary != nullptr; if (fLoadedDll) @@ -537,7 +541,7 @@ static Napi::Value PtyClear(const Napi::CallbackInfo& info) { std::lock_guard lock(g_ptyHandlesMutex); const pty_baton* handle = get_pty_baton(lock, id); - if (handle != nullptr) { + if (handle != nullptr && handle->hpc != nullptr) { HANDLE hLibrary = LoadConptyDll(info, useConptyDll); bool fLoadedDll = hLibrary != nullptr; if (fLoadedDll) @@ -553,6 +557,65 @@ static Napi::Value PtyClear(const Napi::CallbackInfo& info) { return env.Undefined(); } +// The handle is the bytes of an HWND in native byte order, which is how Electron's +// BrowserWindow.getNativeWindowHandle() returns it. +static bool TryGetWindowHandle(const Napi::Value& value, HWND* hwnd) { + if (!value.IsBuffer()) { + return false; + } + const auto buffer = value.As>(); + if (buffer.Length() != sizeof(HWND)) { + return false; + } + std::memcpy(hwnd, buffer.Data(), sizeof(*hwnd)); + return true; +} + +static Napi::Value PtyReparent(const Napi::CallbackInfo& info) { + Napi::Env env(info.Env()); + Napi::HandleScope scope(env); + + if (info.Length() != 3 || + !info[0].IsNumber() || + !info[2].IsBoolean()) { + throw Napi::Error::New(env, "Usage: pty.reparent(id, hwnd, useConptyDll)"); + } + + int id = info[0].As().Int32Value(); + HWND newParent; + if (!TryGetWindowHandle(info[1], &newParent)) { + throw Napi::Error::New(env, "pty.reparent: hwnd must be a Buffer the size of an HWND"); + } + const bool useConptyDll = info[2].As().Value(); + + // ConptyReparentPseudoConsole only exists in the bundled conpty.dll. + if (!useConptyDll) { + return Napi::Boolean::New(env, false); + } + + std::lock_guard lock(g_ptyHandlesMutex); + const pty_baton* handle = get_pty_baton(lock, id); + + bool sent = false; + if (handle != nullptr && handle->hpc != nullptr) { + HANDLE hLibrary = LoadConptyDll(info, useConptyDll); + bool fLoadedDll = hLibrary != nullptr; + if (fLoadedDll) + { + PFNREPARENTPSEUDOCONSOLE const pfnReparentPseudoConsole = (PFNREPARENTPSEUDOCONSOLE)GetProcAddress( + (HMODULE)hLibrary, "ConptyReparentPseudoConsole"); + if (pfnReparentPseudoConsole) + { + // Asks ConPTY to make newParent the owner of the console window returned by + // GetConsoleWindow(); ConPTY applies it asynchronously. + sent = SUCCEEDED(pfnReparentPseudoConsole(handle->hpc, newParent)); + } + } + } + + return Napi::Boolean::New(env, sent); +} + static Napi::Value PtyKill(const Napi::CallbackInfo& info) { Napi::Env env(info.Env()); Napi::HandleScope scope(env); @@ -577,9 +640,11 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) { PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress( (HMODULE)hLibrary, useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole"); - if (pfnClosePseudoConsole) + if (pfnClosePseudoConsole && handle->hpc != nullptr) { pfnClosePseudoConsole(handle->hpc); + // Closing frees the pseudoconsole, but the baton lives until the exit thread erases it. + handle->hpc = nullptr; } } // Defensive: if PtyConnect was never called (or failed before closing the @@ -609,6 +674,7 @@ Napi::Object init(Napi::Env env, Napi::Object exports) { exports.Set("connect", Napi::Function::New(env, PtyConnect)); exports.Set("resize", Napi::Function::New(env, PtyResize)); exports.Set("clear", Napi::Function::New(env, PtyClear)); + exports.Set("reparent", Napi::Function::New(env, PtyReparent)); exports.Set("kill", Napi::Function::New(env, PtyKill)); return exports; }; diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts index 73f83d6ec..5b4f366c0 100644 --- a/src/windowsPtyAgent.ts +++ b/src/windowsPtyAgent.ts @@ -193,6 +193,16 @@ export class WindowsPtyAgent { this._ptyNative.clear(this._pty, this._useConptyDll); } + /** + * Tells ConPTY which top-level window hosts the terminal (ConptyReparentPseudoConsole). The + * window becomes the owner of the console window returned by GetConsoleWindow(), so dialogs + * that console apps parent to it (e.g. WAM sign-in) stack above the host window. Returns + * whether the request was sent to ConPTY, which applies it asynchronously. + */ + public setParentWindow(handle: Buffer): boolean { + return this._ptyNative.reparent(this._pty, handle, this._useConptyDll); + } + public kill(): void { // Prevent deferred connection from completing after kill this._clearConnectionTimeout(); diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts index c76196512..5aa6c843a 100644 --- a/src/windowsTerminal.test.ts +++ b/src/windowsTerminal.test.ts @@ -6,6 +6,7 @@ import * as fs from 'fs'; import * as assert from 'assert'; import { WindowsTerminal } from './windowsTerminal'; +import { IWindowsPtyForkOptions } from './interfaces'; import * as path from 'path'; import * as psList from 'ps-list'; @@ -283,14 +284,134 @@ if (process.platform === 'win32') { }); }); + describe('parent window', () => { + const user32 = `Add-Type -Namespace NodePtyTest -Name W -MemberDefinition '[DllImport("kernel32.dll")] public static extern IntPtr GetConsoleWindow(); [DllImport("user32.dll")] public static extern IntPtr GetAncestor(IntPtr h, uint f); public static ulong U(IntPtr h) { return unchecked((ulong)h.ToInt64()); }'`; + + function scriptArgs(...lines: string[]): string { + return `-NoProfile -NonInteractive -EncodedCommand ${Buffer.from([user32, ...lines].join('; '), 'utf16le').toString('base64')}`; + } + + // Prints the console window and GetAncestor(GetConsoleWindow(), GA_ROOTOWNER), which is the + // parent MSAL/WAM uses for sign-in dialogs. Handles are pointers, so print them unsigned. + function rootOwnerArgs(delayMs: number, keepAliveSeconds: number = 0): string { + return scriptArgs( + `Start-Sleep -Milliseconds ${delayMs}`, + `$c = [NodePtyTest.W]::GetConsoleWindow()`, + `Write-Host ('console=' + [NodePtyTest.W]::U($c) + ' rootOwner=' + [NodePtyTest.W]::U([NodePtyTest.W]::GetAncestor($c, 3)) + ' end')`, + `Start-Sleep -Seconds ${keepAliveSeconds}` + ); + } + + const started: Array<{ exited: Promise, kill: () => void }> = []; + + function startPty(file: string, args: string | string[], options: IWindowsPtyForkOptions = {}): WindowsTerminal { + const term = new WindowsTerminal(file, args, { useConptyDll, ...options }); + let hasExited = false; + const exited = new Promise(resolve => term.once('exit', () => { + hasExited = true; + resolve(); + })); + started.push({ + exited, + kill: () => { + if (!hasExited) { + term.kill(); + } + } + }); + return term; + } + + // Kill and wait for every pty, even after a failed test: ptys left running can keep mocha + // from exiting. + afterEach(async function (): Promise { + this.timeout(30000); + const ptys = started.splice(0); + ptys.forEach(pty => pty.kill()); + await Promise.all(ptys.map(pty => pty.exited)); + }); + + function readOutput(term: WindowsTerminal, pattern: RegExp): Promise { + return new Promise(resolve => { + let output = ''; + term.on('data', (data) => { + output += data; + const match = pattern.exec(output); + if (match) { + resolve(match); + } + }); + }); + } + + async function readWindows(term: WindowsTerminal): Promise<{ console: bigint, rootOwner: bigint }> { + const match = await readOutput(term, /console=(\d+) rootOwner=(\d+) end/); + return { console: BigInt(match[1]), rootOwner: BigInt(match[2]) }; + } + + // Another pty's hidden top-level console window stands in for the host app's window. + async function startHostWindow(): Promise { + return (await readWindows(startPty('powershell.exe', rootOwnerArgs(0, 60)))).console; + } + + // An HWND the way setParentWindow takes it, like Electron's getNativeWindowHandle(): its bytes + // in native byte order. The tests run in 64-bit processes. + function toHandle(hwnd: bigint): Buffer { + const handle = Buffer.alloc(8); + handle.writeBigUInt64LE(hwnd); + return handle; + } + + it('should make the window the owner of the console window when set right after spawn', async function (): Promise { + this.timeout(60000); + const hostWindow = await startHostWindow(); + const term = startPty('powershell.exe', rootOwnerArgs(0)); + // Not deferred, and the shell is only launched later, so this is sent before the shell starts. + assert.strictEqual(term.setParentWindow(toHandle(hostWindow)), useConptyDll); + const windows = await readWindows(term); + assert.strictEqual(windows.rootOwner, useConptyDll ? hostWindow : windows.console); + }); + + it('should change the owner when setParentWindow is called while the shell runs', async function (): Promise { + this.timeout(60000); + const hostWindow = await startHostWindow(); + const term = startPty('powershell.exe', rootOwnerArgs(3000)); + let sent: boolean | undefined; + // The first data event means the shell is running and conpty has been released. + term.once('data', () => sent = term.setParentWindow(toHandle(hostWindow))); + const windows = await readWindows(term); + assert.strictEqual(sent, useConptyDll); + assert.strictEqual(windows.rootOwner, useConptyDll ? hostWindow : windows.console); + }); + + it('should not send setParentWindow once the pseudoconsole is closed', async function (): Promise { + this.timeout(20000); + const term = startPty('cmd.exe', []); + const exited = new Promise(resolve => term.once('exit', () => resolve())); + await new Promise(resolve => term.once('data', () => resolve())); + term.kill(); + assert.strictEqual(term.setParentWindow(Buffer.alloc(8)), false); + await exited; + assert.strictEqual(term.setParentWindow(Buffer.alloc(8)), false); + }); + + it('should reject handles that are not buffers the size of an HWND', () => { + const invalid: unknown[] = [Buffer.alloc(4), Buffer.alloc(9), Buffer.alloc(0), 1, BigInt(1), '0x1234', undefined]; + const term = startPty('cmd.exe', []); + for (const handle of invalid) { + assert.throws(() => term.setParentWindow(handle as Buffer), /hwnd must be a Buffer the size of an HWND/); + } + }); + }); describe('Regression for #921', () => { - it('should not crash with concurrent kills while resizing/clearing', function (done) { + it('should not crash with concurrent kills while resizing/clearing/reparenting', function (done) { this.timeout(60000); const N = 30; const terms: WindowsTerminal[] = []; let ready = 0; let exited = 0; let spamInterval: NodeJS.Timeout | undefined; + const noWindow = Buffer.alloc(8); const cleanup = (err?: Error): void => { if (spamInterval) { clearInterval(spamInterval); @@ -307,6 +428,9 @@ if (process.platform === 'win32') { try { t.clear(); } catch (e) { /* already exited */ } + try { + t.setParentWindow(noWindow); + } catch (e) { /* already exited */ } } }, 1); for (const t of terms) { diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts index 3f88f4dee..c01be4221 100644 --- a/src/windowsTerminal.ts +++ b/src/windowsTerminal.ts @@ -163,6 +163,12 @@ export class WindowsTerminal extends Terminal { }); } + public setParentWindow(handle: Buffer): boolean { + // Not deferred: the pseudoconsole exists as soon as the agent is created, and the owner + // should be requested before the shell starts asking for it. + return this._agent.setParentWindow(handle); + } + public destroy(): void { this._deferNoArgs(() => { this.kill(); diff --git a/typings/node-pty.d.ts b/typings/node-pty.d.ts index a6fc5dd0b..f46c02968 100644 --- a/typings/node-pty.d.ts +++ b/typings/node-pty.d.ts @@ -173,6 +173,23 @@ declare module 'node-pty' { */ clear(): void; + /** + * Sets the handle (HWND) of the top-level window that hosts this terminal. ConPTY makes it the + * owner of the console window returned by `GetConsoleWindow()`, so windows that console + * applications parent to it (for example `GetAncestor(GetConsoleWindow(), GA_ROOTOWNER)`, which + * MSAL/WAM sign-in uses) open on top of the host window instead of behind it. Calling it right + * after spawning sends the request before the process is launched. Call it again whenever the + * terminal moves to a different window. Optional so existing implementations of `IPty` keep + * compiling. + * @param handle The window handle (HWND) as its bytes in native byte order, the way Electron's + * `BrowserWindow.getNativeWindowHandle()` returns it. Throws unless it's a Buffer the size of + * an HWND (8 bytes in a 64-bit process). + * @returns Whether the request was sent to ConPTY, which applies it asynchronously. This is + * always false unless on Windows with `useConptyDll`, and false once the pseudoconsole is + * closed. + */ + setParentWindow?(handle: Buffer): boolean; + /** * Writes data to the pty. * @param data The data to write.