Skip to content

Commit 11cd835

Browse files
Merge pull request #296 from ruturaj-browserstack/PMAA-107-upload-path-traversal-fix
fix(security): validate upload paths to prevent file exfiltration (PM…
2 parents eced804 + 2bbb19d commit 11cd835

6 files changed

Lines changed: 370 additions & 27 deletions

File tree

src/config.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export class Config {
4040
public readonly browserstackLocalOptions: Record<string, any>,
4141
public readonly USE_OWN_LOCAL_BINARY_PROCESS: boolean,
4242
public readonly REMOTE_MCP: boolean,
43+
public readonly UPLOAD_BASE_DIR: string | undefined,
4344
) {}
4445
}
4546

@@ -48,6 +49,9 @@ const config = new Config(
4849
browserstackLocalOptions,
4950
process.env.USE_OWN_LOCAL_BINARY_PROCESS === "true",
5051
process.env.REMOTE_MCP === "true",
52+
process.env.MCP_UPLOAD_BASE_DIR && process.env.MCP_UPLOAD_BASE_DIR.length > 0
53+
? process.env.MCP_UPLOAD_BASE_DIR
54+
: undefined,
5155
);
5256

5357
export default config;

src/lib/upload-validator.ts

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import fs from "fs";
2+
import path from "path";
3+
4+
export interface UploadValidationOptions {
5+
allowedExtensions: readonly string[];
6+
maxSizeBytes: number;
7+
allowedBaseDir?: string;
8+
}
9+
10+
/**
11+
* Canonicalizes and validates a user-supplied upload path. Returns the resolved
12+
* absolute path that callers should stream from. Throws on any rule violation.
13+
*
14+
* Rules enforced:
15+
* - Path resolves (via realpath) to an existing regular file
16+
* - File size is within `maxSizeBytes`
17+
* - File extension is in `allowedExtensions` (case-insensitive)
18+
* - No path segment is a hidden dir/file (starts with `.`); blocks ~/.ssh,
19+
* ~/.aws, .env, etc. even after symlink resolution
20+
* - If `allowedBaseDir` is set, the canonical path must live inside it
21+
*/
22+
export function validateUploadPath(
23+
filePath: string,
24+
options: UploadValidationOptions,
25+
): string {
26+
if (typeof filePath !== "string" || filePath.trim().length === 0) {
27+
throw new Error("Upload rejected: file path is empty.");
28+
}
29+
30+
let canonical: string;
31+
try {
32+
canonical = fs.realpathSync(path.resolve(filePath));
33+
} catch {
34+
throw new Error(`File not found at path: ${filePath}`);
35+
}
36+
37+
let stats: fs.Stats;
38+
try {
39+
stats = fs.statSync(canonical);
40+
} catch {
41+
throw new Error(`File not found at path: ${filePath}`);
42+
}
43+
44+
if (!stats.isFile()) {
45+
throw new Error(
46+
`Upload rejected: path does not point to a regular file: ${filePath}`,
47+
);
48+
}
49+
50+
if (stats.size > options.maxSizeBytes) {
51+
const maxMb = Math.round(options.maxSizeBytes / (1024 * 1024));
52+
throw new Error(
53+
`Upload rejected: file exceeds maximum allowed size of ${maxMb} MB.`,
54+
);
55+
}
56+
57+
const segments = canonical.split(path.sep).filter((s) => s.length > 0);
58+
for (const seg of segments) {
59+
if (seg.startsWith(".") && seg !== "." && seg !== "..") {
60+
throw new Error(
61+
`Upload rejected: path traverses a hidden directory or file ("${seg}"). Move the file to a non-hidden location or set MCP_UPLOAD_BASE_DIR.`,
62+
);
63+
}
64+
}
65+
66+
const ext = path.extname(canonical).toLowerCase();
67+
const allowed = options.allowedExtensions.map((e) => e.toLowerCase());
68+
if (!allowed.includes(ext)) {
69+
throw new Error(
70+
`Upload rejected: file extension "${ext || "(none)"}" is not in the allowed list (${allowed.join(", ")}).`,
71+
);
72+
}
73+
74+
if (options.allowedBaseDir) {
75+
let baseCanonical: string;
76+
try {
77+
baseCanonical = fs.realpathSync(path.resolve(options.allowedBaseDir));
78+
} catch {
79+
throw new Error(
80+
`Upload rejected: configured MCP_UPLOAD_BASE_DIR does not exist (${options.allowedBaseDir}).`,
81+
);
82+
}
83+
const baseWithSep = baseCanonical.endsWith(path.sep)
84+
? baseCanonical
85+
: baseCanonical + path.sep;
86+
if (canonical !== baseCanonical && !canonical.startsWith(baseWithSep)) {
87+
throw new Error(
88+
`Upload rejected: file must be located inside ${baseCanonical}.`,
89+
);
90+
}
91+
}
92+
93+
return canonical;
94+
}
95+
96+
export const APP_BINARY_EXTENSIONS = [
97+
".apk",
98+
".aab",
99+
".ipa",
100+
".app",
101+
".zip",
102+
] as const;
103+
104+
export const TEST_MANAGEMENT_ATTACHMENT_EXTENSIONS = [
105+
".pdf",
106+
".txt",
107+
".md",
108+
".doc",
109+
".docx",
110+
".png",
111+
".jpg",
112+
".jpeg",
113+
".gif",
114+
".csv",
115+
".xls",
116+
".xlsx",
117+
".json",
118+
".html",
119+
".zip",
120+
] as const;
121+
122+
export const ONE_MB = 1024 * 1024;
123+
export const MAX_APP_UPLOAD_BYTES = 4 * 1024 * ONE_MB; // 4 GB — matches BrowserStack app upload limit
124+
export const MAX_ATTACHMENT_UPLOAD_BYTES = 100 * ONE_MB; // 100 MB

