Skip to content

Commit 6da6db7

Browse files
feat: Add xcodebuild extraArgs validation
Adds validation for xcodebuild extraArgs to prevent invalid options. Co-authored-by: cameron.cooke <cameron.cooke@sentry.io>
1 parent 6b23a50 commit 6da6db7

6 files changed

Lines changed: 238 additions & 1 deletion

File tree

src/mcp/tools/device/test_device.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
getSessionAwareToolSchemaShape,
2727
} from '../../../utils/typed-tool-factory.ts';
2828
import { nullifyEmptyStrings } from '../../../utils/schema-helpers.ts';
29+
import { validateXcodebuildExtraArgs } from '../../../utils/xcodebuild-validation.ts';
2930

3031
// Unified schema: XOR between projectPath and workspacePath
3132
const baseSchemaObject = z.object({
@@ -183,6 +184,13 @@ export async function testDeviceLogic(
183184
executor: CommandExecutor = getDefaultCommandExecutor(),
184185
fileSystemExecutor: FileSystemExecutor = getDefaultFileSystemExecutor(),
185186
): Promise<ToolResponse> {
187+
// Validate extraArgs for invalid xcodebuild options
188+
const validation = validateXcodebuildExtraArgs(params.extraArgs);
189+
if (!validation.isValid) {
190+
log('error', `Invalid extraArgs: ${validation.error}`);
191+
return createTextResponse(validation.error!, true);
192+
}
193+
186194
log(
187195
'info',
188196
`Starting test run for scheme ${params.scheme} on platform ${params.platform ?? 'iOS'} (internal)`,

src/mcp/tools/macos/test_macos.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
getSessionAwareToolSchemaShape,
2727
} from '../../../utils/typed-tool-factory.ts';
2828
import { nullifyEmptyStrings } from '../../../utils/schema-helpers.ts';
29+
import { validateXcodebuildExtraArgs } from '../../../utils/xcodebuild-validation.ts';
2930

3031
// Unified schema: XOR between projectPath and workspacePath
3132
const baseSchemaObject = z.object({
@@ -239,6 +240,13 @@ export async function testMacosLogic(
239240
executor: CommandExecutor = getDefaultCommandExecutor(),
240241
fileSystemExecutor: FileSystemExecutor = getDefaultFileSystemExecutor(),
241242
): Promise<ToolResponse> {
243+
// Validate extraArgs for invalid xcodebuild options
244+
const validation = validateXcodebuildExtraArgs(params.extraArgs);
245+
if (!validation.isValid) {
246+
log('error', `Invalid extraArgs: ${validation.error}`);
247+
return createTextResponse(validation.error!, true);
248+
}
249+
242250
log('info', `Starting test run for scheme ${params.scheme} on platform macOS (internal)`);
243251

244252
try {

src/mcp/tools/simulator/__tests__/test_sim.test.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
import { describe, it, expect, beforeEach } from 'vitest';
77
import * as z from 'zod';
88
import { sessionStore } from '../../../../utils/session-store.ts';
9-
import testSim from '../test_sim.ts';
9+
import testSim, { test_simLogic } from '../test_sim.ts';
10+
import { createMockExecutor } from '../../../../test-utils/mock-executors.ts';
1011

1112
describe('test_sim tool', () => {
1213
beforeEach(() => {
@@ -97,4 +98,47 @@ describe('test_sim tool', () => {
9798
expect(result.content[0].text).toContain('simulatorName');
9899
});
99100
});
101+
102+
describe('Validation', () => {
103+
it('should reject invalid xcodebuild options in extraArgs', async () => {
104+
const mockExecutor = createMockExecutor({ success: true, output: 'mock output' });
105+
106+
const result = await test_simLogic(
107+
{
108+
projectPath: '/path/to/project.xcodeproj',
109+
scheme: 'MyScheme',
110+
simulatorName: 'iPhone 16',
111+
configuration: 'Debug',
112+
extraArgs: ['-test-arg', '--snapshot-record'],
113+
},
114+
mockExecutor,
115+
);
116+
117+
expect(result.isError).toBe(true);
118+
expect(result.content[0].text).toContain('-test-arg');
119+
expect(result.content[0].text).toContain('not a recognized xcodebuild argument');
120+
expect(result.content[0].text).toContain('testRunnerEnv');
121+
});
122+
123+
it('should accept valid extraArgs', async () => {
124+
const mockExecutor = createMockExecutor({ success: true, output: 'mock output' });
125+
126+
const result = await test_simLogic(
127+
{
128+
projectPath: '/path/to/project.xcodeproj',
129+
scheme: 'MyScheme',
130+
simulatorName: 'iPhone 16',
131+
configuration: 'Debug',
132+
extraArgs: ['-only-testing:MyTests/MyTestClass'],
133+
},
134+
mockExecutor,
135+
);
136+
137+
// Should not fail due to validation (but might fail for other reasons in mock)
138+
// The key is that it doesn't fail with the validation error message
139+
if (result.isError) {
140+
expect(result.content[0].text).not.toContain('not a recognized xcodebuild argument');
141+
}
142+
});
143+
});
100144
});

src/mcp/tools/simulator/test_sim.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import {
1818
createSessionAwareTool,
1919
getSessionAwareToolSchemaShape,
2020
} from '../../../utils/typed-tool-factory.ts';
21+
import { validateXcodebuildExtraArgs } from '../../../utils/xcodebuild-validation.ts';
22+
import { createTextResponse } from '../../../utils/validation.ts';
2123

2224
// Define base schema object with all fields
2325
const baseSchemaObject = z.object({
@@ -91,6 +93,13 @@ export async function test_simLogic(
9193
params: TestSimulatorParams,
9294
executor: CommandExecutor,
9395
): Promise<ToolResponse> {
96+
// Validate extraArgs for invalid xcodebuild options
97+
const validation = validateXcodebuildExtraArgs(params.extraArgs);
98+
if (!validation.isValid) {
99+
log('error', `Invalid extraArgs: ${validation.error}`);
100+
return createTextResponse(validation.error!, true);
101+
}
102+
94103
// Log warning if useLatestOS is provided with simulatorId
95104
if (params.simulatorId && params.useLatestOS !== undefined) {
96105
log(
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* Tests for xcodebuild validation utilities
3+
*/
4+
5+
import { describe, it, expect } from 'vitest';
6+
import { validateXcodebuildExtraArgs } from '../xcodebuild-validation.ts';
7+
8+
describe('validateXcodebuildExtraArgs', () => {
9+
describe('valid extraArgs', () => {
10+
it('should return valid for undefined extraArgs', () => {
11+
const result = validateXcodebuildExtraArgs(undefined);
12+
expect(result.isValid).toBe(true);
13+
expect(result.error).toBeUndefined();
14+
});
15+
16+
it('should return valid for empty extraArgs array', () => {
17+
const result = validateXcodebuildExtraArgs([]);
18+
expect(result.isValid).toBe(true);
19+
expect(result.error).toBeUndefined();
20+
});
21+
22+
it('should return valid for legitimate xcodebuild options', () => {
23+
const result = validateXcodebuildExtraArgs([
24+
'-only-testing:MyTests/MyTestClass',
25+
'-verbose',
26+
'-dry-run',
27+
]);
28+
expect(result.isValid).toBe(true);
29+
expect(result.error).toBeUndefined();
30+
});
31+
32+
it('should return valid for result bundle path option', () => {
33+
const result = validateXcodebuildExtraArgs([
34+
'-resultBundlePath',
35+
'/path/to/results.xcresult',
36+
]);
37+
expect(result.isValid).toBe(true);
38+
expect(result.error).toBeUndefined();
39+
});
40+
41+
it('should return valid for multiple legitimate options', () => {
42+
const result = validateXcodebuildExtraArgs([
43+
'-only-testing:MyTests/MyTestClass',
44+
'-configuration',
45+
'Debug',
46+
'-sdk',
47+
'iphonesimulator',
48+
]);
49+
expect(result.isValid).toBe(true);
50+
expect(result.error).toBeUndefined();
51+
});
52+
});
53+
54+
describe('invalid extraArgs', () => {
55+
it('should reject -test-arg option', () => {
56+
const result = validateXcodebuildExtraArgs(['-test-arg', '--snapshot-record']);
57+
expect(result.isValid).toBe(false);
58+
expect(result.error).toContain('-test-arg');
59+
expect(result.error).toContain('not a recognized xcodebuild argument');
60+
expect(result.error).toContain('testRunnerEnv');
61+
});
62+
63+
it('should reject --test-arg option (double dash)', () => {
64+
const result = validateXcodebuildExtraArgs(['--test-arg', 'value']);
65+
expect(result.isValid).toBe(false);
66+
expect(result.error).toContain('--test-arg');
67+
});
68+
69+
it('should reject -testArg option (camelCase)', () => {
70+
const result = validateXcodebuildExtraArgs(['-testArg', 'value']);
71+
expect(result.isValid).toBe(false);
72+
expect(result.error).toContain('-testArg');
73+
});
74+
75+
it('should reject --testArg option (camelCase with double dash)', () => {
76+
const result = validateXcodebuildExtraArgs(['--testArg', 'value']);
77+
expect(result.isValid).toBe(false);
78+
expect(result.error).toContain('--testArg');
79+
});
80+
81+
it('should reject when invalid option is mixed with valid options', () => {
82+
const result = validateXcodebuildExtraArgs([
83+
'-only-testing:MyTests/MyTestClass',
84+
'-test-arg',
85+
'--snapshot-record',
86+
'-verbose',
87+
]);
88+
expect(result.isValid).toBe(false);
89+
expect(result.error).toContain('-test-arg');
90+
});
91+
92+
it('should provide helpful error message with testRunnerEnv suggestion', () => {
93+
const result = validateXcodebuildExtraArgs(['-test-arg', 'value']);
94+
expect(result.error).toContain('testRunnerEnv');
95+
expect(result.error).toContain('{ "testRunnerEnv": { "MY_VAR": "value" } }');
96+
});
97+
});
98+
99+
describe('edge cases', () => {
100+
it('should handle array with only invalid option', () => {
101+
const result = validateXcodebuildExtraArgs(['-test-arg']);
102+
expect(result.isValid).toBe(false);
103+
expect(result.error).toContain('-test-arg');
104+
});
105+
106+
it('should detect first invalid option in sequence', () => {
107+
const result = validateXcodebuildExtraArgs(['-test-arg', '--test-arg', '-testArg']);
108+
expect(result.isValid).toBe(false);
109+
// Should return error for first invalid option encountered
110+
expect(result.error).toBeDefined();
111+
});
112+
113+
it('should handle options that look similar but are valid', () => {
114+
// -test is a valid xcodebuild action, not in our invalid list
115+
const result = validateXcodebuildExtraArgs(['-test', '-verbose']);
116+
expect(result.isValid).toBe(true);
117+
});
118+
});
119+
});

src/utils/xcodebuild-validation.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* XcodeBuild Validation Utilities
3+
*
4+
* This module provides validation for xcodebuild command parameters,
5+
* particularly for detecting invalid or unsupported options.
6+
*/
7+
8+
/**
9+
* List of invalid or commonly misused xcodebuild options.
10+
* These options are not recognized by xcodebuild and will cause it to fail.
11+
*/
12+
const INVALID_XCODEBUILD_OPTIONS = new Set([
13+
'-test-arg', // Not a valid xcodebuild option; use testRunnerEnv instead
14+
'--test-arg',
15+
'-testArg',
16+
'--testArg',
17+
]);
18+
19+
/**
20+
* Validates extraArgs for invalid xcodebuild options.
21+
*
22+
* @param extraArgs - Array of extra arguments to validate
23+
* @returns Object with isValid flag and error message if invalid
24+
*/
25+
export function validateXcodebuildExtraArgs(extraArgs?: string[]): {
26+
isValid: boolean;
27+
error?: string;
28+
} {
29+
if (!extraArgs || extraArgs.length === 0) {
30+
return { isValid: true };
31+
}
32+
33+
// Check for invalid options
34+
for (const arg of extraArgs) {
35+
if (INVALID_XCODEBUILD_OPTIONS.has(arg)) {
36+
return {
37+
isValid: false,
38+
error: `Invalid xcodebuild option: '${arg}'. This is not a recognized xcodebuild argument.
39+
40+
If you're trying to pass arguments to the test runner, use the 'testRunnerEnv' parameter instead.
41+
Example: { "testRunnerEnv": { "MY_VAR": "value" } }
42+
43+
For more information, see the xcodebuild man page or documentation.`,
44+
};
45+
}
46+
}
47+
48+
return { isValid: true };
49+
}

0 commit comments

Comments
 (0)