diff --git a/README.md b/README.md index c34a8c4..0e95e0f 100644 --- a/README.md +++ b/README.md @@ -290,6 +290,22 @@ Select the “Installed” tab. Click the “Configure MCP Servers” button at } ``` +### 📂 Upload directory containment + +File-upload tools (app uploads, Test Management attachments) only read files inside the directory the MCP server was launched from (usually your project root). This is a security default: it stops a misbehaving agent from uploading arbitrary files on your machine. + +To upload files from elsewhere (e.g. `~/Downloads`), set `MCP_UPLOAD_BASE_DIR` to the directory your files live in: + + ```json + "env": { + "BROWSERSTACK_USERNAME": "", + "BROWSERSTACK_ACCESS_KEY": "", + "MCP_UPLOAD_BASE_DIR": "/Users/you/Downloads" + } + ``` + +If the server is launched with the filesystem root as its working directory (some MCP clients do this), uploads are rejected until `MCP_UPLOAD_BASE_DIR` is set. + ### 💡 List of BrowserStack MCP Tools As of now we support 44 tools. diff --git a/src/config.ts b/src/config.ts index 18d3d85..0e56a92 100644 --- a/src/config.ts +++ b/src/config.ts @@ -49,6 +49,10 @@ const config = new Config( browserstackLocalOptions, process.env.USE_OWN_LOCAL_BINARY_PROCESS === "true", process.env.REMOTE_MCP === "true", + // Undefined when MCP_UPLOAD_BASE_DIR is unset; validateUploadPath() then + // falls back to the process working directory (fail-closed containment). + // The default lives in the validator — the enforcement point — so it can + // distinguish a defaulted base dir from a configured one. process.env.MCP_UPLOAD_BASE_DIR && process.env.MCP_UPLOAD_BASE_DIR.length > 0 ? process.env.MCP_UPLOAD_BASE_DIR : undefined, diff --git a/src/lib/upload-validator.ts b/src/lib/upload-validator.ts index d516d12..980d8af 100644 --- a/src/lib/upload-validator.ts +++ b/src/lib/upload-validator.ts @@ -17,7 +17,9 @@ 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 + * - The canonical path must live inside `allowedBaseDir` (defaults to the + * process working directory when not provided — containment is fail-closed; + * a defaulted base dir that is the filesystem root is rejected outright) */ export function validateUploadPath( filePath: string, @@ -71,23 +73,30 @@ 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}.`, - ); - } + const usingDefaultBaseDir = options.allowedBaseDir === undefined; + const allowedBaseDir = options.allowedBaseDir ?? process.cwd(); + let baseCanonical: string; + try { + baseCanonical = fs.realpathSync(path.resolve(allowedBaseDir)); + } catch { + throw new Error( + usingDefaultBaseDir + ? `Upload rejected: the process working directory could not be resolved (${allowedBaseDir}). Set MCP_UPLOAD_BASE_DIR to a valid directory.` + : `Upload rejected: configured MCP_UPLOAD_BASE_DIR does not exist (${allowedBaseDir}).`, + ); + } + if (usingDefaultBaseDir && path.parse(baseCanonical).root === baseCanonical) { + throw new Error( + "Upload rejected: the process working directory is the filesystem root, so upload containment cannot be enforced. Set MCP_UPLOAD_BASE_DIR to the directory uploads may come from.", + ); + } + 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}. Set MCP_UPLOAD_BASE_DIR to allow uploads from a different directory.`, + ); } return canonical; diff --git a/tests/tools/upload-validator.test.ts b/tests/tools/upload-validator.test.ts index 51eed47..965a0ec 100644 --- a/tests/tools/upload-validator.test.ts +++ b/tests/tools/upload-validator.test.ts @@ -1,7 +1,7 @@ import fs from "fs"; import os from "os"; import path from "path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { validateUploadPath, APP_BINARY_EXTENSIONS, @@ -32,10 +32,69 @@ describe("validateUploadPath", () => { const resolved = validateUploadPath(file, { allowedExtensions: APP_BINARY_EXTENSIONS, maxSizeBytes: MAX_APP_UPLOAD_BYTES, + allowedBaseDir: workDir, }); expect(resolved).toBe(fs.realpathSync(file)); }); + it("defaults containment to the process working directory (fail-closed)", () => { + const otherDir = fs.mkdtempSync(path.join(os.tmpdir(), "other-cwd-")); + const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(otherDir); + try { + const outside = write("app.apk"); + expect(() => + validateUploadPath(outside, { + allowedExtensions: APP_BINARY_EXTENSIONS, + maxSizeBytes: MAX_APP_UPLOAD_BYTES, + }), + ).toThrow(/must be located inside/); + } finally { + cwdSpy.mockRestore(); + fs.rmSync(otherDir, { recursive: true, force: true }); + } + }); + + it("rejects a defaulted base dir that is the filesystem root", () => { + const cwdSpy = vi + .spyOn(process, "cwd") + .mockReturnValue(path.parse(workDir).root); + try { + const file = write("app.apk"); + expect(() => + validateUploadPath(file, { + allowedExtensions: APP_BINARY_EXTENSIONS, + maxSizeBytes: MAX_APP_UPLOAD_BYTES, + }), + ).toThrow(/filesystem root/); + } finally { + cwdSpy.mockRestore(); + } + }); + + it("honors an explicitly configured filesystem-root base dir", () => { + const file = write("app.apk"); + const resolved = validateUploadPath(file, { + allowedExtensions: APP_BINARY_EXTENSIONS, + maxSizeBytes: MAX_APP_UPLOAD_BYTES, + allowedBaseDir: path.parse(workDir).root, + }); + expect(resolved).toBe(fs.realpathSync(file)); + }); + + it("allows files inside the working directory when no base dir is set", () => { + const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(workDir); + try { + const file = write("app.apk"); + const resolved = validateUploadPath(file, { + allowedExtensions: APP_BINARY_EXTENSIONS, + maxSizeBytes: MAX_APP_UPLOAD_BYTES, + }); + expect(resolved).toBe(fs.realpathSync(file)); + } finally { + cwdSpy.mockRestore(); + } + }); + it("rejects an empty path", () => { expect(() => validateUploadPath(" ", { @@ -185,6 +244,7 @@ describe("validateUploadPath", () => { const resolved = validateUploadPath(file, { allowedExtensions: APP_BINARY_EXTENSIONS, maxSizeBytes: MAX_APP_UPLOAD_BYTES, + allowedBaseDir: workDir, }); expect(resolved).toBe(fs.realpathSync(file)); });