-
Notifications
You must be signed in to change notification settings - Fork 749
feat/regex-replace-guardrail #1312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,92 @@ | ||||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||||
HookEventType, | ||||||||||||||||||||||||||||||||||
PluginContext, | ||||||||||||||||||||||||||||||||||
PluginHandler, | ||||||||||||||||||||||||||||||||||
PluginParameters, | ||||||||||||||||||||||||||||||||||
} from '../types'; | ||||||||||||||||||||||||||||||||||
import { getCurrentContentPart, setCurrentContentPart } from '../utils'; | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
export const handler: PluginHandler = async ( | ||||||||||||||||||||||||||||||||||
context: PluginContext, | ||||||||||||||||||||||||||||||||||
parameters: PluginParameters, | ||||||||||||||||||||||||||||||||||
eventType: HookEventType | ||||||||||||||||||||||||||||||||||
) => { | ||||||||||||||||||||||||||||||||||
let error = null; | ||||||||||||||||||||||||||||||||||
let verdict = true; | ||||||||||||||||||||||||||||||||||
let data: any = null; | ||||||||||||||||||||||||||||||||||
const transformedData: Record<string, any> = { | ||||||||||||||||||||||||||||||||||
request: { | ||||||||||||||||||||||||||||||||||
json: null, | ||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||
response: { | ||||||||||||||||||||||||||||||||||
json: null, | ||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||
let transformed = false; | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||
const regexPattern = parameters.rule; | ||||||||||||||||||||||||||||||||||
const redactText = parameters.redactText || '[REDACTED]'; | ||||||||||||||||||||||||||||||||||
const failOnDetection = parameters.failOnDetection || false; | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
const { content, textArray } = getCurrentContentPart(context, eventType); | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
if (!regexPattern) { | ||||||||||||||||||||||||||||||||||
throw new Error('Missing regex pattern'); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
if (!content) { | ||||||||||||||||||||||||||||||||||
throw new Error('Missing text to match'); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
const regex = new RegExp(regexPattern, 'g'); | ||||||||||||||||||||||||||||||||||
Comment on lines
+34
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚡️ Performance Improvement Issue: Creating new RegExp instance inside the handler for every execution is inefficient
Suggested change
|
||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
// Process all text items in the array | ||||||||||||||||||||||||||||||||||
let hasMatches = false; | ||||||||||||||||||||||||||||||||||
const mappedTextArray: Array<string | null> = []; | ||||||||||||||||||||||||||||||||||
textArray.forEach((text) => { | ||||||||||||||||||||||||||||||||||
if (!text) { | ||||||||||||||||||||||||||||||||||
mappedTextArray.push(null); | ||||||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
// Reset regex for each text when using global flag | ||||||||||||||||||||||||||||||||||
regex.lastIndex = 0; | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
const matches = text.match(regex); | ||||||||||||||||||||||||||||||||||
if (matches && matches.length > 0) { | ||||||||||||||||||||||||||||||||||
hasMatches = true; | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
Comment on lines
+55
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🐛 Bug Fix Issue: The
Suggested change
|
||||||||||||||||||||||||||||||||||
const replacedText = text.replace(regex, redactText); | ||||||||||||||||||||||||||||||||||
mappedTextArray.push(replacedText); | ||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
// Handle transformation | ||||||||||||||||||||||||||||||||||
if (hasMatches) { | ||||||||||||||||||||||||||||||||||
setCurrentContentPart( | ||||||||||||||||||||||||||||||||||
context, | ||||||||||||||||||||||||||||||||||
eventType, | ||||||||||||||||||||||||||||||||||
transformedData, | ||||||||||||||||||||||||||||||||||
mappedTextArray | ||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||
transformed = true; | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
if (failOnDetection && hasMatches) { | ||||||||||||||||||||||||||||||||||
verdict = false; | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
data = { | ||||||||||||||||||||||||||||||||||
regexPattern, | ||||||||||||||||||||||||||||||||||
verdict, | ||||||||||||||||||||||||||||||||||
explanation: transformed | ||||||||||||||||||||||||||||||||||
? `Pattern '${regexPattern}' matched and was replaced with '${redactText}'` | ||||||||||||||||||||||||||||||||||
: `The regex pattern '${regexPattern}' did not match any text.`, | ||||||||||||||||||||||||||||||||||
Comment on lines
+79
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Code Refactor Issue: The explanation message construction could be more readable and maintainable
Suggested change
|
||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||
} catch (e: any) { | ||||||||||||||||||||||||||||||||||
error = e; | ||||||||||||||||||||||||||||||||||
data = { | ||||||||||||||||||||||||||||||||||
explanation: `An error occurred while processing the regex: ${e.message}`, | ||||||||||||||||||||||||||||||||||
regexPattern: parameters.rule, | ||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
return { error, verdict, data, transformedData, transformed }; | ||||||||||||||||||||||||||||||||||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐛 Bug Fix
Issue: Using
any
type weakens type safety and can lead to runtime errorsFix: Use proper typing for the data object
Impact: Prevents potential runtime type errors and improves code maintainability