chore: handle possible path traversals

Author: himanshusinghs

src/common/logger.ts

@@ -56,6 +56,7 @@ export const LogId = {
     exportCloseError: mongoLogId(1_007_005),
     exportedDataListError: mongoLogId(1_007_006),
     exportedDataAutoCompleteError: mongoLogId(1_007_007),
+    exportedDataSessionUninitialized: mongoLogId(1_007_008),
 } as const;
 
 export abstract class LoggerBase {

src/common/sessionExportsManager.ts

@@ -26,8 +26,10 @@ export type SessionExportsManagerConfig = Pick<
     "exportPath" | "exportTimeoutMs" | "exportCleanupIntervalMs"
 >;
 
+const MAX_LOCK_RETRIES = 10;
+
 export class SessionExportsManager {
-    private mutableExports: Export[] = [];
+    private availableExports: Export[] = [];
     private exportsCleanupInterval: NodeJS.Timeout;
     private exportsCleanupInProgress: boolean = false;
 
@@ -56,9 +58,7 @@ export class SessionExportsManager {
     }
 
     public exportNameToResourceURI(nameWithExtension: string): string {
-        if (!path.extname(nameWithExtension)) {
-            throw new Error("Provided export name has no extension");
-        }
+        this.validateExportName(nameWithExtension);
         return `exported-data://${nameWithExtension}`;
     }
 
@@ -73,9 +73,7 @@ export class SessionExportsManager {
     }
 
     public exportFilePath(exportsDirectoryPath: string, exportNameWithExtension: string): string {
-        if (!path.extname(exportNameWithExtension)) {
-            throw new Error("Provided export name has no extension");
-        }
+        this.validateExportName(exportNameWithExtension);
         return path.join(exportsDirectoryPath, exportNameWithExtension);
     }
 
@@ -84,13 +82,14 @@ export class SessionExportsManager {
         // by not acquiring a lock on read. That is because this we require this
         // interface to be fast and just accurate enough for MCP completions
         // API.
-        return this.mutableExports.filter(({ createdAt }) => {
+        return this.availableExports.filter(({ createdAt }) => {
             return !this.isExportExpired(createdAt);
         });
     }
 
     public async readExport(exportNameWithExtension: string): Promise<string> {
         try {
+            this.validateExportName(exportNameWithExtension);
             const exportsDirectoryPath = await this.ensureExportsDirectory();
             const exportFilePath = this.exportFilePath(exportsDirectoryPath, exportNameWithExtension);
             if (await this.isExportFileExpired(exportFilePath)) {
@@ -118,7 +117,9 @@ export class SessionExportsManager {
         jsonExportFormat: JSONExportFormat;
     }): Promise<void> {
         try {
-            const exportNameWithExtension = this.withExtension(exportName, "json");
+            const exportNameWithExtension = this.ensureExtension(exportName, "json");
+            this.validateExportName(exportNameWithExtension);
+
             const inputStream = input.stream();
             const ejsonDocStream = this.docToEJSONStream(this.getEJSONOptionsForFormat(jsonExportFormat));
             await this.withExportsLock<void>(async (exportsDirectoryPath) => {
@@ -146,8 +147,8 @@ export class SessionExportsManager {
                     void input.close();
                     if (pipeSuccessful) {
                         const resourceURI = this.exportNameToResourceURI(exportNameWithExtension);
-                        this.mutableExports = [
-                            ...this.mutableExports,
+                        this.availableExports = [
+                            ...this.availableExports,
                             {
                                 createdAt: (await fs.stat(exportFilePath)).birthtimeMs,
                                 name: exportNameWithExtension,
@@ -216,7 +217,7 @@ export class SessionExportsManager {
                     const exportPath = this.exportFilePath(exportsDirectoryPath, exportName);
                     if (await this.isExportFileExpired(exportPath)) {
                         await fs.unlink(exportPath);
-                        this.mutableExports = this.mutableExports.filter(({ name }) => name !== exportName);
+                        this.availableExports = this.availableExports.filter(({ name }) => name !== exportName);
                         this.session.emit("export-expired", this.exportNameToResourceURI(exportName));
                     }
                 }
@@ -232,6 +233,19 @@ export class SessionExportsManager {
         }
     }
 
+    /**
+     * Small utility to validate provided export name for path traversal or no
+     * extension */
+    private validateExportName(nameWithExtension: string): void {
+        if (!path.extname(nameWithExtension)) {
+            throw new Error("Provided export name has no extension");
+        }
+
+        if (nameWithExtension.includes("..") || nameWithExtension.includes("/") || nameWithExtension.includes("\\")) {
+            throw new Error("Invalid export name: path traversal hinted");
+        }
+    }
+
     /**
      * Small utility to centrally determine if an export is expired or not */
     private async isExportFileExpired(exportFilePath: string): Promise<boolean> {
@@ -252,7 +266,7 @@ export class SessionExportsManager {
 
     /**
      * Ensures the path ends with the provided extension */
-    private withExtension(pathOrName: string, extension: string): string {
+    private ensureExtension(pathOrName: string, extension: string): string {
         const extWithDot = extension.startsWith(".") ? extension : `.${extension}`;
         if (path.extname(pathOrName) === extWithDot) {
             return pathOrName;
@@ -274,7 +288,7 @@ export class SessionExportsManager {
         let releaseLock: (() => Promise<void>) | undefined;
         const exportsDirectoryPath = await this.ensureExportsDirectory();
         try {
-            releaseLock = await lock(exportsDirectoryPath, { retries: 10 });
+            releaseLock = await lock(exportsDirectoryPath, { retries: MAX_LOCK_RETRIES });
             return await callback(exportsDirectoryPath);
         } finally {
             await releaseLock?.();

src/resources/common/exportedData.ts

@@ -18,7 +18,6 @@ export class ExportedData {
             void this.server.mcpServer.server.sendResourceUpdated({
                 uri,
             });
-            this.server.mcpServer.sendResourceListChanged();
         });
         this.server.session.on("export-expired", () => {
             this.server.mcpServer.sendResourceListChanged();
@@ -52,8 +51,11 @@ export class ExportedData {
             if (!sessionId) {
                 // Note that we don't throw error here because this is a
                 // non-critical path and safe to return the most harmless value.
-
-                // TODO: log warn here
+                logger.warning(
+                    LogId.exportedDataSessionUninitialized,
+                    "In ListResourcesCallback of exported-data resource",
+                    "Session not initialized"
+                );
                 return { resources: [] };
             }
 
@@ -84,8 +86,11 @@ export class ExportedData {
             if (!sessionId) {
                 // Note that we don't throw error here because this is a
                 // non-critical path and safe to return the most harmless value.
-
-                // TODO: log warn here
+                logger.warning(
+                    LogId.exportedDataSessionUninitialized,
+                    "In CompleteResourceTemplateCallback of exported-data resource",
+                    "Session not initialized"
+                );
                 return [];
             }