fix: remove the regex replace plugin with the correct regex function parsing #1329

siddharthsambharia-portkey · 2025-09-10T10:55:26Z

Description

This PR fixed the regex replcae guardrail

Motivation

the regex replace guardrail was adding a 'g' flag by itself that was causing the regex to fail OR not work in the desired way

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Refactoring (no functional changes)

How Has This Been Tested?

Unit Tests
Integration Tests
Manual Testing

Screenshots (if applicable)

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Related Issues

#1328

matter-code-review · 2025-09-10T10:55:53Z

Description

Summary By MatterAI

🔄 What Changed

A new utility function parseRegex has been introduced in plugins/default/regexReplace.ts. This function is responsible for correctly parsing a string input into a RegExp object, specifically handling cases where the input includes regex delimiters and flags (e.g., /pattern/flags). It also validates the provided flags against a set of valid JavaScript regex flags. The regexReplace plugin's handler now utilizes parseRegex instead of directly instantiating RegExp.

🔍 Impact of the Change

This change significantly improves the robustness and correctness of regex handling within the regexReplace plugin. Previously, explicit regex flags might have been misinterpreted as part of the pattern. Now, the plugin can correctly interpret and apply flags like 'g' (global), 'i' (case-insensitive), etc., as intended by the user. It also prevents the use of invalid regex flags, leading to more predictable behavior and fewer runtime errors.

📁 Total Files Changed

plugins/default/regexReplace.ts: Introduced parseRegex function (19 additions) and updated RegExp instantiation to use it (1 deletion).

🧪 Test Added

N/A

🔒Security Vulnerabilities

N/A

Motivation

This change is motivated by a bug fix to correct the regex replacement behavior within the plugin by ensuring proper parsing and validation of regex patterns and flags.

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Refactoring (no functional changes)

How Has This Been Tested?

Unit Tests
Integration Tests
Manual Testing

Screenshots (if applicable)

N/A

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Related Issues

.

Tip

Quality Recommendations

Add comprehensive unit tests for the parseRegex function to cover various valid and invalid regex patterns and flag combinations, ensuring its robustness.
Include comments for the parseRegex function explaining its purpose, how it handles different input formats (e.g., /pattern/flags vs. pattern), and the list of valid flags it supports.

Tanka Poem ♫

Regex now parses,
Flags are checked, patterns clear.
No more 'g' default,
Precision in every match,
Code's logic, a clearer path. ✨

Sequence Diagram

sequenceDiagram
    participant PluginHandler as Regex Replace Plugin Handler
    participant ParseRegex as parseRegex(input: string)

    PluginHandler->>+ParseRegex: parseRegex(regexPattern: string)
    Note over ParseRegex: Validate flags (e.g., g, i, m, s, u, y, d)
    alt Valid Regex Pattern and Flags
        ParseRegex-->>-PluginHandler: new RegExp(pattern, flags)
    else Invalid Flags
        ParseRegex-->>-PluginHandler: throw Error('Invalid regex flags')
    end

matter-code-review

Removing the 'g' flag from regex will change behavior from replacing all matches to only the first match. This appears intentional but needs verification.

matter-code-review · 2025-09-10T10:56:01Z

plugins/default/regexReplace.ts

    }

-    const regex = new RegExp(regexPattern, 'g');
+    const regex = new RegExp(regexPattern);


🐛 Bug Fix

Issue: Removing the 'g' (global) flag changes regex behavior from replacing all matches to only replacing the first match in each text item
Fix: Verify this behavioral change is intentional, as it fundamentally alters the plugin's functionality
Impact: Users expecting global replacement will now only get first-match replacement

Suggested change

const regex = new RegExp(regexPattern);

const regex = new RegExp(regexPattern, 'g');

Alternative: If the intention is to replace only the first match globally, consider documenting this change or providing a configuration option for users who need global replacement.

✅ Resolved: Fixed by implementing parseRegex function that properly handles user-provided flags including global 'g' flag when specified

matter-code-review

Good implementation of regex parsing with flag validation. The new parseRegex function properly handles user-provided flags while maintaining security.

matter-code-review · 2025-09-11T12:13:32Z

plugins/default/regexReplace.ts


