From 4af71a45f9c740b292b0b454ba11bcc0494874b8 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:03:34 +0800 Subject: [PATCH 1/4] fix(process): reconcile native lifetime and selective Linux UID facts Settle absent and zombie candidates before retaining baseline or token-read failures. Preserve fail-closed behavior for live and unknown generations, and stop retrying exited candidates. Bind Linux effective UID to stat-bracketed status reads only for unreadable-token candidates, preserving partial identity facts and avoiding full-table UID enrichment. --- src/process/LinuxProcessStatus.test.ts | 21 +++ src/process/LinuxProcessStatus.ts | 19 +++ src/process/ProcessOwnership.identity.test.ts | 21 ++- .../ProcessOwnership.procIdentity.test.ts | 148 +++++++++++++++++- .../ProcessOwnership.sweepExit.test.ts | 74 +++++++++ src/process/ProcessOwnership.ts | 63 ++++---- src/process/ProcessOwnershipObservation.ts | 112 ++++++++++--- 7 files changed, 394 insertions(+), 64 deletions(-) create mode 100644 src/process/LinuxProcessStatus.test.ts create mode 100644 src/process/LinuxProcessStatus.ts create mode 100644 src/process/ProcessOwnership.sweepExit.test.ts diff --git a/src/process/LinuxProcessStatus.test.ts b/src/process/LinuxProcessStatus.test.ts new file mode 100644 index 0000000000..46f7688447 --- /dev/null +++ b/src/process/LinuxProcessStatus.test.ts @@ -0,0 +1,21 @@ +import { expect, it } from "vitest"; + +import { parseLinuxProcessEffectiveUid } from "./LinuxProcessStatus.js"; + +it("parses the effective UID from the single native status Uid row", () => { + expect( + parseLinuxProcessEffectiveUid( + "Name:\tworker\nUid:\t1000\t1001\t1002\t1003\nGid:\t20\t20\t20\t20\n", + ), + ).toBe(1001); +}); + +it.each([ + "Name:\tworker\n", + "Uid:\t1000\t1001\t1002\n", + "Uid:\t1000\t1001\t1002\t1003\nUid:\t1000\t1001\t1002\t1003\n", + "Uid:\t1000\t-1\t1002\t1003\n", + "Uid:\t1000\t9007199254740992\t1002\t1003\n", +])("does not infer an effective UID from malformed status: %s", (status) => { + expect(parseLinuxProcessEffectiveUid(status)).toBeUndefined(); +}); diff --git a/src/process/LinuxProcessStatus.ts b/src/process/LinuxProcessStatus.ts new file mode 100644 index 0000000000..60d8f2809e --- /dev/null +++ b/src/process/LinuxProcessStatus.ts @@ -0,0 +1,19 @@ +/** Effective UID parsed from one Linux `/proc//status` record. */ +export const parseLinuxProcessEffectiveUid = ( + value: string, +): number | undefined => { + let effectiveUid: number | undefined; + for (const line of value.split("\n")) { + if (!line.startsWith("Uid:")) continue; + if (effectiveUid !== undefined) return undefined; + const match = /^Uid:\s+([0-9]+)\s+([0-9]+)\s+([0-9]+)\s+([0-9]+)\s*$/u.exec( + line, + ); + if (match === null) return undefined; + const values = match.slice(1).map(Number); + if (values.some((uid) => !Number.isSafeInteger(uid) || uid < 0)) + return undefined; + effectiveUid = values[1]; + } + return effectiveUid; +}; diff --git a/src/process/ProcessOwnership.identity.test.ts b/src/process/ProcessOwnership.identity.test.ts index 1d518fda2c..5b5c69f988 100644 --- a/src/process/ProcessOwnership.identity.test.ts +++ b/src/process/ProcessOwnership.identity.test.ts @@ -32,7 +32,11 @@ describe("Linux process start identities", () => { state: "S", command: process.execPath, } satisfies ProcessTableEntry; - const identities = await host.processIdentities?.([observedProcess]); + const identities = await host.processIdentities?.( + [observedProcess], + undefined, + new Set([observedProcess.pid]), + ); expect(identities?.get(observedProcess.pid)).toMatchObject({ state: "readable", identity: expect.stringMatching(/^linux-starttime:\d+$/u), @@ -43,8 +47,9 @@ describe("Linux process start identities", () => { processGroupId: expect.any(Number), }, }); - expect(identities?.get(observedProcess.pid)).not.toHaveProperty( + expect(identities?.get(observedProcess.pid)).toHaveProperty( "effectiveUid", + process.geteuid!(), ); const baseline = await host.captureBaseline?.(); expect(baseline).toContainEqual({ @@ -110,9 +115,11 @@ it("validates and cleans an exact-token owner without a capture baseline", async expect(signalGroup).toHaveBeenCalledWith(process.processGroupId, "SIGKILL"); }); -it.skipIf(typeof process.getuid !== "function")( - "uses a same-generation native UID instead of the process-table UID for unreadable tokens", - async () => { +it + .skipIf(typeof process.getuid !== "function") + .each(["EINVAL", "EACCES: permission denied"])( + "uses a same-generation native UID instead of the process-table UID for unreadable tokens: %s", + async (diagnostic) => { const callerUid = process.getuid!(); const foreignUid = callerUid === 0 ? 1 : 0; const verify = async (rowUid: number, effectiveUid: number) => { @@ -132,7 +139,7 @@ it.skipIf(typeof process.getuid !== "function")( runTokens: () => Promise.resolve( new Map([ - [906, { state: "unavailable", reason: "EINVAL" } as const], + [906, { state: "unavailable", reason: diagnostic } as const], ]), ), processIdentities: (entries) => @@ -161,7 +168,7 @@ it.skipIf(typeof process.getuid !== "function")( }); await expect(verify(callerUid, foreignUid)).resolves.toMatchObject({ cleaned: true, - unverified: [{ pid: 906, diagnostic: "EINVAL" }], + unverified: [{ pid: 906, diagnostic }], }); }, ); diff --git a/src/process/ProcessOwnership.procIdentity.test.ts b/src/process/ProcessOwnership.procIdentity.test.ts index 6b2eaa3eb6..fff53db724 100644 --- a/src/process/ProcessOwnership.procIdentity.test.ts +++ b/src/process/ProcessOwnership.procIdentity.test.ts @@ -9,8 +9,14 @@ vi.mock("node:fs/promises", async (importOriginal) => { return { ...original, readFile: filesystem.readFile }; }); -import { createSystemProcessOwnershipHost } from "./ProcessOwnershipObservation.js"; -import type { ProcessTableEntry } from "./ProcessOwnership.js"; +import { + createSystemProcessOwnershipHost, + readProcessOwnershipFacts, +} from "./ProcessOwnershipObservation.js"; +import type { + ProcessOwnershipHost, + ProcessTableEntry, +} from "./ProcessOwnership.js"; const entry: ProcessTableEntry = { pid: 42, @@ -23,6 +29,144 @@ const entry: ProcessTableEntry = { const filesystemError = (code: string): NodeJS.ErrnoException => Object.assign(new Error(code), { code }); +const procStat = ( + startTime: number, + parentPid = 1, + processGroupId = 42, + pid = 42, +) => { + const fields = ["S", String(parentPid), String(processGroupId), "42"]; + while (fields.length < 19) fields.push("0"); + fields.push(String(startTime)); + return `${String(pid)} (fixture) ${fields.join(" ")}`; +}; + +it("binds Linux effective UID to the same native process generation", async () => { + const host = createSystemProcessOwnershipHost("linux", {}); + filesystem.readFile + .mockResolvedValueOnce(procStat(100, 10, 42)) + .mockResolvedValueOnce("Name:\tfixture\nUid:\t1000\t1001\t1002\t1003\n") + .mockResolvedValueOnce(procStat(100, 11, 43)); + + await expect( + host.processIdentities?.([entry], undefined, new Set([entry.pid])), + ).resolves.toEqual( + new Map([ + [ + 42, + { + state: "readable", + identity: "linux-starttime:100", + effectiveUid: 1001, + lifecycle: { state: "observed", lifecycle: "non-zombie" }, + relation: { + state: "observed", + parentPid: 11, + processGroupId: 43, + }, + }, + ], + ]), + ); + expect(filesystem.readFile.mock.calls.map(([path]) => path)).toEqual([ + "/proc/42/stat", + "/proc/42/status", + "/proc/42/stat", + ]); +}); + +it("keeps native identity when status UID is unavailable", async () => { + const host = createSystemProcessOwnershipHost("linux", {}); + filesystem.readFile + .mockResolvedValueOnce(procStat(100)) + .mockRejectedValueOnce(filesystemError("EACCES")); + + await expect( + host.processIdentities?.([entry], undefined, new Set([entry.pid])), + ).resolves.toEqual( + new Map([ + [ + 42, + { + state: "readable", + identity: "linux-starttime:100", + lifecycle: { state: "observed", lifecycle: "non-zombie" }, + relation: { + state: "observed", + parentPid: 1, + processGroupId: 42, + }, + }, + ], + ]), + ); + expect(filesystem.readFile).toHaveBeenCalledTimes(2); +}); + +it("does not attach a UID when the process generation changes around status", async () => { + const host = createSystemProcessOwnershipHost("linux", {}); + filesystem.readFile + .mockResolvedValueOnce(procStat(100)) + .mockResolvedValueOnce("Uid:\t1000\t1001\t1002\t1003\n") + .mockResolvedValueOnce(procStat(101)); + + await expect( + host.processIdentities?.([entry], undefined, new Set([entry.pid])), + ).resolves.toEqual( + new Map([ + [ + 42, + { + state: "unavailable", + reason: "process_identity_changed_during_uid_observation", + }, + ], + ]), + ); +}); + +it("reads Linux effective UID only for token-unavailable rows", async () => { + const rows = [entry, { ...entry, pid: 43, processGroupId: 43 }]; + const statReads = new Map(); + const uidStatusPids: number[] = []; + filesystem.readFile.mockImplementation(async (path) => { + const match = /^\/proc\/(\d+)\/(stat|status)$/u.exec(String(path)); + if (match === null) throw new Error(`unexpected path: ${String(path)}`); + const pid = Number(match[1]); + if (match[2] === "status") { + uidStatusPids.push(pid); + return "Uid:\t1000\t1001\t1002\t1003\n"; + } + const reads = (statReads.get(pid) ?? 0) + 1; + statReads.set(pid, reads); + return procStat(100, 1, pid, pid); + }); + const systemHost = createSystemProcessOwnershipHost("linux", {}); + const host: ProcessOwnershipHost = { + ...systemHost, + runTokens: async () => + new Map([ + [entry.pid, { state: "unavailable", reason: "EACCES" }], + [43, { state: "readable", runId: undefined }], + ]), + }; + + const facts = await readProcessOwnershipFacts(rows, host); + + expect(uidStatusPids).toEqual([entry.pid]); + expect(facts.map(({ identityAfter }) => identityAfter)).toMatchObject([ + { state: "readable", effectiveUid: 1001 }, + { state: "readable" }, + ]); + expect(facts[1]?.identityAfter).not.toHaveProperty("effectiveUid"); + expect(statReads).toEqual( + new Map([ + [entry.pid, 3], + [43, 2], + ]), + ); +}); + it("reports absence only when /proc lookup and signal-zero both report ESRCH", async () => { const host = createSystemProcessOwnershipHost("linux", {}); const signalProbe = vi.spyOn(process, "kill").mockImplementation(() => { diff --git a/src/process/ProcessOwnership.sweepExit.test.ts b/src/process/ProcessOwnership.sweepExit.test.ts new file mode 100644 index 0000000000..fb0b9b2f84 --- /dev/null +++ b/src/process/ProcessOwnership.sweepExit.test.ts @@ -0,0 +1,74 @@ +import { expect, it, vi } from "vitest"; +import { + verifyNoTokenOwnedProcesses, + type ProcessIdentityObservation, + type ProcessOwnershipHost, + type ProcessTableEntry, +} from "./ProcessOwnership.js"; +import { ProcessOwnershipEvidenceOwner } from "./ProcessOwnershipEvidence.js"; + +it.each(["absent", "zombie", "live", "unavailable"] as const)( + "settles a baseline candidate against its final native lifetime: %s", + async (lifetime) => { + const row: ProcessTableEntry = { + pid: 910, + parentPid: 1, + processGroupId: 910, + state: "S", + command: "baseline-neighbor", + }; + let tokenRead = false; + const identity = (): ProcessIdentityObservation => { + if (lifetime === "absent") + return { state: "absent", reason: "not-in-snapshot" }; + if (lifetime === "unavailable") + return { state: "unavailable", reason: "native identity denied" }; + return { + state: "readable", + identity: "new-generation", + lifecycle: { + state: "observed", + lifecycle: + tokenRead && lifetime === "zombie" ? "zombie" : "non-zombie", + }, + relation: { state: "observed", parentPid: 1, processGroupId: 910 }, + }; + }; + const signalGroup = vi.fn(); + const host: ProcessOwnershipHost = { + platform: "darwin", + listProcesses: async () => [row], + processIdentities: async (members) => + new Map(members.map(({ pid }) => [pid, identity()])), + runTokens: async (members) => { + tokenRead = true; + return new Map( + members.map(({ pid }) => [ + pid, + { state: "unavailable" as const, reason: "EACCES" }, + ]), + ); + }, + environment: async () => ({}), + signalGroup, + }; + const result = await verifyNoTokenOwnedProcesses( + "run", + host, + [{ pid: row.pid, identity: "baseline-generation" }], + { + leaderPid: 100, + processGroupId: 100, + evidenceOwner: new ProcessOwnershipEvidenceOwner(), + }, + ); + if (lifetime === "absent" || lifetime === "zombie") + expect(result).toEqual({ cleaned: true, signaled: false }); + else + expect(result).toMatchObject({ + cleaned: false, + failures: [{ pid: row.pid }], + }); + expect(signalGroup).not.toHaveBeenCalled(); + }, +); diff --git a/src/process/ProcessOwnership.ts b/src/process/ProcessOwnership.ts index 3cc3d6ee22..307b20220c 100644 --- a/src/process/ProcessOwnership.ts +++ b/src/process/ProcessOwnership.ts @@ -179,6 +179,8 @@ export interface ProcessOwnershipHost { processIdentities?( processes: readonly ProcessTableEntry[], signal?: AbortSignal, + /** Linux UID enrichment is selective; omitted means UID remains unknown. */ + effectiveUidPids?: ReadonlySet, ): Promise>; /** Snapshot identities before launch, honoring startup cancellation. */ captureBaseline?(signal?: AbortSignal): Promise; @@ -965,6 +967,14 @@ const sanitizedTokenReadFailure = (diagnostic: string | undefined): string => { return "other_unavailable"; }; +const processIdentityHasExited = ( + identity: ProcessIdentityObservation, +): boolean => + identity.state === "absent" || + (identity.state === "readable" && + identity.lifecycle?.state === "observed" && + identity.lifecycle.lifecycle === "zombie"); + const processOwnershipFailures = async ( members: readonly ProcessTableEntry[], runId: string, @@ -975,13 +985,7 @@ const processOwnershipFailures = async ( const facts = await readProcessOwnershipFacts(members, host); const failures: ProcessOwnershipValidationFailure[] = []; for (const fact of facts) { - if (fact.identityAfter.state === "absent") continue; - if ( - fact.identityAfter.state === "readable" && - fact.identityAfter.lifecycle?.state === "observed" && - fact.identityAfter.lifecycle.lifecycle === "zombie" - ) - continue; + if (processIdentityHasExited(fact.identityAfter)) continue; const relation = fact.identityAfter.state === "readable" && fact.identityAfter.relation?.state === "observed" @@ -1289,14 +1293,17 @@ const scanWithOwnershipEvidence = async ( observationRows, host, ); - const baselineIdentityFailures: ProcessOwnershipValidationFailure[] = []; + const baselineIdentityFailures = new Map< + number, + ProcessOwnershipValidationFailure + >(); const candidates = observationRows.filter((process) => { if (retainedPids.has(process.pid)) return true; if (!baseline.has(process.pid)) return true; const before = baseline.get(process.pid); const now = firstIdentities.get(process.pid); if (before === null || now?.state !== "readable") { - baselineIdentityFailures.push({ + baselineIdentityFailures.set(process.pid, { pid: process.pid, reason: "process-identity-unavailable", diagnostic: "process identity was unavailable during token validation", @@ -1320,21 +1327,18 @@ const scanWithOwnershipEvidence = async ( }, ); let facts = initialFacts; - for ( - let attempt = 0; - attempt < TOKEN_READ_RETRY_ATTEMPTS && - facts.some(({ token }) => token.state === "unavailable"); - attempt += 1 - ) { + for (let attempt = 0; attempt < TOKEN_READ_RETRY_ATTEMPTS; attempt += 1) { + const retryRows = facts + .filter( + ({ token, identityAfter }) => + token.state === "unavailable" && + !processIdentityHasExited(identityAfter), + ) + .map(({ process }) => process); + if (retryRows.length === 0) break; await new Promise((resolve) => setTimeout(resolve, TOKEN_READ_RETRY_DELAY_MS), ); - const pendingPids = new Set( - facts - .filter(({ token }) => token.state === "unavailable") - .map(({ process }) => process.pid), - ); - const retryRows = candidates.filter(({ pid }) => pendingPids.has(pid)); const retries = await readProcessOwnershipFacts(retryRows, host); const retryByPid = new Map(retries.map((fact) => [fact.process.pid, fact])); facts = facts.map((fact) => retryByPid.get(fact.process.pid) ?? fact); @@ -1356,24 +1360,26 @@ const scanWithOwnershipEvidence = async ( const retainedByPid = new Map( retained.map((candidate) => [candidate.pid, candidate]), ); - const failures: ProcessOwnershipValidationFailure[] = [ - ...baselineIdentityFailures, - ]; + const failures: ProcessOwnershipValidationFailure[] = []; const unverified: OwnershipSweepUnverifiedProcess[] = []; const owned: ProcessTableEntry[] = []; const identities = new Map(); for (const fact of facts) { + // The table row and baseline predate the final native observation. An + // exited candidate needs no ownership proof or signal, even when its + // token became unreadable while exiting. Unknown native state stays a + // cleanup failure; table omission alone is never proof of exit. + if (processIdentityHasExited(fact.identityAfter)) continue; + const baselineFailure = baselineIdentityFailures.get(fact.process.pid); + if (baselineFailure !== undefined) failures.push(baselineFailure); if (fact.state === "replaced") failures.push({ pid: fact.process.pid, reason: "process-identity-unavailable", diagnostic: "process identity changed during token validation", }); - else if ( - fact.state === "unavailable" && - fact.identityAfter.state !== "absent" - ) + else if (fact.state === "unavailable") failures.push({ pid: fact.process.pid, reason: "process-identity-unavailable", @@ -1392,7 +1398,6 @@ const scanWithOwnershipEvidence = async ( reason: "run-token-mismatch", }); if (fact.token.state !== "unavailable") continue; - if (fact.identityAfter.state === "absent") continue; const relationBefore = fact.identityBefore.state === "readable" ? fact.identityBefore.relation diff --git a/src/process/ProcessOwnershipObservation.ts b/src/process/ProcessOwnershipObservation.ts index 592833855d..01439da94f 100644 --- a/src/process/ProcessOwnershipObservation.ts +++ b/src/process/ProcessOwnershipObservation.ts @@ -1,5 +1,6 @@ import { readFile } from "node:fs/promises"; import { parseLinuxProcessStat } from "./LinuxProcessStat.js"; +import { parseLinuxProcessEffectiveUid } from "./LinuxProcessStatus.js"; import { AnalysisCapabilityUnavailableError } from "../domain/analysisErrorCore.js"; import { launcherIdentityFailure } from "./ProcessOwnershipIdentity.js"; @@ -498,6 +499,7 @@ const readIdentityFacts = async ( host: ProcessOwnershipHost, phase: "before" | "after", signal?: AbortSignal, + effectiveUidPids?: ReadonlySet, ): Promise> => { signal?.throwIfAborted(); if (host.processIdentities === undefined) @@ -511,10 +513,11 @@ const readIdentityFacts = async ( ]), ); try { - const observations = - signal === undefined - ? await host.processIdentities(processes) - : await host.processIdentities(processes, signal); + const observations = await host.processIdentities( + processes, + signal, + effectiveUidPids, + ); signal?.throwIfAborted(); return new Map( processes.map(({ pid }) => [pid, identityFact(observations, pid, phase)]), @@ -583,11 +586,17 @@ export const readProcessOwnershipFacts = async ( } signal?.throwIfAborted(); + const effectiveUidPids = new Set( + processes + .filter(({ pid }) => tokens.get(pid)?.state !== "readable") + .map(({ pid }) => pid), + ); const identityAfter = await readIdentityFacts( processes, host, "after", signal, + effectiveUidPids, ); signal?.throwIfAborted(); const facts = processes.map((process): ProcessOwnershipFact => { @@ -699,7 +708,7 @@ export const createSystemProcessOwnershipHost = ( }; const processIdentities: NonNullable< ProcessOwnershipHost["processIdentities"] - > = async (processes, signal) => { + > = async (processes, signal, effectiveUidPids) => { if (processes.length === 0) return new Map(); if (platform === "linux") return new Map( @@ -716,27 +725,78 @@ export const createSystemProcessOwnershipHost = ( ...(signal === undefined ? {} : { signal }), }); const parsed = parseLinuxProcessStat(stat); - return [ - pid, - parsed === undefined || parsed.pid !== pid - ? ({ - state: "unavailable", - reason: "malformed_proc_stat", - } as const) - : ({ - state: "readable", - identity: `linux-starttime:${parsed.startTime}`, - lifecycle: { - state: "observed", - lifecycle: parsed.lifecycle, - }, - relation: { - state: "observed", - parentPid: parsed.parentPid, - processGroupId: parsed.processGroupId, - }, - } as const), - ] as const; + if (parsed === undefined || parsed.pid !== pid) + return [ + pid, + { + state: "unavailable", + reason: "malformed_proc_stat", + } as const, + ] as const; + + const readableIdentity = ( + facts: typeof parsed, + effectiveUid?: number, + ) => ({ + state: "readable" as const, + identity: `linux-starttime:${facts.startTime}`, + ...(effectiveUid === undefined ? {} : { effectiveUid }), + lifecycle: { + state: "observed" as const, + lifecycle: facts.lifecycle, + }, + relation: { + state: "observed" as const, + parentPid: facts.parentPid, + processGroupId: facts.processGroupId, + }, + }); + + if (!effectiveUidPids?.has(pid)) + return [pid, readableIdentity(parsed)] as const; + + let status: string; + try { + status = await readFile(`/proc/${String(pid)}/status`, { + encoding: "utf8", + ...(signal === undefined ? {} : { signal }), + }); + } catch (cause: unknown) { + if (isExpectedAbort(cause, signal)) throw cause; + return [pid, readableIdentity(parsed)] as const; + } + signal?.throwIfAborted(); + const effectiveUid = parseLinuxProcessEffectiveUid(status); + if (effectiveUid === undefined) + return [pid, readableIdentity(parsed)] as const; + + let afterStatus: string; + try { + afterStatus = await readFile(`/proc/${String(pid)}/stat`, { + encoding: "utf8", + ...(signal === undefined ? {} : { signal }), + }); + } catch (cause: unknown) { + if (isExpectedAbort(cause, signal)) throw cause; + return [pid, readableIdentity(parsed)] as const; + } + signal?.throwIfAborted(); + const verified = parseLinuxProcessStat(afterStatus); + if ( + verified !== undefined && + verified.pid === pid && + verified.startTime !== parsed.startTime + ) + return [ + pid, + { + state: "unavailable", + reason: "process_identity_changed_during_uid_observation", + } as const, + ] as const; + if (verified === undefined || verified.pid !== pid) + return [pid, readableIdentity(parsed)] as const; + return [pid, readableIdentity(verified, effectiveUid)] as const; } catch (cause: unknown) { if (isExpectedAbort(cause, signal)) throw cause; if ( From 0949313d8fa222dfd48d33828e19924d55775981 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:03:45 +0800 Subject: [PATCH 2/4] fix(schema): compose portable stack-safe base64 constraints Replace the lookahead-based pattern with separate allOf/not JSON Schema rules for quartet length, alphabet, padding and canonical pad bits. Share the runtime predicates with HAR and padded base64 parsing. Validate converted advertised leaves with the MCP SDK validator, preserve permissive versus canonical semantics, and exercise every pattern near the 32 MiB encoded-input budget in all JavaScript modes. --- src/browser/history/HarCapture.ts | 4 +- src/domain/go/goBinary.ts | 10 +-- src/domain/native/binaryLayout.ts | 4 +- src/domain/native/recordedCrash.ts | 4 +- src/domain/paddedBase64.ts | 5 +- src/domain/stringPatterns.test.ts | 80 ++++++++++++++----- src/domain/stringPatterns.ts | 48 ++++++++--- .../mcp/advertisedPatternsVMode.test.ts | 66 +++++++++++++++ 8 files changed, 174 insertions(+), 47 deletions(-) diff --git a/src/browser/history/HarCapture.ts b/src/browser/history/HarCapture.ts index 9e164e92d9..ec55644d0e 100644 --- a/src/browser/history/HarCapture.ts +++ b/src/browser/history/HarCapture.ts @@ -2,7 +2,7 @@ import { createHash } from "node:crypto"; import { LosslessNumber } from "lossless-json"; import { parseCaptureJson } from "./CaptureJson.js"; import type { JsonValue } from "../../domain/jsonValue.js"; -import { CANONICAL_BASE64_PATTERN } from "../../domain/stringPatterns.js"; +import { isCanonicalBase64 } from "../../domain/stringPatterns.js"; import { WEB_NETWORK_CAPTURE_LIMITS, type WebNetworkCapture, @@ -265,7 +265,7 @@ const integerLexeme = (literal: string): boolean => { const decodeHarBase64 = (value: string, pointer: string): Buffer => { const compact = value.replace(/[\t\n\r ]/g, ""); - if (compact.length % 4 !== 0 || !CANONICAL_BASE64_PATTERN.test(compact)) + if (!isCanonicalBase64(compact)) throw new CaptureFormatError( "format", "HAR content declares base64 but its text is not valid canonical base64 (ASCII whitespace is allowed).", diff --git a/src/domain/go/goBinary.ts b/src/domain/go/goBinary.ts index 8714159c38..550d7a8348 100644 --- a/src/domain/go/goBinary.ts +++ b/src/domain/go/goBinary.ts @@ -1,7 +1,7 @@ import { z } from "zod"; import { digestSchema } from "../digests.js"; import { localPathStringSchema } from "../localPath.js"; -import { CANONICAL_BASE64_PATTERN } from "../stringPatterns.js"; +import { canonicalBase64Schema } from "../stringPatterns.js"; const sourceRangeSchema = z.strictObject({ offset: z.number().int().nonnegative(), @@ -25,9 +25,7 @@ export const goModuleMetadataSchema = z.strictObject({ dependencies: z.array(goModuleSchema), settings: z.array(z.strictObject({ key: z.string(), value: z.string() })), unparsed_lines: z.array(z.string()), - unparsed_line_bytes_base64: z.array( - z.string().regex(CANONICAL_BASE64_PATTERN), - ), + unparsed_line_bytes_base64: z.array(canonicalBase64Schema), complete: z.boolean(), }); @@ -43,9 +41,9 @@ export const goBuildInfoSchema = z.strictObject({ header_offset: z.number().int().nonnegative(), encoding: z.enum(["inline", "pointer"]), go_version: z.string().nullable(), - go_version_bytes_base64: z.string().regex(CANONICAL_BASE64_PATTERN), + go_version_bytes_base64: canonicalBase64Schema, module_text: z.string().nullable(), - module_bytes_base64: z.string().regex(CANONICAL_BASE64_PATTERN), + module_bytes_base64: canonicalBase64Schema, version_location: sourceRangeSchema, module_location: sourceRangeSchema, module: goModuleMetadataSchema, diff --git a/src/domain/native/binaryLayout.ts b/src/domain/native/binaryLayout.ts index cc37376dd9..c1a694cc2d 100644 --- a/src/domain/native/binaryLayout.ts +++ b/src/domain/native/binaryLayout.ts @@ -2,13 +2,13 @@ import { z } from "zod"; import { canBorrowParsedJsonSnapshot } from "../immutableJson.js"; import { localPathStringSchema } from "../localPath.js"; -import { CANONICAL_BASE64_PATTERN } from "../stringPatterns.js"; +import { canonicalBase64Schema } from "../stringPatterns.js"; const unsignedHex = z.string().regex(/^0x(?:0|[1-9a-f][0-9a-f]{0,15})$/); const index = z.number().int().nonnegative(); const scalar = z.union([z.string(), z.number().int()]); const range = z.strictObject({ offset: unsignedHex, bytes: unsignedHex }); -const canonicalBase64 = z.string().regex(CANONICAL_BASE64_PATTERN); +const canonicalBase64 = canonicalBase64Schema; const name = z.strictObject({ display: z.string(), bytes_base64: canonicalBase64.nullable(), diff --git a/src/domain/native/recordedCrash.ts b/src/domain/native/recordedCrash.ts index 8bd04cac14..0163a85329 100644 --- a/src/domain/native/recordedCrash.ts +++ b/src/domain/native/recordedCrash.ts @@ -2,13 +2,13 @@ import { z } from "zod"; import type { BinaryArchitecture } from "../binaryTargetTypes.js"; import { localPathStringSchema } from "../localPath.js"; -import { CANONICAL_BASE64_PATTERN } from "../stringPatterns.js"; +import { canonicalBase64Schema } from "../stringPatterns.js"; import { recordedNoteCoverageIssues } from "./recordedCrashNoteCoverage.js"; const hex = z.string().regex(/^0x(?:0|[1-9a-f][0-9a-f]{0,15})$/); const index = z.number().int().nonnegative(); const signed32 = z.number().int().min(-2147483648).max(2147483647); -const base64 = z.string().regex(CANONICAL_BASE64_PATTERN); +const base64 = canonicalBase64Schema; const range = z.strictObject({ offset: hex, bytes: hex }); const diagnostics = z.strictObject({ stdout: z.string(), diff --git a/src/domain/paddedBase64.ts b/src/domain/paddedBase64.ts index 395bdf5b8e..b4402f6b5e 100644 --- a/src/domain/paddedBase64.ts +++ b/src/domain/paddedBase64.ts @@ -1,9 +1,8 @@ -import { PADDED_BASE64_PATTERN } from "./stringPatterns.js"; +import { isPaddedBase64 } from "./stringPatterns.js"; /** Validate padded syntax without imposing canonical zero pad bits. */ export const paddedBase64ByteLength = (value: string): number | undefined => { - if (value.length % 4 !== 0 || !PADDED_BASE64_PATTERN.test(value)) - return undefined; + if (!isPaddedBase64(value)) return undefined; const padding = value.endsWith("==") ? 2 : value.endsWith("=") ? 1 : 0; return (value.length / 4) * 3 - padding; }; diff --git a/src/domain/stringPatterns.test.ts b/src/domain/stringPatterns.test.ts index 7537742bd3..4410ad7b95 100644 --- a/src/domain/stringPatterns.test.ts +++ b/src/domain/stringPatterns.test.ts @@ -1,10 +1,16 @@ import { expect, it } from "vitest"; +import { z } from "zod"; import { - CANONICAL_BASE64_PATTERN, + BASE64_QUARTET_LENGTH_PATTERN, + INVALID_BASE64_CHARACTER_PATTERN, + INVALID_BASE64_PADDING_PATTERN, + INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN, IDENTIFIER_PATTERN, - PADDED_BASE64_PATTERN, STABLE_IDENTIFIER_PATTERN, + canonicalBase64Schema, + isCanonicalBase64, + isPaddedBase64, } from "./stringPatterns.js"; it("preserves identifier grammars under Unicode and Unicode-set compilation", () => { @@ -43,7 +49,7 @@ it("preserves identifier grammars under Unicode and Unicode-set compilation", () } }); -it("preserves canonical base64 padding and pad-bit validation", () => { +it("preserves canonical and permissive padded base64 validation", () => { const previous = /^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/][AQgw]==|[A-Za-z0-9+/]{2}[AEIMQUYcgkosw048]=)?$/; const samples = [ @@ -71,28 +77,62 @@ it("preserves canonical base64 padding and pad-bit validation", () => { for (const first of alphabet) for (const second of alphabet) samples.push(`${first}${second}==`, `A${first}${second}=`); - for (const flag of ["u", "v"]) { - const projected = new RegExp(CANONICAL_BASE64_PATTERN.source, flag); - for (const sample of samples) - expect(projected.test(sample), JSON.stringify({ flag, sample })).toBe( - previous.test(sample), - ); + for (const sample of samples) { + expect(isCanonicalBase64(sample), JSON.stringify({ sample })).toBe( + previous.test(sample), + ); + expect(isPaddedBase64(sample), JSON.stringify({ sample })).toBe( + sample.length % 4 === 0 && /^[A-Za-z0-9+/]*(?:==|=)?$/.test(sample), + ); } }); -it("validates large canonical base64 under JSON Schema Unicode modes", () => { - // Near the 32 MiB capture input budget: a quartet expansion alone still - // overflows at this size, especially in Unicode-set client validators. +it("advertises every portable base64 constraint and keeps large inputs stack-safe", () => { + const jsonSchema = z.toJSONSchema(canonicalBase64Schema, { + target: "draft-2020-12", + }); + expect(jsonSchema).toMatchObject({ + allOf: expect.arrayContaining([ + { pattern: BASE64_QUARTET_LENGTH_PATTERN.source }, + { not: { pattern: INVALID_BASE64_CHARACTER_PATTERN.source } }, + { not: { pattern: INVALID_BASE64_PADDING_PATTERN.source } }, + { not: { pattern: INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN.source } }, + ]), + }); + const body = Buffer.alloc(24 * 1024 * 1024 - 3, 0x81); const encoded = body.toString("base64"); for (const flag of ["", "u", "v"]) { - const pattern = new RegExp(CANONICAL_BASE64_PATTERN.source, flag); - expect(pattern.test(encoded), flag).toBe(true); - expect(pattern.test(`${encoded.slice(0, -1)}!`), flag).toBe(false); - expect(pattern.test(`${encoded.slice(0, -3)}R==`), flag).toBe(false); - expect(pattern.test(`${encoded}A`), flag).toBe(false); - const padded = new RegExp(PADDED_BASE64_PATTERN.source, flag); - expect(padded.test(encoded), flag).toBe(true); - expect(padded.test(`${encoded.slice(0, -1)}!`), flag).toBe(false); + expect( + new RegExp(BASE64_QUARTET_LENGTH_PATTERN.source, flag).test(encoded), + flag, + ).toBe(true); + expect( + new RegExp(INVALID_BASE64_CHARACTER_PATTERN.source, flag).test(encoded), + flag, + ).toBe(false); + expect( + new RegExp(INVALID_BASE64_PADDING_PATTERN.source, flag).test(encoded), + flag, + ).toBe(false); + expect( + new RegExp(INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN.source, flag).test( + encoded, + ), + flag, + ).toBe(false); + expect( + new RegExp(INVALID_BASE64_CHARACTER_PATTERN.source, flag).test( + `${encoded.slice(0, -1)}!`, + ), + flag, + ).toBe(true); + expect( + new RegExp(INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN.source, flag).test( + `${encoded.slice(0, -3)}R==`, + ), + flag, + ).toBe(true); } + expect(isCanonicalBase64(encoded)).toBe(true); }); diff --git a/src/domain/stringPatterns.ts b/src/domain/stringPatterns.ts index ab8994f17f..718cc1d1d4 100644 --- a/src/domain/stringPatterns.ts +++ b/src/domain/stringPatterns.ts @@ -1,20 +1,44 @@ +import { z } from "zod"; + /** Portable ASCII identifier starting with a letter. */ export const IDENTIFIER_PATTERN = /^[A-Za-z][A-Za-z0-9._\x2d]*$/u; /** Stable evidence identifier, also admitting namespace and path separators. */ export const STABLE_IDENTIFIER_PATTERN = /^[A-Za-z][A-Za-z0-9._:\x2f\x2d]*$/u; -// Repeated alphabet quartets exhaust V8's regexp stack on large bodies, -// including under JSON Schema's Unicode-set mode. Check alphabet and terminal -// padding separately, leaving an unconstrained fixed-width loop for length. -const PADDED_BASE64_PREFIX = String.raw`^(?!.*[^A-Za-z0-9+\x2f=])(?!.*=[^=])(?!.*===)`; +// These rules are separate JSON Schema constraints. A single RE2-compatible +// regex for alphabet and quartet validity requires a repeated alphabet group; +// V8's Unicode-set engine overflows its regexp stack on that group near the +// provider's 32 MiB encoded-input budget. Wildcard quartets and literal bad +// token probes stay stack-safe at that size and compose exactly with `allOf`. +export const BASE64_QUARTET_LENGTH_PATTERN = /^(?:....)*$/; +export const INVALID_BASE64_CHARACTER_PATTERN = /[^A-Za-z0-9+\x2f=]/; +export const INVALID_BASE64_PADDING_PATTERN = /(?:=[^=]|===)/; +export const INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN = + /(?:[^AQgw]==$|[BCDFGHJKLNOPRSTVWXZabdefhijlmnpqrtuvxyz1235679+\x2f]=$)/; -/** Padded base64 syntax, allowing nonzero pad bits and the empty encoding. */ -export const PADDED_BASE64_PATTERN = new RegExp( - `${PADDED_BASE64_PREFIX}(?:....)*$`, -); +export const isPaddedBase64 = (value: string): boolean => + value.length % 4 === 0 && + !INVALID_BASE64_CHARACTER_PATTERN.test(value) && + !INVALID_BASE64_PADDING_PATTERN.test(value); -/** Canonical padded base64, including zero pad bits and the empty encoding. */ -export const CANONICAL_BASE64_PATTERN = new RegExp( - `${PADDED_BASE64_PREFIX}(?!.*[^AQgw]==$)(?!.*[^AEIMQUYcgkosw048=]=$)(?:....)*$`, -); +export const isCanonicalBase64 = (value: string): boolean => + isPaddedBase64(value) && + !INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN.test(value); + +const paddedBase64SchemaRules = [ + { pattern: BASE64_QUARTET_LENGTH_PATTERN.source }, + { not: { pattern: INVALID_BASE64_CHARACTER_PATTERN.source } }, + { not: { pattern: INVALID_BASE64_PADDING_PATTERN.source } }, +]; + +/** Canonical padded base64 with zero pad bits, including the empty encoding. */ +export const canonicalBase64Schema = z + .string() + .refine(isCanonicalBase64, "Expected canonical padded base64") + .meta({ + allOf: [ + ...paddedBase64SchemaRules, + { not: { pattern: INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN.source } }, + ], + }); diff --git a/tests/boundary/mcp/advertisedPatternsVMode.test.ts b/tests/boundary/mcp/advertisedPatternsVMode.test.ts index b48fe27aa0..1291d79d9a 100644 --- a/tests/boundary/mcp/advertisedPatternsVMode.test.ts +++ b/tests/boundary/mcp/advertisedPatternsVMode.test.ts @@ -1,7 +1,14 @@ import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { AjvJsonSchemaValidator } from "@modelcontextprotocol/client/validators/ajv"; import { expect, it, onTestFinished } from "vitest"; import { createServer } from "../../../src/server/createServer.js"; +import { + BASE64_QUARTET_LENGTH_PATTERN, + INVALID_BASE64_CHARACTER_PATTERN, + INVALID_BASE64_PADDING_PATTERN, + INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN, +} from "../../../src/domain/stringPatterns.js"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; interface AdvertisedPattern { @@ -10,6 +17,33 @@ interface AdvertisedPattern { readonly pattern: string; } +const collectCanonicalBase64Leaves = ( + value: unknown, + leaves: Record[], +): void => { + if (value === null || typeof value !== "object") return; + if (Array.isArray(value)) { + value.forEach((entry) => collectCanonicalBase64Leaves(entry, leaves)); + return; + } + const record = value as Record; + const allOf = record.allOf; + if ( + Array.isArray(allOf) && + allOf.some( + (entry) => + entry !== null && + typeof entry === "object" && + "pattern" in entry && + entry.pattern === BASE64_QUARTET_LENGTH_PATTERN.source, + ) + ) + leaves.push(record); + Object.values(record).forEach((entry) => + collectCanonicalBase64Leaves(entry, leaves), + ); +}; + const collectPatterns = ( value: unknown, where: string, @@ -48,12 +82,29 @@ it("advertises portable NUL escapes and patterns that compile in all JS modes", const advertised = (await client.listTools()).tools; const patterns: AdvertisedPattern[] = []; + const canonicalBase64Leaves: Record[] = []; for (const tool of advertised) { collectPatterns(tool.inputSchema, "inputSchema", tool.name, patterns); + collectCanonicalBase64Leaves(tool.inputSchema, canonicalBase64Leaves); if (tool.outputSchema !== undefined) collectPatterns(tool.outputSchema, "outputSchema", tool.name, patterns); + if (tool.outputSchema !== undefined) + collectCanonicalBase64Leaves(tool.outputSchema, canonicalBase64Leaves); } + for (const pattern of [ + BASE64_QUARTET_LENGTH_PATTERN, + INVALID_BASE64_CHARACTER_PATTERN, + INVALID_BASE64_PADDING_PATTERN, + INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN, + ]) + expect( + patterns.some(({ pattern: advertisedPattern }) => + advertisedPattern.includes(pattern.source), + ), + pattern.source, + ).toBe(true); + // Annex B and `u` mode accept an unescaped `-`, `/` or `[` inside a character // class, so REA's own schema validation cannot observe the defect. `v` mode // rejects them, and a client that compiles advertised patterns rejects the @@ -87,4 +138,19 @@ it("advertises portable NUL escapes and patterns that compile in all JS modes", ), ).toEqual([]); expect(patterns.some(({ pattern }) => pattern.includes("\\x00"))).toBe(true); + + expect(canonicalBase64Leaves.length).toBeGreaterThan(0); + const sdkValidator = new AjvJsonSchemaValidator(); + const validators = canonicalBase64Leaves.map((schema) => + sdkValidator.getValidator(schema), + ); + for (const value of ["", "AA==", "AQID", "AB==", "AAA", "AA$=", "AA\n=="]) + expect( + validators.map((validate) => validate(value).valid), + JSON.stringify(value), + ).toEqual( + canonicalBase64Leaves.map( + () => value === "" || value === "AA==" || value === "AQID", + ), + ); }); From aacf740a047078e79c71d528e8e188afe2ba1aff Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:03:54 +0800 Subject: [PATCH 3/4] fix(ci): correlate ownership trace threads and preserve setup failures Follow CLONE_THREAD edges across strace -ff files when excluding the ownership process's stat/status reads. Keep provider child processes outside the allowance and document the same-process attribution limit. Validate setup result array shapes and preserve the original failed-plan JSON instead of masking it with a diagnostic TypeError. --- scripts/verify-package-setup-plan.d.mts | 8 + scripts/verify-package-setup-plan.mjs | 23 +++ scripts/verify-package-setup.mjs | 25 ++- .../recorded-crash/trace-ownership.d.mts | 6 + .../verify/recorded-crash/trace-ownership.mjs | 127 +++++++++++++++ scripts/verify/recorded-crash/trace.mjs | 50 ++---- .../packageSetupPlanDiagnostics.test.ts | 39 +++++ .../recordedCrashTraceClassifier.test.ts | 144 ++++++++++++++++++ 8 files changed, 383 insertions(+), 39 deletions(-) create mode 100644 scripts/verify-package-setup-plan.d.mts create mode 100644 scripts/verify-package-setup-plan.mjs create mode 100644 scripts/verify/recorded-crash/trace-ownership.d.mts create mode 100644 scripts/verify/recorded-crash/trace-ownership.mjs create mode 100644 tests/boundary/filesystem/packageSetupPlanDiagnostics.test.ts create mode 100644 tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts diff --git a/scripts/verify-package-setup-plan.d.mts b/scripts/verify-package-setup-plan.d.mts new file mode 100644 index 0000000000..08fe24aed8 --- /dev/null +++ b/scripts/verify-package-setup-plan.d.mts @@ -0,0 +1,8 @@ +/** Preserve an unexpected JSON result while describing the failed plan check. */ +export function setupPlanFailureMessage(options: { + readonly planned: unknown; + readonly plannedExecution: { readonly status: number | null }; + readonly plannedClaudeConfig: string; + readonly plannedCodexConfig: string; + readonly plannedCursorConfig: string; +}): string; diff --git a/scripts/verify-package-setup-plan.mjs b/scripts/verify-package-setup-plan.mjs new file mode 100644 index 0000000000..4734607367 --- /dev/null +++ b/scripts/verify-package-setup-plan.mjs @@ -0,0 +1,23 @@ +/** Describe a failed package setup-plan check without masking its result shape. */ +export const setupPlanFailureMessage = ({ + planned, + plannedExecution, + plannedClaudeConfig, + plannedCodexConfig, + plannedCursorConfig, +}) => { + const actionKinds = (actions) => + Array.isArray(actions) + ? actions.map((action) => action?.kind ?? action) + : actions; + return `packaged setup plan was not read-only: ${JSON.stringify({ + status: planned?.status, + exitCode: plannedExecution.status, + plannedKinds: actionKinds(planned?.plannedActions), + appliedKinds: actionKinds(planned?.appliedActions), + result: planned, + claudeConfig: plannedClaudeConfig, + codexConfig: plannedCodexConfig, + cursorConfig: plannedCursorConfig, + })}`; +}; diff --git a/scripts/verify-package-setup.mjs b/scripts/verify-package-setup.mjs index ad23548cb3..e44c4f5c88 100644 --- a/scripts/verify-package-setup.mjs +++ b/scripts/verify-package-setup.mjs @@ -21,6 +21,7 @@ import { } from "./lib/verify-package-core.mjs"; import { parse as parseJsonc } from "jsonc-parser"; import { skillReferenceIssues } from "./lib/docs-facts.mjs"; +import { setupPlanFailureMessage } from "./verify-package-setup-plan.mjs"; export const terminalModesRestored = (termios) => { if (typeof termios !== "string") return false; @@ -250,23 +251,37 @@ const verifySetupPlan = async ({ ); const planned = json(plannedExecution.stdout); if (supportedSetupHost) { + const plannedActions = Array.isArray(planned?.plannedActions) + ? planned.plannedActions + : undefined; + const appliedActions = Array.isArray(planned?.appliedActions) + ? planned.appliedActions + : undefined; const plannedClaudeConfig = await readFile(claudeConfig, "utf8"); const plannedCodexConfig = await readFile(codexTarget, "utf8"); const plannedCursorConfig = await readFile(cursorConfig, "utf8"); const plannedOpenCodeConfig = await readFile(opencodeConfig, "utf8"); if ( - planned.status !== "planned" || + planned?.status !== "planned" || plannedExecution.status !== 0 || - planned.appliedActions.length !== 0 || - !planned.plannedActions.some(({ kind }) => kind === "configure_client") || - !planned.plannedActions.some(({ kind }) => kind === "install_skill") || + plannedActions === undefined || + appliedActions === undefined || + appliedActions.length !== 0 || + !plannedActions.some((action) => action?.kind === "configure_client") || + !plannedActions.some((action) => action?.kind === "install_skill") || plannedClaudeConfig !== '{"existing":true}\n' || plannedCodexConfig !== CODEX_ORIGINAL || plannedCursorConfig !== '{"existing":true}\n' || plannedOpenCodeConfig !== opencodeOriginal ) throw new Error( - `packaged setup plan was not read-only: ${JSON.stringify({ status: planned.status, exitCode: plannedExecution.status, plannedKinds: planned.plannedActions.map(({ kind }) => kind), appliedKinds: planned.appliedActions.map(({ kind }) => kind), claudeConfig: plannedClaudeConfig, codexConfig: plannedCodexConfig, cursorConfig: plannedCursorConfig })}`, + setupPlanFailureMessage({ + planned, + plannedExecution, + plannedClaudeConfig, + plannedCodexConfig, + plannedCursorConfig, + }), ); } return { planned }; diff --git a/scripts/verify/recorded-crash/trace-ownership.d.mts b/scripts/verify/recorded-crash/trace-ownership.d.mts new file mode 100644 index 0000000000..0daeba3737 --- /dev/null +++ b/scripts/verify/recorded-crash/trace-ownership.d.mts @@ -0,0 +1,6 @@ +/** Assert the documented procfs allowance over strace -ff process/thread files. */ +export function assertHistoricalPidReadsAreOwnershipOnly(options: { + readonly traces: readonly { readonly pid: number; readonly text: string }[]; + readonly sentinelPid: number; + readonly psExecutables: ReadonlySet; +}): string[]; diff --git a/scripts/verify/recorded-crash/trace-ownership.mjs b/scripts/verify/recorded-crash/trace-ownership.mjs new file mode 100644 index 0000000000..0267dcdaa8 --- /dev/null +++ b/scripts/verify/recorded-crash/trace-ownership.mjs @@ -0,0 +1,127 @@ +import assert from "node:assert/strict"; + +const ownershipPsArguments = + '["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat=,command="]'; + +/** + * Allow only the procfs paths used by the process-ownership scan: ps may + * inspect its process-table fields, and its caller's thread group may read + * native identity and UID records. With strace -ff, libuv worker threads get + * separate files; follow CLONE_THREAD edges, never ordinary child-process + * edges. strace cannot distinguish the purpose of a stat/status read inside + * the same Node thread group, so this is deliberately limited to those two + * files; provider Python/GDB children remain outside the permit. + */ +export const assertHistoricalPidReadsAreOwnershipOnly = ({ + traces, + sentinelPid, + psExecutables, +}) => { + const processGroups = new Map(); + const findGroup = (pid) => { + const parent = processGroups.get(pid); + if (parent === undefined) { + processGroups.set(pid, pid); + return pid; + } + if (parent === pid) return pid; + const root = findGroup(parent); + processGroups.set(pid, root); + return root; + }; + const joinThreadGroup = (left, right) => { + const leftRoot = findGroup(left); + const rightRoot = findGroup(right); + if (leftRoot !== rightRoot) processGroups.set(rightRoot, leftRoot); + }; + + const cloneEvents = []; + for (const trace of traces) { + let unfinished; + for (const rawLine of trace.text.split("\n")) { + const line = rawLine.trimStart(); + const start = /^(clone3?|fork|vfork)\((.*)$/u.exec( + line, + ); + if (start !== null) { + unfinished = { name: start[1], text: start[2] }; + continue; + } + const resumed = /^<\.\.\. (clone3?|fork|vfork) resumed>(.*)$/u.exec(line); + if (resumed !== null && resumed[1] === unfinished?.name) { + cloneEvents.push({ + parentPid: trace.pid, + text: `${unfinished.text}${resumed[2]}`, + }); + unfinished = undefined; + continue; + } + const complete = + /^(clone3?|fork|vfork)\((.*\)) = (\d+)(?:<[^>]*>)?$/u.exec(line); + if (complete !== null) + cloneEvents.push({ + parentPid: trace.pid, + childPid: Number(complete[3]), + text: complete[2], + }); + } + } + + for (const event of cloneEvents) { + const childPid = + event.childPid ?? /\) = (\d+)(?:<[^>]*>)?$/u.exec(event.text)?.[1]; + if (childPid === undefined) continue; + event.childPid = Number(childPid); + if (/\bCLONE_THREAD\b/u.test(event.text)) + joinThreadGroup(event.parentPid, event.childPid); + } + + const psPids = new Set(); + for (const trace of traces) { + const successfulPsExec = trace.text.split("\n").some((line) => { + const executable = /^execve\("([^"]+)"/u.exec(line)?.[1]; + return ( + executable !== undefined && + psExecutables.has(executable) && + line.includes(ownershipPsArguments) && + line.endsWith(" = 0") + ); + }); + if (successfulPsExec) psPids.add(trace.pid); + } + + const psParentGroups = new Set( + cloneEvents + .filter(({ childPid }) => childPid !== undefined && psPids.has(childPid)) + .map(({ parentPid }) => findGroup(parentPid)), + ); + + const ownershipReads = []; + const pidPrefix = `/proc/${sentinelPid}/`; + for (const trace of traces) { + const isPsProcess = psPids.has(trace.pid); + const isOwnershipProcessThread = psParentGroups.has(findGroup(trace.pid)); + const pidReads = trace.text + .split("\n") + .filter((line) => line.includes(pidPrefix)); + for (const line of pidReads) { + const psAllowedPath = new RegExp( + `/proc/${sentinelPid}/(?:stat|status|cmdline|wchan|environ)"`, + "u", + ).test(line); + const ownershipThreadPath = new RegExp( + `/proc/${sentinelPid}/(?:stat|status)"`, + "u", + ).test(line); + const psRead = isPsProcess && psAllowedPath; + const ownershipThreadRead = + isOwnershipProcessThread && ownershipThreadPath; + assert.ok( + psRead || ownershipThreadRead, + `Historical PID caused a provider lookup in trace.${trace.pid}: ${line}`, + ); + ownershipReads.push(line); + } + } + return ownershipReads; +}; diff --git a/scripts/verify/recorded-crash/trace.mjs b/scripts/verify/recorded-crash/trace.mjs index 7cf5835087..44ef4f9632 100644 --- a/scripts/verify/recorded-crash/trace.mjs +++ b/scripts/verify/recorded-crash/trace.mjs @@ -3,6 +3,7 @@ import { readdir, readFile, realpath } from "node:fs/promises"; import { fileURLToPath } from "node:url"; import { isAbsolute, join, resolve } from "node:path"; import { runOwnedCommand } from "../../../dist/process/OwnedCommand.js"; +import { assertHistoricalPidReadsAreOwnershipOnly } from "./trace-ownership.mjs"; /** Check attempted syscalls, including failed attempts, on a recorded PID that collides with an owned live sentinel. */ export async function verifyCoreInspectionTrace({ @@ -68,11 +69,22 @@ export async function verifyCoreInspectionTrace({ { timeoutMs: 90_000, diagnosticBytes: 1024 * 1024 }, ); const executions = []; - const ownershipReads = []; let gdbObserved = false; - for (const file of await readdir(root)) { - if (!file.startsWith(`inspection-${mode}.trace.`)) continue; - const text = await readFile(join(root, file), "utf8"); + const traces = await Promise.all( + (await readdir(root)) + .filter((file) => file.startsWith(`inspection-${mode}.trace.`)) + .map(async (file) => ({ + file, + pid: Number(file.slice(`inspection-${mode}.trace.`.length)), + text: await readFile(join(root, file), "utf8"), + })), + ); + const ownershipReads = assertHistoricalPidReadsAreOwnershipOnly({ + traces, + sentinelPid: sentinel.pid, + psExecutables: psPaths, + }); + for (const { file, text } of traces) { // Keep the source executable present: GDB 16 otherwise silently hides a // core-only violation when its automatic executable lookup finds nothing. const recordedExecutable = JSON.stringify(join(root, "fixture-program")); @@ -80,36 +92,6 @@ export async function verifyCoreInspectionTrace({ !text.split("\n").some((line) => line.includes(recordedExecutable)), `Inspection attempted access to the recorded executable: ${file}`, ); - const ownershipTrace = text - .split("\n") - .some( - (line) => - /^execve\("\/(?:usr\/)?bin\/ps",/u.test(line) && - line.includes( - '["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat=,command="]', - ) && - line.endsWith(" = 0"), - ); - const pidReads = text - .split("\n") - .filter((line) => line.includes(`/proc/${sentinel.pid}/`)); - if (ownershipTrace) { - for (const line of pidReads) { - assert.ok( - new RegExp( - `/proc/${sentinel.pid}/(?:stat|status|cmdline|wchan|environ)"`, - "u", - ).test(line), - `Unexpected ownership observation of the sentinel: ${line}`, - ); - ownershipReads.push(line); - } - } else - assert.deepEqual( - pidReads, - [], - `Historical PID caused a provider lookup in ${file}: ${pidReads.join("\n")}`, - ); assert.ok( !/\bptrace\(/u.test(text), `Inspection attempted ptrace: ${file}`, diff --git a/tests/boundary/filesystem/packageSetupPlanDiagnostics.test.ts b/tests/boundary/filesystem/packageSetupPlanDiagnostics.test.ts new file mode 100644 index 0000000000..7de7c1d420 --- /dev/null +++ b/tests/boundary/filesystem/packageSetupPlanDiagnostics.test.ts @@ -0,0 +1,39 @@ +import { expect, it } from "vitest"; + +import { setupPlanFailureMessage } from "../../../scripts/verify-package-setup-plan.mjs"; + +it("preserves setup failure details when an error result has no action arrays", () => { + const result = { + status: "needs_human", + remediation: "Review the host setup requirements.", + }; + const message = setupPlanFailureMessage({ + planned: result, + plannedExecution: { status: 1 }, + plannedClaudeConfig: "unchanged", + plannedCodexConfig: "unchanged", + plannedCursorConfig: "unchanged", + }); + + expect(message).toContain("packaged setup plan was not read-only"); + expect(message).toContain('"status":"needs_human"'); + expect(message).toContain("Review the host setup requirements."); + expect(message).not.toContain("TypeError"); +}); + +it("still formats action kinds for a structured setup result", () => { + const message = setupPlanFailureMessage({ + planned: { + status: "planned", + plannedActions: [{ kind: "configure_client" }], + appliedActions: [], + }, + plannedExecution: { status: 0 }, + plannedClaudeConfig: "unchanged", + plannedCodexConfig: "unchanged", + plannedCursorConfig: "unchanged", + }); + + expect(message).toContain('"plannedKinds":["configure_client"]'); + expect(message).toContain('"appliedKinds":[]'); +}); diff --git a/tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts b/tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts new file mode 100644 index 0000000000..f03fbd5ba1 --- /dev/null +++ b/tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts @@ -0,0 +1,144 @@ +import { expect, it } from "vitest"; + +import { assertHistoricalPidReadsAreOwnershipOnly } from "../../../scripts/verify/recorded-crash/trace-ownership.mjs"; + +const psExec = + 'execve("/usr/bin/ps", ["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat=,command="], 0x7fff) = 0'; + +it("correlates the ps child trace with its parent identity read", () => { + const ownershipReads = assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: [ + "vfork() = 1201", + "clone3({flags=CLONE_VM|CLONE_SIGHAND|CLONE_THREAD}, 88) = 1202", + 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY|O_CLOEXEC) = 21', + ].join("\n"), + }, + { + pid: 1202, + text: [ + 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY|O_CLOEXEC) = 21', + 'openat(AT_FDCWD, "/proc/4008/status", O_RDONLY|O_CLOEXEC) = 22', + ].join("\n"), + }, + { + pid: 1201, + text: [ + psExec, + 'openat(AT_FDCWD, "/proc/4008/status", O_RDONLY) = 3', + 'openat(AT_FDCWD, "/proc/4008/cmdline", O_RDONLY) = 4', + ].join("\n"), + }, + ], + }); + + expect(ownershipReads).toHaveLength(5); +}); + +it("joins unfinished and resumed CLONE_THREAD records", () => { + const ownershipReads = assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: [ + "vfork() = 1201", + "clone3({flags=CLONE_VM|CLONE_SIGHAND|CLONE_THREAD, child_tid=0x3 ", + "<... clone3 resumed>) = 1202", + ].join("\n"), + }, + { + pid: 1202, + text: 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY) = 3', + }, + { pid: 1201, text: psExec }, + ], + }); + + expect(ownershipReads).toHaveLength(1); +}); + +it("rejects provider-style reads outside the ps child and stat-only caller", () => { + const traces = [ + { + pid: 1200, + text: [ + "vfork() = 1201", + 'openat(AT_FDCWD, "/proc/4008/cmdline", O_RDONLY) = 21', + ].join("\n"), + }, + { pid: 1201, text: psExec }, + ]; + + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + traces, + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + }), + ).toThrow("Historical PID caused a provider lookup"); +}); + +it("does not trust a ps command with different arguments or failed exec", () => { + for (const text of [ + 'execve("/usr/bin/ps", ["ps", "-ef"], 0x7fff) = 0', + psExec.replace(" = 0", " = -1 EACCES"), + ]) + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: `vfork() = 1201\nopenat(AT_FDCWD, "/proc/4008/stat", O_RDONLY) = 3`, + }, + { pid: 1201, text }, + ], + }), + ).toThrow("Historical PID caused a provider lookup"); +}); + +it("allows only known procfs fields inside the exact ps child", () => { + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { pid: 1200, text: "vfork() = 1201" }, + { + pid: 1201, + text: `${psExec}\nopenat(AT_FDCWD, "/proc/4008/exe", O_RDONLY) = 3`, + }, + ], + }), + ).toThrow("Historical PID caused a provider lookup"); +}); + +it("does not grant the ownership process thread permit to a forked child", () => { + for (const field of ["stat", "status"]) + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: ["vfork() = 1201", "clone3({flags=SIGCHLD}, 88) = 1203"].join( + "\n", + ), + }, + { pid: 1201, text: psExec }, + { + pid: 1203, + text: `openat(AT_FDCWD, "/proc/4008/${field}", O_RDONLY) = 3`, + }, + ], + }), + ).toThrow("Historical PID caused a provider lookup"); +}); From 7510c1a3c80ada0e0fe1b498231d9957c91e0178 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 09:23:30 -0700 Subject: [PATCH 4/4] fix(environment): bound observed values in parameter descriptions A parameter description lists at most ten observed values, each cut at 64 characters, followed by a count of the rest. tools/list and the environment card are served whole, so a capture with many or very long path values no longer inflates them; the manifest keeps every value. Co-Authored-By: Claude Opus 5.5 --- docs/agent-environments.md | 3 +- .../environment-serve/spec.md | 3 +- .../environment/environmentTools.test.ts | 29 +++++++++++++++++++ .../environment/environmentTools.ts | 23 +++++++++++++-- 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/docs/agent-environments.md b/docs/agent-environments.md index abecf2aaec..5cab3ad8e9 100644 --- a/docs/agent-environments.md +++ b/docs/agent-environments.md @@ -178,7 +178,8 @@ connecting to `/mcp`: `capabilities.tools` lists the same names, descriptions and input schemas that `tools/list` advertises over MCP. Path parameters are required strings whose description names the segment index, the templating rule and the observed -values; query parameter names are optional strings; write methods accept an +values (at most ten, each cut at 64 characters, with a count of the rest; the +manifest keeps them all); query parameter names are optional strings; write methods accept an optional `body` object described by the recorded top-level key types. Each input keeps its own argument: a query name equal to a path parameter's name is advertised as `query_`, and the write body as `request_body` when a query diff --git a/docs/specs/agent-environments/environment-serve/spec.md b/docs/specs/agent-environments/environment-serve/spec.md index 2347ccfab8..edca804133 100644 --- a/docs/specs/agent-environments/environment-serve/spec.md +++ b/docs/specs/agent-environments/environment-serve/spec.md @@ -26,7 +26,8 @@ later increment. capture's records, so no further record-coverage refusal is defined. - Build an `McpServer` from the manifest alone: each tool's input schema lists every path parameter as a required string described by its segment index, - rule and observed values, every observed query name as an optional string, + rule and observed values (at most ten, each cut at 64 characters, with a + count of the rest), every observed query name as an optional string, and an optional `body` object for `POST`, `PUT` and `PATCH` (a query name that collides with a path parameter becomes `query_`, and the body becomes `request_body` when a query parameter is named `body`), closed with diff --git a/src/application/environment/environmentTools.test.ts b/src/application/environment/environmentTools.test.ts index a048db1ec5..a809a06669 100644 --- a/src/application/environment/environmentTools.test.ts +++ b/src/application/environment/environmentTools.test.ts @@ -6,6 +6,8 @@ import { environmentCard, environmentToolDefinitions, environmentToolInputSchema, + MAX_DESCRIBED_VALUE_CHARS, + MAX_DESCRIBED_VALUES, } from "./environmentTools.js"; const source = environmentServeSource(); @@ -55,6 +57,33 @@ describe("environmentToolDefinitions", () => { }); }); + it("bounds a parameter's observed values in its description, leaving the full list to the manifest", () => { + const users = tool("get_users_by_id"); + const long = "x".repeat(1000); + const values = [ + long, + ...Array.from({ length: 24 }, (_, i) => `v${String(i)}`), + ]; + const [param] = users.params; + if (param === undefined) + throw new Error("Fixture tool must have a path parameter"); + const schema = environmentToolInputSchema({ + ...users, + params: [{ ...param, observed_values: values }], + }); + const description = schema.properties.id?.description ?? ""; + expect(description).toContain( + `${"x".repeat(MAX_DESCRIBED_VALUE_CHARS)}…, v0,`, + ); + expect(description).not.toContain( + "x".repeat(MAX_DESCRIBED_VALUE_CHARS + 1), + ); + expect(description).toContain( + `v${String(MAX_DESCRIBED_VALUES - 2)} and 15 more in the manifest`, + ); + expect(description).not.toContain(`v${String(MAX_DESCRIBED_VALUES - 1)},`); + }); + it("derives optional string query parameters and no body for reads", () => { const search = environmentToolInputSchema(tool("get_search")); expect(search.required).toEqual([]); diff --git a/src/application/environment/environmentTools.ts b/src/application/environment/environmentTools.ts index 18eea3af9c..24a4e5b829 100644 --- a/src/application/environment/environmentTools.ts +++ b/src/application/environment/environmentTools.ts @@ -28,8 +28,27 @@ export interface EnvironmentToolDefinition { readonly tool: EnvironmentManifestTool; } -const observed = (values: readonly string[]): string => - values.length === 0 ? "none" : values.join(", "); +/** A parameter description lists at most this many observed values, each cut to this many characters; the manifest keeps them all. */ +export const MAX_DESCRIBED_VALUES = 10; +export const MAX_DESCRIBED_VALUE_CHARS = 64; + +/** The value's first characters, never ending inside a surrogate pair, marked when cut. */ +const cut = (value: string): string => { + if (value.length <= MAX_DESCRIBED_VALUE_CHARS) return value; + const last = value.charCodeAt(MAX_DESCRIBED_VALUE_CHARS - 1); + const end = + last >= 0xd800 && last <= 0xdbff + ? MAX_DESCRIBED_VALUE_CHARS - 1 + : MAX_DESCRIBED_VALUE_CHARS; + return `${value.slice(0, end)}…`; +}; + +const observed = (values: readonly string[]): string => { + if (values.length === 0) return "none"; + const shown = values.slice(0, MAX_DESCRIBED_VALUES).map(cut).join(", "); + const rest = values.length - MAX_DESCRIBED_VALUES; + return rest > 0 ? `${shown} and ${String(rest)} more in the manifest` : shown; +}; const bodyDescription = (tool: EnvironmentManifestTool): string => tool.body_shape === null