From fadf4e35f04dd03a30120227a5debc7e0bf047ad Mon Sep 17 00:00:00 2001 From: xy200303 <3483421977@qq.com> Date: Wed, 10 Jun 2026 22:38:10 +0800 Subject: [PATCH 1/3] fix: reject malformed numeric CLI options --- .prettierrc.json | 3 +++ src/commands/artifacts-command.ts | 9 +-------- src/commands/option-parsers.ts | 29 ++++++++++++++++++++++++----- 3 files changed, 28 insertions(+), 13 deletions(-) create mode 100644 .prettierrc.json diff --git a/.prettierrc.json b/.prettierrc.json new file mode 100644 index 00000000..168d9d2a --- /dev/null +++ b/.prettierrc.json @@ -0,0 +1,3 @@ +{ + "endOfLine": "auto" +} diff --git a/src/commands/artifacts-command.ts b/src/commands/artifacts-command.ts index dfbd24af..d60c29cd 100644 --- a/src/commands/artifacts-command.ts +++ b/src/commands/artifacts-command.ts @@ -30,6 +30,7 @@ import { parseCommanderProgram, } from "./commander-utils.js"; import { setStderrDevLogStorageRootDirectory } from "../runtime/stderr-dev-log.js"; +import { parseNonNegativeInt } from "./option-parsers.js"; interface WriteTarget { write(chunk: string): unknown; @@ -391,14 +392,6 @@ function parseHarnessKindOption(value: string): AgentHarnessKind { ); } -function parseNonNegativeInt(value: string): number { - const parsed = Number.parseInt(value, 10); - if (!Number.isFinite(parsed) || parsed < 0) { - throw new InvalidArgumentError("value must be a non-negative integer"); - } - return parsed; -} - function writeJson(stdout: WriteTarget, value: unknown): void { stdout.write(`${JSON.stringify(value, null, 2)}\n`); } diff --git a/src/commands/option-parsers.ts b/src/commands/option-parsers.ts index 120f78eb..041a97be 100644 --- a/src/commands/option-parsers.ts +++ b/src/commands/option-parsers.ts @@ -14,9 +14,28 @@ import { MIN_ANTHROPIC_THINKING_BUDGET_TOKENS } from "../bootstrap/config/defaul export class InvalidArgumentError extends Error {} +const DECIMAL_INTEGER_PATTERN = /^[+-]?\d+$/u; +const DECIMAL_NUMBER_PATTERN = /^[+-]?(?:\d+\.?\d*|\.\d+)(?:e[+-]?\d+)?$/iu; + +function parseDecimalInteger(value: string): number { + const normalized = value.trim(); + if (!DECIMAL_INTEGER_PATTERN.test(normalized)) { + return Number.NaN; + } + return Number(normalized); +} + +function parseFiniteNumber(value: string): number { + const normalized = value.trim(); + if (!DECIMAL_NUMBER_PATTERN.test(normalized)) { + return Number.NaN; + } + return Number(normalized); +} + export function parsePositiveInt(value: string): number { - const parsed = Number.parseInt(value, 10); - if (!Number.isInteger(parsed) || parsed <= 0) { + const parsed = parseDecimalInteger(value); + if (!Number.isSafeInteger(parsed) || parsed <= 0) { throw new Error(`Expected positive integer, got: ${value}`); } return parsed; @@ -32,8 +51,8 @@ export function parseOperatingMode(value: string): AgentOperatingMode { } export function parseNonNegativeInt(value: string): number { - const parsed = Number.parseInt(value, 10); - if (!Number.isInteger(parsed) || parsed < 0) { + const parsed = parseDecimalInteger(value); + if (!Number.isSafeInteger(parsed) || parsed < 0) { throw new Error(`Expected non-negative integer, got: ${value}`); } return parsed; @@ -107,7 +126,7 @@ export function parseToolSearchIndexProfile( } export function parseNumber(value: string): number { - const parsed = Number.parseFloat(value); + const parsed = parseFiniteNumber(value); if (!Number.isFinite(parsed)) { throw new Error(`Expected number, got: ${value}`); } From 9ef8010a1119260110ba5214c2fd002ac0e292f2 Mon Sep 17 00:00:00 2001 From: xy200303 <3483421977@qq.com> Date: Thu, 18 Jun 2026 20:57:20 +0800 Subject: [PATCH 2/3] Fix Windows test compatibility after main merge --- packages/utils/src/path.test.ts | 236 +++++++++++++++++-------------- packages/utils/src/shell.test.ts | 9 +- 2 files changed, 135 insertions(+), 110 deletions(-) diff --git a/packages/utils/src/path.test.ts b/packages/utils/src/path.test.ts index e1206b95..27487b9b 100644 --- a/packages/utils/src/path.test.ts +++ b/packages/utils/src/path.test.ts @@ -220,6 +220,10 @@ async function makeWorkspace(): Promise { return fs.realpath(dir); } +// Windows symlink creation requires Developer Mode or elevated privileges. +// Linux/macOS CI still exercises these resolver paths. +const symlinkIt = process.platform === "win32" ? it.skip : it; + describe("resolveExistingPathInWorkspace", () => { it("returns the real path of an existing file inside the workspace", async () => { const root = await makeWorkspace(); @@ -232,23 +236,26 @@ describe("resolveExistingPathInWorkspace", () => { } }); - it("resolves a symlink target that lives inside the workspace", async () => { - const root = await makeWorkspace(); - try { - await fs.writeFile(path.join(root, "real.txt"), "data"); - await fs.symlink( - path.join(root, "real.txt"), - path.join(root, "link.txt"), - ); - const result = await resolveExistingPathInWorkspace(root, "link.txt"); - // Returns the resolved real target. - expect(result).toBe(path.join(root, "real.txt")); - } finally { - await fs.rm(root, { recursive: true }); - } - }); - - it("throws when a symlink points outside the workspace", async () => { + symlinkIt( + "resolves a symlink target that lives inside the workspace", + async () => { + const root = await makeWorkspace(); + try { + await fs.writeFile(path.join(root, "real.txt"), "data"); + await fs.symlink( + path.join(root, "real.txt"), + path.join(root, "link.txt"), + ); + const result = await resolveExistingPathInWorkspace(root, "link.txt"); + // Returns the resolved real target. + expect(result).toBe(path.join(root, "real.txt")); + } finally { + await fs.rm(root, { recursive: true }); + } + }, + ); + + symlinkIt("throws when a symlink points outside the workspace", async () => { const root = await makeWorkspace(); const outside = await makeWorkspace(); try { @@ -266,28 +273,31 @@ describe("resolveExistingPathInWorkspace", () => { } }); - it("throws when the symlink's parent dir escapes the workspace", async () => { - const root = await makeWorkspace(); - const outside = await makeWorkspace(); - try { - // 'sub' inside root is a symlink to an outside dir; addressing - // sub/inner.txt makes the symlink parent check fail. - await fs.mkdir(path.join(outside, "real-dir")); - await fs.writeFile(path.join(outside, "real-dir", "inner.txt"), "y"); - await fs.symlink( - path.join(outside, "real-dir"), - path.join(root, "sub"), - "dir", - ); - // 'sub' itself is a symlink whose realpath is outside. - await expect(resolveExistingPathInWorkspace(root, "sub")).rejects.toThrow( - "Path escapes workspace root", - ); - } finally { - await fs.rm(root, { recursive: true }); - await fs.rm(outside, { recursive: true }); - } - }); + symlinkIt( + "throws when the symlink's parent dir escapes the workspace", + async () => { + const root = await makeWorkspace(); + const outside = await makeWorkspace(); + try { + // 'sub' inside root is a symlink to an outside dir; addressing + // sub/inner.txt makes the symlink parent check fail. + await fs.mkdir(path.join(outside, "real-dir")); + await fs.writeFile(path.join(outside, "real-dir", "inner.txt"), "y"); + await fs.symlink( + path.join(outside, "real-dir"), + path.join(root, "sub"), + "dir", + ); + // 'sub' itself is a symlink whose realpath is outside. + await expect( + resolveExistingPathInWorkspace(root, "sub"), + ).rejects.toThrow("Path escapes workspace root"); + } finally { + await fs.rm(root, { recursive: true }); + await fs.rm(outside, { recursive: true }); + } + }, + ); it("rejects for a non-existent path (ENOENT from realpath)", async () => { const root = await makeWorkspace(); @@ -302,7 +312,7 @@ describe("resolveExistingPathInWorkspace", () => { }); describe("resolveAddressedExistingPathInWorkspace", () => { - it("returns the addressed (non-real) resolved path", async () => { + symlinkIt("returns the addressed (non-real) resolved path", async () => { const root = await makeWorkspace(); try { await fs.writeFile(path.join(root, "real.txt"), "data"); @@ -321,7 +331,7 @@ describe("resolveAddressedExistingPathInWorkspace", () => { } }); - it("throws when symlink escapes workspace", async () => { + symlinkIt("throws when symlink escapes workspace", async () => { const root = await makeWorkspace(); const outside = await makeWorkspace(); try { @@ -341,7 +351,7 @@ describe("resolveAddressedExistingPathInWorkspace", () => { }); describe("resolveAddressedPathEntryInWorkspace", () => { - it("returns the resolved path early for a symlink entry", async () => { + symlinkIt("returns the resolved path early for a symlink entry", async () => { const root = await makeWorkspace(); try { await fs.writeFile(path.join(root, "real.txt"), "data"); @@ -373,27 +383,30 @@ describe("resolveAddressedPathEntryInWorkspace", () => { } }); - it("returns resolved symlink even if it escapes (parent check passes)", async () => { - const root = await makeWorkspace(); - const outside = await makeWorkspace(); - try { - await fs.writeFile(path.join(outside, "secret.txt"), "x"); - // Symlink directly in root; its parent (root) is in the workspace, so the - // parent check passes and the symlink branch returns early. - await fs.symlink( - path.join(outside, "secret.txt"), - path.join(root, "link.txt"), - ); - const result = await resolveAddressedPathEntryInWorkspace( - root, - "link.txt", - ); - expect(result).toBe(path.join(root, "link.txt")); - } finally { - await fs.rm(root, { recursive: true }); - await fs.rm(outside, { recursive: true }); - } - }); + symlinkIt( + "returns resolved symlink even if it escapes (parent check passes)", + async () => { + const root = await makeWorkspace(); + const outside = await makeWorkspace(); + try { + await fs.writeFile(path.join(outside, "secret.txt"), "x"); + // Symlink directly in root; its parent (root) is in the workspace, so the + // parent check passes and the symlink branch returns early. + await fs.symlink( + path.join(outside, "secret.txt"), + path.join(root, "link.txt"), + ); + const result = await resolveAddressedPathEntryInWorkspace( + root, + "link.txt", + ); + expect(result).toBe(path.join(root, "link.txt")); + } finally { + await fs.rm(root, { recursive: true }); + await fs.rm(outside, { recursive: true }); + } + }, + ); }); describe("resolveWritablePathInWorkspace", () => { @@ -421,50 +434,59 @@ describe("resolveWritablePathInWorkspace", () => { } }); - it("follows a symlinked parent dir to an existing location inside workspace", async () => { - const root = await makeWorkspace(); - try { - await fs.mkdir(path.join(root, "actual")); - await fs.symlink(path.join(root, "actual"), path.join(root, "linkdir")); - const result = await resolveWritablePathInWorkspace( - root, - "linkdir/file.txt", - ); - // Nearest existing path is the symlink's real target, then file.txt. - expect(result).toBe(path.join(root, "actual", "file.txt")); - } finally { - await fs.rm(root, { recursive: true }); - } - }); - - it("throws when the nearest existing ancestor escapes the workspace", async () => { - const root = await makeWorkspace(); - const outside = await makeWorkspace(); - try { - await fs.symlink(outside, path.join(root, "escape"), "dir"); - await expect( - resolveWritablePathInWorkspace(root, "escape/child.txt"), - ).rejects.toThrow("Path escapes workspace root"); - } finally { - await fs.rm(root, { recursive: true }); - await fs.rm(outside, { recursive: true }); - } - }); - - it("rethrows non-ENOENT errors from realpath (ELOOP cycle)", async () => { - const root = await makeWorkspace(); - try { - // a -> b, b -> a: realpath fails with ELOOP, which is not ENOENT, so - // findNearestExistingPath rethrows it (covers the non-ENOENT branch). - await fs.symlink(path.join(root, "b"), path.join(root, "a")); - await fs.symlink(path.join(root, "a"), path.join(root, "b")); - await expect( - resolveWritablePathInWorkspace(root, "a/child.txt"), - ).rejects.toThrow(/ELOOP|Symlink cycle|escapes workspace/); - } finally { - await fs.rm(root, { recursive: true }); - } - }); + symlinkIt( + "follows a symlinked parent dir to an existing location inside workspace", + async () => { + const root = await makeWorkspace(); + try { + await fs.mkdir(path.join(root, "actual")); + await fs.symlink(path.join(root, "actual"), path.join(root, "linkdir")); + const result = await resolveWritablePathInWorkspace( + root, + "linkdir/file.txt", + ); + // Nearest existing path is the symlink's real target, then file.txt. + expect(result).toBe(path.join(root, "actual", "file.txt")); + } finally { + await fs.rm(root, { recursive: true }); + } + }, + ); + + symlinkIt( + "throws when the nearest existing ancestor escapes the workspace", + async () => { + const root = await makeWorkspace(); + const outside = await makeWorkspace(); + try { + await fs.symlink(outside, path.join(root, "escape"), "dir"); + await expect( + resolveWritablePathInWorkspace(root, "escape/child.txt"), + ).rejects.toThrow("Path escapes workspace root"); + } finally { + await fs.rm(root, { recursive: true }); + await fs.rm(outside, { recursive: true }); + } + }, + ); + + symlinkIt( + "rethrows non-ENOENT errors from realpath (ELOOP cycle)", + async () => { + const root = await makeWorkspace(); + try { + // a -> b, b -> a: realpath fails with ELOOP, which is not ENOENT, so + // findNearestExistingPath rethrows it (covers the non-ENOENT branch). + await fs.symlink(path.join(root, "b"), path.join(root, "a")); + await fs.symlink(path.join(root, "a"), path.join(root, "b")); + await expect( + resolveWritablePathInWorkspace(root, "a/child.txt"), + ).rejects.toThrow(/ELOOP|Symlink cycle|escapes workspace/); + } finally { + await fs.rm(root, { recursive: true }); + } + }, + ); it("resolves a path whose ancestor chain walks up to the workspace root", async () => { const root = await makeWorkspace(); diff --git a/packages/utils/src/shell.test.ts b/packages/utils/src/shell.test.ts index a4a9c0f8..470f3842 100644 --- a/packages/utils/src/shell.test.ts +++ b/packages/utils/src/shell.test.ts @@ -3,6 +3,9 @@ import os from "node:os"; import { enforceOutputLimit, runShell } from "./shell.js"; const BIG_LIMIT = 1_000_000; +const longRunningCommand = `${JSON.stringify( + process.execPath, +)} -e "setTimeout(() => {}, 5000)"`; // Some runShell cases assert POSIX shell semantics (`;` chaining, `exit N`, // `for`/`seq`) that cmd.exe does not interpret. Skip those on Windows; the @@ -151,7 +154,7 @@ describe("runShell", () => { }); it("times out a long-running command and kills it", async () => { - const result = await runShell("sleep 5", { + const result = await runShell(longRunningCommand, { cwd, timeoutMs: 100, outputLimit: BIG_LIMIT, @@ -164,7 +167,7 @@ describe("runShell", () => { it("interrupts a running command when the signal aborts mid-flight", async () => { const controller = new AbortController(); - const promise = runShell("sleep 5", { + const promise = runShell(longRunningCommand, { cwd, timeoutMs: 10_000, outputLimit: BIG_LIMIT, @@ -190,7 +193,7 @@ describe("runShell", () => { }); it("returns exitCode -1 when the close code is null (killed by timeout)", async () => { - const result = await runShell("sleep 5", { + const result = await runShell(longRunningCommand, { cwd, timeoutMs: 50, outputLimit: BIG_LIMIT, From 1ea926efafc2837deb78e39a20987466b94ecd1f Mon Sep 17 00:00:00 2001 From: xy200303 Date: Tue, 15 Sep 2026 14:50:55 +0800 Subject: [PATCH 3/3] test(cli): cover the partial-parse rejections this PR introduces The previous cases only exercised fully non-numeric input ("abc"), which the old Number.parseInt / Number.parseFloat implementation rejected too, so the behaviour this PR actually changes had no coverage. Add cases for the inputs the old code accepted by truncating them ("12abc", "3.14xyz", "12px", "0items", "0x10"), for integer-only rejection of scientific notation, and for the Number.isSafeInteger boundaries the implementation now relies on. Also pin the inputs that must keep working (scientific notation and sign prefixes) so the stricter grammar cannot silently regress. Verified: the new cases fail against the pre-fix implementation (5 failures) and the full file passes on this branch (48 tests). --- src/commands/option-parsers.test.ts | 43 ++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/commands/option-parsers.test.ts b/src/commands/option-parsers.test.ts index c6605760..117b5bf3 100644 --- a/src/commands/option-parsers.test.ts +++ b/src/commands/option-parsers.test.ts @@ -36,11 +36,21 @@ describe("parsePositiveInt", () => { expect(() => parsePositiveInt("abc")).toThrow("positive integer"); }); - it("rejects partially parsed and unsafe integer inputs", () => { - for (const value of ["12px", "1.5", "1e3", "9007199254740992"]) { + it("rejects partially parsed numeric prefixes instead of truncating them", () => { + for (const value of ["12px", "12abc", "3.14xyz", "1.5", "1e3", "0x10"]) { expect(() => parsePositiveInt(value)).toThrow("positive integer"); } }); + + it("rejects integers beyond the safe integer range", () => { + for (const value of ["9007199254740992", "9007199254740993", "1e21"]) { + expect(() => parsePositiveInt(value)).toThrow("positive integer"); + } + }); + + it("accepts the largest safe integer", () => { + expect(parsePositiveInt("9007199254740991")).toBe(9007199254740991); + }); }); describe("parseNonNegativeInt", () => { @@ -53,7 +63,19 @@ describe("parseNonNegativeInt", () => { }); it("rejects partially parsed values", () => { - expect(() => parseNonNegativeInt("0items")).toThrow("non-negative"); + for (const value of ["0items", "3.14xyz", "1e3"]) { + expect(() => parseNonNegativeInt(value)).toThrow("non-negative"); + } + }); + + it("rejects values beyond the safe integer range", () => { + expect(() => parseNonNegativeInt("9007199254740992")).toThrow( + "non-negative", + ); + }); + + it("accepts the largest safe integer", () => { + expect(parseNonNegativeInt("9007199254740991")).toBe(9007199254740991); }); }); @@ -67,10 +89,23 @@ describe("parseNumber", () => { }); it("rejects malformed numeric values", () => { - for (const value of ["3.14ms", "1.2.3", "Infinity"]) { + for (const value of [ + "3.14ms", + "3.14xyz", + "1.2.3", + "Infinity", + "-Infinity", + "NaN", + ]) { expect(() => parseNumber(value)).toThrow("Expected number"); } }); + + it("still accepts scientific notation and sign prefixes", () => { + expect(parseNumber("1e3")).toBeCloseTo(1000); + expect(parseNumber("-2.5e-3")).toBeCloseTo(-0.0025); + expect(parseNumber("+7")).toBe(7); + }); }); describe("parseOperatingMode", () => {