From 9b3ffb8ce9c3d8015459adba2d6ee8c214224696 Mon Sep 17 00:00:00 2001
From: Savio Dias <savio@browserstack.com>
Date: Thu, 10 Sep 2026 15:11:19 +0530
Subject: [PATCH 1/2] feat(security): require MCP_UPLOAD_BASE_DIR for the
 upload-PRD tool

uploadProductRequirementFile now refuses unless the operator has set
MCP_UPLOAD_BASE_DIR to an allowed directory; when set, uploads are confined to
that directory (existing validateUploadPath containment). This makes file
uploads opt-in instead of accepting arbitrary local paths.

- upload-file.ts: refuse with clear guidance when MCP_UPLOAD_BASE_DIR is unset;
  file_path schema now documents the base-dir requirement.
- tests: gate refuses when unset, and passes through when set.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 src/tools/testmanagement-utils/upload-file.ts | 25 +++++++++++--
 tests/tools/uploadFile.test.ts                | 36 +++++++++++++++++++
 2 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 tests/tools/uploadFile.test.ts

diff --git a/src/tools/testmanagement-utils/upload-file.ts b/src/tools/testmanagement-utils/upload-file.ts
index a9d662a9..cee14d54 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,27 @@ export async function uploadFile(
 ): Promise<CallToolResult> {
   const { project_identifier, file_path } = args;
 
+  // File upload is opt-in: it is available only when the operator has declared
+  // an allowed directory via MCP_UPLOAD_BASE_DIR. Without it, refuse rather than
+  // allow uploads from arbitrary paths.
+  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");
+  });
+});

From fca19f5901097fb99d1bd6c2f3c43dccddc911c1 Mon Sep 17 00:00:00 2001
From: Savio Dias <savio@browserstack.com>
Date: Thu, 10 Sep 2026 15:17:02 +0530
Subject: [PATCH 2/2] chore: drop redundant comment on the upload base-dir gate

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 src/tools/testmanagement-utils/upload-file.ts | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/tools/testmanagement-utils/upload-file.ts b/src/tools/testmanagement-utils/upload-file.ts
index cee14d54..cc8ed173 100644
--- a/src/tools/testmanagement-utils/upload-file.ts
+++ b/src/tools/testmanagement-utils/upload-file.ts
@@ -43,9 +43,6 @@ export async function uploadFile(
 ): Promise<CallToolResult> {
   const { project_identifier, file_path } = args;
 
-  // File upload is opt-in: it is available only when the operator has declared
-  // an allowed directory via MCP_UPLOAD_BASE_DIR. Without it, refuse rather than
-  // allow uploads from arbitrary paths.
   if (!appConfig.UPLOAD_BASE_DIR) {
     return {
       content: [