fix: correct package name and output path validation in utils.ts (#377)

- Allow hyphens in validatePackageName so iOS bundle identifiers like
  "com.some-company.app" aren't rejected. Apple's CFBundleIdentifier
  format permits hyphens; the regex previously only allowed
  alphanumeric characters, ".", and "_". This is shared with Android
  package names via android.ts/ios.ts/iphone-simulator.ts.

- Resolve symlinks on the allowed roots in validateOutputPath. On
  macOS, os.tmpdir() lives under /var/folders/..., and /var is a
  symlink to /private/var, so the default temp directory never
  matched its own allow-list entry, causing screenshot/recording
  saves to the default temp path to be rejected.

- Add test/utils.ts covering both fixes plus the existing validators,
  which previously had zero test coverage.
This commit is contained in:
Tsur Pinhas
2026-07-11 17:56:09 +02:00
committed by GitHub
parent 90fb660786
commit 36060d3cfb
2 changed files with 119 additions and 2 deletions
+18 -2
View File
@@ -4,7 +4,10 @@ import fs from "node:fs";
import { ActionableError } from "./robot";
export function validatePackageName(packageName: string): void {
if (!/^[a-zA-Z0-9._]+$/.test(packageName)) {
// iOS bundle identifiers (which flow through this same "packageName" parameter
// for launch/terminate) are allowed to contain hyphens per Apple's CFBundleIdentifier
// format, e.g. "com.some-company.app".
if (!/^[a-zA-Z0-9._-]+$/.test(packageName)) {
throw new ActionableError(`Invalid package name: "${packageName}"`);
}
}
@@ -15,6 +18,16 @@ export function validateLocale(locale: string): void {
}
}
function resolveRoot(root: string): string {
const resolved = path.resolve(root);
try {
return fs.realpathSync(resolved);
} catch {
return resolved;
}
}
function getAllowedRoots(): string[] {
const roots = [
os.tmpdir(),
@@ -27,7 +40,10 @@ function getAllowedRoots(): string[] {
roots.push("/private/tmp");
}
return roots.map(r => path.resolve(r));
// Resolve symlinks so roots line up with resolveWithSymlinks() below. On macOS,
// os.tmpdir() itself lives under /var/folders/..., and /var is a symlink to
// /private/var, so without this the real temp directory never matches here.
return roots.map(resolveRoot);
}
function isPathUnderRoot(filePath: string, root: string): boolean {
+101
View File
@@ -0,0 +1,101 @@
import { test, expect } from "@playwright/test";
import path from "node:path";
import os from "node:os";
import {
validatePackageName,
validateLocale,
validateFileExtension,
validateOutputPath,
} from "../src/utils";
test.describe("utils", () => {
test.describe("validatePackageName", () => {
test("should accept a standard android package name", () => {
expect(() => validatePackageName("com.example.app")).not.toThrow();
});
test("should accept a package name containing an underscore", () => {
expect(() => validatePackageName("com.example.my_app")).not.toThrow();
});
test("should accept an ios bundle id containing a hyphen", () => {
// CFBundleIdentifier explicitly permits hyphens, e.g. "com.some-company.app"
expect(() => validatePackageName("com.some-company.app")).not.toThrow();
});
test("should reject a package name containing a space", () => {
expect(() => validatePackageName("com.example app")).toThrow();
});
test("should reject a package name containing shell metacharacters", () => {
expect(() => validatePackageName("com.example.app; rm -rf /")).toThrow();
});
test("should reject an empty string", () => {
expect(() => validatePackageName("")).toThrow();
});
});
test.describe("validateLocale", () => {
test("should accept a single locale", () => {
expect(() => validateLocale("en-US")).not.toThrow();
});
test("should accept a comma-separated list of locales", () => {
expect(() => validateLocale("en-US, fr-FR")).not.toThrow();
});
test("should reject a locale containing shell metacharacters", () => {
expect(() => validateLocale("en-US; rm -rf /")).toThrow();
});
});
test.describe("validateFileExtension", () => {
test("should accept a matching extension", () => {
expect(() => validateFileExtension("/tmp/screenshot.png", [".png"], "save_screenshot")).not.toThrow();
});
test("should be case-insensitive", () => {
expect(() => validateFileExtension("/tmp/screenshot.PNG", [".png"], "save_screenshot")).not.toThrow();
});
test("should reject a non-matching extension", () => {
expect(() => validateFileExtension("/tmp/screenshot.txt", [".png", ".jpg"], "save_screenshot")).toThrow();
});
test("should reject a missing extension", () => {
expect(() => validateFileExtension("/tmp/screenshot", [".png"], "save_screenshot")).toThrow();
});
});
test.describe("validateOutputPath", () => {
test("should accept a path under the os temp directory", () => {
const target = path.join(os.tmpdir(), "mobile-mcp-test-output.png");
expect(() => validateOutputPath(target)).not.toThrow();
});
test("should accept a path under the current working directory", () => {
const target = path.join(process.cwd(), "mobile-mcp-test-output.png");
expect(() => validateOutputPath(target)).not.toThrow();
});
test("should reject a path outside of the allowed directories", () => {
const outside = process.platform === "win32"
? "C:\\Windows\\mobile-mcp-test-output.png"
: "/etc/mobile-mcp-test-output.png";
expect(() => validateOutputPath(outside)).toThrow();
});
test("should reject a path that escapes cwd via traversal", () => {
// A single ".." isn't reliably outside every allowed root (cwd can end up
// nested under os.tmpdir() in some environments). Climbing 32 levels always
// bottoms out at the filesystem root, which can never be a descendant of cwd,
// os.tmpdir(), or any other allowed root - regardless of where either lives.
const escape = new Array(32).fill("..").join(path.sep);
const target = path.join(process.cwd(), escape, "mobile-mcp-test-output.png");
expect(() => validateOutputPath(target)).toThrow();
});
});
});