fix(windows): reject stale parent PID links in shutdown snapshots (#19149)

* fix(windows): reject stale parent PID links in exit snapshots

* refactor(windows): share the walk's pid index in the stale-link filter

Resolve parent links through the same index the descendant walk builds, so a
table that repeats a pid answers both the same way, and drop the non-null
assertion on the walk by keeping the "cannot see" null contract.

Pin the two filter branches nothing exercised: the root surviving its own
recycled ppid, and the root's start bounding a link whose claimed parent
denied its creation time.

* test(windows): pin the root creation-time floor and its tie

The floor clause survived deletion: for a chain of timestamped rows the
per-parent check already enforces order transitively, so it only does work
below a row that denied its creation time -- admitted unchecked, and its
children then find no parent time to compare against either. Cover that chain
with a child at the root's exact timestamp, which a same-millisecond spawn
produces routinely, and one that predates the root.

Also pin that pruning a link drops the unidentified rows beneath it from the
count, since a retained one would cap the verdict at unverifiable over a
process the root never owned.

Record why ties pass, what the floor is for, and the clock monotonicity the
filter assumes.

* docs(windows): say why the pid index is shared with the walk

The index is not reused across the two calls -- the walk indexes the filtered
array -- so name the actual reason: a repeated pid must resolve first-wins, the
way the walk resolves it, rather than last-wins as a Map over the rows would.

* docs(windows): describe why both pid lookups share one index

* docs(windows): put each pruning rationale on the code it justifies

---------

Co-authored-by: Merge Sim <sim@local>
This commit is contained in:
Brennan Benson
2026-09-06 17:36:29 -07:00
committed by GitHub
co-authored by Merge Sim
parent 6fd03a74ef
commit 41934759ea
2 changed files with 128 additions and 3 deletions
@@ -19,6 +19,101 @@ function snapshot(
}
describe('captureWindowsDescendantSnapshot', () => {
it('does not claim an older process whose former parent PID was reused by the root', async () => {
const olderProcess = { pid: 50244, ppid: 36084, creationTimeMs: 1788659167395 }
const captured = await captureWindowsDescendantSnapshot(36084, {
readTable: async () => [
{ pid: 36084, ppid: 60976, creationTimeMs: 1788733587893 },
olderProcess
]
})
expect(captured?.descendants).toEqual([])
await expect(
verifyWindowsDescendantSnapshotExit(captured!, { readTable: async () => [olderProcess] })
).resolves.toBe('exited')
})
it('prunes a stale parent link and its subtree at any depth', async () => {
const captured = await captureWindowsDescendantSnapshot(100, {
readTable: async () => [
{ pid: 100, ppid: 1, creationTimeMs: 5 },
{ pid: 200, ppid: 100, creationTimeMs: 10 },
{ pid: 300, ppid: 200, creationTimeMs: 7 },
{ pid: 400, ppid: 300, creationTimeMs: 12 },
{ pid: 500, ppid: 100, creationTimeMs: 4 },
{ pid: 600, ppid: 500, creationTimeMs: 13 },
{ pid: 700, ppid: 200, creationTimeMs: 10 }
]
})
expect(captured?.descendants).toEqual([
{ pid: 700, creationTimeMs: 10 },
{ pid: 200, creationTimeMs: 10 }
])
})
it('keeps the root when its own parent PID was reused by a newer process', async () => {
// The root's retained ppid now names a process created after it. Pruning the
// root drops the whole snapshot, so its own link is never evidence about it.
const captured = await captureWindowsDescendantSnapshot(100, {
readTable: async () => [
{ pid: 100, ppid: 900, creationTimeMs: 5 },
{ pid: 900, ppid: 1, creationTimeMs: 50 },
{ pid: 200, ppid: 100, creationTimeMs: 7 }
],
now: () => 42
})
expect(captured).toEqual({
root: { pid: 100, creationTimeMs: 5 },
descendants: [{ pid: 200, creationTimeMs: 7 }],
unidentifiedCount: 0,
capturedAtMs: 42
})
})
it('bounds a link by the root when the claimed parent denied its creation time', async () => {
// 300 has no creation time for a child to be compared against, so the root's
// start is the only bound left: 350 ties with it, which a same-millisecond
// spawn does routinely, while 360 predates the whole tree.
const captured = await captureWindowsDescendantSnapshot(100, {
readTable: async () => [
{ pid: 100, ppid: 1, creationTimeMs: 5 },
{ pid: 300, ppid: 100 },
{ pid: 350, ppid: 300, creationTimeMs: 5 },
{ pid: 360, ppid: 300, creationTimeMs: 2 }
],
now: () => 42
})
expect(captured).toEqual({
root: { pid: 100, creationTimeMs: 5 },
descendants: [{ pid: 350, creationTimeMs: 5 }],
unidentifiedCount: 1,
capturedAtMs: 42
})
})
it('drops an unidentified row whose parent link was pruned', async () => {
// 250 denied its creation time, but 200's claim on the root is impossible, so
// 250 was never in this tree: counting it would cap the verdict at
// unverifiable over a process the root does not own.
const captured = await captureWindowsDescendantSnapshot(100, {
readTable: async () => [
{ pid: 100, ppid: 1, creationTimeMs: 10 },
{ pid: 200, ppid: 100, creationTimeMs: 5 },
{ pid: 250, ppid: 200 }
]
})
expect(captured?.descendants).toEqual([])
expect(captured?.unidentifiedCount).toBe(0)
await expect(
verifyWindowsDescendantSnapshotExit(captured!, { readTable: async () => [] })
).resolves.toBe('exited')
})
it('walks the whole subtree and keeps only rows a later read can re-identify', async () => {
const captured = await captureWindowsDescendantSnapshot(100, {
// 400 is a grandchild; 300 denied a creation-time query, so no later read
@@ -1,3 +1,4 @@
import { getProcessTableIndex } from '../shared/process-table-index'
import type { DescendantTreeVerdict } from './pty-descendant-exit-verification'
import { windowsDescendantsFromRows } from './providers/windows-foreground-process-rows'
import { readWindowsProcessTableFresh } from './windows/windows-process-table'
@@ -57,6 +58,9 @@ function delay(ms: number): Promise<void> {
* Snapshot a Windows root's descendants while it is still alive. Resolves null
* (never rejects) when the table is unreadable or the root is absent — the same
* contract as the POSIX walk, because "cannot see" is never "nothing is there".
*
* Stale parent links are pruned by creation time, so a backwards clock step
* between two spawns can drop a live descendant — accepted over a certain stall.
*/
export async function captureWindowsDescendantSnapshot(
rootPid: number,
@@ -69,9 +73,35 @@ export async function captureWindowsDescendantSnapshot(
// One table read, not a walk plus an identity read: each is bounded in
// seconds, and this runs inside the close ladder's budget.
const table = await (deps.readTable ?? readWindowsProcessTableFresh)().catch(() => null)
const descendants = table && windowsDescendantsFromRows(table, rootPid)
const root = table?.find((row) => row.pid === rootPid)
if (!descendants || typeof root?.creationTimeMs !== 'number') {
if (!table) {
return null
}
// One index for both lookups, so a repeated pid resolves to the same row for
// the root and for a parent link: `byPid` is first-wins, a Map is not.
const rowsByPid = getProcessTableIndex(table).byPid
const root = rowsByPid.get(rootPid)
if (typeof root?.creationTimeMs !== 'number') {
return null
}
const rootCreationTimeMs = root.creationTimeMs
// Windows keeps a process's original parent PID after that parent exits, so a
// reused PID is not ancestry: no real child predates the parent it claims.
// The root's start backstops the undefined-time bypass, which admits a row
// unchecked and leaves its children no parent time to compare against. Ties
// pass -- FILETIMEs truncated to ms make a same-millisecond parent and child
// collide exactly, so `>` would drop true descendants.
const currentRows = table.filter((row) => {
const parentCreationTimeMs = rowsByPid.get(row.ppid)?.creationTimeMs
return (
// Its own ppid can be recycled too, and a pruned root loses the snapshot.
row.pid === rootPid ||
row.creationTimeMs === undefined ||
(row.creationTimeMs >= rootCreationTimeMs &&
(parentCreationTimeMs === undefined || row.creationTimeMs >= parentCreationTimeMs))
)
})
const descendants = windowsDescendantsFromRows(currentRows, rootPid)
if (!descendants) {
return null
}
return {