src/tools/appautomate-utils/native-execution/appautomate.ts

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ import { apiClient } from "../../../lib/apiClient.js";
44
import { customFuzzySearch } from "../../../lib/fuzzy.js";
55
import { getBrowserStackAuth } from "../../../lib/get-auth.js";
66
import { BrowserStackConfig } from "../../../lib/types.js";
7+
import {
8+
validateUploadPath,
9+
UploadValidationOptions,
10+
APP_BINARY_EXTENSIONS,
11+
MAX_APP_UPLOAD_BYTES,
12+
} from "../../../lib/upload-validator.js";
13+
import appConfig from "../../../config.js";
714

815
interface Device {
916
device: string;
@@ -139,14 +146,14 @@ export async function uploadApp(
139146
username: string,
140147
password: string,
141148
): Promise<string> {
142-
const filePath = appPath;
143-
144-
if (!fs.existsSync(filePath)) {
145-
throw new Error(`File not found at path: ${filePath}`);
146-
}
149+
const safePath = validateUploadPath(appPath, {
150+
allowedExtensions: APP_BINARY_EXTENSIONS,
151+
maxSizeBytes: MAX_APP_UPLOAD_BYTES,
152+
allowedBaseDir: appConfig.UPLOAD_BASE_DIR,
153+
});
147154

148155
const formData = new FormData();
149-
formData.append("file", fs.createReadStream(filePath));
156+
formData.append("file", fs.createReadStream(safePath));
150157

151158
const response = await apiClient.post<UploadResponse>({
152159
url: "https://api-cloud.browserstack.com/app-automate/upload",
@@ -171,13 +178,16 @@ async function uploadFileToBrowserStack(
171178
endpoint: string,
172179
responseKey: string,
173180
config: BrowserStackConfig,
181+
validation: Pick<UploadValidationOptions, "allowedExtensions">,
174182
): Promise<string> {
175-
if (!fs.existsSync(filePath)) {
176-
throw new Error(`File not found at path: ${filePath}`);
177-
}
183+
const safePath = validateUploadPath(filePath, {
184+
allowedExtensions: validation.allowedExtensions,
185+
maxSizeBytes: MAX_APP_UPLOAD_BYTES,
186+
allowedBaseDir: appConfig.UPLOAD_BASE_DIR,
187+
});
178188

179189
const formData = new FormData();
180-
formData.append("file", fs.createReadStream(filePath));
190+
formData.append("file", fs.createReadStream(safePath));
181191

182192
const authHeader =
183193
"Basic " +
@@ -211,6 +221,7 @@ export async function uploadEspressoApp(
211221
"https://api-cloud.browserstack.com/app-automate/espresso/v2/app",
212222
"app_url",
213223
config,
224+
{ allowedExtensions: [".apk", ".aab"] },
214225
);
215226
}
216227

@@ -224,6 +235,7 @@ export async function uploadEspressoTestSuite(
224235
"https://api-cloud.browserstack.com/app-automate/espresso/v2/test-suite",
225236
"test_suite_url",
226237
config,
238+
{ allowedExtensions: [".apk"] },
227239
);
228240
}
229241

@@ -237,6 +249,7 @@ export async function uploadXcuiApp(
237249
"https://api-cloud.browserstack.com/app-automate/xcuitest/v2/app",
238250
"app_url",
239251
config,
252+
{ allowedExtensions: [".ipa"] },
240253
);
241254
}
242255

@@ -250,6 +263,7 @@ export async function uploadXcuiTestSuite(
250263
"https://api-cloud.browserstack.com/app-automate/xcuitest/v2/test-suite",
251264
"test_suite_url",
252265
config,
266+
{ allowedExtensions: [".zip"] },
253267
);
254268
}
255269

src/tools/applive-utils/upload-app.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { apiClient } from "../../lib/apiClient.js";
22
import FormData from "form-data";
33
import fs from "fs";
4+
import {
5+
validateUploadPath,
6+
APP_BINARY_EXTENSIONS,
7+
MAX_APP_UPLOAD_BYTES,
8+
} from "../../lib/upload-validator.js";
9+
import appConfig from "../../config.js";
410

511
interface UploadResponse {
612
app_url: string;
@@ -11,12 +17,14 @@ export async function uploadApp(
1117
username: string,
1218
password: string,
1319
): Promise<UploadResponse> {
14-
if (!fs.existsSync(filePath)) {
15-
throw new Error(`File not found at path: ${filePath}`);
16-
}
20+
const safePath = validateUploadPath(filePath, {
21+
allowedExtensions: APP_BINARY_EXTENSIONS,
22+
maxSizeBytes: MAX_APP_UPLOAD_BYTES,
23+
allowedBaseDir: appConfig.UPLOAD_BASE_DIR,
24+
});
1725

1826
const formData = new FormData();
19-
formData.append("file", fs.createReadStream(filePath));
27+
formData.append("file", fs.createReadStream(safePath));
2028

2129
try {
2230
const response = await apiClient.post<UploadResponse>({

src/tools/testmanagement-utils/upload-file.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ import { signedUrlMap } from "../../lib/inmemory-store.js";
1010
import { projectIdentifierToId } from "./TCG-utils/api.js";
1111
import { BrowserStackConfig } from "../../lib/types.js";
1212
import { getTMBaseURL } from "../../lib/tm-base-url.js";
13+
import {
14+
validateUploadPath,
15+
TEST_MANAGEMENT_ATTACHMENT_EXTENSIONS,
16+
MAX_ATTACHMENT_UPLOAD_BYTES,
17+
} from "../../lib/upload-validator.js";
18+
import appConfig from "../../config.js";
1319

1420
/**
1521
* Schema for the upload file tool
@@ -35,26 +41,22 @@ export async function uploadFile(
3541
const { project_identifier, file_path } = args;
3642

3743
try {
38-
// Validate file exists
39-
if (!fs.existsSync(file_path)) {
40-
return {
41-
content: [
42-
{
43-
type: "text",
44-
text: `File ${file_path} does not exist.`,
45-
},
46-
],
47-
isError: true,
48-
};
49-
}
44+
// Canonicalize path and enforce upload safety rules (extension, size,
45+
// hidden-directory traversal, optional base-dir containment).
46+
const safePath = validateUploadPath(file_path, {
47+
allowedExtensions: TEST_MANAGEMENT_ATTACHMENT_EXTENSIONS,
48+
maxSizeBytes: MAX_ATTACHMENT_UPLOAD_BYTES,
49+
allowedBaseDir: appConfig.UPLOAD_BASE_DIR,
50+
});
51+
5052
// Get the project ID
5153
const projectIdResponse = await projectIdentifierToId(
5254
project_identifier,
5355
config,
5456
);
5557

5658
const formData = new FormData();
57-
formData.append("attachments[]", fs.createReadStream(file_path));
59+
formData.append("attachments[]", fs.createReadStream(safePath));
5860

5961
const tmBaseUrl = await getTMBaseURL(config);
6062
const uploadUrl = `${tmBaseUrl}/api/v1/projects/${projectIdResponse}/generic/attachments/ai_uploads`;

0 commit comments

Comments
 (0)