diff --git a/bridge/process/ProcessRunTokenReader.swift b/bridge/process/ProcessRunTokenReader.swift index d5152ab50c..599fbdb1b4 100644 --- a/bridge/process/ProcessRunTokenReader.swift +++ b/bridge/process/ProcessRunTokenReader.swift @@ -15,6 +15,8 @@ struct ProcessIdentityObservation: Encodable { let effective_uid: UInt32? let lifecycle: String? let relation: ProcessRelationObservation? + /** Display-only kernel accounting name (p_comm, at most MAXCOMLEN bytes). */ + let name: String? let reason: String? } @@ -31,6 +33,7 @@ struct ProcessMetadata { let parentPid: Int32 let processGroupId: Int32 let pointerSize: Int + let name: String } struct ProcessSnapshot { @@ -92,7 +95,8 @@ func readProcessSnapshot( lifecycle: process.kp_proc.p_stat == SZOMB ? "zombie" : "non-zombie", parentPid: process.kp_eproc.e_ppid, processGroupId: process.kp_eproc.e_pgid, - pointerSize: process.kp_proc.p_flag & P_LP64 != 0 ? 8 : 4 + pointerSize: process.kp_proc.p_flag & P_LP64 != 0 ? 8 : 4, + name: processName(process) ) ) }, @@ -103,6 +107,13 @@ func readProcessSnapshot( return ProcessSnapshot(metadata: [:], failure: "process_table_failed_\(ENOMEM)") } +/** Decode the NUL-terminated accounting name; invalid UTF-8 is replaced for display. */ +private func processName(_ process: kinfo_proc) -> String { + withUnsafeBytes(of: process.kp_proc.p_comm) { bytes in + String(decoding: bytes.prefix { $0 != 0 }, as: UTF8.self) + } +} + func readProcessIdentities(pids: [Int32]) -> [ProcessIdentityObservation] { readProcessIdentities(pids: pids, snapshot: readProcessSnapshot()) } @@ -130,6 +141,7 @@ func readProcessIdentities( effective_uid: nil, lifecycle: nil, relation: nil, + name: nil, reason: "not-in-snapshot" ) } @@ -144,6 +156,7 @@ func readProcessIdentities( parent_pid: metadata.parentPid, process_group_id: metadata.processGroupId ), + name: metadata.name, reason: nil ) } @@ -157,6 +170,7 @@ private func unavailableIdentity(pid: Int32, reason: String) -> ProcessIdentityO effective_uid: nil, lifecycle: nil, relation: nil, + name: nil, reason: reason ) } diff --git a/docs/testing.md b/docs/testing.md index 02ad5fec77..8e2cfda3f7 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -754,9 +754,14 @@ membership changes, a cached lineage token is read again on the next refresh, an a group that reappears after it was processed is revalidated once, a further reappearance stays unresolved, lineage publishes one row per PID, and lineage does not publish a verified empty descendant list while that row remains unexamined. A readable token with unavailable -identity stays unresolved. Native UID facts, rather than an earlier process-table +identity stays unresolved. Both the sweep and the pre-signal group check re-read +an unreadable live token a bounded number of times. The sweep classifies the +PID's current process; the pre-signal re-read stays bound to the first observed +generation, so a reused PID stays unresolved there. A token that stays +unreadable fails closed with the native name, parent, group and identity that +bracketed it. Native UID facts, rather than an earlier process-table row, determine whether a token denial concerns another user. Darwin tests compile -the helper and exercise snapshot growth, absence, lifecycle, and UID observations; +the helper and exercise snapshot growth, absence, lifecycle, name and UID observations; controlled Linux procfs tests do not establish real Linux exit or PID-reuse behavior. For atomic writes, exercise cancellation on both sides of the documented commit diff --git a/src/process/DarwinProcessRunTokenReader.ts b/src/process/DarwinProcessRunTokenReader.ts index 7de6ebe20b..ffb8faf3d0 100644 --- a/src/process/DarwinProcessRunTokenReader.ts +++ b/src/process/DarwinProcessRunTokenReader.ts @@ -73,6 +73,7 @@ const identityResponseSchema = z.strictObject({ process_group_id: z.int().nonnegative(), }) .optional(), + name: z.string().optional(), reason: z.null().optional(), }), z.strictObject({ @@ -819,6 +820,9 @@ export const createDarwinProcessRunTokenReader = ( processGroupId: result.relation.process_group_id, }, }), + ...(result.name === undefined || result.name === "" + ? {} + : { name: result.name }), } : result.state === "absent" ? { state: "absent", reason: result.reason } diff --git a/src/process/LinuxProcessStat.test.ts b/src/process/LinuxProcessStat.test.ts index c195baf352..8f8ff7d1af 100644 --- a/src/process/LinuxProcessStat.test.ts +++ b/src/process/LinuxProcessStat.test.ts @@ -31,6 +31,7 @@ it("parses command names containing parentheses and retains native relation fact parseLinuxProcessStat(statRecord({ command: "worker ) old ) name" })), ).toEqual({ pid: 42, + name: "worker ) old ) name", startTime: "987654", lifecycle: "non-zombie", parentPid: 17, @@ -62,6 +63,7 @@ it("retains identity when Linux teardown reports unavailable group and session I ), ).toEqual({ pid: 42, + name: "worker", startTime: "987654", lifecycle: "dead", parentPid: 0, @@ -114,6 +116,7 @@ it.each(["invalid", "-1", "4.1", "9007199254740992"])( (flags) => { expect(parseLinuxProcessStat(statRecord({ flags }))).toEqual({ pid: 42, + name: "worker", startTime: "987654", lifecycle: undefined, parentPid: 17, diff --git a/src/process/LinuxProcessStat.ts b/src/process/LinuxProcessStat.ts index 02dd1aaed4..88e66e0d14 100644 --- a/src/process/LinuxProcessStat.ts +++ b/src/process/LinuxProcessStat.ts @@ -3,6 +3,8 @@ import type { ProcessLifecycle } from "./ProcessLifecycle.js"; /** Native process facts parsed from one Linux `/proc//stat` record. */ export interface LinuxProcessStat { readonly pid: number; + /** The kernel's short command name (comm), possibly truncated; display only. */ + readonly name: string; readonly startTime: string; readonly lifecycle: ProcessLifecycle | undefined; readonly parentPid: number; @@ -81,6 +83,7 @@ export const parseLinuxProcessStat = ( return { pid, + name: value.slice(commandStart + 1, commandEnd), startTime, lifecycle, parentPid, diff --git a/src/process/ProcessOwnership.nativeExit.test.ts b/src/process/ProcessOwnership.nativeExit.test.ts index 6d691ada0e..2d17c2cb8b 100644 --- a/src/process/ProcessOwnership.nativeExit.test.ts +++ b/src/process/ProcessOwnership.nativeExit.test.ts @@ -644,7 +644,8 @@ it("does not signal a stale group after its last member exits during revalidatio host, ), ).toEqual({ cleaned: true, signaled: false }); - expect(tokenReads).toBe(2); + // The unreadable pre-signal token is read once more before exit settles it. + expect(tokenReads).toBe(3); expect(signalGroup).not.toHaveBeenCalled(); }); diff --git a/src/process/ProcessOwnership.procIdentity.test.ts b/src/process/ProcessOwnership.procIdentity.test.ts index 1fe4c05369..c433fd1035 100644 --- a/src/process/ProcessOwnership.procIdentity.test.ts +++ b/src/process/ProcessOwnership.procIdentity.test.ts @@ -107,6 +107,7 @@ it("binds Linux credentials to the same native process generation", async () => { state: "readable", identity: "linux-starttime:100", + name: "fixture", effectiveUid: 1001, // Kernel ptrace checks also compare the real and saved UIDs. environmentRestriction: "credentials", @@ -175,6 +176,7 @@ it("keeps native identity when status UID is unavailable", async () => { { state: "readable", identity: "linux-starttime:100", + name: "fixture", lifecycle: { state: "observed", lifecycle: "non-zombie" }, relation: { state: "observed", diff --git a/src/process/ProcessOwnership.tokenReread.test.ts b/src/process/ProcessOwnership.tokenReread.test.ts new file mode 100644 index 0000000000..9ff7758ba6 --- /dev/null +++ b/src/process/ProcessOwnership.tokenReread.test.ts @@ -0,0 +1,408 @@ +import { expect, it, vi } from "vitest"; + +import { + cleanupOwnedProcessGroup, + type OwnedProcessGroup, + type ProcessIdentityObservation, + type ProcessOwnershipHost, + type ProcessRunTokenObservation, + type ProcessTableEntry, +} from "./ProcessOwnership.js"; +import { ProcessOwnershipEvidenceOwner } from "./ProcessOwnershipEvidence.js"; +import { TOKEN_READ_RETRY_ATTEMPTS } from "./ProcessOwnershipObservation.js"; + +const RUN_ID = "verifier-run"; +// The verifier's packaged CLI leader has exited; its child stays in the group. +const LEADER = 100; + +const member = (pid: number): ProcessTableEntry => ({ + pid, + parentPid: LEADER, + processGroupId: LEADER, + state: "S", + command: `child-${String(pid)}`, +}); + +const PATH_PADDING = "malformed_procargs_path_padding"; + +/** + * Darwin host whose members stay in the leader's group until it is signaled. + * `token` decides each member's run-token read by the 1-based batch number. + */ +const darwinHost = (options: { + readonly members: readonly number[]; + readonly token: (pid: number, batch: number) => ProcessRunTokenObservation; + readonly identity?: (pid: number, batch: number) => string | undefined; +}) => { + let signaled = false; + let tokenBatches = 0; + const tokenReads: number[][] = []; + const signalGroup = vi.fn((processGroupId: number) => { + if (processGroupId === LEADER) signaled = true; + }); + const host: ProcessOwnershipHost = { + platform: "darwin", + listProcesses: async () => (signaled ? [] : options.members.map(member)), + environment: async () => ({}), + runTokens: async (entries) => { + tokenBatches += 1; + tokenReads.push(entries.map(({ pid }) => pid)); + return new Map( + entries.map(({ pid }) => [pid, options.token(pid, tokenBatches)]), + ); + }, + processIdentities: async (entries) => + new Map( + entries.map(({ pid }) => { + const identity = + options.identity?.(pid, tokenBatches) ?? `start-${String(pid)}`; + return [ + pid, + signaled || pid === LEADER || !options.members.includes(pid) + ? { state: "absent", reason: "not-in-snapshot" } + : { + state: "readable", + identity, + name: "node", + lifecycle: { state: "observed", lifecycle: "non-zombie" }, + relation: { + state: "observed", + parentPid: LEADER, + processGroupId: LEADER, + }, + }, + ]; + }), + ), + signalGroup, + }; + return { host, signalGroup, tokenReads }; +}; + +// A verifier command sweeps with one evidence owner for its whole lifetime. +const ownership = (sweep: boolean): OwnedProcessGroup => { + const group = { runId: RUN_ID, leaderPid: LEADER, processGroupId: LEADER }; + return sweep + ? { + ...group, + sweepTokenOwnedProcesses: true, + evidenceOwner: new ProcessOwnershipEvidenceOwner(), + captureBaseline: [], + } + : group; +}; + +const owned: ProcessRunTokenObservation = { state: "readable", runId: RUN_ID }; +const unreadable: ProcessRunTokenObservation = { + state: "unavailable", + reason: PATH_PADDING, +}; + +// With a full sweep, batch 1 is the sweep and batch 2 the pre-signal group +// check. Without one, the group check reads first. +it.each([ + { sweep: false, groupCheck: 1 }, + { sweep: true, groupCheck: 2 }, +])( + "re-reads a transiently unreadable member before signaling its group: %j", + async ({ sweep, groupCheck }) => { + const { host, signalGroup, tokenReads } = darwinHost({ + members: [101], + token: (_pid, batch) => (batch === groupCheck ? unreadable : owned), + }); + + await expect( + cleanupOwnedProcessGroup(ownership(sweep), host), + ).resolves.toEqual({ cleaned: true, signaled: true }); + expect(signalGroup).toHaveBeenCalledWith(LEADER, "SIGKILL"); + expect(tokenReads[groupCheck]).toEqual([101]); + }, +); + +it.each([false, true])( + "fails closed and names a member whose token stays unreadable (sweep %s)", + async (sweep) => { + const { host, signalGroup, tokenReads } = darwinHost({ + members: [101], + token: () => unreadable, + }); + + const result = await cleanupOwnedProcessGroup(ownership(sweep), host); + + expect(result).toEqual({ + cleaned: false, + // The summary names the candidate from the native facts it carries. + reason: expect.stringContaining( + `live candidates 101=${PATH_PADDING} (name "node", ppid 100, pgid 100, identity start-101)`, + ), + failures: [ + { + pid: 101, + reason: "environment-unreadable", + diagnostic: PATH_PADDING, + identityAfter: expect.objectContaining({ + identity: "start-101", + name: "node", + relation: { + state: "observed", + parentPid: LEADER, + processGroupId: LEADER, + }, + }), + }, + ], + }); + expect(signalGroup).not.toHaveBeenCalled(); + // One read plus the bounded re-reads for the sweep and the group check. + expect(tokenReads).toHaveLength( + (1 + TOKEN_READ_RETRY_ATTEMPTS) * (sweep ? 2 : 1), + ); + }, +); + +it("accepts a member that exits while its unreadable token is re-read", async () => { + let exited = false; + const { host, signalGroup } = darwinHost({ + members: [101], + token: () => unreadable, + identity: (_pid, batch) => { + if (batch >= 1) exited = true; + return undefined; + }, + }); + const listProcesses = host.listProcesses; + const exitingHost: ProcessOwnershipHost = { + ...host, + listProcesses: async () => (exited ? [] : listProcesses()), + processIdentities: async (entries, signal, credentials) => + exited + ? new Map( + entries.map(({ pid }) => [ + pid, + { state: "absent", reason: "not-in-snapshot" } as const, + ]), + ) + : (host.processIdentities?.(entries, signal, credentials) ?? new Map()), + }; + + await expect( + cleanupOwnedProcessGroup(ownership(false), exitingHost), + ).resolves.toEqual({ cleaned: true, signaled: false }); + expect(signalGroup).not.toHaveBeenCalled(); +}); + +it.each([ + { + // The re-read's bracket straddles PID reuse; its own reads disagree. + reuse: "during the re-read", + replacement: () => (_pid: number, batch: number) => + batch >= 2 ? "start-101-reused" : undefined, + }, + { + // The PID is reused before the re-read starts, so both of its identity + // reads agree; only the first-read generation can reject its token. + reuse: "before the re-read", + replacement: () => { + let readsAfterFirstToken = 0; + return (_pid: number, batch: number) => + batch >= 1 && ++readsAfterFirstToken > 1 + ? "start-101-reused" + : undefined; + }, + }, +])( + "keeps a PID reused $reuse unresolved instead of trusting the replacement's token", + async ({ replacement }) => { + const { host, signalGroup } = darwinHost({ + members: [101], + token: (_pid, batch) => (batch === 1 ? unreadable : owned), + identity: replacement(), + }); + + await expect( + cleanupOwnedProcessGroup(ownership(false), host), + ).resolves.toMatchObject({ + cleaned: false, + failures: [{ pid: 101, reason: "process-identity-unavailable" }], + }); + expect(signalGroup).not.toHaveBeenCalled(); + }, +); + +it("keeps a PID reuse found by the first read unresolved across token re-reads", async () => { + // Linux reports the vanished token with ESRCH, and the first read's exit + // check already sees a zombie replacement. That known change must not be + // re-read into the replacement's exited facts. + let tokenReads = 0; + let identityReadsAfterToken = 0; + const signalGroup = vi.fn(); + const host: ProcessOwnershipHost = { + platform: "linux", + listProcesses: async () => [member(101)], + environment: async () => ({}), + runTokens: async (entries) => { + tokenReads += 1; + return new Map( + entries.map(({ pid }) => [ + pid, + { state: "unavailable", reason: "vanished", code: "ESRCH" } as const, + ]), + ); + }, + processIdentities: async (entries) => + new Map( + entries.map(({ pid }) => { + const replaced = tokenReads > 0 && ++identityReadsAfterToken > 1; + return [ + pid, + { + state: "readable", + identity: replaced ? "start-101-reused" : "start-101", + lifecycle: { + state: "observed", + lifecycle: replaced ? "zombie" : "non-zombie", + }, + relation: { + state: "observed", + parentPid: LEADER, + processGroupId: LEADER, + }, + }, + ]; + }), + ), + signalGroup, + }; + + await expect( + cleanupOwnedProcessGroup(ownership(false), host), + ).resolves.toMatchObject({ + cleaned: false, + failures: [{ pid: 101, reason: "process-identity-unavailable" }], + }); + expect(tokenReads).toBe(1); + expect(signalGroup).not.toHaveBeenCalled(); +}); + +it("keeps a readable member that changes groups while another member's token is re-read", async () => { + // 101 is readable and owned on the first read; 102 needs one re-read. + // While cleanup waits for 102, 101 moves to group 200 and must not escape. + const MOVED_GROUP = 200; + const groups = new Map([ + [101, LEADER], + [102, LEADER], + ]); + let tokenBatches = 0; + const signalGroup = vi.fn((processGroupId: number) => { + for (const [pid, group] of groups) + if (group === processGroupId) groups.delete(pid); + }); + const host: ProcessOwnershipHost = { + platform: "darwin", + listProcesses: async () => + [...groups].map(([pid, processGroupId]) => ({ + ...member(pid), + processGroupId, + })), + environment: async () => ({}), + runTokens: async (entries) => { + tokenBatches += 1; + if (tokenBatches === 2) groups.set(101, MOVED_GROUP); + return new Map( + entries.map(({ pid }) => [ + pid, + pid === 102 && tokenBatches === 1 ? unreadable : owned, + ]), + ); + }, + processIdentities: async (entries) => + new Map( + entries.map(({ pid }) => { + const processGroupId = groups.get(pid); + return [ + pid, + processGroupId === undefined + ? { state: "absent", reason: "not-in-snapshot" } + : { + state: "readable", + identity: `start-${String(pid)}`, + lifecycle: { state: "observed", lifecycle: "non-zombie" }, + relation: { + state: "observed", + parentPid: LEADER, + processGroupId, + }, + }, + ]; + }), + ), + signalGroup, + }; + + await expect( + cleanupOwnedProcessGroup(ownership(false), host), + ).resolves.toMatchObject({ + cleaned: false, + failures: [{ pid: 101, reason: "process-identity-unavailable" }], + }); + expect(signalGroup).not.toHaveBeenCalled(); + expect(groups.get(101)).toBe(MOVED_GROUP); +}); + +it("reports only the persistent member when group members have mixed token outcomes", async () => { + const { host, signalGroup } = darwinHost({ + members: [101, 102], + token: (pid, batch) => (pid === 102 || batch === 1 ? unreadable : owned), + }); + + const result = await cleanupOwnedProcessGroup(ownership(false), host); + + expect(result).toMatchObject({ + cleaned: false, + reason: expect.stringContaining( + `live candidates 102=${PATH_PADDING} (name "node", ppid 100, pgid 100, identity start-102)`, + ), + failures: [{ pid: 102, reason: "environment-unreadable" }], + }); + expect(result.cleaned === false && result.reason).not.toContain("101="); + expect(signalGroup).not.toHaveBeenCalled(); +}); + +it("re-reads a member whose refresh turns unreadable within the same bound", async () => { + // 102 needs a re-read; 101's own second read, its refresh, is transient. + let reads101 = 0; + const { host, signalGroup } = darwinHost({ + members: [101, 102], + token: (pid, batch) => + (pid === 102 && batch === 1) || (pid === 101 && ++reads101 === 2) + ? unreadable + : owned, + }); + + await expect( + cleanupOwnedProcessGroup(ownership(false), host), + ).resolves.toEqual({ cleaned: true, signaled: true }); + expect(signalGroup).toHaveBeenCalledWith(LEADER, "SIGKILL"); +}); + +it("signals on a cleanup retry once the retained member's token becomes readable", async () => { + let readable = false; + const { host, signalGroup } = darwinHost({ + members: [101], + token: () => (readable ? owned : unreadable), + }); + const group = ownership(true); + + await expect(cleanupOwnedProcessGroup(group, host)).resolves.toMatchObject({ + cleaned: false, + failures: [{ pid: 101, reason: "environment-unreadable" }], + }); + expect(signalGroup).not.toHaveBeenCalled(); + + readable = true; + await expect(cleanupOwnedProcessGroup(group, host)).resolves.toEqual({ + cleaned: true, + signaled: true, + }); + expect(signalGroup).toHaveBeenCalledWith(LEADER, "SIGKILL"); +}); diff --git a/src/process/ProcessOwnership.ts b/src/process/ProcessOwnership.ts index eb47356f7a..198b0dfaa6 100644 --- a/src/process/ProcessOwnership.ts +++ b/src/process/ProcessOwnership.ts @@ -7,9 +7,12 @@ import { errorMessage, readNativeProcessIdentityMap, readProcessOwnershipFacts, + reobserveUnavailableTokenFacts, settleExitingProcessIdentities, settleVanishedTokenFacts, systemProcessOwnershipHost as systemHost, + TOKEN_READ_RETRY_ATTEMPTS, + TOKEN_READ_RETRY_DELAY_MS, } from "./ProcessOwnershipObservation.js"; import { launcherIdentityFailure } from "./ProcessOwnershipIdentity.js"; import { descendantsOf, liveProcesses } from "./ProcessOwnershipProcessTree.js"; @@ -101,6 +104,11 @@ export type ProcessIdentityObservation = readonly lifecycle?: ProcessLifecycleObservation; /** Native relation facts read with this identity, never copied from a process-table row. */ readonly relation?: ProcessNativeRelationObservation; + /** + * Short native process name (Darwin p_comm, Linux comm), possibly + * truncated. It names a process in diagnostics; never identity or lookup. + */ + readonly name?: string; } | { readonly state: "absent"; @@ -221,6 +229,14 @@ interface ProcessOwnershipValidationFailure { | "run-token-mismatch" | "process-identity-unavailable"; readonly diagnostic?: string; + /** + * Native identity read after an unreadable token. It names the live + * process in the failure; it is never ownership or signal authority. + */ + readonly identityAfter?: Extract< + ProcessIdentityObservation, + { readonly state: "readable" } + >; } /** @@ -1246,8 +1262,8 @@ const cleanupValidationFailure = ( .join(", "); const candidates = unreadable .map( - ({ pid, diagnostic }) => - `${String(pid)}=${sanitizedTokenReadFailure(diagnostic)}`, + ({ pid, diagnostic, identityAfter }) => + `${String(pid)}=${sanitizedTokenReadFailure(diagnostic)}${unreadableCandidateFacts(identityAfter)}`, ) .sort((left, right) => compareUnicodeCodePoints(left, right)); return { @@ -1265,6 +1281,39 @@ const cleanupValidationFailure = ( }; }; +/** Keep the native identity that bracketed an unreadable token read. */ +const unreadableTokenFailure = ( + fact: ProcessOwnershipFact, + diagnostic: string, +): ProcessOwnershipValidationFailure => ({ + pid: fact.process.pid, + reason: "environment-unreadable", + diagnostic, + ...(fact.identityAfter.state === "readable" + ? { identityAfter: fact.identityAfter } + : {}), +}); + +/** Name an unreadable candidate from its native facts; display only. */ +const unreadableCandidateFacts = ( + identity: ProcessOwnershipValidationFailure["identityAfter"], +): string => { + if (identity === undefined) return ""; + const facts = [ + ...(identity.name === undefined + ? [] + : [`name ${JSON.stringify(identity.name)}`]), + ...(identity.relation?.state === "observed" + ? [ + `ppid ${String(identity.relation.parentPid)}`, + `pgid ${String(identity.relation.processGroupId)}`, + ] + : []), + `identity ${identity.identity}`, + ]; + return ` (${facts.join(", ")})`; +}; + const sanitizedTokenReadFailure = (diagnostic: string | undefined): string => { if (diagnostic === "environment_unavailable") return diagnostic; if (diagnostic === "platform_binary_environment_withheld") return diagnostic; @@ -1319,10 +1368,17 @@ const processOwnershipFailures = async ( }> => { const runId = ownership.runId; if (host.processIdentities !== undefined) { + // Re-read transiently unreadable tokens before signaling, as the sweep + // does; one exec-time read must not decide this group's cleanup. const facts = await settleVanishedTokenFacts( - await readProcessOwnershipFacts(members, host, undefined, { + await reobserveUnavailableTokenFacts( + await readProcessOwnershipFacts(members, host, undefined, { + exitSettlement, + }), + host, exitSettlement, - }), + { generation: "first-observed", runId }, + ), host, exitSettlement, ); @@ -1393,11 +1449,7 @@ const processOwnershipFailures = async ( continue; } if (fact.token.state === "unavailable") { - failures.push({ - pid: fact.process.pid, - reason: "environment-unreadable", - diagnostic: fact.token.reason, - }); + failures.push(unreadableTokenFailure(fact, fact.token.reason)); } else if (fact.token.runId !== runId) { failures.push({ pid: fact.process.pid, reason: "run-token-mismatch" }); } else { @@ -1496,9 +1548,6 @@ interface UnreadableTokenCandidate { readonly diagnostic: string; } -const TOKEN_READ_RETRY_ATTEMPTS = 3; -const TOKEN_READ_RETRY_DELAY_MS = 40; - /** Darwin reports this when the kernel withholds a platform binary's environment. */ const PLATFORM_BINARY_ENVIRONMENT_WITHHELD = "platform_binary_environment_withheld"; @@ -1697,30 +1746,14 @@ const scanWithOwnershipEvidence = async ( exitSettlement, }, ); - let facts = initialFacts; - for (let attempt = 0; attempt < TOKEN_READ_RETRY_ATTEMPTS; attempt += 1) { - const retryRows = facts - .filter( - ({ token, identityAfter }) => - token.state === "unavailable" && - !processIdentityHasExited(identityAfter) && - !processIdentityIsExiting(identityAfter), - ) - .map(({ process }) => process); - if (retryRows.length === 0) break; - await delay(TOKEN_READ_RETRY_DELAY_MS); - const retries = await readProcessOwnershipFacts( - retryRows, - host, - undefined, - { exitSettlement }, - ); - const retryByPid = new Map(retries.map((fact) => [fact.process.pid, fact])); - facts = facts.map((fact) => retryByPid.get(fact.process.pid) ?? fact); - } - // Recheck final token-open races at the shared native identity boundary. - facts = await settleVanishedTokenFacts(facts, host, exitSettlement); + const facts = await settleVanishedTokenFacts( + await reobserveUnavailableTokenFacts(initialFacts, host, exitSettlement, { + generation: "current", + }), + host, + exitSettlement, + ); const processGroupIds = new Set([ ...(relation === undefined ? [] : [relation.processGroupId]), @@ -1820,11 +1853,7 @@ const scanWithOwnershipEvidence = async ( fact.identityAfter.state === "readable" && fact.identityAfter.identity === retainedCandidate.identity) ) - failures.push({ - pid: fact.process.pid, - reason: "environment-unreadable", - diagnostic: fact.token.reason, - }); + failures.push(unreadableTokenFailure(fact, fact.token.reason)); } for (const candidate of retained) { diff --git a/src/process/ProcessOwnershipObservation.ts b/src/process/ProcessOwnershipObservation.ts index 10f67cd8fc..57d960e814 100644 --- a/src/process/ProcessOwnershipObservation.ts +++ b/src/process/ProcessOwnershipObservation.ts @@ -1063,6 +1063,104 @@ export const readProcessOwnershipFacts = async ( return facts; }; +/** Re-reads after a live fact's run token was unavailable. */ +export const TOKEN_READ_RETRY_ATTEMPTS = 3; +/** Delay before each re-read. */ +export const TOKEN_READ_RETRY_DELAY_MS = 40; + +/** Which process a token re-read describes; the caller owns the run token. */ +export type TokenReobservationScope = + /** A sweep classifies whichever process now holds each PID. */ + | { readonly generation: "current" } + /** A pre-signal check stays on the generation it first read. */ + | { readonly generation: "first-observed"; readonly runId: string }; + +/** + * Re-read facts whose token was unavailable while their process was neither + * exited nor exiting. Darwin KERN_PROCARGS2 samples the argument length and + * stack top separately from its address-space read, and exec publishes the + * new length before it copies the new strings out (xnu sysctl_procargsx, + * exec_copyout_strings). A read during exec can therefore see a zero-filled or + * stale strings area, and exec keeps the PID and start time that bracket the + * read. Each attempt is a complete identity/token/identity read, and a fact + * that stays unreadable keeps its observation so its caller fails closed. + * + * A pre-signal check re-reads only same-generation facts. Every attempt also + * re-reads its owned members, so a member that moves groups or turns + * unreadable while another waits is seen within the same bound. A re-read that + * sees another generation marks that fact changed, because a reused PID is + * not proof that the first generation exited. Changed, exited, exiting and + * foreign-token facts are final and never re-read. + */ +export const reobserveUnavailableTokenFacts = async ( + facts: readonly ProcessOwnershipFact[], + host: ProcessOwnershipHost, + exitSettlement: ProcessExitSettlementBudget, + scope: TokenReobservationScope, +): Promise => { + const settled = ({ identityAfter }: ProcessOwnershipFact) => + processIdentityHasExited(identityAfter) || + processIdentityIsExiting(identityAfter); + const transient = (fact: ProcessOwnershipFact) => + fact.token.state === "unavailable" && + !settled(fact) && + (scope.generation === "current" || fact.state === "same-generation"); + const reread = (fact: ProcessOwnershipFact) => + transient(fact) || + (scope.generation === "first-observed" && + fact.state === "same-generation" && + !settled(fact) && + fact.token.state === "readable" && + fact.token.runId === scope.runId); + const bind = ( + fact: ProcessOwnershipFact, + next: ProcessOwnershipFact | undefined, + ): ProcessOwnershipFact => { + if (next === undefined || scope.generation === "current") + return next ?? fact; + if (fact.identityAfter.state !== "readable") return fact; + const first = fact.identityAfter.identity; + if (next.identityAfter.state === "absent") return next; + if ( + [next.identityBefore, next.identityAfter].some( + (identity) => + identity.state === "readable" && identity.identity !== first, + ) + ) + return { + ...fact, + state: "unavailable", + identityAfter: { + state: "unavailable", + reason: "process_identity_changed_during_token_reread", + }, + }; + // An inconclusive read keeps an unreadable fact for the next attempt but + // replaces an owned one, so stale membership cannot authorize a signal. + return next.state === "same-generation" || fact.token.state === "readable" + ? next + : fact; + }; + + let current = facts; + for (let attempt = 0; attempt < TOKEN_READ_RETRY_ATTEMPTS; attempt += 1) { + if (!current.some(transient)) break; + await delay(TOKEN_READ_RETRY_DELAY_MS); + const next = new Map( + ( + await readProcessOwnershipFacts( + current.filter(reread).map(({ process }) => process), + host, + undefined, + { exitSettlement }, + ) + ).map((fact) => [fact.process.pid, fact]), + ); + current = current.map((fact) => bind(fact, next.get(fact.process.pid))); + } + return current; +}; + const isExpectedAbort = ( cause: unknown, signal: AbortSignal | undefined, @@ -1179,6 +1277,7 @@ export const createSystemProcessOwnershipHost = ( ) => ({ state: "readable" as const, identity: `linux-starttime:${facts.startTime}`, + ...(facts.name === "" ? {} : { name: facts.name }), ...(effectiveUid === undefined ? {} : { effectiveUid }), ...(environmentRestriction === undefined ? {} diff --git a/tests/boundary/process/darwinProcessRunTokenReader.test.ts b/tests/boundary/process/darwinProcessRunTokenReader.test.ts index dfd4edc423..247d3e3639 100644 --- a/tests/boundary/process/darwinProcessRunTokenReader.test.ts +++ b/tests/boundary/process/darwinProcessRunTokenReader.test.ts @@ -1389,6 +1389,7 @@ it.skipIf(!onDarwin)( zombieLifecycleIsSeparateFromIdentity: true, effectiveUidComesFromNativeIdentity: true, nativeRelationIsSeparateFromIdentity: true, + nativeNameIsSeparateFromIdentity: true, }); } finally { await rm(root, { recursive: true, force: true }); diff --git a/tests/fixtures/processRunTokenSnapshotProbe.swift b/tests/fixtures/processRunTokenSnapshotProbe.swift index 77a758d705..393809c9e8 100644 --- a/tests/fixtures/processRunTokenSnapshotProbe.swift +++ b/tests/fixtures/processRunTokenSnapshotProbe.swift @@ -45,7 +45,7 @@ struct ProcessRunTokenSnapshotProbe { let zombieIdentity = readProcessIdentities( pids: [42], snapshot: ProcessSnapshot( - metadata: [42: ProcessMetadata(identity: "start:42", effectiveUid: 501, lifecycle: "zombie", parentPid: 7, processGroupId: 9, pointerSize: 8)], + metadata: [42: ProcessMetadata(identity: "start:42", effectiveUid: 501, lifecycle: "zombie", parentPid: 7, processGroupId: 9, pointerSize: 8, name: "probe-name")], failure: nil ) ) @@ -58,7 +58,8 @@ struct ProcessRunTokenSnapshotProbe { "invalidPidIsUnavailable": invalidIdentity.first?.state == "unavailable" && invalidIdentity.first?.reason == "invalid_process_id", "zombieLifecycleIsSeparateFromIdentity": zombieIdentity.first?.state == "readable" && zombieIdentity.first?.identity == "start:42" && zombieIdentity.first?.lifecycle == "zombie", "effectiveUidComesFromNativeIdentity": zombieIdentity.first?.effective_uid == 501, - "nativeRelationIsSeparateFromIdentity": zombieIdentity.first?.relation?.parent_pid == 7 && zombieIdentity.first?.relation?.process_group_id == 9 + "nativeRelationIsSeparateFromIdentity": zombieIdentity.first?.relation?.parent_pid == 7 && zombieIdentity.first?.relation?.process_group_id == 9, + "nativeNameIsSeparateFromIdentity": zombieIdentity.first?.name == "probe-name" && absentIdentities.first?.name == nil && failedIdentities.first?.name == nil ] let data = try JSONSerialization.data(withJSONObject: output, options: [.sortedKeys]) FileHandle.standardOutput.write(data) diff --git a/tests/support/hostCleanup.ts b/tests/support/hostCleanup.ts index 443ba85817..811c065509 100644 --- a/tests/support/hostCleanup.ts +++ b/tests/support/hostCleanup.ts @@ -50,8 +50,9 @@ const expectHostCleanupReport = (report: { process.platform === "linux" ? "environment_errno_(?:EACCES|EPERM)" : "environment_unavailable"; + // Native identity facts may follow; a JSON-quoted name can hold parentheses. const candidate = - "[1-9][0-9]*=(?:environment_unavailable|environment_errno_(?:EACCES|EPERM))"; + '[1-9][0-9]*=(?:environment_unavailable|environment_errno_(?:EACCES|EPERM))(?: \\((?:[^()"]|"(?:[^"\\\\]|\\\\.)*")*\\))?'; expect(reason).toMatch( new RegExp( `^process ownership token could not be read for [1-9][0-9]* live process\\(es\\): ` +