feat(security): require MCP_UPLOAD_BASE_DIR for the upload-PRD tool

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<CallToolResult> {
   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,

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");
+  });
+});