diff --git a/binding.gyp b/binding.gyp index 5f63978b07ab50aaf7523219a2170ec737a6b5db..bbd9e06136e8922f40b5779e35d4fc835f1479ab 100644 --- a/binding.gyp +++ b/binding.gyp @@ -1,13 +1,10 @@ { 'target_defaults': { 'dependencies': [ - " #include #include +#include #include +#include #include #include @@ -47,6 +49,25 @@ #include #endif +/* Orca: glibc 2.32-2.34 relocated pthread_sigmask/openpty/forkpty into libc + * under new symbol versions, so building on a newer glibc produces references + * (GLIBC_2.32/2.34) absent on Ubuntu 20.04 (glibc 2.31) and the app fails to + * launch. Pin these to the pre-merge version glibc still ships as a compat + * alias; the binding.gyp ldflags force libutil/libpthread into DT_NEEDED so + * those aliases are actually loaded on the target. */ +#if defined(__linux__) +# if defined(__x86_64__) +# define ORCA_GLIBC_COMPAT_VERSION "GLIBC_2.2.5" +# elif defined(__aarch64__) +# define ORCA_GLIBC_COMPAT_VERSION "GLIBC_2.17" +# endif +# ifdef ORCA_GLIBC_COMPAT_VERSION +__asm__(".symver openpty,openpty@" ORCA_GLIBC_COMPAT_VERSION); +__asm__(".symver forkpty,forkpty@" ORCA_GLIBC_COMPAT_VERSION); +__asm__(".symver pthread_sigmask,pthread_sigmask@" ORCA_GLIBC_COMPAT_VERSION); +# endif +#endif + /* Some platforms name VWERASE and VDISCARD differently */ #if !defined(VWERASE) && defined(VWERSE) #define VWERASE VWERSE @@ -237,13 +258,23 @@ pty_getproc(int, char *); #endif #if defined(__APPLE__) || defined(__OpenBSD__) +struct pty_spawn_error { + const char* step; + int errnum; + std::string detail_name; + std::string detail_value; +}; + +static std::string +pty_format_spawn_error(const pty_spawn_error&); + static void pty_posix_spawn(char** argv, char** env, const struct termios *termp, const struct winsize *winp, int* master, pid_t* pid, - int* err); + pty_spawn_error* err); #endif struct DelBuf { @@ -367,10 +398,11 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) { argv[i + 3] = strdup(arg.c_str()); } - int err = -1; - pty_posix_spawn(argv, env, term, &winp, &master, &pid, &err); - if (err != 0) { - throw Napi::Error::New(napiEnv, "posix_spawnp failed."); + pty_spawn_error spawn_error = { NULL, 0, "", "" }; + pty_posix_spawn(argv, env, term, &winp, &master, &pid, &spawn_error); + if (spawn_error.errnum != 0) { + std::string spawn_message = pty_format_spawn_error(spawn_error); + throw Napi::Error::New(napiEnv, spawn_message); } if (pty_nonblock(master) == -1) { throw Napi::Error::New(napiEnv, "Could not set master fd to nonblocking."); @@ -684,15 +716,73 @@ pty_getproc(int fd, char *tty) { #endif #if defined(__APPLE__) +static const char* +pty_errno_name(int errnum) { + switch (errnum) { + case E2BIG: return "E2BIG"; + case EACCES: return "EACCES"; + case EAGAIN: return "EAGAIN"; + case EMFILE: return "EMFILE"; + case ENFILE: return "ENFILE"; + case ENOENT: return "ENOENT"; + case ENOMEM: return "ENOMEM"; + default: return "errno"; + } +} + +static void +pty_set_spawn_error(pty_spawn_error* err, + const char* step, + int errnum, + const char* detail_name = NULL, + const char* detail_value = NULL) { + err->step = step; + err->errnum = errnum; + err->detail_name = detail_name ? detail_name : ""; + err->detail_value = detail_value ? detail_value : ""; +} + +static std::string +pty_format_spawn_error(const pty_spawn_error& err) { + char errno_buf[64]; + snprintf(errno_buf, sizeof(errno_buf), "%d", err.errnum); + + std::string message = "node-pty: "; + message += err.step ? err.step : "unknown"; + message += " failed: "; + message += pty_errno_name(err.errnum); + message += " (errno "; + message += errno_buf; + message += ", "; + message += strerror(err.errnum); + message += ")"; + + if (!err.detail_name.empty()) { + message += " - "; + message += err.detail_name; + message += "='"; + message += err.detail_value; + message += "'"; + } + + return message; +} + static void pty_posix_spawn(char** argv, char** env, const struct termios *termp, const struct winsize *winp, int* master, pid_t* pid, - int* err) { - int low_fds[3]; + pty_spawn_error* err) { + int low_fds[3] = {-1, -1, -1}; size_t count = 0; + int res = -1; + int slave = -1; + posix_spawn_file_actions_t acts; + bool acts_initialized = false; + posix_spawnattr_t attrs; + bool attrs_initialized = false; for (; count < 3; count++) { low_fds[count] = posix_openpt(O_RDWR); @@ -706,80 +796,118 @@ pty_posix_spawn(char** argv, char** env, POSIX_SPAWN_SETSID; *master = posix_openpt(O_RDWR); if (*master == -1) { - return; + pty_set_spawn_error(err, "posix_openpt", errno); + goto done; } - int res = grantpt(*master) || unlockpt(*master); + res = grantpt(*master); if (res == -1) { - return; + pty_set_spawn_error(err, "grantpt", errno); + goto done; + } + + res = unlockpt(*master); + if (res == -1) { + pty_set_spawn_error(err, "unlockpt", errno); + goto done; } // Use TIOCPTYGNAME instead of ptsname() to avoid threading problems. - int slave; char slave_pty_name[128]; res = ioctl(*master, TIOCPTYGNAME, slave_pty_name); if (res == -1) { - return; + pty_set_spawn_error(err, "ioctl_TIOCPTYGNAME", errno); + goto done; } slave = open(slave_pty_name, O_RDWR | O_NOCTTY); if (slave == -1) { - return; + pty_set_spawn_error(err, "open_slave", errno, "slave", slave_pty_name); + goto done; } if (termp) { res = tcsetattr(slave, TCSANOW, termp); if (res == -1) { - return; + pty_set_spawn_error(err, "tcsetattr", errno, "slave", slave_pty_name); + goto done; }; } if (winp) { res = ioctl(slave, TIOCSWINSZ, winp); if (res == -1) { - return; + pty_set_spawn_error(err, "ioctl_TIOCSWINSZ", errno, "slave", slave_pty_name); + goto done; } } - posix_spawn_file_actions_t acts; - posix_spawn_file_actions_init(&acts); + res = posix_spawn_file_actions_init(&acts); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawn_file_actions_init", res); + goto done; + } + acts_initialized = true; posix_spawn_file_actions_adddup2(&acts, slave, STDIN_FILENO); posix_spawn_file_actions_adddup2(&acts, slave, STDOUT_FILENO); posix_spawn_file_actions_adddup2(&acts, slave, STDERR_FILENO); posix_spawn_file_actions_addclose(&acts, slave); posix_spawn_file_actions_addclose(&acts, *master); - posix_spawnattr_t attrs; - posix_spawnattr_init(&attrs); - *err = posix_spawnattr_setflags(&attrs, flags); - if (*err != 0) { + res = posix_spawnattr_init(&attrs); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_init", res); + goto done; + } + attrs_initialized = true; + res = posix_spawnattr_setflags(&attrs, flags); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_setflags", res); goto done; } sigset_t signal_set; /* Reset all signal the child to their default behavior */ sigfillset(&signal_set); - *err = posix_spawnattr_setsigdefault(&attrs, &signal_set); - if (*err != 0) { + res = posix_spawnattr_setsigdefault(&attrs, &signal_set); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_setsigdefault", res); goto done; } /* Reset the signal mask for all signals */ sigemptyset(&signal_set); - *err = posix_spawnattr_setsigmask(&attrs, &signal_set); - if (*err != 0) { + res = posix_spawnattr_setsigmask(&attrs, &signal_set); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawnattr_setsigmask", res); goto done; } do - *err = posix_spawn(pid, argv[0], &acts, &attrs, argv, env); - while (*err == EINTR); + res = posix_spawn(pid, argv[0], &acts, &attrs, argv, env); + while (res == EINTR); + if (res != 0) { + pty_set_spawn_error(err, "posix_spawn", res, "helper", argv[0]); + } done: - posix_spawn_file_actions_destroy(&acts); - posix_spawnattr_destroy(&attrs); + if (acts_initialized) { + posix_spawn_file_actions_destroy(&acts); + } + if (attrs_initialized) { + posix_spawnattr_destroy(&attrs); + } + if (slave != -1) { + close(slave); + } + if (err->errnum != 0 && *master != -1) { + close(*master); + *master = -1; + } - for (; count > 0; count--) { - close(low_fds[count]); + for (size_t i = 0; i <= count && i < 3; i++) { + if (low_fds[i] != -1) { + close(low_fds[i]); + } } } #endif diff --git a/src/win/conpty.cc b/src/win/conpty.cc index 7b286d3d644c26141df516929703aa6e129df4b2..ec6bf3932c65b89c013ff133dc6bf46a6a4082ce 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -44,12 +45,29 @@ struct pty_baton { HANDLE hOut; HPCON hpc; - HANDLE hShell; + HANDLE hShell = nullptr; + // Orca: the shell's pid, captured at spawn. The ownership guard compares + // against this rather than calling GetProcessId(hShell), because the exit + // watcher closes hShell on another thread -- reading it there is an + // invalid-handle operation, and under strict handle checks that is fatal. + DWORD shellPid = 0; + + // Orca: job object owning this pty's whole process tree. Null when the OS + // refused to create or assign one (an outer job without breakaway rights), + // in which case callers fall back to their pre-job behaviour. + HANDLE hJob = nullptr; pty_baton(int _id, HANDLE _hIn, HANDLE _hOut, HPCON _hpc) : id(_id), hIn(_hIn), hOut(_hOut), hpc(_hpc) {}; }; static std::vector> ptyHandles; +// Orca: guards the job accessors below against the exit watcher thread. It does +// NOT make the whole table safe -- PtyResize/PtyClear/PtyKill read it unlocked, +// as they always have -- but it closes the window this patch opened, where the +// watcher can close hShell/hJob and free the baton between a lookup and its use. +// Handle VALUES are recycled aggressively, so an unguarded read could pass the +// shell-pid check against an unrelated process and terminate the wrong job. +static std::mutex ptyJobMutex; static volatile LONG ptyCounter; static pty_baton* get_pty_baton(int id) { @@ -102,8 +120,27 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) { // Get process exit code. GetExitCodeProcess(baton->hShell, (LPDWORD)(&exit_event->exit_code)); // Clean up handles - CloseHandle(baton->hShell); - assert(remove_pty_baton(baton->id)); + // Orca: release the job once the shell is gone. Without kill-on-close this + // only frees the handle -- anything the user backgrounded is orphaned, as + // it was before this patch. + { + std::lock_guard guard(ptyJobMutex); + CloseHandle(baton->hShell); + baton->hShell = nullptr; + if (baton->hJob != nullptr) { + CloseHandle(baton->hJob); + baton->hJob = nullptr; + } + // Why inside the lock: erasing frees the baton the job accessors hold a + // pointer to. Note remove_pty_baton must not be an assert() argument -- + // NDEBUG would compile the call away and leak every baton. + const bool removed = remove_pty_baton(baton->id); + assert(removed); + (void)removed; + } + // Why the lock ends here: BlockingCall below waits on the JS thread, and the + // JS thread can be waiting on ptyJobMutex inside PtyTerminateJob. Holding + // the lock across it deadlocks. Do not widen this scope. auto status = tsfn.BlockingCall(exit_event, callback); // In main thread switch (status) { @@ -409,6 +446,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { throw errorWithCode(info, "UpdateProcThreadAttribute failed"); } + // Orca: resolve the DLL BEFORE creating anything. It throws when conpty.dll + // is missing -- a real state, and one this branch hit during development -- + // and every throw between CreateProcessW and SetupExitCallback leaks the job, + // process and thread handles AND leaves an untracked shell tree running, + // once per attempt. Validating first means the only throw after creation is + // the resume failure, which cleans up after itself. + HANDLE hLibrary = LoadConptyDll(info, useConptyDll); + bool fLoadedDll = hLibrary != nullptr; + PROCESS_INFORMATION piClient{}; fSuccess = !!CreateProcessW( nullptr, @@ -416,7 +462,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { nullptr, // lpProcessAttributes nullptr, // lpThreadAttributes false, // bInheritHandles VERY IMPORTANT that this is false - EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, // dwCreationFlags + // Orca: CREATE_SUSPENDED so the shell is inside its job before it can + // spawn anything. Assigning after the fact leaves a window in which a + // fast child escapes the job and outlives the pane. + EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT | CREATE_SUSPENDED, // dwCreationFlags envArg, // lpEnvironment mutableCwd.get(), // lpCurrentDirectory &siEx.StartupInfo, // lpStartupInfo @@ -426,8 +475,47 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { throw errorWithCode(info, "Cannot create process"); } - HANDLE hLibrary = LoadConptyDll(info, useConptyDll); - bool fLoadedDll = hLibrary != nullptr; + // Orca: own the tree with a handle instead of inferring it later from a + // parent-pid walk. A pid walk cannot survive pid reuse and cannot see a + // descendant that reparented, which is why detached agent children outlived + // their pane and held the worktree directory open. + // + // Deliberately WITHOUT JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE. Measured on + // Windows 11: with that flag, closing the handle when the shell exits also + // kills whatever the user left running, so typing `exit` in a pane reaped a + // `start /b` server that used to survive. This job exists to make an + // EXPLICIT teardown exact, not to redefine what a clean exit means. + HANDLE hJob = CreateJobObjectW(nullptr, nullptr); + if (hJob != nullptr) { + // Why BREAKAWAY_OK and not a bare job: with no limits set, a child asking + // for CREATE_BREAKAWAY_FROM_JOB is refused with ERROR_ACCESS_DENIED. + // Installers, msiexec and some updater and service-control paths spawn that + // way deliberately, so a bare job breaks them ONLY inside an Orca terminal. + // With this flag a child has to ask, so ordinary descendants stay owned. + JOBOBJECT_EXTENDED_LIMIT_INFORMATION jobLimits{}; + jobLimits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_BREAKAWAY_OK; + if (!SetInformationJobObject(hJob, JobObjectExtendedLimitInformation, &jobLimits, sizeof(jobLimits)) || + !AssignProcessToJobObject(hJob, piClient.hProcess)) { + // Why tolerate failure: an outer job without JOB_OBJECT_LIMIT_BREAKAWAY_OK + // (some EDR and container hosts) refuses the assignment. The pty must + // still start; ownership just degrades to the older best-effort path. + CloseHandle(hJob); + hJob = nullptr; + } + } + // Safe to run now: either it is in the job, or we accepted that it is not. + if (ResumeThread(piClient.hThread) == static_cast(-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"); + } + if (useConptyDll && fLoadedDll) { PFNRELEASEPSEUDOCONSOLE const pfnReleasePseudoConsole = (PFNRELEASEPSEUDOCONSOLE)GetProcAddress( @@ -440,6 +528,8 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { // Update handle handle->hShell = piClient.hProcess; + handle->shellPid = piClient.dwProcessId; + handle->hJob = hJob; // Close the thread handle to avoid resource leak CloseHandle(piClient.hThread); @@ -567,6 +657,143 @@ 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 && expectedShellPid != 0 && + handle->shellPid == expectedShellPid; +} + +/** + * Orca: kill this pty's entire tree in one syscall. + * + * Replaces "scrape the process table, walk parent pids, hope none were + * recycled, then taskkill /T /F". Returns false when no job was assigned so + * the caller knows to fall back rather than assume the tree is gone. + */ +static Napi::Value PtyTerminateJob(const Napi::CallbackInfo& info) { + Napi::Env env(info.Env()); + Napi::HandleScope scope(env); + + if (info.Length() != 2 || !info[0].IsNumber() || !info[1].IsNumber()) { + throw Napi::Error::New(env, "Usage: pty.terminateJob(id, shellPid)"); + } + + // Held across the lookup AND the Win32 call: the watcher thread can otherwise + // close these handles and free the baton in between. + std::lock_guard guard(ptyJobMutex); + const pty_baton* handle = get_pty_baton(info[0].As().Int32Value()); + if (!ownsShell(handle, info[1].As().Uint32Value())) { + return Napi::Boolean::New(env, false); + } + return Napi::Boolean::New(env, !!TerminateJobObject(handle->hJob, 1)); +} + +/** + * Orca: the pids still alive in this pty's tree, straight from the kernel. + * + * Descendant liveness for a tree that is still tracked, including children that + * detached from the console. Once the shell exits the baton is gone, so this + * returns null rather than an empty list -- null means "no answer", never + * "they died". Also returns null when no job was assigned. + * + * Does not include the ConPTY console host: CreatePseudoConsole spawns it + * before this job exists, so it is not a member and ClosePseudoConsole is what + * reaps it. + */ +static Napi::Value PtyListJobProcessIds(const Napi::CallbackInfo& info) { + Napi::Env env(info.Env()); + Napi::HandleScope scope(env); + + if (info.Length() != 2 || !info[0].IsNumber() || !info[1].IsNumber()) { + throw Napi::Error::New(env, "Usage: pty.listJobProcessIds(id, shellPid)"); + } + + // Held across the lookup AND the Win32 call: the watcher thread can otherwise + // close these handles and free the baton in between. + std::lock_guard guard(ptyJobMutex); + const pty_baton* handle = get_pty_baton(info[0].As().Int32Value()); + if (!ownsShell(handle, info[1].As().Uint32Value())) { + return env.Null(); + } + + // Grow until the buffer holds every pid: the count can change between calls, + // and a truncated list would read as "these children are gone". + DWORD capacity = 64; + for (int attempt = 0; attempt < 8; attempt++) { + const size_t bytes = sizeof(JOBOBJECT_BASIC_PROCESS_ID_LIST) + sizeof(ULONG_PTR) * capacity; + std::vector buffer(bytes, 0); + auto* list = reinterpret_cast(buffer.data()); + if (QueryInformationJobObject(handle->hJob, JobObjectBasicProcessIdList, list, static_cast(bytes), nullptr)) { + auto pids = Napi::Array::New(env, list->NumberOfProcessIdsInList); + for (DWORD i = 0; i < list->NumberOfProcessIdsInList; i++) { + pids.Set(i, Napi::Number::New(env, static_cast(list->ProcessIdList[i]))); + } + return pids; + } + if (GetLastError() != ERROR_MORE_DATA) { + return env.Null(); + } + capacity *= 4; + } + return env.Null(); +} + +/** + * Orca: put THIS process in a kill-on-close job, so its whole descendant tree + * dies with it. + * + * Why here and not per-pty: a per-pty job cannot carry KILL_ON_JOB_CLOSE, + * because its handle is released when the shell exits and that would reap + * whatever the user had backgrounded. This job's handle is released only when + * the process itself dies, so it reaps a crashed host without changing what a + * clean shell exit means. Children inherit job membership, so every pty the + * caller later spawns is covered without further work, and the per-pty jobs + * simply nest inside this one. + * + * The handle is deliberately never closed: it must outlive every caller, and + * process teardown is what releases it. + */ +static Napi::Value PtyAssignCurrentProcessToJob(const Napi::CallbackInfo& info) { + Napi::Env env(info.Env()); + Napi::HandleScope scope(env); + + // Why locked: two callers racing here would each create a job, put the + // process in both, and leak the first handle -- and since the handle is what + // keeps a kill-on-close job alive, a leaked one is never released. A worker + // thread with its own N-API env shares these statics, so "only JS calls it" + // is not a guarantee. + static HANDLE hHostJob = nullptr; + std::lock_guard guard(ptyJobMutex); + if (hHostJob != nullptr) { + return Napi::Boolean::New(env, true); + } + + HANDLE job = CreateJobObjectW(nullptr, nullptr); + if (job == nullptr) { + return Napi::Boolean::New(env, false); + } + JOBOBJECT_EXTENDED_LIMIT_INFORMATION limits{}; + // BREAKAWAY_OK for the same reason as the per-pty job: without it a child + // asking for CREATE_BREAKAWAY_FROM_JOB is refused outright. + limits.BasicLimitInformation.LimitFlags = + JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE | JOB_OBJECT_LIMIT_BREAKAWAY_OK; + if (!SetInformationJobObject(job, JobObjectExtendedLimitInformation, &limits, sizeof(limits)) || + !AssignProcessToJobObject(job, GetCurrentProcess())) { + // An outer job that forbids nesting refuses this; the caller degrades. + CloseHandle(job); + return Napi::Boolean::New(env, false); + } + hHostJob = job; + return Napi::Boolean::New(env, true); +} + /** * Init */ @@ -577,6 +804,9 @@ 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)); + exports.Set("terminateJob", Napi::Function::New(env, PtyTerminateJob)); + exports.Set("listJobProcessIds", Napi::Function::New(env, PtyListJobProcessIds)); + exports.Set("assignCurrentProcessToJob", Napi::Function::New(env, PtyAssignCurrentProcessToJob)); return exports; };