Skip to content

Commit 8b9986b

Browse files
committed
fix(security): harden pipeline planning, pointer traversal, and concurrency
- expressions: never execute $js during planning/dry-run. `pipeline run --dry-run` is the command a cautious user runs to preview an unfamiliar pipeline; it must not run embedded JavaScript. Planning now returns the expression placeholder instead of calling new Function. - schema (getByJsonPointer): block __proto__/constructor/prototype and require own properties, so a crafted $from/$input path cannot pull object internals (e.g. constructor) out of step output and feed them downstream. - scheduler: clamp --concurrency to a maximum (64) to bound fan-out so a single run cannot launch an unbounded number of concurrent API calls / downloads. Note: the runtime new Function sinks in script/js and $js (arbitrary host code execution) are intentionally left unchanged here — remediating them is a design decision (sandbox vs. literal-only code) for the maintainers; see PR notes. https://claude.ai/code/session_017ZGQCjwNQF5Pz96gLUnnG1
1 parent d24f203 commit 8b9986b

3 files changed

Lines changed: 17 additions & 20 deletions

File tree

packages/cli/src/pipeline/expressions.ts

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -289,25 +289,10 @@ function resolvePlannedExpression(
289289
return combineResolved(undefined, undefined, false, false);
290290
}
291291
if ("$js" in expression) {
292-
const argsExpressions = (expression.args ?? {}) as Record<string, PipelineInputExpression>;
293-
const hasFrom = Object.values(argsExpressions).some((v) => isRecord(v) && "$from" in v);
294-
if (hasFrom) return combineResolved({ ...expression }, { ...expression }, false);
295-
const code = expression.$js as string;
296-
const resolvedArgs: Record<string, unknown> = {};
297-
let sensitive = false;
298-
for (const [key, argExpr] of Object.entries(argsExpressions)) {
299-
const resolved = resolvePlannedExpression(argExpr, pipeline, runtimeInput);
300-
resolvedArgs[key] = resolved.value;
301-
sensitive = sensitive || resolved.sensitive;
302-
}
303-
try {
304-
// eslint-disable-next-line @typescript-eslint/no-implied-eval
305-
const fn = new Function("args", `return (${code})`);
306-
const value = fn(resolvedArgs);
307-
return combineResolved(value, sensitive ? REDACTED : value, sensitive);
308-
} catch {
309-
return combineResolved({ ...expression }, { ...expression }, false);
310-
}
292+
// Planning / dry-run must be a non-executing preview: never run user
293+
// JavaScript here. Surface the expression as an unresolved placeholder so a
294+
// `--dry-run` of an untrusted pipeline cannot trigger code execution.
295+
return combineResolved({ ...expression }, { ...expression }, false);
311296
}
312297
return combineResolved(expression, expression, false);
313298
}

packages/cli/src/pipeline/scheduler.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,16 @@ export function orderReports(
7070
return [...reports].sort((a, b) => (index.get(a.id) ?? 0) - (index.get(b.id) ?? 0));
7171
}
7272

73+
const MAX_CONCURRENCY = 64;
74+
7375
export function normalizeConcurrency(value: number | undefined): number {
7476
if (value === undefined) return 1;
7577
if (!Number.isInteger(value) || value < 1) {
7678
throw new PipelineError("pipeline_validation_error", "Invalid pipeline execution options", {
7779
details: { issues: ["concurrency must be a positive integer"] },
7880
});
7981
}
80-
return value;
82+
// Cap fan-out so a single run cannot launch an unbounded number of concurrent
83+
// API calls / downloads and exhaust sockets, file descriptors, or memory.
84+
return Math.min(value, MAX_CONCURRENCY);
8185
}

packages/cli/src/pipeline/schema.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,14 @@ export function getByJsonPointer(value: unknown, pointer: string): unknown {
9191
continue;
9292
}
9393
if (isRecord(current)) {
94+
// A JSON pointer over data must not reach object internals. Block
95+
// prototype-polluting keys and only follow own properties so a crafted
96+
// `$from`/`$input` path cannot pull out `constructor`/`__proto__` and feed
97+
// it into downstream consumers.
98+
if (segment === "__proto__" || segment === "constructor" || segment === "prototype") {
99+
return undefined;
100+
}
101+
if (!Object.prototype.hasOwnProperty.call(current, segment)) return undefined;
94102
current = current[segment];
95103
continue;
96104
}

0 commit comments

Comments
 (0)