diff --git a/src/tools/testmanagement-utils/upload-file.ts b/src/tools/testmanagement-utils/upload-file.ts index a9d662a9..cc8ed173 100644 --- a/src/tools/testmanagement-utils/upload-file.ts +++ b/src/tools/testmanagement-utils/upload-file.ts @@ -28,7 +28,10 @@ export const UploadFileSchema = z.object({ ), file_path: z .string() - .describe("Full path to the file that should be uploaded"), + .describe( + "Full path to the file that should be uploaded. Must be inside the " + + "directory configured via the MCP_UPLOAD_BASE_DIR environment variable.", + ), }); /** @@ -40,9 +43,24 @@ export async function uploadFile( ): Promise { const { project_identifier, file_path } = args; + if (!appConfig.UPLOAD_BASE_DIR) { + return { + content: [ + { + type: "text", + text: + "File upload is disabled. Set the MCP_UPLOAD_BASE_DIR environment " + + "variable to a directory that contains the files you want to upload, " + + "then restart the MCP server. Uploads are restricted to that directory.", + }, + ], + isError: true, + }; + } + try { // Canonicalize path and enforce upload safety rules (extension, size, - // hidden-directory traversal, optional base-dir containment). + // hidden-directory traversal, base-dir containment). const safePath = validateUploadPath(file_path, { allowedExtensions: TEST_MANAGEMENT_ATTACHMENT_EXTENSIONS, maxSizeBytes: MAX_ATTACHMENT_UPLOAD_BYTES, diff --git a/tests/tools/uploadFile.test.ts b/tests/tools/uploadFile.test.ts new file mode 100644 index 00000000..262f34fd --- /dev/null +++ b/tests/tools/uploadFile.test.ts @@ -0,0 +1,36 @@ +import { describe, it, expect, vi } from "vitest"; + +const { cfgMock } = vi.hoisted(() => ({ + cfgMock: { UPLOAD_BASE_DIR: undefined as string | undefined }, +})); +vi.mock("../../src/config.js", () => ({ default: cfgMock })); + +import { uploadFile } from "../../src/tools/testmanagement-utils/upload-file.js"; + +const bsConfig: any = { + "browserstack-username": "u", + "browserstack-access-key": "k", +}; + +describe("uploadFile — MCP_UPLOAD_BASE_DIR requirement", () => { + it("refuses the upload when MCP_UPLOAD_BASE_DIR is not set", async () => { + cfgMock.UPLOAD_BASE_DIR = undefined; + const res = await uploadFile( + { project_identifier: "P1", file_path: "/tmp/whatever.pdf" }, + bsConfig, + ); + expect(res.isError).toBe(true); + expect(res.content[0].text).toContain("File upload is disabled"); + expect(res.content[0].text).toContain("MCP_UPLOAD_BASE_DIR"); + }); + + it("passes the gate when set (any later failure is not the gate)", async () => { + cfgMock.UPLOAD_BASE_DIR = "/tmp"; + const res = await uploadFile( + { project_identifier: "P1", file_path: "/etc/hostname" }, // outside/ext-invalid + bsConfig, + ); + expect(res.isError).toBe(true); + expect(res.content[0].text).not.toContain("File upload is disabled"); + }); +});