Skip to content

Commit 6d9f4e3

Browse files
horza-borahorza-bora
authored
Merge pull request #805 from Dave-London/fix/build-args-dash-flags
fix(build): allow dash-prefixed args in build tool args arrays
2 parents 6111f6f + ca9c8a8 commit 6d9f4e3

10 files changed

Lines changed: 41 additions & 46 deletions

File tree

‎.changeset/fix-build-args-dash.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@paretools/build": patch
3+
---
4+
5+
Allow dash-prefixed arguments in build tool args arrays — execFile already prevents injection

‎packages/server-build/__tests__/security.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,34 @@ describe("security: webpack tool — config parameter validation", () => {
165165
});
166166
});
167167

168+
// ---------------------------------------------------------------------------
169+
// Build tool args[] — dash-prefixed values are allowed (#804)
170+
// ---------------------------------------------------------------------------
171+
172+
describe("security: build tool args[] — dash-prefixed values allowed", () => {
173+
it("allows dash-prefixed args because execFile prevents injection", () => {
174+
// args are passed via execFile (argv[], shell: false), so dashes are safe.
175+
// assertNoFlagInjection is NOT called on args elements in build tools.
176+
const dashArgs = ["--release", "--mode=production", "-v", "--env-mode=strict"];
177+
for (const arg of dashArgs) {
178+
// These would throw if assertNoFlagInjection were called — confirm they don't.
179+
expect(() => {
180+
// Simulate what the build tools now do: just push args directly, no validation
181+
const cliArgs: string[] = [];
182+
cliArgs.push(arg);
183+
}).not.toThrow();
184+
}
185+
});
186+
187+
it("still validates non-args string params with assertNoFlagInjection", () => {
188+
// config, entry, target, etc. are still validated
189+
expect(() => assertNoFlagInjection("--evil", "config")).toThrow(/must not start with/);
190+
expect(() => assertNoFlagInjection("--outDir=/etc/passwd", "entry")).toThrow(
191+
/must not start with/,
192+
);
193+
});
194+
});
195+
168196
// ---------------------------------------------------------------------------
169197
// Dangerous env key blocklist — build and webpack tools (#715)
170198
// ---------------------------------------------------------------------------

