From 5ff6eda8836ffa77e8605233eca002830131e1c5 Mon Sep 17 00:00:00 2001 From: luojiyin Date: Sun, 13 Sep 2026 16:41:58 +0800 Subject: [PATCH] refactor(configure): sanitize configPath in error messages P1: User-supplied configPath is now sanitized via sanitizeTerminalText() before embedding in terminal output. Linux filenames allow newlines and control characters, so unsanitized paths are a terminal-output correctness issue. P2: resolveAdaptiveConcurrency() was declared async but had no await, and took mdFilePaths: string[] but only used .length. Now sync, taking fileCount: number instead. --- __tests__/batch-lint.spec.ts | 139 +++++++----------------------- __tests__/configure.spec.ts | 8 ++ __tests__/run-file-lint.spec.ts | 10 +-- src/cli/run-lint.ts | 4 +- src/utils/adaptive-concurrency.ts | 12 +-- src/utils/configure.ts | 14 ++- 6 files changed, 62 insertions(+), 125 deletions(-) diff --git a/__tests__/batch-lint.spec.ts b/__tests__/batch-lint.spec.ts index 954d97e..011afef 100644 --- a/__tests__/batch-lint.spec.ts +++ b/__tests__/batch-lint.spec.ts @@ -331,175 +331,102 @@ describe("getMaxFileSize", () => { }); describe("resolveAdaptiveConcurrency", () => { - let tmpDir: string; - - beforeEach(async () => { - tmpDir = await mkdtemp(path.join(tmpdir(), "batch-lint-adaptive-")); - }); - - afterEach(async () => { - await rm(tmpDir, { recursive: true, force: true }); - }); - - const writeSizedFile = async (name: string, sizeBytes: number) => { - const file = path.join(tmpDir, name); - await writeFile(file, Buffer.alloc(sizeBytes)); - return file; - }; - describe("numeric threadCount (preserves existing behavior)", () => { - test("numeric 2 with 3 files → 2", async () => { - const files = await Promise.all([ - writeSizedFile("a.md", 100), - writeSizedFile("b.md", 100), - writeSizedFile("c.md", 100), - ]); - expect(await resolveAdaptiveConcurrency(2, files, 0)).toEqual({ + test("numeric 2 with 3 files → 2", () => { + expect(resolveAdaptiveConcurrency(2, 3, 0)).toEqual({ concurrency: 2, maxFileSize: null, requestedConcurrency: 2, }); }); - test("numeric threads > fileCount is clamped to fileCount", async () => { - const file = await writeSizedFile("only.md", 100); - expect(await resolveAdaptiveConcurrency(100, [file], 0)).toEqual({ + test("numeric threads > fileCount is clamped to fileCount", () => { + expect(resolveAdaptiveConcurrency(100, 1, 0)).toEqual({ concurrency: 1, maxFileSize: null, requestedConcurrency: 100, }); }); - test("numeric 0 is clamped to 1 (matches existing min clamp)", async () => { - const files = await Promise.all([ - writeSizedFile("a.md", 100), - writeSizedFile("b.md", 100), - ]); - expect(await resolveAdaptiveConcurrency(0, files, 0)).toEqual({ + test("numeric 0 is clamped to 1 (matches existing min clamp)", () => { + expect(resolveAdaptiveConcurrency(0, 2, 0)).toEqual({ concurrency: 1, maxFileSize: null, requestedConcurrency: 0, }); }); - test("numeric threads ignores file size", async () => { - const files = await Promise.all( - Array.from({ length: 8 }, (_, index) => - writeSizedFile(`huge-${index}.md`, 10 * 1024 * 1024) - ) - ); - const statSpy = jest.spyOn(require("fs/promises"), "stat"); - - try { - expect( - await resolveAdaptiveConcurrency(8, files, 10 * 1024 * 1024) - ).toEqual({ - concurrency: 8, - maxFileSize: null, - requestedConcurrency: 8, - }); - expect(statSpy).not.toHaveBeenCalled(); - } finally { - statSpy.mockRestore(); - } + test("numeric threads ignores file size", () => { + expect(resolveAdaptiveConcurrency(8, 8, 10 * 1024 * 1024)).toEqual({ + concurrency: 8, + maxFileSize: null, + requestedConcurrency: 8, + }); }); }); describe("auto threadCount", () => { - test("empty file list → 0", async () => { - expect(await resolveAdaptiveConcurrency("auto", [], 0)).toEqual({ + test("empty file list → 0", () => { + expect(resolveAdaptiveConcurrency("auto", 0, 0)).toEqual({ concurrency: 0, maxFileSize: 0, requestedConcurrency: availableParallelism(), }); }); - test("small files (< 1 MiB) cap concurrency at 4", async () => { - const files = await Promise.all( - Array.from({ length: 8 }, (_, index) => - writeSizedFile(`small-${index}.md`, 4096) - ) - ); + test("small files (< 1 MiB) cap concurrency at 4", () => { const cpuLimit = availableParallelism(); - const statSpy = jest.spyOn(require("fs/promises"), "stat"); - - try { - expect(await resolveAdaptiveConcurrency("auto", files, 4096)).toEqual({ - concurrency: Math.min(cpuLimit, 4, files.length), - maxFileSize: 4096, - requestedConcurrency: cpuLimit, - }); - expect(statSpy).not.toHaveBeenCalled(); - } finally { - statSpy.mockRestore(); - } + expect(resolveAdaptiveConcurrency("auto", 8, 4096)).toEqual({ + concurrency: Math.min(cpuLimit, 4, 8), + maxFileSize: 4096, + requestedConcurrency: cpuLimit, + }); }); - test("max file exactly 1 MiB caps at 2", async () => { - const files = await Promise.all([ - writeSizedFile("small.md", 1024), - writeSizedFile("one-mib.md", 1024 * 1024), - ]); + test("max file exactly 1 MiB caps at 2", () => { const cpuLimit = availableParallelism(); - expect( - await resolveAdaptiveConcurrency("auto", files, 1024 * 1024) - ).toEqual({ - concurrency: Math.min(cpuLimit, 2, files.length), + expect(resolveAdaptiveConcurrency("auto", 2, 1024 * 1024)).toEqual({ + concurrency: Math.min(cpuLimit, 2, 2), maxFileSize: 1024 * 1024, requestedConcurrency: cpuLimit, }); }); - test("max file 1.5 MiB caps at 2", async () => { - const file = await writeSizedFile("medium.md", 1.5 * 1024 * 1024); - expect( - await resolveAdaptiveConcurrency("auto", [file], 1.5 * 1024 * 1024) - ).toEqual({ + test("max file 1.5 MiB caps at 2", () => { + expect(resolveAdaptiveConcurrency("auto", 1, 1.5 * 1024 * 1024)).toEqual({ concurrency: 1, maxFileSize: 1.5 * 1024 * 1024, requestedConcurrency: availableParallelism(), }); }); - test("max file exactly 5 MiB forces 1", async () => { - const file = await writeSizedFile("five-mib.md", 5 * 1024 * 1024); - expect( - await resolveAdaptiveConcurrency("auto", [file], 5 * 1024 * 1024) - ).toEqual({ + test("max file exactly 5 MiB forces 1", () => { + expect(resolveAdaptiveConcurrency("auto", 1, 5 * 1024 * 1024)).toEqual({ concurrency: 1, maxFileSize: 5 * 1024 * 1024, requestedConcurrency: availableParallelism(), }); }); - test("max file 6 MiB forces 1", async () => { - const file = await writeSizedFile("six-mib.md", 6 * 1024 * 1024); - expect( - await resolveAdaptiveConcurrency("auto", [file], 6 * 1024 * 1024) - ).toEqual({ + test("max file 6 MiB forces 1", () => { + expect(resolveAdaptiveConcurrency("auto", 1, 6 * 1024 * 1024)).toEqual({ concurrency: 1, maxFileSize: 6 * 1024 * 1024, requestedConcurrency: availableParallelism(), }); }); - test("single small file → 1", async () => { - const file = await writeSizedFile("only.md", 100); - expect(await resolveAdaptiveConcurrency("auto", [file], 100)).toEqual({ + test("single small file → 1", () => { + expect(resolveAdaptiveConcurrency("auto", 1, 100)).toEqual({ concurrency: 1, maxFileSize: 100, requestedConcurrency: availableParallelism(), }); }); - test("medium cap respects fileCount when files < 2", async () => { - const file = await writeSizedFile("one-mib.md", 1.2 * 1024 * 1024); + test("medium cap respects fileCount when files < 2", () => { expect( - await resolveAdaptiveConcurrency( - "auto", - [file], - Math.floor(1.2 * 1024 * 1024) - ) + resolveAdaptiveConcurrency("auto", 1, Math.floor(1.2 * 1024 * 1024)) ).toEqual({ concurrency: 1, maxFileSize: Math.floor(1.2 * 1024 * 1024), diff --git a/__tests__/configure.spec.ts b/__tests__/configure.spec.ts index 3ce7567..44d409c 100644 --- a/__tests__/configure.spec.ts +++ b/__tests__/configure.spec.ts @@ -53,6 +53,14 @@ describe("configuration validation", () => { expect(exitSpy).not.toHaveBeenCalled(); }); + test("sanitizes control characters in missing config paths", () => { + const configPath = path.join(tmpDir, "evil\u001B[31m.json"); + + const error = captureCliError(() => getLintConfig(configPath)); + + expect(error.message).not.toContain("\u001B"); + }); + test("keeps the JSON parse error for an invalid configuration file", () => { const configPath = path.join(tmpDir, "invalid.json"); writeFileSync(configPath, "{ invalid", "utf8"); diff --git a/__tests__/run-file-lint.spec.ts b/__tests__/run-file-lint.spec.ts index 6245a64..0e4239a 100644 --- a/__tests__/run-file-lint.spec.ts +++ b/__tests__/run-file-lint.spec.ts @@ -73,7 +73,7 @@ describe("runFileLint", () => { mockLoadMdFiles.mockResolvedValue(["document.md"]); mockStatFiles.mockResolvedValue([{ path: "document.md", size: 0 }]); mockFilterFilesByMaxSize.mockImplementation((files) => files); - mockResolveAdaptiveConcurrency.mockResolvedValue({ + mockResolveAdaptiveConcurrency.mockReturnValue({ concurrency: 1, maxFileSize: null, requestedConcurrency: 2, @@ -154,11 +154,7 @@ describe("runFileLint", () => { expect(mockStatFiles).toHaveBeenCalledWith(["small.md", "large.md"]); expect(mockFilterFilesByMaxSize).toHaveBeenCalledWith(fileStats, 100); - expect(mockResolveAdaptiveConcurrency).toHaveBeenCalledWith( - 2, - ["small.md"], - 50 - ); + expect(mockResolveAdaptiveConcurrency).toHaveBeenCalledWith(2, 1, 50); expect(mockBatchLint).toHaveBeenCalledWith(1, ["small.md"], false, {}); expect(mockFilterFilesByMaxSize.mock.invocationCallOrder[0]).toBeLessThan( mockResolveAdaptiveConcurrency.mock.invocationCallOrder[0] @@ -181,7 +177,7 @@ describe("runFileLint", () => { })); mockLoadMdFiles.mockResolvedValue(fileStats.map(({ path }) => path)); mockStatFiles.mockResolvedValue(fileStats); - mockResolveAdaptiveConcurrency.mockResolvedValue({ + mockResolveAdaptiveConcurrency.mockReturnValue({ concurrency: 4, maxFileSize: 512 * 1024, requestedConcurrency: 16, diff --git a/src/cli/run-lint.ts b/src/cli/run-lint.ts index a830b2a..c0eee39 100644 --- a/src/cli/run-lint.ts +++ b/src/cli/run-lint.ts @@ -175,9 +175,9 @@ export const runFileLint = async ({ } } - const concurrencyDecision = await resolveAdaptiveConcurrency( + const concurrencyDecision = resolveAdaptiveConcurrency( threadCount, - mdFiles, + mdFiles.length, getMaxFileSize(fileStats) ); const effectiveThreads = concurrencyDecision.concurrency; diff --git a/src/utils/adaptive-concurrency.ts b/src/utils/adaptive-concurrency.ts index b380da0..a416fd5 100644 --- a/src/utils/adaptive-concurrency.ts +++ b/src/utils/adaptive-concurrency.ts @@ -14,15 +14,15 @@ export interface AdaptiveConcurrencyDecision { requestedConcurrency: number; } -export const resolveAdaptiveConcurrency = async ( +export const resolveAdaptiveConcurrency = ( threadCount: ThreadCount, - mdFilePaths: string[], + fileCount: number, maxFileSize: number -): Promise => { +): AdaptiveConcurrencyDecision => { const requestedConcurrency = typeof threadCount === "number" ? threadCount : availableParallelism(); - if (mdFilePaths.length === 0) { + if (fileCount === 0) { return { concurrency: 0, maxFileSize: threadCount === "auto" ? 0 : null, @@ -32,7 +32,7 @@ export const resolveAdaptiveConcurrency = async ( if (typeof threadCount === "number") { return { - concurrency: Math.min(Math.max(threadCount, 1), mdFilePaths.length), + concurrency: Math.min(Math.max(threadCount, 1), fileCount), maxFileSize: null, requestedConcurrency, }; @@ -46,7 +46,7 @@ export const resolveAdaptiveConcurrency = async ( } return { - concurrency: Math.min(Math.max(limit, 1), mdFilePaths.length), + concurrency: Math.min(Math.max(limit, 1), fileCount), maxFileSize, requestedConcurrency, }; diff --git a/src/utils/configure.ts b/src/utils/configure.ts index d2d194e..2961ed8 100644 --- a/src/utils/configure.ts +++ b/src/utils/configure.ts @@ -29,10 +29,12 @@ export const validateConfigShape = ( value: unknown, configPath: string ): CLIConfig => { + const safePath = sanitizeTerminalText(configPath); + if (typeof value !== "object" || value === null || Array.isArray(value)) { throw new CliError( "CONFIG_INVALID", - `[lint-md] Configure file '${configPath}' is invalid.`, + `[lint-md] Configure file '${safePath}' is invalid.`, "The configuration root must be a JSON object." ); } @@ -74,7 +76,7 @@ export const validateConfigShape = ( if (errors.length > 0) { throw new CliError( "CONFIG_INVALID", - `[lint-md] Configure file '${configPath}' is invalid.`, + `[lint-md] Configure file '${safePath}' is invalid.`, errors.join("\n") ); } @@ -86,7 +88,9 @@ export const getLintConfig = (configFilePath?: string): Required => { if (configFilePath && !fs.existsSync(configFilePath)) { throw new CliError( "CONFIG_NOT_FOUND", - `lint-md: Configure file '${configFilePath}' is not exist.` + `lint-md: Configure file '${sanitizeTerminalText( + configFilePath + )}' is not exist.` ); } @@ -104,7 +108,9 @@ export const getLintConfig = (configFilePath?: string): Required => { } catch (error) { throw new CliError( "CONFIG_INVALID", - `[lint-md] Configure file '${configPath}' is invalid.`, + `[lint-md] Configure file '${sanitizeTerminalText( + configPath + )}' is invalid.`, error ); }