+function parseRegex(input: string): RegExp {
+  // Valid JavaScript regex flags
+  const validFlags = /^[gimsuyd]*$/;


🔒 Security Issue Fix

Issue: The regex flag validation includes 'y' and 'd' flags which are not supported in all JavaScript environments and could cause runtime errors
Fix: Remove unsupported flags to ensure cross-environment compatibility
Impact: Prevents potential runtime errors in older JavaScript environments

Suggested change

const validFlags = /^[gimsuyd]*$/;

const validFlags = /^[gimsu]*$/;

matter-code-review · 2025-09-11T12:13:32Z

plugins/default/regexReplace.ts

+  // Valid JavaScript regex flags
+  const validFlags = /^[gimsuyd]*$/;
+
+  const match = input.match(/^\/(.+?)\/([gimsuyd]*)$/);


🔒 Security Issue Fix

Issue: The regex pattern matching includes 'y' and 'd' flags in the capture group which could cause inconsistent behavior
Fix: Remove unsupported flags from the pattern matching to align with validation
Impact: Ensures consistent flag handling across the function

Suggested change

const match = input.match(/^\/(.+?)\/([gimsuyd]*)$/);

const match = input.match(/^\/(.+?)\/([gimsu]*)$/);

matter-code-review · 2025-09-11T12:13:32Z

plugins/default/regexReplace.ts

+  if (match) {
+    const [, pattern, flags] = match;
+
+    if (flags && !validFlags.test(flags)) {


🐛 Bug Fix

Issue: Empty flags string will pass validation but could cause confusion in error messages
Fix: Add explicit check for empty flags to provide clearer logic flow
Impact: Improves code clarity and prevents unnecessary validation of empty strings

Suggested change

if (flags && !validFlags.test(flags)) {

if (flags && flags.length > 0 && !validFlags.test(flags)) {

narengogi

nice optimization removing the global search from the regex matches

siddharthsambharia-portkey · 2025-09-15T11:19:46Z

nice optimization removing the global search from the regex matches

🙏

fix regex replace

8a058b5

matter-code-review bot reviewed Sep 10, 2025

View reviewed changes

fix/regex-replace-guardrail

daa9533

siddharthsambharia-portkey changed the title ~~fix: remove the "g" flag from regex replace plugin~~ fix: remove the regex replace plugin with the correct regex function parsing Sep 11, 2025

matter-code-review bot reviewed Sep 11, 2025

View reviewed changes

narengogi linked an issue Sep 15, 2025 that may be closed by this pull request

Regex Replace Guardrails #1328

Closed

narengogi approved these changes Sep 15, 2025

View reviewed changes

VisargD approved these changes Sep 16, 2025

View reviewed changes

VisargD merged commit f400d72 into Portkey-AI:main Sep 16, 2025
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: remove the regex replace plugin with the correct regex function parsing #1329

fix: remove the regex replace plugin with the correct regex function parsing #1329

Uh oh!

siddharthsambharia-portkey commented Sep 10, 2025

Uh oh!

matter-code-review bot commented Sep 10, 2025 •

edited

Loading

Quality Recommendations

Uh oh!

matter-code-review bot left a comment

Uh oh!

matter-code-review bot Sep 10, 2025

Uh oh!

matter-code-review bot Sep 11, 2025

Uh oh!

matter-code-review bot left a comment

Uh oh!

matter-code-review bot Sep 11, 2025

Uh oh!

matter-code-review bot Sep 11, 2025

Uh oh!

matter-code-review bot Sep 11, 2025

Uh oh!

narengogi left a comment

Uh oh!

siddharthsambharia-portkey commented Sep 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	const regex = new RegExp(regexPattern);
	const regex = new RegExp(regexPattern, 'g');

	const validFlags = /^[gimsuyd]*$/;
	const validFlags = /^[gimsu]*$/;

	const match = input.match(/^\/(.+?)\/([gimsuyd]*)$/);
	const match = input.match(/^\/(.+?)\/([gimsu]*)$/);

	if (flags && !validFlags.test(flags)) {
	if (flags && flags.length > 0 && !validFlags.test(flags)) {

fix: remove the regex replace plugin with the correct regex function parsing #1329

fix: remove the regex replace plugin with the correct regex function parsing #1329

Uh oh!

Conversation

siddharthsambharia-portkey commented Sep 10, 2025

Description

Motivation

Type of Change

How Has This Been Tested?

Screenshots (if applicable)

Checklist

Related Issues

Uh oh!

matter-code-review bot commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Summary By MatterAI

🔄 What Changed

🔍 Impact of the Change

📁 Total Files Changed

🧪 Test Added

🔒Security Vulnerabilities

Motivation

Type of Change

How Has This Been Tested?

Screenshots (if applicable)

Checklist

Related Issues

Quality Recommendations

Tanka Poem ♫

Sequence Diagram

Uh oh!

matter-code-review bot left a comment

Choose a reason for hiding this comment

Uh oh!

matter-code-review bot Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

matter-code-review bot Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

matter-code-review bot left a comment

Choose a reason for hiding this comment

Uh oh!

matter-code-review bot Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

matter-code-review bot Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

matter-code-review bot Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

narengogi left a comment

Choose a reason for hiding this comment

Uh oh!

siddharthsambharia-portkey commented Sep 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

matter-code-review bot commented Sep 10, 2025 •

edited

Loading