From d68e78743f287086e48cfdc5b017c718b076a54c Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 10 Sep 2025 22:59:10 +0200 Subject: [PATCH 1/3] fix: remove dep on export input for export name With this commit we're removing the dependency on export tool input for constructing the name of the export without affecting the user experience in any way. --- src/common/exportsManager.ts | 18 +----------------- src/resources/common/exportedData.ts | 2 +- src/tools/mongodb/read/export.ts | 2 +- .../integration/resources/exportedData.test.ts | 4 ++-- tests/unit/common/exportsManager.test.ts | 17 +---------------- 5 files changed, 6 insertions(+), 37 deletions(-) diff --git a/src/common/exportsManager.ts b/src/common/exportsManager.ts index f83b07ee6..56ba39b2c 100644 --- a/src/common/exportsManager.ts +++ b/src/common/exportsManager.ts @@ -163,7 +163,7 @@ export class ExportsManager extends EventEmitter { }): Promise { try { this.assertIsNotShuttingDown(); - const exportNameWithExtension = validateExportName(ensureExtension(exportName, "json")); + const exportNameWithExtension = ensureExtension(exportName, "json"); if (this.storedExports[exportNameWithExtension]) { return Promise.reject( new Error("Export with same name is either already available or being generated.") @@ -373,22 +373,6 @@ export function ensureExtension(pathOrName: string, extension: string): string { return `${pathOrName}${extWithDot}`; } -/** - * Small utility to decoding and validating provided export name for path - * traversal or no extension */ -export function validateExportName(nameWithExtension: string): string { - const decodedName = decodeURIComponent(nameWithExtension); - if (!path.extname(decodedName)) { - throw new Error("Provided export name has no extension"); - } - - if (decodedName.includes("..") || decodedName.includes("/") || decodedName.includes("\\")) { - throw new Error("Invalid export name: path traversal hinted"); - } - - return decodedName; -} - export function isExportExpired(createdAt: number, exportTimeoutMs: number): boolean { return Date.now() - createdAt > exportTimeoutMs; } diff --git a/src/resources/common/exportedData.ts b/src/resources/common/exportedData.ts index b1b5ed2cf..d7fa55f4f 100644 --- a/src/resources/common/exportedData.ts +++ b/src/resources/common/exportedData.ts @@ -72,7 +72,7 @@ export class ExportedData { private autoCompleteExportName: CompleteResourceTemplateCallback = (value) => { try { return this.session.exportsManager.availableExports - .filter(({ exportName }) => exportName.startsWith(value)) + .filter(({ exportName, exportTitle }) => exportName.startsWith(value) || exportTitle.includes(value)) .map(({ exportName }) => exportName); } catch (error) { this.session.logger.error({ diff --git a/src/tools/mongodb/read/export.ts b/src/tools/mongodb/read/export.ts index 784f0e14f..e2ac194b3 100644 --- a/src/tools/mongodb/read/export.ts +++ b/src/tools/mongodb/read/export.ts @@ -81,7 +81,7 @@ export class ExportTool extends MongoDBToolBase { }); } - const exportName = `${database}.${collection}.${new ObjectId().toString()}.json`; + const exportName = `${new ObjectId().toString()}.json`; const { exportURI, exportPath } = await this.session.exportsManager.createJSONExport({ input: cursor, diff --git a/tests/integration/resources/exportedData.test.ts b/tests/integration/resources/exportedData.test.ts index df48f515a..394bed254 100644 --- a/tests/integration/resources/exportedData.test.ts +++ b/tests/integration/resources/exportedData.test.ts @@ -155,10 +155,10 @@ describeWithMongoDB( }, argument: { name: "exportName", - value: "b", + value: "big", }, }); - expect(completeResponse.completion.total).toEqual(1); + expect(completeResponse.completion.total).toBeGreaterThanOrEqual(1); }); }); }, diff --git a/tests/unit/common/exportsManager.test.ts b/tests/unit/common/exportsManager.test.ts index 264d3230a..08f115568 100644 --- a/tests/unit/common/exportsManager.test.ts +++ b/tests/unit/common/exportsManager.test.ts @@ -5,12 +5,7 @@ import type { FindCursor } from "mongodb"; import { Long } from "mongodb"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { ExportsManagerConfig } from "../../../src/common/exportsManager.js"; -import { - ensureExtension, - isExportExpired, - ExportsManager, - validateExportName, -} from "../../../src/common/exportsManager.js"; +import { ensureExtension, isExportExpired, ExportsManager } from "../../../src/common/exportsManager.js"; import type { AvailableExport } from "../../../src/common/exportsManager.js"; import { config } from "../../../src/common/config.js"; import { ROOT_DIR } from "../../accuracy/sdk/constants.js"; @@ -611,16 +606,6 @@ describe("#ensureExtension", () => { }); }); -describe("#validateExportName", () => { - it("should return decoded name when name is valid", () => { - expect(validateExportName(encodeURIComponent("Test Name.json"))).toEqual("Test Name.json"); - }); - it("should throw when name is invalid", () => { - expect(() => validateExportName("NoExtension")).toThrow("Provided export name has no extension"); - expect(() => validateExportName("../something.json")).toThrow("Invalid export name: path traversal hinted"); - }); -}); - describe("#isExportExpired", () => { it("should return true if export is expired", () => { const createdAt = Date.now() - 1000; From 71f9dd4fde44aba6109ba235af9d80bd024208ed Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 10 Sep 2025 23:18:43 +0200 Subject: [PATCH 2/3] chore: lower case comparison --- src/resources/common/exportedData.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/resources/common/exportedData.ts b/src/resources/common/exportedData.ts index d7fa55f4f..2ae4ba80e 100644 --- a/src/resources/common/exportedData.ts +++ b/src/resources/common/exportedData.ts @@ -72,7 +72,12 @@ export class ExportedData { private autoCompleteExportName: CompleteResourceTemplateCallback = (value) => { try { return this.session.exportsManager.availableExports - .filter(({ exportName, exportTitle }) => exportName.startsWith(value) || exportTitle.includes(value)) + .filter(({ exportName, exportTitle }) => { + const lcExportName = exportName.toLowerCase(); + const lcExportTitle = exportTitle.toLowerCase(); + const lcValue = value.toLowerCase(); + return lcExportName.startsWith(lcValue) || lcExportTitle.includes(lcValue); + }) .map(({ exportName }) => exportName); } catch (error) { this.session.logger.error({ From e41d3ea26f31a114c70e320f68ccea352911f3f1 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 11 Sep 2025 00:10:00 +0200 Subject: [PATCH 3/3] chore: fix tests and normalize the input --- src/common/exportsManager.ts | 8 ++++++-- tests/unit/common/exportsManager.test.ts | 8 ++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/common/exportsManager.ts b/src/common/exportsManager.ts index 56ba39b2c..ad90f8cc2 100644 --- a/src/common/exportsManager.ts +++ b/src/common/exportsManager.ts @@ -127,7 +127,7 @@ export class ExportsManager extends EventEmitter { public async readExport(exportName: string): Promise { try { this.assertIsNotShuttingDown(); - exportName = decodeURIComponent(exportName); + exportName = decodeAndNormalize(exportName); const exportHandle = this.storedExports[exportName]; if (!exportHandle) { throw new Error("Requested export has either expired or does not exist."); @@ -163,7 +163,7 @@ export class ExportsManager extends EventEmitter { }): Promise { try { this.assertIsNotShuttingDown(); - const exportNameWithExtension = ensureExtension(exportName, "json"); + const exportNameWithExtension = decodeAndNormalize(ensureExtension(exportName, "json")); if (this.storedExports[exportNameWithExtension]) { return Promise.reject( new Error("Export with same name is either already available or being generated.") @@ -363,6 +363,10 @@ export class ExportsManager extends EventEmitter { } } +export function decodeAndNormalize(text: string): string { + return decodeURIComponent(text).normalize("NFKC"); +} + /** * Ensures the path ends with the provided extension */ export function ensureExtension(pathOrName: string, extension: string): string { diff --git a/tests/unit/common/exportsManager.test.ts b/tests/unit/common/exportsManager.test.ts index 08f115568..81759e0ad 100644 --- a/tests/unit/common/exportsManager.test.ts +++ b/tests/unit/common/exportsManager.test.ts @@ -25,14 +25,10 @@ const exportsManagerConfig: ExportsManagerConfig = { function getExportNameAndPath({ uniqueExportsId = new ObjectId().toString(), uniqueFileId = new ObjectId().toString(), - database = "foo", - collection = "bar", }: | { uniqueExportsId?: string; uniqueFileId?: string; - database?: string; - collection?: string; } | undefined = {}): { sessionExportsPath: string; @@ -41,7 +37,7 @@ function getExportNameAndPath({ exportURI: string; uniqueExportsId: string; } { - const exportName = `${database}.${collection}.${uniqueFileId}.json`; + const exportName = `${uniqueFileId}.json`; // This is the exports directory for a session. const sessionExportsPath = path.join(exportsPath, uniqueExportsId); const exportPath = path.join(sessionExportsPath, exportName); @@ -243,7 +239,7 @@ describe("ExportsManager unit test", () => { }); it("should handle encoded name", async () => { - const { exportName, exportURI } = getExportNameAndPath({ database: "some database", collection: "coll" }); + const { exportName, exportURI } = getExportNameAndPath({ uniqueFileId: "1FOO 2BAR" }); const { cursor } = createDummyFindCursor([]); const exportAvailableNotifier = getExportAvailableNotifier(encodeURI(exportURI), manager); await manager.createJSONExport({