Skip to content

Commit 5191978

Browse files
committed
refactor: improve preprocessing expression
1 parent 19c8dd3 commit 5191978

File tree

2 files changed

+50
-17
lines changed

2 files changed

+50
-17
lines changed

packages/utilities/src/memory_calculator.ts

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,32 +96,49 @@ const roundToClosestPowerOf2 = (num: number): number | undefined => {
9696
};
9797

9898
/**
99-
* Replaces `{{variable}}` placeholders in an expression string with `runOptions.variable`.
100-
*
101-
* This function also validates that the variable is one of the allowed 'runOptions' keys.
99+
* Replaces `{{variable}}` placeholders in an expression string with the variable name.
100+
* Enforces strict validation to only allow `input.*` paths or whitelisted `runOptions.*` keys.
102101
*
103102
* @example
104103
* // Returns "runOptions.memoryMbytes + 1024"
105-
* preprocessDefaultMemoryExpression("{{memoryMbytes}} + 1024");
104+
* preprocessDefaultMemoryExpression("{{runOptions.memoryMbytes}} + 1024");
106105
*
107-
* @param defaultMemoryMbytes The raw string expression, e.g., "{{memoryMbytes}} * 2".
106+
* @param defaultMemoryMbytes The raw string expression, e.g., "{{runOptions.memoryMbytes}} * 2".
108107
* @returns A safe, processed expression for evaluation, e.g., "runOptions.memoryMbytes * 2".
109108
*/
110109
const preprocessDefaultMemoryExpression = (defaultMemoryMbytes: string): string => {
111-
// This regex captures the variable name inside {{...}}
112-
const variableRegex = /{{\s*([a-zA-Z0-9_]+)\s*}}/g;
110+
const variableRegex = /{{\s*([a-zA-Z0-9_.]+)\s*}}/g;
113111

114112
const processedExpression = defaultMemoryMbytes.replace(
115113
variableRegex,
116114
(_, variableName: string) => {
117-
// Check if the captured variable name is in our allowlist
118-
if (!ALLOWED_RUN_OPTION_KEYS.has(variableName as keyof ActorRunOptions)) {
115+
// 1. Validate that the variable starts with either 'input.' or 'runOptions.'
116+
if (!variableName.startsWith('runOptions.') && !variableName.startsWith('input.')) {
119117
throw new Error(
120-
`Invalid variable '{{${variableName}}}' in expression.`,
118+
`Invalid variable '{{${variableName}}}' in expression. Variables must start with 'input.' or 'runOptions.'.`,
121119
);
122120
}
123121

124-
return `runOptions.${variableName}`;
122+
// 2. Check if the variable is accessing Input (e.g. {{input.someValue}})
123+
// We do not validate the specific property name because input is dynamic.
124+
if (variableName.startsWith('input.')) {
125+
return variableName;
126+
}
127+
128+
if (variableName.startsWith('runOptions.')) {
129+
const key = variableName.slice('runOptions.'.length);
130+
if (!ALLOWED_RUN_OPTION_KEYS.has(key as keyof ActorRunOptions)) {
131+
throw new Error(
132+
`Invalid variable '{{${variableName}}}' in expression. Only the following runOptions are allowed: ${Array.from(ALLOWED_RUN_OPTION_KEYS).map((k) => `runOptions.${k}`).join(', ')}.`,
133+
);
134+
}
135+
return variableName;
136+
}
137+
138+
// 3. Throw error for unrecognized variables (e.g. {{process.env}})
139+
throw new Error(
140+
`Invalid variable '{{${variableName}}}' in expression.`,
141+
);
125142
},
126143
);
127144

test/memory_calculator.test.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,35 @@ describe('calculateDefaultMemoryFromExpression', () => {
5656
});
5757

5858
describe('Preprocessing with {{variable}}', () => {
59-
it('correctly replaces {{variable}} with valid runOptions.variable', () => {
59+
it('should throw error if variable doesn\'t start with .runOptions or .input', () => {
6060
const context = { input: {}, runOptions: { memoryMbytes: 16 } };
61-
const expr = '{{memoryMbytes}} * 1024';
61+
const expr = '{{unexistingVariable}} * 1024';
62+
expect(() => calculateDefaultMemoryFromExpression(expr, context))
63+
.toThrow(`Invalid variable '{{unexistingVariable}}' in expression. Variables must start with 'input.' or 'runOptions.'.`);
64+
});
65+
66+
it('correctly replaces {{runOptions.variable}} with valid runOptions.variable', () => {
67+
const context = { input: {}, runOptions: { memoryMbytes: 16 } };
68+
const expr = '{{runOptions.memoryMbytes}} * 1024';
6269
// 16 * 1024 = 16384, which is 2^14
6370
const result = calculateDefaultMemoryFromExpression(expr, context);
6471
expect(result).toBe(16384);
6572
});
6673

67-
it('should throw error for invalid variable in {{variable}} syntax', () => {
68-
const context = { input: {}, runOptions: { memoryMbytes: 16 } };
69-
const expr = '{{unexistingVariable}} * 1024';
74+
it('correctly replaces {{input.variable}} with valid input.variable', () => {
75+
const context = { input: { value: 16 }, runOptions: { } };
76+
const expr = '{{input.value}} * 1024';
77+
// 16 * 1024 = 16384, which is 2^14
78+
const result = calculateDefaultMemoryFromExpression(expr, context);
79+
expect(result).toBe(16384);
80+
});
81+
82+
it('correctly throw error if runOptions variable is invalid', () => {
83+
const context = { input: { value: 16 }, runOptions: { } };
84+
const expr = '{{runOptions.customVariable}} * 1024';
85+
// 16 * 1024 = 16384, which is 2^14
7086
expect(() => calculateDefaultMemoryFromExpression(expr, context))
71-
.toThrow(`Invalid variable '{{unexistingVariable}}' in expression.`);
87+
.toThrow(`Invalid variable '{{runOptions.customVariable}}' in expression. Only the following runOptions are allowed:`);
7288
});
7389
});
7490

0 commit comments

Comments
 (0)