Skip to content

Commit c3e9b0f

Browse files
committed
address gemini review feedbacks
1 parent ac5f55b commit c3e9b0f

2 files changed

Lines changed: 40 additions & 33 deletions

File tree

packages/agentflow/src/core/utils/fieldVisibility.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,12 @@ describe('conditionMatches', () => {
7979
expect(conditionMatches('test', '[invalid')).toBe(false)
8080
})
8181

82+
it('rejects oversized regex patterns to mitigate ReDoS', () => {
83+
const longPattern = '(a+)'.repeat(60) // exceeds 200 char limit
84+
expect(conditionMatches('aaa', longPattern)).toBe(false)
85+
expect(conditionMatches(['aaa'], longPattern)).toBe(false)
86+
})
87+
8288
it('scalar boolean strict equality', () => {
8389
expect(conditionMatches(true, true)).toBe(true)
8490
expect(conditionMatches(true, false)).toBe(false)

packages/agentflow/src/core/utils/fieldVisibility.ts

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,19 @@ import type { InputParam } from '../types'
66
/** Detects characters that signal intentional regex usage (excludes `.` and `\` which appear in normal names). */
77
const REGEX_INTENT = /[|()^$*+?[\]]/
88

9+
/** Maximum length for regex patterns to mitigate ReDoS from untrusted input. */
10+
const MAX_REGEX_LENGTH = 200
11+
12+
/** Timeout-safe regex test: compiles and tests the pattern, returning false for invalid or oversized patterns. */
13+
function safeRegexTest(pattern: string, value: string): boolean {
14+
if (pattern.length > MAX_REGEX_LENGTH) return false
15+
try {
16+
return new RegExp(pattern).test(value)
17+
} catch {
18+
return false
19+
}
20+
}
21+
922
/**
1023
* Check if a ground value matches a comparison value using the same matrix
1124
* as the UI's _showHideOperation in genericHelper.js.
@@ -19,11 +32,7 @@ export function conditionMatches(groundValue: unknown, comparisonValue: unknown)
1932
return (groundValue as unknown[]).some((val) => {
2033
if (comparisonValue === val) return true
2134
if (REGEX_INTENT.test(comparisonValue)) {
22-
try {
23-
return new RegExp(comparisonValue).test(String(val))
24-
} catch {
25-
return false
26-
}
35+
return safeRegexTest(comparisonValue, String(val))
2736
}
2837
return false
2938
})
@@ -41,11 +50,7 @@ export function conditionMatches(groundValue: unknown, comparisonValue: unknown)
4150
if (typeof comparisonValue === 'string') {
4251
if (comparisonValue === groundValue) return true
4352
if (REGEX_INTENT.test(comparisonValue)) {
44-
try {
45-
return new RegExp(comparisonValue).test(String(groundValue))
46-
} catch {
47-
return false
48-
}
53+
return safeRegexTest(comparisonValue, String(groundValue))
4954
}
5055
return false
5156
}
@@ -59,47 +64,43 @@ export function conditionMatches(groundValue: unknown, comparisonValue: unknown)
5964
return false
6065
}
6166

67+
/** Resolve a ground value from inputValues, parsing JSON-encoded arrays when found. */
68+
function resolveGroundValue(inputValues: Record<string, unknown>, rawPath: string, arrayIndex?: number): unknown {
69+
const path = arrayIndex !== undefined ? rawPath.replace('$index', String(arrayIndex)) : rawPath
70+
let value: unknown = get(inputValues, path, '')
71+
if (typeof value === 'string' && value.startsWith('[') && value.endsWith(']')) {
72+
try {
73+
value = JSON.parse(value)
74+
} catch {
75+
// keep as string
76+
}
77+
}
78+
return value
79+
}
80+
6281
/**
6382
* Evaluate whether a single param should be visible given current input values.
6483
*/
6584
export function evaluateParamVisibility(param: InputParam, inputValues: Record<string, unknown>, arrayIndex?: number): boolean {
66-
let display = true
67-
6885
if (param.show) {
6986
for (const [rawPath, comparisonValue] of Object.entries(param.show)) {
70-
const path = arrayIndex !== undefined ? rawPath.replace('$index', String(arrayIndex)) : rawPath
71-
let groundValue: unknown = get(inputValues, path, '')
72-
if (typeof groundValue === 'string' && groundValue.startsWith('[') && groundValue.endsWith(']')) {
73-
try {
74-
groundValue = JSON.parse(groundValue)
75-
} catch {
76-
// keep as string
77-
}
78-
}
87+
const groundValue = resolveGroundValue(inputValues, rawPath, arrayIndex)
7988
if (!conditionMatches(groundValue, comparisonValue)) {
80-
display = false
89+
return false
8190
}
8291
}
8392
}
8493

8594
if (param.hide) {
8695
for (const [rawPath, comparisonValue] of Object.entries(param.hide)) {
87-
const path = arrayIndex !== undefined ? rawPath.replace('$index', String(arrayIndex)) : rawPath
88-
let groundValue: unknown = get(inputValues, path, '')
89-
if (typeof groundValue === 'string' && groundValue.startsWith('[') && groundValue.endsWith(']')) {
90-
try {
91-
groundValue = JSON.parse(groundValue)
92-
} catch {
93-
// keep as string
94-
}
95-
}
96+
const groundValue = resolveGroundValue(inputValues, rawPath, arrayIndex)
9697
if (conditionMatches(groundValue, comparisonValue)) {
97-
display = false
98+
return false
9899
}
99100
}
100101
}
101102

102-
return display
103+
return true
103104
}
104105

105106
/**

0 commit comments

Comments
 (0)