From 17e2fbfa861414236a66dda8fd138c0dd9cdcf38 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 20 Jun 2026 02:31:40 +0800 Subject: [PATCH] fix(test): harden script probe bounds (#95060) Merged via squash. Prepared head SHA: 3a51c3c2d7d9d9aaeac6e1a56deee56afafa39b3 Co-authored-by: vincentkoc <25068+vincentkoc@users.noreply.github.com> Co-authored-by: vincentkoc <25068+vincentkoc@users.noreply.github.com> Reviewed-by: @vincentkoc --- scripts/e2e/kitchen-sink-rpc-walk.mjs | 14 +- scripts/update-clawtributors.ts | 11 +- test/scripts/kitchen-sink-rpc-walk.test.ts | 34 +++++ test/scripts/update-clawtributors.test.ts | 155 ++++++++++++--------- 4 files changed, 141 insertions(+), 73 deletions(-) diff --git a/scripts/e2e/kitchen-sink-rpc-walk.mjs b/scripts/e2e/kitchen-sink-rpc-walk.mjs index 9685cafaedc..8ad3ea41c02 100644 --- a/scripts/e2e/kitchen-sink-rpc-walk.mjs +++ b/scripts/e2e/kitchen-sink-rpc-walk.mjs @@ -980,7 +980,7 @@ export async function readBoundedResponseText(response, byteLimit, timeoutPromis const contentLength = response.headers?.get?.("content-length"); if (contentLength && /^\d+$/u.test(contentLength)) { const parsedContentLength = Number(contentLength); - if (Number.isSafeInteger(parsedContentLength) && parsedContentLength > resolvedByteLimit) { + if (!Number.isSafeInteger(parsedContentLength) || parsedContentLength > resolvedByteLimit) { await response.body?.cancel?.().catch(() => undefined); throw createFetchBodyTooLargeError(resolvedByteLimit); } @@ -1873,12 +1873,14 @@ async function samplePosixProcessTree(pid, run, commandLineNeedles) { if (hasMalformedProcessTreeRows(malformedRows, rootTreeRows)) { return null; } + const rootRow = rootTreeRows.find((row) => row.processId === safePid) ?? null; const descendants = rootTreeRows.filter((row) => row.processId !== safePid); - const commandMatches = descendants.filter((row) => + const matchesCommandNeedles = (row) => commandLineNeedles.every((needle) => row.command.toLowerCase().includes(needle.toLowerCase()), - ), - ); + ); + const commandMatches = descendants.filter(matchesCommandNeedles); + const rootCommandMatches = rootRow && matchesCommandNeedles(rootRow) ? [rootRow] : []; const gatewayTitleMatches = descendants.filter((row) => row.command.toLowerCase().includes("openclaw-gateway"), ); @@ -1887,7 +1889,9 @@ async function samplePosixProcessTree(pid, run, commandLineNeedles) { ? commandMatches : gatewayTitleMatches.length > 0 ? gatewayTitleMatches - : descendants, + : descendants.length > 0 + ? descendants + : rootCommandMatches, ); if (!selected) { return null; diff --git a/scripts/update-clawtributors.ts b/scripts/update-clawtributors.ts index de20121eac9..fc7c6aede3e 100644 --- a/scripts/update-clawtributors.ts +++ b/scripts/update-clawtributors.ts @@ -570,10 +570,13 @@ async function readAvatarProbeBuffer( response: Response, timeoutPromise?: Promise, ): Promise { - const contentLength = Number(response.headers.get("content-length") ?? 0); - if (Number.isFinite(contentLength) && contentLength > AVATAR_PROBE_MAX_BYTES) { - await response.body?.cancel().catch(() => undefined); - throw new Error(`avatar probe exceeded ${AVATAR_PROBE_MAX_BYTES} bytes`); + const contentLengthRaw = response.headers.get("content-length"); + if (contentLengthRaw && /^\d+$/u.test(contentLengthRaw)) { + const contentLength = Number(contentLengthRaw); + if (!Number.isSafeInteger(contentLength) || contentLength > AVATAR_PROBE_MAX_BYTES) { + await response.body?.cancel().catch(() => undefined); + throw new Error(`avatar probe exceeded ${AVATAR_PROBE_MAX_BYTES} bytes`); + } } const reader = response.body?.getReader?.(); diff --git a/test/scripts/kitchen-sink-rpc-walk.test.ts b/test/scripts/kitchen-sink-rpc-walk.test.ts index 17f9439fe1d..8b763874d37 100644 --- a/test/scripts/kitchen-sink-rpc-walk.test.ts +++ b/test/scripts/kitchen-sink-rpc-walk.test.ts @@ -1739,6 +1739,25 @@ describe("kitchen-sink RPC process sampling", () => { }); }); + it("samples the POSIX gateway root when command-line needles match", async () => { + const sample = await sampleProcess(4321, { + platform: "darwin", + posixCommandLineNeedles: ["gateway", "--port", "19080"], + runCommand: async () => ({ + stdout: + " 4321 1 262144 12.5 node dist/index.js gateway --port 19080 --bind loopback\n", + stderr: "", + }), + }); + + expect(sample).toEqual({ + aggregateRssMiB: 256, + cpuPercent: 12.5, + processId: 4321, + rssMiB: 256, + }); + }); + it("falls back to the POSIX gateway process title when the port arg is rewritten", async () => { const sample = await sampleProcess(4321, { platform: "darwin", @@ -1904,6 +1923,21 @@ describe("kitchen-sink RPC process sampling", () => { expect(response.text).not.toHaveBeenCalled(); }); + it("rejects unsafe decimal HTTP content lengths before reading", async () => { + const response = { + headers: new Headers({ + "content-length": "9007199254740992", + }), + text: vi.fn(async () => "not read"), + }; + + await expect(readBoundedResponseText(response, 1024)).rejects.toMatchObject({ + code: "ETOOBIG", + message: "fetch response body exceeded 1024 bytes", + }); + expect(response.text).not.toHaveBeenCalled(); + }); + it("streams HTTP probe responses with non-decimal content-length values", async () => { let readStarted = false; let canceled = false; diff --git a/test/scripts/update-clawtributors.test.ts b/test/scripts/update-clawtributors.test.ts index 9a802bf40b3..d36f77e383a 100644 --- a/test/scripts/update-clawtributors.test.ts +++ b/test/scripts/update-clawtributors.test.ts @@ -13,71 +13,99 @@ afterEach(() => { vi.resetModules(); }); -describe("update-clawtributors", () => { - it("cancels stalled avatar probe body reads at the probe timeout", async () => { - const readme = [ - "# Fixture", - "", - "Thanks to all clawtributors:", - "", - "", - "", - "", - ].join("\n"); - let writtenReadme = ""; - vi.doMock("node:fs", () => ({ - readFileSync: vi.fn((path: string) => { - if (path.endsWith("scripts/clawtributors-map.json")) { - return "{}\n"; - } - if (path.endsWith("README.md")) { - return readme; - } - throw new Error(`unexpected read: ${path}`); - }), - writeFileSync: vi.fn((path: string, data: string) => { - if (path.endsWith("README.md")) { - writtenReadme = data; - return; - } - throw new Error(`unexpected write: ${path}`); - }), - })); - const contributor = { - login: "octo", - name: "Octo", - html_url: "https://github.com/octo", - avatar_url: "https://avatars.githubusercontent.com/u/1?v=4", - contributions: 3, - }; - const execSync = vi.fn((cmd: string) => { - if (cmd === 'gh api "repos/openclaw/openclaw/contributors?per_page=100&anon=1" --paginate') { - return `${JSON.stringify([contributor])}\n`; +function mockClawtributorsFixture() { + const readme = [ + "# Fixture", + "", + "Thanks to all clawtributors:", + "", + "", + "", + "", + ].join("\n"); + let writtenReadme = ""; + vi.doMock("node:fs", () => ({ + readFileSync: vi.fn((path: string) => { + if (path.endsWith("scripts/clawtributors-map.json")) { + return "{}\n"; } - if (cmd === "git log --reverse --format=%aN%x1f%aE%x1f%aI --numstat") { - return ""; + if (path.endsWith("README.md")) { + return readme; } - if ( - cmd === - "gh pr list -R openclaw/openclaw --state merged --limit 5000 --json author --jq '.[].author.login'" - ) { - return ""; + throw new Error(`unexpected read: ${path}`); + }), + writeFileSync: vi.fn((path: string, data: string) => { + if (path.endsWith("README.md")) { + writtenReadme = data; + return; } - if (cmd === "git rev-list --max-parents=0 HEAD") { - return "root-sha\n"; - } - if (cmd === "git log --format=%aI -1 root-sha") { - return "2024-01-01T00:00:00Z\n"; - } - throw new Error(`unexpected command: ${cmd}`); - }); - vi.doMock("node:child_process", () => ({ - execFileSync: vi.fn(() => { - throw new Error("unexpected execFileSync"); - }), - execSync, - })); + throw new Error(`unexpected write: ${path}`); + }), + })); + const contributor = { + login: "octo", + name: "Octo", + html_url: "https://github.com/octo", + avatar_url: "https://avatars.githubusercontent.com/u/1?v=4", + contributions: 3, + }; + const execSync = vi.fn((cmd: string) => { + if (cmd === 'gh api "repos/openclaw/openclaw/contributors?per_page=100&anon=1" --paginate') { + return `${JSON.stringify([contributor])}\n`; + } + if (cmd === "git log --reverse --format=%aN%x1f%aE%x1f%aI --numstat") { + return ""; + } + if ( + cmd === + "gh pr list -R openclaw/openclaw --state merged --limit 5000 --json author --jq '.[].author.login'" + ) { + return ""; + } + if (cmd === "git rev-list --max-parents=0 HEAD") { + return "root-sha\n"; + } + if (cmd === "git log --format=%aI -1 root-sha") { + return "2024-01-01T00:00:00Z\n"; + } + throw new Error(`unexpected command: ${cmd}`); + }); + vi.doMock("node:child_process", () => ({ + execFileSync: vi.fn(() => { + throw new Error("unexpected execFileSync"); + }), + execSync, + })); + return { + readWrittenReadme: () => writtenReadme, + }; +} +async function importUpdateClawtributors() { + const scriptUrl = pathToFileURL(resolve(originalCwd, "scripts/update-clawtributors.ts")).href; + await import(`${scriptUrl}?case=${Date.now()}`); +} + +describe("update-clawtributors", () => { + it("rejects unsafe avatar probe content lengths before reading the body", async () => { + const fixture = mockClawtributorsFixture(); + const arrayBuffer = vi.fn(async () => new ArrayBuffer(0)); + vi.stubGlobal("fetch", (() => + Promise.resolve({ + ok: true, + headers: new Headers({ "content-length": "9007199254740992" }), + arrayBuffer, + } as unknown as Response)) as typeof fetch); + vi.spyOn(console, "log").mockImplementation(() => undefined); + + await importUpdateClawtributors(); + + expect(arrayBuffer).not.toHaveBeenCalled(); + expect(fixture.readWrittenReadme()).toContain("https://github.com/octo"); + }); + + it("cancels stalled avatar probe body reads at the probe timeout", async () => { + const fixture = mockClawtributorsFixture(); let signal: AbortSignal | undefined; let canceled = false; let markFetchStarted!: () => void; @@ -104,8 +132,7 @@ describe("update-clawtributors", () => { vi.spyOn(console, "log").mockImplementation(() => undefined); vi.useFakeTimers(); - const scriptUrl = pathToFileURL(resolve(originalCwd, "scripts/update-clawtributors.ts")).href; - const imported = import(`${scriptUrl}?case=${Date.now()}`); + const imported = importUpdateClawtributors(); await fetchStarted; await vi.advanceTimersByTimeAsync(8000); @@ -114,6 +141,6 @@ describe("update-clawtributors", () => { expect(signal?.aborted).toBe(true); expect(canceled).toBe(true); - expect(writtenReadme).toContain("https://github.com/octo"); + expect(fixture.readWrittenReadme()).toContain("https://github.com/octo"); }); });