From 23b3bc13a061c7fdf2b68505ce5101a030b38783 Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Tue, 15 Sep 2026 18:48:51 +0530 Subject: [PATCH 1/5] fix(security): make MCP_UPLOAD_BASE_DIR mandatory for all upload tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Directory containment in validateUploadPath was opt-in: the base-dir prefix check only ran when allowedBaseDir was set, and MCP_UPLOAD_BASE_DIR is unset by default. With it unset, any caller-supplied absolute path to a non-hidden, allow-listed-extension file was accepted and streamed to BrowserStack — an arbitrary file read/exfiltration on a default install (GHSA-j4xm-vw5v-c87v). Only the upload-PRD tool (upload-file.ts) had its own refuse gate; the App Automate (uploadApp, uploadEspressoApp) and App Live (app upload) paths went through the same opt-in validator and remained exposed. Make containment mandatory in validateUploadPath itself: refuse the upload unless allowedBaseDir is configured, then enforce that the canonical path lives inside it. This covers every upload tool uniformly via the single shared validator. Behavior change: App Automate app upload, Espresso app upload, and App Live sessions now require MCP_UPLOAD_BASE_DIR to be set (same requirement already in place for the upload-PRD tool). Co-Authored-By: Claude Opus 4.8 --- src/lib/upload-validator.ts | 46 +++++++++++++++++----------- tests/tools/upload-validator.test.ts | 23 ++++++++++++++ 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/lib/upload-validator.ts b/src/lib/upload-validator.ts index d516d129..4114bd57 100644 --- a/src/lib/upload-validator.ts +++ b/src/lib/upload-validator.ts @@ -17,7 +17,8 @@ export interface UploadValidationOptions { * - File extension is in `allowedExtensions` (case-insensitive) * - No path segment is a hidden dir/file (starts with `.`); blocks ~/.ssh, * ~/.aws, .env, etc. even after symlink resolution - * - If `allowedBaseDir` is set, the canonical path must live inside it + * - `allowedBaseDir` is mandatory: uploads are refused unless it is configured + * (via MCP_UPLOAD_BASE_DIR), and the canonical path must live inside it */ export function validateUploadPath( filePath: string, @@ -71,23 +72,32 @@ export function validateUploadPath( ); } - if (options.allowedBaseDir) { - let baseCanonical: string; - try { - baseCanonical = fs.realpathSync(path.resolve(options.allowedBaseDir)); - } catch { - throw new Error( - `Upload rejected: configured MCP_UPLOAD_BASE_DIR does not exist (${options.allowedBaseDir}).`, - ); - } - const baseWithSep = baseCanonical.endsWith(path.sep) - ? baseCanonical - : baseCanonical + path.sep; - if (canonical !== baseCanonical && !canonical.startsWith(baseWithSep)) { - throw new Error( - `Upload rejected: file must be located inside ${baseCanonical}.`, - ); - } + // Directory containment is mandatory. Without a configured base dir there is + // nothing confining the (possibly absolute) path, so any readable file on the + // host could be streamed off it — refuse rather than fall back to "no check". + if (!options.allowedBaseDir) { + throw new Error( + "Upload rejected: file uploads are disabled because MCP_UPLOAD_BASE_DIR is not set. " + + "Set MCP_UPLOAD_BASE_DIR to a directory containing the files you want to upload, then " + + "restart the MCP server. Uploads are restricted to that directory.", + ); + } + + let baseCanonical: string; + try { + baseCanonical = fs.realpathSync(path.resolve(options.allowedBaseDir)); + } catch { + throw new Error( + `Upload rejected: configured MCP_UPLOAD_BASE_DIR does not exist (${options.allowedBaseDir}).`, + ); + } + const baseWithSep = baseCanonical.endsWith(path.sep) + ? baseCanonical + : baseCanonical + path.sep; + if (canonical !== baseCanonical && !canonical.startsWith(baseWithSep)) { + throw new Error( + `Upload rejected: file must be located inside ${baseCanonical}.`, + ); } return canonical; diff --git a/tests/tools/upload-validator.test.ts b/tests/tools/upload-validator.test.ts index 51eed475..f423f194 100644 --- a/tests/tools/upload-validator.test.ts +++ b/tests/tools/upload-validator.test.ts @@ -32,10 +32,32 @@ describe("validateUploadPath", () => { const resolved = validateUploadPath(file, { allowedExtensions: APP_BINARY_EXTENSIONS, maxSizeBytes: MAX_APP_UPLOAD_BYTES, + allowedBaseDir: workDir, }); expect(resolved).toBe(fs.realpathSync(file)); }); + it("refuses when no base directory is configured (MCP_UPLOAD_BASE_DIR unset)", () => { + const file = write("app.apk"); + expect(() => + validateUploadPath(file, { + allowedExtensions: APP_BINARY_EXTENSIONS, + maxSizeBytes: MAX_APP_UPLOAD_BYTES, + }), + ).toThrow(/MCP_UPLOAD_BASE_DIR is not set/); + }); + + it("refuses when the configured base directory does not exist", () => { + const file = write("app.apk"); + expect(() => + validateUploadPath(file, { + allowedExtensions: APP_BINARY_EXTENSIONS, + maxSizeBytes: MAX_APP_UPLOAD_BYTES, + allowedBaseDir: path.join(os.tmpdir(), "no-such-base-dir-xyz"), + }), + ).toThrow(/does not exist/); + }); + it("rejects an empty path", () => { expect(() => validateUploadPath(" ", { @@ -185,6 +207,7 @@ describe("validateUploadPath", () => { const resolved = validateUploadPath(file, { allowedExtensions: APP_BINARY_EXTENSIONS, maxSizeBytes: MAX_APP_UPLOAD_BYTES, + allowedBaseDir: workDir, }); expect(resolved).toBe(fs.realpathSync(file)); }); From 82339575e788662ca043ef396ab1335c89b78800 Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Wed, 16 Sep 2026 12:13:24 +0530 Subject: [PATCH 2/5] refactor(security): drop redundant comment and shorten the upload gate message Co-Authored-By: Claude Opus 4.8 --- src/lib/upload-validator.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/lib/upload-validator.ts b/src/lib/upload-validator.ts index 4114bd57..86fa0aa9 100644 --- a/src/lib/upload-validator.ts +++ b/src/lib/upload-validator.ts @@ -72,14 +72,10 @@ export function validateUploadPath( ); } - // Directory containment is mandatory. Without a configured base dir there is - // nothing confining the (possibly absolute) path, so any readable file on the - // host could be streamed off it — refuse rather than fall back to "no check". if (!options.allowedBaseDir) { throw new Error( - "Upload rejected: file uploads are disabled because MCP_UPLOAD_BASE_DIR is not set. " + - "Set MCP_UPLOAD_BASE_DIR to a directory containing the files you want to upload, then " + - "restart the MCP server. Uploads are restricted to that directory.", + "Upload rejected: MCP_UPLOAD_BASE_DIR is not set. Set it to the directory " + + "containing the files to upload, then restart the MCP server.", ); } From 933172d86f438f7905894506113e3d7d7715f0ed Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Wed, 16 Sep 2026 18:41:19 +0530 Subject: [PATCH 3/5] docs+refactor(security): document MCP_UPLOAD_BASE_DIR and tighten upload types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on the mandatory upload base-dir change: - README: add an "Enabling File & App Uploads" section explaining MCP_UPLOAD_BASE_DIR — what it is, an example env entry, and the four tools that require it (uploadProductRequirementFile, takeAppScreenshot, runAppTestsOnBrowserStack, runAppLiveSession). Without this the change looks like the upload tools "just broke" on a default install. - upload-validator: make `allowedBaseDir` a required key (value may be undefined) so every call site must consciously thread the configured base dir — a missing one becomes a compile error instead of a silent runtime always-refuse. - App/test path tool descriptions now note the file must live inside MCP_UPLOAD_BASE_DIR, so the model doesn't confidently pass paths that will be refused. Co-Authored-By: Claude Opus 4.8 --- README.md | 27 +++++++++++++++++++ src/lib/upload-validator.ts | 5 +++- .../native-execution/constants.ts | 6 +++-- src/tools/appautomate.ts | 2 +- src/tools/applive.ts | 2 +- 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index d8c349e2..7fff2135 100644 --- a/README.md +++ b/README.md @@ -292,6 +292,33 @@ Select the “Installed” tab. Click the “Configure MCP Servers” button at } ``` +### 📁 Enabling File & App Uploads (`MCP_UPLOAD_BASE_DIR`) + +Tools that read a local file or app binary and upload it to BrowserStack are **disabled by default** and refuse to run until you set the `MCP_UPLOAD_BASE_DIR` environment variable. It must point to a directory containing the files you want to upload; uploads are then restricted to (and canonicalized within) that directory, so a caller cannot exfiltrate arbitrary files from the host. + +Tools that require it: + +- `uploadProductRequirementFile` (Test Management PRD upload) +- `takeAppScreenshot` (App Automate app upload) +- `runAppTestsOnBrowserStack` (App Automate app + test-suite upload) +- `runAppLiveSession` (App Live app upload) + +Add it to the server `env` block alongside your credentials, for example: + +```json +{ + "command": "npx", + "args": ["-y", "@browserstack/mcp-server@latest"], + "env": { + "BROWSERSTACK_USERNAME": "", + "BROWSERSTACK_ACCESS_KEY": "", + "MCP_UPLOAD_BASE_DIR": "/absolute/path/to/your/uploads" + } +} +``` + +If it is not set, the tools above return: _"Upload rejected: MCP_UPLOAD_BASE_DIR is not set…"_ — set the variable and restart the MCP server. + ### 💡 List of BrowserStack MCP Tools As of now we support 45 tools. diff --git a/src/lib/upload-validator.ts b/src/lib/upload-validator.ts index 86fa0aa9..1a8b839a 100644 --- a/src/lib/upload-validator.ts +++ b/src/lib/upload-validator.ts @@ -4,7 +4,10 @@ import path from "path"; export interface UploadValidationOptions { allowedExtensions: readonly string[]; maxSizeBytes: number; - allowedBaseDir?: string; + // Required key (value may be undefined when MCP_UPLOAD_BASE_DIR is unset) so + // every call site must consciously thread the configured base dir — a missing + // one is a compile error, not a silent runtime always-refuse. + allowedBaseDir: string | undefined; } /** diff --git a/src/tools/appautomate-utils/native-execution/constants.ts b/src/tools/appautomate-utils/native-execution/constants.ts index 8b1a5360..58d5aa10 100644 --- a/src/tools/appautomate-utils/native-execution/constants.ts +++ b/src/tools/appautomate-utils/native-execution/constants.ts @@ -26,7 +26,8 @@ export const RUN_APP_AUTOMATE_SCHEMA = { " xcodebuild clean -scheme YOUR_SCHEME && \\\n" + " xcodebuild archive -scheme YOUR_SCHEME -configuration Release -archivePath build/app.xcarchive && \\\n" + " xcodebuild -exportArchive -archivePath build/app.xcarchive -exportPath build/ipa -exportOptionsPlist exportOptions.plist\n\n" + - "If in other directory, provide existing app path", + "If in other directory, provide existing app path.\n" + + "The resolved file must be located inside the directory set in MCP_UPLOAD_BASE_DIR.", ), testSuitePath: z .string() @@ -38,7 +39,8 @@ export const RUN_APP_AUTOMATE_SCHEMA = { " xcodebuild test-without-building -scheme YOUR_SCHEME -destination 'generic/platform=iOS' && \\\n" + " cd ~/Library/Developer/Xcode/DerivedData/*/Build/Products/Debug-iphonesimulator/ && \\\n" + " zip -r Tests.zip *.xctestrun *-Runner.app\n\n" + - "If in other directory, provide existing test file path", + "If in other directory, provide existing test file path.\n" + + "The resolved file must be located inside the directory set in MCP_UPLOAD_BASE_DIR.", ), devices: z .array(MobileDeviceSchema) diff --git a/src/tools/appautomate.ts b/src/tools/appautomate.ts index 36d05826..0fbd0e29 100644 --- a/src/tools/appautomate.ts +++ b/src/tools/appautomate.ts @@ -333,7 +333,7 @@ export default function addAppAutomationTools( appPath: z .string() .describe( - "The path to the .apk or .ipa file. Required for app installation.", + "The path to the .apk or .ipa file. Required for app installation. Must be located inside the directory set in MCP_UPLOAD_BASE_DIR.", ), }, { diff --git a/src/tools/applive.ts b/src/tools/applive.ts index e188e06e..a8e159a3 100644 --- a/src/tools/applive.ts +++ b/src/tools/applive.ts @@ -103,7 +103,7 @@ export default function addAppLiveTools( appPath: z .string() .describe( - "The path to the .ipa or .apk file to install on the device. Always ask the user for the app path, do not assume it.", + "The path to the .ipa or .apk file to install on the device. Always ask the user for the app path, do not assume it. Must be located inside the directory set in MCP_UPLOAD_BASE_DIR.", ), }, { From 6c0f52aafafe9136f6034951db0f674e4f4eccb3 Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Wed, 16 Sep 2026 18:55:54 +0530 Subject: [PATCH 4/5] refactor: drop redundant comment on allowedBaseDir Co-Authored-By: Claude Opus 4.8 --- src/lib/upload-validator.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/lib/upload-validator.ts b/src/lib/upload-validator.ts index 1a8b839a..fe420b66 100644 --- a/src/lib/upload-validator.ts +++ b/src/lib/upload-validator.ts @@ -4,9 +4,6 @@ import path from "path"; export interface UploadValidationOptions { allowedExtensions: readonly string[]; maxSizeBytes: number; - // Required key (value may be undefined when MCP_UPLOAD_BASE_DIR is unset) so - // every call site must consciously thread the configured base dir — a missing - // one is a compile error, not a silent runtime always-refuse. allowedBaseDir: string | undefined; } From 488ffce2b607b230f2afc3f1027fe3cbaf9b74a0 Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Wed, 16 Sep 2026 18:58:44 +0530 Subject: [PATCH 5/5] docs: condense MCP_UPLOAD_BASE_DIR note to a single line Co-Authored-By: Claude Opus 4.8 --- README.md | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/README.md b/README.md index 7fff2135..8322f079 100644 --- a/README.md +++ b/README.md @@ -292,32 +292,7 @@ Select the “Installed” tab. Click the “Configure MCP Servers” button at } ``` -### 📁 Enabling File & App Uploads (`MCP_UPLOAD_BASE_DIR`) - -Tools that read a local file or app binary and upload it to BrowserStack are **disabled by default** and refuse to run until you set the `MCP_UPLOAD_BASE_DIR` environment variable. It must point to a directory containing the files you want to upload; uploads are then restricted to (and canonicalized within) that directory, so a caller cannot exfiltrate arbitrary files from the host. - -Tools that require it: - -- `uploadProductRequirementFile` (Test Management PRD upload) -- `takeAppScreenshot` (App Automate app upload) -- `runAppTestsOnBrowserStack` (App Automate app + test-suite upload) -- `runAppLiveSession` (App Live app upload) - -Add it to the server `env` block alongside your credentials, for example: - -```json -{ - "command": "npx", - "args": ["-y", "@browserstack/mcp-server@latest"], - "env": { - "BROWSERSTACK_USERNAME": "", - "BROWSERSTACK_ACCESS_KEY": "", - "MCP_UPLOAD_BASE_DIR": "/absolute/path/to/your/uploads" - } -} -``` - -If it is not set, the tools above return: _"Upload rejected: MCP_UPLOAD_BASE_DIR is not set…"_ — set the variable and restart the MCP server. +> **File & app uploads:** tools that upload a local file/app (`uploadProductRequirementFile`, `takeAppScreenshot`, `runAppTestsOnBrowserStack`, `runAppLiveSession`) require the `MCP_UPLOAD_BASE_DIR` env var set to a directory containing those files; uploads are restricted to it. ### 💡 List of BrowserStack MCP Tools