From 2e5889057a4174d5389b21dca0d345a669d127bb Mon Sep 17 00:00:00 2001 From: LogicDuke Date: Wed, 19 Aug 2026 22:34:01 +0200 Subject: [PATCH] fix(process-transport): handle non-extensible internal promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A promise is not private between the allocation that makes it and the next statement: an ordinary `async_hooks` init hook receives each newly allocated promise as its own resource and can seal it there, so the own `constructor` this module installs to keep an `await` on its fast path may throw before it can land. Combined with a persistently replaced `Promise.prototype.constructor` and `Promise.prototype.then`, every internal `await` in the termination chain is then pushed into thenable assimilation and left with no continuation, so an ordinary timeout — and the mandatory hardening rejection — never settle. Internal promises are now allocated from an `InternalPromise` whose prototype is created and frozen at module load with `constructor` fixed to the captured intrinsic, so the recognition test is answered one link before `Promise.prototype` and needs no own property on the instance. The platform termination steps report through that capability instead of through the promise the runtime makes for an `async` function, which cannot be protected at all. `protectPromiseResolution` stays as a secondary layer whose failure is now survivable rather than silent. Co-Authored-By: Claude Opus 5 (1M context) --- src/adapters/process-transport.ts | 323 ++++++++++++++------ tests/adapters/process-transport.test.ts | 287 +++++++++++++++-- tests/adapters/transport-invariants.test.ts | 6 +- 3 files changed, 499 insertions(+), 117 deletions(-) diff --git a/src/adapters/process-transport.ts b/src/adapters/process-transport.ts index fc6513e..40fe497 100644 --- a/src/adapters/process-transport.ts +++ b/src/adapters/process-transport.ts @@ -199,33 +199,110 @@ function protectChildDispatch(child: ChildProcess): void { } /** - * Make one promise this module owns safe to `await` after hostile code has run. + * The promise kind this module awaits. * * `await` does not read `then` off an ordinary native promise: `PromiseResolve` * recognises the promise as its own kind and hands it straight to the internal - * reaction machinery. The recognition test is `Get(promise, "constructor")`, and - * for a promise carrying no own `constructor` that read walks up to + * reaction machinery. The recognition test is `Get(promise, "constructor")`, + * and for a promise carrying no own `constructor` that read walks up to * `Promise.prototype.constructor` — an ordinary, writable property of an * ordinary, mutable object. Replace it with anything that is not the intrinsic * and the fast path is abandoned: the awaited value is then resolved as a plain - * thenable, which *does* read `then`, reaching whatever a hostile path installed - * on `Promise.prototype` in the meantime. A replacement that installs no - * continuation leaves the `await` suspended for good, and everything waiting + * thenable, which *does* read `then`, reaching whatever a hostile path + * installed on `Promise.prototype` in the meantime. A replacement that installs + * no continuation leaves the `await` suspended for good, and everything waiting * behind it with it — including the mandatory hardening rejection and the * ordinary timeout settlement, neither of which has any further deadline left * to rescue it. * - * An own `constructor` fixed to the captured {@link NativePromise} closes that - * lookup at its source. The recognition test then reads a property no later - * code can shadow, answers with the intrinsic, and the thenable path — with its - * `then` read — is never entered. `Symbol.species` is not consulted on this - * route at all, so no mutation of it is reachable from an `await` of a promise - * that passed through here. + * {@link protectPromiseResolution} answers that lookup with an own property. It + * cannot answer it *unconditionally*: defining a property requires an + * extensible target, and a promise this module allocates is not private between + * the allocation and the next statement. Ordinary Node facilities — an + * `async_hooks` `init` hook is the reachable one, which receives each newly + * allocated promise as its own resource — can observe it first and seal it, and + * the definition then throws before it can land. Nothing in this file can + * prevent that, because the observation happens inside the allocation itself. * - * Applied to promises this module has *just* created, which are extensible and - * carry no own `constructor`, so the definition cannot fail; the guard is here - * only so that this helper can never become a rejection path of its own. - * Non-configurable, so nothing that runs later can take it back off again. + * The lookup is a *chain* walk, though, and only its last step is the mutable + * one. A promise whose immediate prototype is an object this module owns never + * reaches `Promise.prototype` at all: the walk stops one link earlier, at a + * `constructor` fixed to the captured {@link NativePromise} on a prototype + * created and frozen at module load, before any hostile path can run. That + * answer needs no own property on the instance, so sealing the instance — the + * one thing this module cannot prevent — no longer decides anything. + * + * Every promise this module *awaits itself* is therefore allocated from here, + * and {@link internalStep} exists so that the promises the runtime allocates + * for `async` functions are never among them. `Symbol.species` is not consulted + * on the `await` route at all, so no mutation of it is reachable either. + * + * The prototype's `constructor` is redefined rather than added, so a sealed + * prototype could not defeat this step either: the class definition already + * gave it that own property, and redefining a configurable own property does + * not require extensibility. + */ +class InternalPromise extends Promise {} +// `void`: both intrinsics return their own first argument, which here is typed +// as a promise because a promise prototype is one. +void objectDefineProperty(InternalPromise.prototype, 'constructor', { + configurable: false, + enumerable: false, + value: NativePromise, + writable: false, +}); +void objectFreeze(InternalPromise.prototype); +void objectFreeze(InternalPromise); + +/** + * Report one internal asynchronous step through a promise this module owns. + * + * An `async` function's own promise comes from the runtime's intrinsic + * capability, so it inherits `constructor` straight from `Promise.prototype` + * and carries no own one. Awaiting it is exactly the lookup {@link + * InternalPromise} exists to avoid, and it cannot be repaired after the fact: + * the promise may already be sealed, and the only way to observe it — `then` — + * is the property under mutation. So no `async` function's promise is ever + * awaited here. The step reports its result through the capability it is + * handed, and the promise the runtime made for it is never consulted. + * + * That makes the step responsible for absorbing its own faults into `fail`, + * which every one of them does. The absorber below is a second layer for a + * programmer defect only: settlement never depends on it being installed, so a + * fault in the intrinsic's own species prologue costs nothing here. + */ +function internalStep( + step: (settle: (value: T) => void, fail: (reason: unknown) => void) => Promise, +): Promise { + return new InternalPromise((resolve, reject) => { + const running = step(resolve, reject); + try { + void reflectApply(promiseThen, running, [undefined, reject]); + } catch { + // See the doc comment: the step has already reported through `resolve` + // or `reject`, so nothing is waiting on this attachment. + } + }); +} + +/** + * Fix one promise's `constructor` lookup with an own property. + * + * A second layer over {@link InternalPromise}, and the only one available for a + * promise handed to a caller outside this module. It answers the same + * recognition test one step earlier in the chain, and non-configurably, so + * nothing that runs later can take it back off again. + * + * **It can fail, and its failure is survivable.** Defining a property requires + * an extensible target, and an ordinary Node facility can seal a promise inside + * the allocation that produced it — before any statement of this module runs + * against it. The definition then throws. Returning the promise unchanged keeps + * this helper from becoming a rejection path of its own; what makes that + * *safe*, rather than a silent downgrade, is that every promise this module + * awaits itself already answers the same lookup from a prototype it owns, where + * no own property is needed. A promise that only leaves this module — the + * settled exchange handed back by {@link resolved} — is not awaited here, and + * how an external caller consumes it is that caller's own runtime. */ function protectPromiseResolution(promise: Promise): Promise { try { @@ -237,12 +314,13 @@ function protectPromiseResolution(promise: Promise): Promise { writable: false, }); } catch { - // Unreachable for a freshly created promise; see the doc comment. Returning - // the promise unchanged keeps this helper total. + // Reachable: the target may have been sealed inside its own allocation. + // See the doc comment for why returning it unchanged is survivable. } return promise; } +/** An already-settled promise for a caller outside this module. */ function resolved(value: T): Promise { return protectPromiseResolution( new NativePromise((resolve) => { @@ -251,6 +329,23 @@ function resolved(value: T): Promise { ); } +/** + * An already-settled promise this module goes on to `await` itself. + * + * Separate from {@link resolved} because the two have different consumers and + * therefore different requirements. This one is allocated from {@link + * InternalPromise}, whose prototype answers the recognition test without + * needing an own property on the instance, so an `await` of it stays on its + * fast path even when the instance was sealed inside its own allocation. + */ +function internallyResolved(value: T): Promise { + return protectPromiseResolution( + new InternalPromise((resolve) => { + resolve(value); + }), + ); +} + function onEvent( emitter: EventEmitter, event: string, @@ -458,9 +553,9 @@ function hasEnded(child: ChildProcess): boolean { /** Resolve true when the child ends within `ms`, false when it outlives it. */ function waitForExit(child: ChildProcess, ms: number): Promise { if (hasEnded(child)) { - return resolved(true); + return internallyResolved(true); } - const exited = new NativePromise((resolve) => { + const exited = new InternalPromise((resolve) => { let done = false; const finish = (value: boolean): void => { if (done) { @@ -578,7 +673,7 @@ function runTaskkill( taskkill: { readonly executable: string; readonly systemRoot: string }, pid: number, ): Promise { - const issued = new NativePromise((resolve) => { + const issued = new InternalPromise((resolve) => { let killer: ChildProcess; try { const decimalPid = reflectApply(numberToString, pid, []); @@ -675,42 +770,61 @@ function runTaskkill( * `setsid` itself has left that group and is not reached — which is why the * returned scope says *requested*, never *completed*. */ -async function terminatePosix( +function terminatePosix( child: ChildProcess, pid: number, graceMs: number, ): Promise { - if (hasEnded(child)) { - return TERMINATION_SCOPE.DIRECT_CHILD_ONLY; - } + return internalStep(async (settle, fail) => { + try { + if (hasEnded(child)) { + settle(TERMINATION_SCOPE.DIRECT_CHILD_ONLY); + return; + } - let groupReached = signalProcessGroup(pid, 'SIGTERM'); - if (!groupReached) { - killDirectChild(child, 'SIGTERM'); - } - if (await waitForExit(child, graceMs)) { - return groupReached - ? TERMINATION_SCOPE.PROCESS_GROUP_REQUESTED - : TERMINATION_SCOPE.DIRECT_CHILD_ONLY; - } + let groupReached = signalProcessGroup(pid, 'SIGTERM'); + if (!groupReached) { + killDirectChild(child, 'SIGTERM'); + } + if (await waitForExit(child, graceMs)) { + settle( + groupReached + ? TERMINATION_SCOPE.PROCESS_GROUP_REQUESTED + : TERMINATION_SCOPE.DIRECT_CHILD_ONLY, + ); + return; + } - // The grace timer and child exit can become ready in the same event-loop - // turn. Once the child is observed ended, its numeric process-group ID may - // be reused, so it must not receive the escalation signal. - if (hasEnded(child)) { - return groupReached - ? TERMINATION_SCOPE.PROCESS_GROUP_REQUESTED - : TERMINATION_SCOPE.DIRECT_CHILD_ONLY; - } + // The grace timer and child exit can become ready in the same event-loop + // turn. Once the child is observed ended, its numeric process-group ID may + // be reused, so it must not receive the escalation signal. + if (hasEnded(child)) { + settle( + groupReached + ? TERMINATION_SCOPE.PROCESS_GROUP_REQUESTED + : TERMINATION_SCOPE.DIRECT_CHILD_ONLY, + ); + return; + } - if (!signalProcessGroup(pid, 'SIGKILL')) { - killDirectChild(child, 'SIGKILL'); - groupReached = false; - } - await waitForExit(child, graceMs); - return groupReached - ? TERMINATION_SCOPE.PROCESS_GROUP_REQUESTED - : TERMINATION_SCOPE.DIRECT_CHILD_ONLY; + if (!signalProcessGroup(pid, 'SIGKILL')) { + killDirectChild(child, 'SIGKILL'); + groupReached = false; + } + await waitForExit(child, graceMs); + settle( + groupReached + ? TERMINATION_SCOPE.PROCESS_GROUP_REQUESTED + : TERMINATION_SCOPE.DIRECT_CHILD_ONLY, + ); + } catch (error) { + // Every observation above reads the handle, and a hostile accessor can + // fault on any of them. The reason reaches the caller unchanged: this is + // the same rejection the `async` form produced, reported through the + // capability instead of through a promise nothing here may await. + fail(error); + } + }); } /** @@ -721,33 +835,44 @@ async function terminatePosix( * `taskkill` cannot start, fails, or times out, the direct child is terminated * and the scope degrades to `DIRECT_CHILD_ONLY` — descendants are not claimed. */ -async function terminateWindows( +function terminateWindows( child: ChildProcess, pid: number, graceMs: number, ): Promise { - // Once the leader has ended, its numeric PID may identify an unrelated - // process. Safety outranks reaching descendants that outlived the leader. - if (hasEnded(child)) { - return TERMINATION_SCOPE.DIRECT_CHILD_ONLY; - } + return internalStep(async (settle, fail) => { + try { + // Once the leader has ended, its numeric PID may identify an unrelated + // process. Safety outranks reaching descendants that outlived the leader. + if (hasEnded(child)) { + settle(TERMINATION_SCOPE.DIRECT_CHILD_ONLY); + return; + } - const taskkill = resolveTaskkill(); - if (taskkill === null) { - killDirectChild(child); - await waitForExit(child, graceMs); - return TERMINATION_SCOPE.DIRECT_CHILD_ONLY; - } + const taskkill = resolveTaskkill(); + if (taskkill === null) { + killDirectChild(child); + await waitForExit(child, graceMs); + settle(TERMINATION_SCOPE.DIRECT_CHILD_ONLY); + return; + } - const issued = await runTaskkill(taskkill, pid); - if (!issued) { - killDirectChild(child); - await waitForExit(child, graceMs); - return TERMINATION_SCOPE.DIRECT_CHILD_ONLY; - } + const issued = await runTaskkill(taskkill, pid); + if (!issued) { + killDirectChild(child); + await waitForExit(child, graceMs); + settle(TERMINATION_SCOPE.DIRECT_CHILD_ONLY); + return; + } - await waitForExit(child, graceMs); - return TERMINATION_SCOPE.PROCESS_TREE_REQUESTED; + await waitForExit(child, graceMs); + settle(TERMINATION_SCOPE.PROCESS_TREE_REQUESTED); + } catch (error) { + // Same contract as the POSIX strategy: the handle reads above can fault, + // and the reason reaches the caller unchanged. + fail(error); + } + }); } /** @@ -770,29 +895,47 @@ async function terminateWindows( * * Awaiting instead settles this function with a {@link TerminationScope}, which * is a string. Resolving a capability with a primitive reads nothing at all, so - * the assimilation step that made the lookup reachable no longer occurs. The - * awaited promise itself goes through {@link protectPromiseResolution} so that - * the `await` cannot be pushed off its own fast path either. Rejection - * behaviour is unchanged: a platform strategy that faults still rejects this - * function's promise with the same value, for the same callers to handle. + * the assimilation step that made the lookup reachable no longer occurs. + * + * That leaves the `await` itself, which decides whether it may skip + * assimilation by reading `constructor` off the awaited promise. An own + * property answers that read only while the promise is extensible, and a + * promise is not private between its allocation and the next statement, so the + * platform strategies report through {@link internalStep} — an {@link + * InternalPromise}, whose prototype answers the read with no own property + * involved. This function reports the same way, for the same reason: its own + * promise is awaited by {@link releaseUnprotectedChild} and by {@link + * runTermination}, and the promise the runtime would have made for an `async` + * function is not one either of them could safely await. + * + * Rejection behaviour is unchanged: a platform strategy that faults still + * rejects this function's promise with the same value, for the same callers to + * handle. */ -async function terminate( +function terminate( child: ChildProcess, platform: TransportPlatform, graceMs: number, ): Promise { - const pid = child.pid; - if (pid === undefined) { - // Never started, so nothing beyond the handle can be reached. Reported as - // degraded rather than as a successful group or tree request. - return TERMINATION_SCOPE.DIRECT_CHILD_ONLY; - } - const scope = await protectPromiseResolution( - platform === 'posix' - ? terminatePosix(child, pid, graceMs) - : terminateWindows(child, pid, graceMs), - ); - return scope; + return internalStep(async (settle, fail) => { + try { + const pid = child.pid; + if (pid === undefined) { + // Never started, so nothing beyond the handle can be reached. Reported + // as degraded rather than as a successful group or tree request. + settle(TERMINATION_SCOPE.DIRECT_CHILD_ONLY); + return; + } + const scope = await protectPromiseResolution( + platform === 'posix' + ? terminatePosix(child, pid, graceMs) + : terminateWindows(child, pid, graceMs), + ); + settle(scope); + } catch (error) { + fail(error); + } + }); } /** @@ -1145,9 +1288,9 @@ export function invokeAgentProcess( /** Resolve true on close, false when the bounded close wait expires. */ function awaitClose(ms: number): Promise { if (closed) { - return resolved(true); + return internallyResolved(true); } - const observed = new NativePromise((resolveWait) => { + const observed = new InternalPromise((resolveWait) => { const waiter = scheduleTimeout(() => { notifyClosed = null; resolveWait(false); diff --git a/tests/adapters/process-transport.test.ts b/tests/adapters/process-transport.test.ts index 7066613..a443445 100644 --- a/tests/adapters/process-transport.test.ts +++ b/tests/adapters/process-transport.test.ts @@ -1101,28 +1101,40 @@ process.exit(0); * call would be answered by the first lookup and would say nothing about the * rest of the lifecycle — which is where both of these live. * - * Four modes, two axes. The path is either the mandatory hardening failure + * Six modes, three axes. The path is either the mandatory hardening failure * (whose rejection waits on the release, which waits on termination) or an * ordinary timeout (whose settlement waits on termination directly, with the - * exchange deadline already spent). The mutation is either `then` alone or - * `then` together with `constructor`, because the two defeat settlement through + * exchange deadline already spent). The mutation is `then` alone, or `then` + * together with `constructor`, because the two defeat settlement through * different steps: `then` alone is reached only through assimilation, while * mutating `constructor` as well pushes every `await` in the termination chain * off its fast path and into that same assimilation. * + * The third axis is whether newly allocated promises are *sealed*. It exists + * because the own-property answer to that `constructor` read is not something + * this transport can install unconditionally: defining a property needs an + * extensible target, and an ordinary `async_hooks` init hook receives each + * promise as its own resource inside the allocation that made it. Seal it there + * and the definition throws before it lands. Sealing alone is harmless and + * mutating alone is survivable; it is the two together that leave an internal + * `await` with no answer but assimilation and no continuation at the end of it. + * * The probe carries its own counterfactuals, run while the hook is still armed * and after the transport has already settled. They reproduce the pre-repair * shape (`return promise`), the half-repaired shape (`return await promise`), - * and the repaired shape (`return await` a promise carrying an own - * `constructor`) against the identical staged runtime, and report which of them - * settles. That is what makes the staged condition provably lethal at the exact - * moment the transport survived it, rather than merely present. + * the own-property shape (`return await` a promise the committed helper tried + * to give an own `constructor`), and the prototype shape (`return await` a + * promise whose prototype answers the read) against the identical staged + * runtime, and report which of them settles. That is what makes the staged + * condition provably lethal at the exact moment the transport survived it, + * rather than merely present. * * Everything between arming and restoring is written in callbacks. An `await` * there would be the very thing under test, and a probe that suspended on its * own instrumentation would report a pending transport. */ const PERSISTENT_PROMISE_PROBE_SCRIPT = ` +import { createHook } from 'node:async_hooks'; import { ChildProcess } from 'node:child_process'; import { tmpdir } from 'node:os'; @@ -1136,15 +1148,30 @@ const MODES = [ 'hardening-persistent-ctor-then', 'timeout-persistent-then', 'timeout-persistent-ctor-then', + 'hardening-persistent-sealed-ctor-then', + 'timeout-persistent-sealed-ctor-then', ]; if (!MODES.includes(mode)) { console.log('MODE_INVALID=' + String(mode)); process.exit(3); } const HARDENING = - mode === 'hardening-persistent-then' || mode === 'hardening-persistent-ctor-then'; + mode === 'hardening-persistent-then' || + mode === 'hardening-persistent-ctor-then' || + mode === 'hardening-persistent-sealed-ctor-then'; const MUTATE_CTOR = - mode === 'hardening-persistent-ctor-then' || mode === 'timeout-persistent-ctor-then'; + mode === 'hardening-persistent-ctor-then' || + mode === 'timeout-persistent-ctor-then' || + mode === 'hardening-persistent-sealed-ctor-then' || + mode === 'timeout-persistent-sealed-ctor-then'; +// The third axis. A promise is not private between the allocation that makes it +// and the next statement of the code that asked for one: an ordinary +// 'async_hooks' init hook receives each newly allocated promise as its own +// resource and may seal it there. Every own-property protection the transport +// would install on a promise it just created then throws instead of landing. +const SEAL = + mode === 'hardening-persistent-sealed-ctor-then' || + mode === 'timeout-persistent-sealed-ctor-then'; // Intrinsics captured before anything is installed over them. This probe has to // keep observing, timing, and cleaning up while its own substitution is in @@ -1154,6 +1181,8 @@ const REAL_APPLY = Reflect.apply; const REAL_DEFINE = Object.defineProperty; const REAL_PROMISE = Promise; const REAL_CTOR_DESCRIPTOR = Object.getOwnPropertyDescriptor(Promise.prototype, 'constructor'); +const REAL_IS_EXTENSIBLE = Object.isExtensible; +const REAL_PREVENT_EXTENSIONS = Object.preventExtensions; const realSetTimeout = setTimeout; const realClearTimeout = clearTimeout; @@ -1240,6 +1269,65 @@ if (HARDENING) { } } +// --------------------------------------------------------------------------- +// Whether the transport's own-property protection actually lands. +// +// The transport captures 'Object.defineProperty' at module load, so replacing +// it here — before the import — is what makes the capture this counted one. The +// filter is exact: only a definition of 'constructor' whose value is the real +// Promise intrinsic is a protection attempt, which is the only definition the +// transport makes with that shape. An attempt against a target that is no +// longer extensible is a protection *failure*, and the count of those is the +// evidence that this probe staged the case under audit rather than merely +// mentioning it. The wrapper is otherwise transparent: it forwards every +// argument and propagates the intrinsic's own throw unchanged. +let protectionAttempts = 0; +let protectionFailures = 0; +function countedDefineProperty(target, key, descriptor) { + if ( + key === 'constructor' && + descriptor !== null && + typeof descriptor === 'object' && + descriptor.value === REAL_PROMISE + ) { + protectionAttempts += 1; + let extensible = true; + try { extensible = REAL_IS_EXTENSIBLE(target); } catch { extensible = true; } + if (!extensible) protectionFailures += 1; + } + return REAL_DEFINE(target, key, descriptor); +} +REAL_DEFINE(Object, 'defineProperty', { + value: countedDefineProperty, writable: true, enumerable: false, configurable: true, +}); + +// --------------------------------------------------------------------------- +// THE SEALING FACILITY. +// +// Nothing exotic and nothing installed over an intrinsic: 'async_hooks' is an +// ordinary Node facility, and for a resource of type PROMISE the resource it +// hands the init callback *is* the promise, at a point inside the allocation +// itself — before the statement that asked for the promise has resumed. Sealing +// it there is what makes a later own-property definition impossible, and no +// capture in the transport can prevent it, because the transport never gets to +// see the promise first. +// +// Created here, enabled only after the transport module is resident, and left +// enabled for the whole exchange. +// --------------------------------------------------------------------------- +let sealedPromises = 0; +const sealHook = createHook({ + init(id, type, triggerId, resource) { + if (type !== 'PROMISE') return; + try { + REAL_PREVENT_EXTENSIONS(resource); + sealedPromises += 1; + } catch { + // A resource this facility cannot seal simply is not part of the staging. + } + }, +}); + // --------------------------------------------------------------------------- // THE TRANSPORT, loaded before anything hostile is installed. // @@ -1288,15 +1376,38 @@ function installHostileConstructor() { configurable: true, }); } +// Asked by allocating a promise and looking at it. A hook that had stopped +// firing would hand back an extensible one, so this reports the facility's +// present effect rather than the fact that enable() was once called. The +// promise is discarded with no continuation and no rejection. +function sealArmed() { + if (!SEAL) return true; + let probePromise; + try { + probePromise = new REAL_PROMISE(() => {}); + } catch { + return false; + } + try { + return !REAL_IS_EXTENSIBLE(probePromise); + } catch { + return false; + } +} function armed() { return Promise.prototype.then !== REAL_THEN && - (!MUTATE_CTOR || Promise.prototype.constructor === HOSTILE_CONSTRUCTOR); + (!MUTATE_CTOR || Promise.prototype.constructor === HOSTILE_CONSTRUCTOR) && + sealArmed(); } function restoreIntrinsics() { REAL_DEFINE(Promise.prototype, 'then', { value: REAL_THEN, writable: true, enumerable: false, configurable: true, }); REAL_DEFINE(Promise.prototype, 'constructor', REAL_CTOR_DESCRIPTOR); + if (SEAL) sealHook.disable(); + REAL_DEFINE(Object, 'defineProperty', { + value: REAL_DEFINE, writable: true, enumerable: false, configurable: true, + }); } /** @@ -1354,17 +1465,40 @@ function controlLater(value) { realSetTimeout(() => { resolve(value); }, 1); }); } +// The committed shape of 'protectPromiseResolution', reproduced exactly — +// including the catch that swallows a definition which could not land. Against +// an extensible target this is the protection that survives a mutated +// 'Promise.prototype'; against a sealed one it silently returns a promise +// carrying nothing, which is the defect under audit. function protectLikeRepair(promise) { - REAL_DEFINE(promise, 'constructor', { - configurable: false, enumerable: false, value: REAL_PROMISE, writable: false, - }); + try { + REAL_DEFINE(promise, 'constructor', { + configurable: false, enumerable: false, value: REAL_PROMISE, writable: false, + }); + } catch { + // Exactly what the committed helper does. + } return promise; } +// The repaired shape: the answer comes from a prototype this script owns and +// froze before anything hostile ran, so no own property on the instance — and +// therefore no extensible instance — is required for it. +class OwnedPromise extends REAL_PROMISE {} +REAL_DEFINE(OwnedPromise.prototype, 'constructor', { + configurable: false, enumerable: false, value: REAL_PROMISE, writable: false, +}); +Object.freeze(OwnedPromise.prototype); +function controlLaterOwned(value) { + return new OwnedPromise((resolve) => { + realSetTimeout(() => { resolve(value); }, 1); + }); +} async function controlThenableReturn() { return controlLater('thenable-return'); } async function controlAwaitUnprotected() { return await controlLater('await-unprotected'); } async function controlAwaitProtected() { return await protectLikeRepair(controlLater('await-protected')); } +async function controlAwaitOwned() { return await controlLaterOwned('await-owned'); } const environment = {}; for (const name of ['HOMEDRIVE', 'HOMEPATH', 'LOGONSERVER', 'PATH', 'SYSTEMDRIVE', @@ -1391,8 +1525,11 @@ const limits = { maxStderrBytes: 16384, }; +if (SEAL) sealHook.enable(); if (MUTATE_CTOR) installHostileConstructor(); installPersistentHostileThen(); +const sealedBeforeCall = sealedPromises; +const protectionFailuresBeforeCall = protectionFailures; const exchange = invokeAgentProcess(spec, limits); const hookCallsAfterCall = hookCalls; @@ -1415,11 +1552,14 @@ observe(exchange, 12000, (result) => { controls.awaitUnprotected = b.kind; observe(controlAwaitProtected(), 400, (c) => { controls.awaitProtected = c.kind; - // Only now, with every measurement taken while the substitution was in - // place, is the runtime handed back intact. - const stillArmedAtEnd = armed(); - restoreIntrinsics(); - report(stillArmedAtEnd); + observe(controlAwaitOwned(), 400, (d) => { + controls.awaitOwned = d.kind; + // Only now, with every measurement taken while the substitution was + // in place, is the runtime handed back intact. + const stillArmedAtEnd = armed(); + restoreIntrinsics(); + report(stillArmedAtEnd); + }); }); }); }); @@ -1429,8 +1569,15 @@ function report(stillArmedAtEnd) { let restored = false; try { restored = Promise.prototype.then === REAL_THEN && - Promise.prototype.constructor === REAL_PROMISE; + Promise.prototype.constructor === REAL_PROMISE && + Object.defineProperty === REAL_DEFINE && + REAL_IS_EXTENSIBLE(new REAL_PROMISE(() => {})); } catch { restored = false; } + console.log('SEALED_BEFORE_CALL=' + sealedBeforeCall); + console.log('SEALED_PROMISES=' + sealedPromises); + console.log('PROTECTION_ATTEMPTS=' + protectionAttempts); + console.log('PROTECTION_FAILURES_BEFORE_CALL=' + protectionFailuresBeforeCall); + console.log('PROTECTION_FAILURES=' + protectionFailures); console.log('HOOK_INSTALLED=' + hookInstalled); console.log('HOOK_CALLS_AFTER_CALL=' + hookCallsAfterCall); console.log('HOOK_CALLS_AT_SETTLEMENT=' + hookCallsAtSettlement); @@ -1441,6 +1588,7 @@ function report(stillArmedAtEnd) { console.log('CONTROL_THENABLE_RETURN=' + String(controls.thenableReturn)); console.log('CONTROL_AWAIT_UNPROTECTED=' + String(controls.awaitUnprotected)); console.log('CONTROL_AWAIT_PROTECTED=' + String(controls.awaitProtected)); + console.log('CONTROL_AWAIT_OWNED=' + String(controls.awaitOwned)); console.log('SETTLEMENT=' + settlement.kind); console.log('PUBLIC_SETTLEMENTS=' + publicSettlements.count); if (settlement.kind === 'resolved') { @@ -1969,10 +2117,11 @@ async function runPersistentPromiseProbe(mode: string): Promise { * `expectLethal` names the counterfactual shapes this mode's staged runtime is * required to defeat. Requiring them to *fail* is what keeps the mode honest: a * probe whose substitution had quietly stopped biting would otherwise report a - * pass for a transport that was never actually tested. The repaired shape is - * required to survive in every mode, at the same moment, against the same - * runtime — so the difference between the two is attributable to the shape - * alone and not to the staging. + * pass for a transport that was never actually tested. Whatever a mode leaves + * off that list is required to survive, at the same moment and against the same + * runtime — so the difference between the shapes is attributable to the shape + * alone and not to the staging. `AWAIT_OWNED`, the shape the transport now uses + * for every promise it awaits itself, is on no mode's lethal list. */ function expectPersistentMutationSurvived( probe: ProbeResult, @@ -1991,7 +2140,12 @@ function expectPersistentMutationSurvived( expect(probe.stdout).toMatch(/^HOOK_CALLS_AT_SETTLEMENT=0$/m); // The counterfactuals, run against this same armed runtime after the // transport had already settled. - for (const shape of ['THENABLE_RETURN', 'AWAIT_UNPROTECTED', 'AWAIT_PROTECTED']) { + for (const shape of [ + 'THENABLE_RETURN', + 'AWAIT_UNPROTECTED', + 'AWAIT_PROTECTED', + 'AWAIT_OWNED', + ]) { const lethal = expectLethal.includes(shape); expect(probe.stdout).toMatch( new RegExp(`^CONTROL_${shape}=${lethal ? 'pending' : 'resolved'}$`, 'm'), @@ -3081,6 +3235,91 @@ describe('invokeAgentProcess — adversarial', () => { expect(probe.stdout).not.toContain('SETTLEMENT=rejected'); }, 60_000); + /** + * The evidence a *sealed* mode owes on top of the shared set. + * + * Three separate claims, because a probe that staged only some of them would + * report a pass for a case it never ran: + * + * - the sealing facility was actually effective, and stayed effective for + * the whole exchange rather than being answered once and stepping aside; + * - the transport's own-property protection was actually *attempted* while + * that was true, and actually *failed* — which is the exact branch whose + * silent `return promise` this repair exists to make safe; and + * - the shape the committed helper degrades to on that branch is provably + * unable to settle against this same runtime, while the shape the + * transport now uses still settles. + */ + function expectSealedProtectionFailed(probe: ProbeResult): void { + // Sealing happened, and it happened to promises allocated after the + // transport module was already resident. + expect(probe.stdout).toMatch(/^SEALED_PROMISES=[1-9][0-9]*$/m); + // The protection was attempted under the seal and could not land. Before + // the exchange began nothing had failed yet, so every counted failure is + // one this exchange actually reached. + expect(probe.stdout).toMatch(/^PROTECTION_FAILURES_BEFORE_CALL=0$/m); + expect(probe.stdout).toMatch(/^PROTECTION_FAILURES=[1-9][0-9]*$/m); + expect(probe.stdout).toMatch(/^PROTECTION_ATTEMPTS=[1-9][0-9]*$/m); + } + + it('settles a hardening failure when the protective constructor cannot be installed', async () => { + const probe = await runPersistentPromiseProbe('hardening-persistent-sealed-ctor-then'); + + // The third mechanism, and the one an own-property protection cannot + // answer at all. `Object.defineProperty` needs an extensible target, and a + // promise is not private between the allocation that makes it and the next + // statement: an ordinary `async_hooks` init hook receives it there and + // seals it. The committed helper's `catch` then hands back a promise + // carrying nothing, and under the same `constructor`+`then` mutation this + // PR exists to tolerate, the release the mandatory rejection waits on never + // reports. + // + // The counterfactual list is the statement of that: with this runtime + // staged, the pre-repair shape, the merely-awaited shape, *and* the + // own-property shape all hang. Only a promise whose prototype answers the + // recognition test still settles. + expectPersistentMutationSurvived(probe, [ + 'THENABLE_RETURN', + 'AWAIT_UNPROTECTED', + 'AWAIT_PROTECTED', + ]); + expectSealedProtectionFailed(probe); + // The staged failure was genuinely reached and genuinely mandatory. + expect(probe.stdout).toMatch(/^HARDENING_POISONED=[1-9][0-9]*$/m); + // The exact hardening failure, by identity, not by message. + expect(probe.stdout).toContain('SETTLEMENT=rejected'); + expect(probe.stdout).toContain('DETAIL=forced post-spawn hardening failure'); + expect(probe.stdout).toMatch(/^ERROR_IDENTITY=true$/m); + // A rejection is not an AgentExchange, and the mandatory failure is never + // reported as a failure to spawn. + expect(probe.stdout).not.toContain('SPAWN_FAILED'); + expect(probe.stdout).not.toContain('SETTLEMENT=resolved'); + }, 60_000); + + it('settles an ordinary timeout when the protective constructor cannot be installed', async () => { + const probe = await runPersistentPromiseProbe('timeout-persistent-sealed-ctor-then'); + + // The same third mechanism on the path with nothing left to rescue it: the + // exchange deadline has already fired, so it is what *started* the + // termination rather than anything that could still end the wait, and + // `terminating` is latched so no later cause can start a second lifecycle. + expectPersistentMutationSurvived(probe, [ + 'THENABLE_RETURN', + 'AWAIT_UNPROTECTED', + 'AWAIT_PROTECTED', + ]); + expectSealedProtectionFailed(probe); + // Nothing forced a failure here; this is the ordinary path. + expect(probe.stdout).toMatch(/^HARDENING_POISONED=0$/m); + // The intended outcome, with the termination it initiated actually + // reported rather than left at NOT_REQUIRED, and no scope invented for it. + expect(probe.stdout).toContain('SETTLEMENT=resolved'); + expect(probe.stdout).toContain('DETAIL=TIMED_OUT'); + expect(probe.stdout).not.toMatch(/^SCOPE=NOT_REQUIRED$/m); + expect(probe.stdout).not.toMatch(/^SCOPE=undefined$/m); + expect(probe.stdout).not.toContain('SETTLEMENT=rejected'); + }, 60_000); + it('settles a hardening failure whose bounded termination attempt itself fails', async () => { const probe = await runHardeningSettlementProbe('terminate-fault'); diff --git a/tests/adapters/transport-invariants.test.ts b/tests/adapters/transport-invariants.test.ts index 2669784..19eb266 100644 --- a/tests/adapters/transport-invariants.test.ts +++ b/tests/adapters/transport-invariants.test.ts @@ -237,8 +237,8 @@ describe('termination vocabulary claims no more than the OS provides', () => { }); it('invalidates a POSIX group target before either possible signal', () => { - const start = IMPLEMENTATION_SOURCE.indexOf('async function terminatePosix'); - const end = IMPLEMENTATION_SOURCE.indexOf('async function terminateWindows'); + const start = IMPLEMENTATION_SOURCE.indexOf('function terminatePosix'); + const end = IMPLEMENTATION_SOURCE.indexOf('function terminateWindows'); const implementation = IMPLEMENTATION_SOURCE.slice(start, end); const firstGuard = implementation.indexOf('if (hasEnded(child))'); const termSignal = implementation.indexOf("signalProcessGroup(pid, 'SIGTERM')"); @@ -257,7 +257,7 @@ describe('termination vocabulary claims no more than the OS provides', () => { }); it('invalidates a Windows PID before resolving or spawning taskkill', () => { - const start = IMPLEMENTATION_SOURCE.indexOf('async function terminateWindows'); + const start = IMPLEMENTATION_SOURCE.indexOf('function terminateWindows'); const end = IMPLEMENTATION_SOURCE.indexOf('/** Dispatch termination', start); const implementation = IMPLEMENTATION_SOURCE.slice(start, end); const guard = implementation.indexOf('if (hasEnded(child))');