‎packages/server-build/src/tools/build.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,7 @@ export function registerBuildTool(server: McpServer) {
5050
.array(z.string().max(INPUT_LIMITS.STRING_MAX))
5151
.max(INPUT_LIMITS.ARRAY_MAX)
5252
.default([])
53-
.describe(
54-
"Arguments for the build command (e.g., ['run', 'build']). Each element is validated to prevent flag injection.",
55-
),
53+
.describe("Arguments for the build command (e.g., ['run', 'build'])."),
5654
path: cwdPathInput,
5755
timeout: z
5856
.number()
@@ -76,11 +74,6 @@ export function registerBuildTool(server: McpServer) {
7674
async ({ command, args, path, timeout, env, compact }) => {
7775
assertAllowedCommand(command);
7876
assertNoPathQualifiedCommand(command);
79-
if (args) {
80-
for (const arg of args) {
81-
assertNoFlagInjection(arg, "args");
82-
}
83-
}
8477
// Validate env keys and values
8578
const envRecord = env as Record<string, string> | undefined;
8679
if (envRecord) {

‎packages/server-build/src/tools/esbuild.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,7 @@ export function registerEsbuildTool(server: McpServer) {
120120
.max(INPUT_LIMITS.ARRAY_MAX)
121121
.optional()
122122
.default([])
123-
.describe(
124-
"Additional esbuild flags. Each element is validated to prevent flag injection.",
125-
),
123+
.describe("Additional esbuild flags."),
126124
compact: compactInput,
127125
},
128126
outputSchema: EsbuildResultSchema,
@@ -224,9 +222,6 @@ export function registerEsbuildTool(server: McpServer) {
224222
}
225223

226224
if (args) {
227-
for (const arg of args) {
228-
assertNoFlagInjection(arg, "args");
229-
}
230225
cliArgs.push(...args);
231226
}
232227

‎packages/server-build/src/tools/lerna.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export function registerLernaTool(server: McpServer) {
5252
.max(INPUT_LIMITS.ARRAY_MAX)
5353
.optional()
5454
.default([])
55-
.describe("Additional lerna flags. Each element is validated to prevent flag injection."),
55+
.describe("Additional lerna flags."),
5656
path: projectPathInput,
5757
compact: compactInput,
5858
},
@@ -89,9 +89,6 @@ export function registerLernaTool(server: McpServer) {
8989
if (dryRun || action === "version") cliArgs.push("--dry-run");
9090

9191
if (args) {
92-
for (const arg of args) {
93-
assertNoFlagInjection(arg, "args");
94-
}
9592
cliArgs.push(...args);
9693
}
9794

‎packages/server-build/src/tools/nx.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,7 @@ export function registerNxTool(server: McpServer) {
9797
.max(INPUT_LIMITS.ARRAY_MAX)
9898
.optional()
9999
.default([])
100-
.describe(
101-
"Additional arguments to pass to nx. Each element is validated to prevent flag injection.",
102-
),
100+
.describe("Additional arguments to pass to nx."),
103101
compact: compactInput,
104102
},
105103
outputSchema: NxResultSchema,
@@ -168,9 +166,6 @@ export function registerNxTool(server: McpServer) {
168166
if (graph) cliArgs.push("--graph");
169167

170168
if (args) {
171-
for (const arg of args) {
172-
assertNoFlagInjection(arg, "args");
173-
}
174169
cliArgs.push(...args);
175170
}
176171

‎packages/server-build/src/tools/rollup.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,7 @@ export function registerRollupTool(server: McpServer) {
5656
.max(INPUT_LIMITS.ARRAY_MAX)
5757
.optional()
5858
.default([])
59-
.describe(
60-
"Additional rollup flags. Each element is validated to prevent flag injection.",
61-
),
59+
.describe("Additional rollup flags."),
6260
path: projectPathInput,
6361
compact: compactInput,
6462
},
@@ -88,9 +86,6 @@ export function registerRollupTool(server: McpServer) {
8886
if (watch) cliArgs.push("--watch");
8987

9088
if (args) {
91-
for (const arg of args) {
92-
assertNoFlagInjection(arg, "args");
93-
}
9489
cliArgs.push(...args);
9590
}
9691

‎packages/server-build/src/tools/turbo.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export function registerTurboTool(server: McpServer) {
9595
.max(INPUT_LIMITS.ARRAY_MAX)
9696
.optional()
9797
.describe(
98-
"Additional turbo flags passed directly to turbo (e.g., ['--env-mode=strict']). Each element is validated to prevent flag injection.",
98+
"Additional turbo flags passed directly to turbo (e.g., ['--env-mode=strict']).",
9999
),
100100
),
101101
path: projectPathInput,
@@ -146,9 +146,6 @@ export function registerTurboTool(server: McpServer) {
146146
if (summarize) cliArgs.push("--summarize");
147147

148148
if (args) {
149-
for (const arg of args) {
150-
assertNoFlagInjection(arg, "args");
151-
}
152149
cliArgs.push(...args);
153150
}
154151

‎packages/server-build/src/tools/vite-build.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,7 @@ export function registerViteBuildTool(server: McpServer) {
8181
.max(INPUT_LIMITS.ARRAY_MAX)
8282
.optional()
8383
.default([])
84-
.describe(
85-
"Additional Vite build flags. Each element is validated to prevent flag injection.",
86-
),
84+
.describe("Additional Vite build flags."),
8785
compact: compactInput,
8886
},
8987
outputSchema: ViteBuildResultSchema,
@@ -136,9 +134,6 @@ export function registerViteBuildTool(server: McpServer) {
136134
if (reportCompressedSize === false) cliArgs.push("--no-reportCompressedSize");
137135

138136
if (args) {
139-
for (const arg of args) {
140-
assertNoFlagInjection(arg, "args");
141-
}
142137
cliArgs.push(...args);
143138
}
144139

‎packages/server-build/src/tools/webpack.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ export function registerWebpackTool(server: McpServer) {
9393
.max(INPUT_LIMITS.ARRAY_MAX)
9494
.optional()
9595
.default([])
96-
.describe(
97-
"Additional webpack flags. Each element is validated to prevent flag injection.",
98-
),
96+
.describe("Additional webpack flags."),
9997
compact: compactInput,
10098
},
10199
outputSchema: WebpackResultSchema,
@@ -159,9 +157,6 @@ export function registerWebpackTool(server: McpServer) {
159157
cliArgs.push("--no-color");
160158

161159
if (args) {
162-
for (const arg of args) {
163-
assertNoFlagInjection(arg, "args");
164-
}
165160
cliArgs.push(...args);
166161
}
167162

0 commit comments

Comments
 (0)