This is an automated email from the ASF dual-hosted git repository.
dengliming pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu-dashboard.git
The following commit(s) were added to refs/heads/master by this push:
new 608f1f42 fix: parse plugin handle check rules without eval (#707)
608f1f42 is described below
commit 608f1f42478742034d6ed2f29d220f60f9f9c343
Author: Bhaskar Gurram <[email protected]>
AuthorDate: Mon Oct 19 05:11:04 2026 -0400
fix: parse plugin handle check rules without eval (#707)
* fix: parse plugin handle check rules without eval
Plugin handle metadata stores an optional validation rule (extObj.rule)
as a regular expression literal such as "/^(true|false)$/". The plugin,
namespace plugin, selector, rule handle and discovery proxy selector
forms passed that admin-editable string to eval(), so any JavaScript
stored in a rule ran in the browser of every user who opened one of
those forms.
Add parseRegExpRule() in src/utils/regExpRule.js. It splits the
"/source/flags" literal using JavaScript literal rules (escaped "/" and
"/" inside a character class do not end the source), accepts only the
standard RegExp flags and builds the pattern with new RegExp(). Anything
that is not a valid regular expression literal yields undefined, and the
forms then add no pattern rule for that field instead of executing it.
All seven eval(checkRule) call sites now use the helper.
Fixes #613
* fix: reject line terminators and surface invalid check rules
A JavaScript regular expression literal cannot contain a line terminator
(LF, CR, U+2028, U+2029), escaped or not, so eval() rejected such rules
with a SyntaxError. parseRegExpRule() accepted them and built a RegExp
from the raw source. It now returns undefined for them, matching what a
regular expression literal allows.
A non-empty rule that is not a valid literal made eval() throw while the
form rendered. After the switch to parseRegExpRule() the forms dropped
the pattern check for such a field without any sign. Add
checkRulePattern(), used by all seven form call sites: an empty rule
still adds no check, a valid rule returns its RegExp, and an invalid
rule returns a pattern that matches nothing, so any non-empty value
fails validation with the existing message that shows the rule, and the
rule is reported once with console.error, as the plugin handle model
does for extObj JSON it cannot parse.
* fix: cap the invalid-rule report set, restore console spies, use const
---
src/routes/Plugin/Common/CommonRuleHandle.js | 7 +-
src/routes/Plugin/Common/Selector.js | 13 +-
src/routes/Plugin/Discovery/ProxySelectorModal.js | 13 +-
src/routes/System/NamespacePlugin/AddModal.js | 7 +-
src/routes/System/Plugin/AddModal.js | 7 +-
src/utils/regExpRule.js | 119 ++++++++++++++++
src/utils/regExpRule.test.js | 162 ++++++++++++++++++++++
7 files changed, 307 insertions(+), 21 deletions(-)
diff --git a/src/routes/Plugin/Common/CommonRuleHandle.js
b/src/routes/Plugin/Common/CommonRuleHandle.js
index 272d0ad1..458fb87e 100644
--- a/src/routes/Plugin/Common/CommonRuleHandle.js
+++ b/src/routes/Plugin/Common/CommonRuleHandle.js
@@ -20,6 +20,7 @@ import { Button, Form, Input, Popconfirm, Select, Tooltip }
from "antd";
import classnames from "classnames";
import styles from "../index.less";
import { getIntlContent } from "../../../utils/IntlUtils";
+import { checkRulePattern } from "../../../utils/regExpRule";
const FormItem = Form.Item;
const { Option } = Select;
@@ -125,10 +126,10 @@ export default class CommonRuleHandle extends Component {
item.label,
});
}
- if (checkRule) {
+ const checkPattern = checkRulePattern(checkRule);
+ if (checkPattern) {
rules.push({
- // eslint-disable-next-line no-eval
- pattern: eval(checkRule),
+ pattern: checkPattern,
message: `${getIntlContent(
"SHENYU.PLUGIN.RULE.INVALID",
)}:(${checkRule})`,
diff --git a/src/routes/Plugin/Common/Selector.js
b/src/routes/Plugin/Common/Selector.js
index 652d9701..3d076f3f 100644
--- a/src/routes/Plugin/Common/Selector.js
+++ b/src/routes/Plugin/Common/Selector.js
@@ -48,6 +48,7 @@ import {
formatDateString,
formatTimeString,
} from "../../../utils/utils";
+import { checkRulePattern } from "../../../utils/regExpRule";
import DiscoveryImportModal from "../Discovery/DiscoveryImportModal";
import EditableFormTable from "../Discovery/DiscoveryUpstreamTable.js";
import { buildHandle } from "./selectorHandle";
@@ -753,10 +754,10 @@ class AddModal extends Component {
item.label,
});
}
- if (checkRule) {
+ const checkPattern = checkRulePattern(checkRule);
+ if (checkPattern) {
rules.push({
- // eslint-disable-next-line no-eval
- pattern: eval(checkRule),
+ pattern: checkPattern,
message: `${getIntlContent(
"SHENYU.PLUGIN.RULE.INVALID",
)}:(${checkRule})`,
@@ -1531,10 +1532,10 @@ class AddModal extends Component {
item.label,
});
}
- if (checkRule) {
+ const checkPattern = checkRulePattern(checkRule);
+ if (checkPattern) {
rules.push({
- // eslint-disable-next-line no-eval
- pattern: eval(checkRule),
+ pattern: checkPattern,
message: `${getIntlContent(
"SHENYU.PLUGIN.RULE.INVALID",
)}:(${checkRule})`,
diff --git a/src/routes/Plugin/Discovery/ProxySelectorModal.js
b/src/routes/Plugin/Discovery/ProxySelectorModal.js
index 63c5242e..986001fb 100644
--- a/src/routes/Plugin/Discovery/ProxySelectorModal.js
+++ b/src/routes/Plugin/Discovery/ProxySelectorModal.js
@@ -35,6 +35,7 @@ import { getIntlContent } from "../../../utils/IntlUtils";
import styles from "../index.less";
import ProxySelectorCopy from "./ProxySelectorCopy.js";
import { findKeyByValue } from "../../../utils/utils";
+import { checkRulePattern } from "../../../utils/regExpRule";
import EditableFormTable from "./DiscoveryUpstreamTable";
import { getDefaultValueList, getDiscoveryProps } from "./optionalFields";
@@ -390,10 +391,10 @@ export class ProxySelectorModalComponent extends
Component {
) + item.label,
});
}
- if (checkRule) {
+ const checkPattern = checkRulePattern(checkRule);
+ if (checkPattern) {
rules.push({
- // eslint-disable-next-line no-eval
- pattern: eval(checkRule),
+ pattern: checkPattern,
message: `${getIntlContent(
"SHENYU.PLUGIN.RULE.INVALID",
)}:(${checkRule})`,
@@ -623,10 +624,10 @@ export class ProxySelectorModalComponent extends
Component {
) + item.label,
});
}
- if (checkRule) {
+ const checkPattern = checkRulePattern(checkRule);
+ if (checkPattern) {
rules.push({
- // eslint-disable-next-line no-eval
- pattern: eval(checkRule),
+ pattern: checkPattern,
message: `${getIntlContent(
"SHENYU.PLUGIN.RULE.INVALID",
)}:(${checkRule})`,
diff --git a/src/routes/System/NamespacePlugin/AddModal.js
b/src/routes/System/NamespacePlugin/AddModal.js
index 5e3ad6f1..9ac9aeea 100644
--- a/src/routes/System/NamespacePlugin/AddModal.js
+++ b/src/routes/System/NamespacePlugin/AddModal.js
@@ -24,6 +24,7 @@ import {
getConfigFieldValue,
serializePluginConfig,
} from "../../../utils/pluginConfig";
+import { checkRulePattern } from "../../../utils/regExpRule";
const { Option } = Select;
const FormItem = Form.Item;
@@ -172,10 +173,10 @@ class AddModal extends Component {
message: getIntlContent("SHENYU.COMMON.PLEASEINPUT"),
});
}
- if (checkRule) {
+ const checkPattern = checkRulePattern(checkRule);
+ if (checkPattern) {
rules.push({
- // eslint-disable-next-line no-eval
- pattern: eval(checkRule),
+ pattern: checkPattern,
message: `${getIntlContent(
"SHENYU.PLUGIN.RULE.INVALID",
)}:(${checkRule})`,
diff --git a/src/routes/System/Plugin/AddModal.js
b/src/routes/System/Plugin/AddModal.js
index cb46adb0..314084bb 100644
--- a/src/routes/System/Plugin/AddModal.js
+++ b/src/routes/System/Plugin/AddModal.js
@@ -32,6 +32,7 @@ import {
getConfigFieldValue,
serializePluginConfig,
} from "../../../utils/pluginConfig";
+import { checkRulePattern } from "../../../utils/regExpRule";
const { Option } = Select;
const FormItem = Form.Item;
@@ -174,10 +175,10 @@ class AddModal extends Component {
message: getIntlContent("SHENYU.COMMON.PLEASEINPUT"),
});
}
- if (checkRule) {
+ const checkPattern = checkRulePattern(checkRule);
+ if (checkPattern) {
rules.push({
- // eslint-disable-next-line no-eval
- pattern: eval(checkRule),
+ pattern: checkPattern,
message: `${getIntlContent(
"SHENYU.PLUGIN.RULE.INVALID",
)}:(${checkRule})`,
diff --git a/src/utils/regExpRule.js b/src/utils/regExpRule.js
new file mode 100644
index 00000000..2a16a1f7
--- /dev/null
+++ b/src/utils/regExpRule.js
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Plugin handle metadata (extObj.rule) stores optional validation rules as
+// regular expression literals such as "/^(true|false)$/" or "/^[a-z]+$/i".
+// The value is admin-editable data, so it must be parsed, never evaluated.
+const VALID_FLAGS = /^[dgimsuvy]*$/;
+
+// JavaScript line terminators: LF, CR, LINE SEPARATOR, PARAGRAPH SEPARATOR.
+const LINE_TERMINATOR = /[\n\r\u2028\u2029]/;
+
+// Splits "/source/flags" at the first "/" that closes the literal, following
+// JavaScript regular expression literal rules: "\/" and "/" inside a
+// character class do not end the literal, and a line terminator (escaped or
+// not) is not allowed anywhere in the literal.
+function splitRegExpLiteral(literal) {
+ if (literal[0] !== "/") {
+ return null;
+ }
+ let inClass = false;
+ for (let i = 1; i < literal.length; i += 1) {
+ const char = literal[i];
+ if (LINE_TERMINATOR.test(char)) {
+ return null;
+ }
+ if (char === "\\") {
+ i += 1;
+ if (LINE_TERMINATOR.test(literal[i])) {
+ return null;
+ }
+ } else if (char === "[") {
+ inClass = true;
+ } else if (char === "]") {
+ inClass = false;
+ } else if (char === "/" && !inClass) {
+ return { source: literal.slice(1, i), flags: literal.slice(i + 1) };
+ }
+ }
+ return null;
+}
+
+/**
+ * Converts a stored check rule into a RegExp without executing it as code.
+ *
+ * @param {*} rule rule in "/source/flags" form
+ * @returns {RegExp|undefined} the parsed RegExp, or undefined when the rule
+ * is empty or is not a valid regular expression literal
+ */
+export function parseRegExpRule(rule) {
+ if (typeof rule !== "string") {
+ return undefined;
+ }
+ const literal = splitRegExpLiteral(rule.trim());
+ if (!literal || !literal.source || !VALID_FLAGS.test(literal.flags)) {
+ return undefined;
+ }
+ try {
+ return new RegExp(literal.source, literal.flags);
+ } catch (e) {
+ // Invalid pattern or flag combination (e.g. duplicated flags).
+ return undefined;
+ }
+}
+
+// Rules already reported, so each invalid rule is logged once. Capped so a
+// long-lived page that sees many distinct invalid rules cannot grow it
+// without bound.
+const MAX_REPORTED_INVALID_RULES = 100;
+const reportedInvalidRules = new Set();
+
+/**
+ * Returns the pattern a form field should be validated against for a stored
+ * check rule.
+ *
+ * An empty rule means the field has no pattern check. A rule that is not a
+ * valid regular expression literal used to make eval() throw while the form
+ * rendered; it now produces a pattern that matches nothing, so any non-empty
+ * value fails validation with the rule shown in the message, and the problem
+ * is reported once in the console instead of validation being skipped.
+ *
+ * @param {*} rule rule in "/source/flags" form
+ * @returns {RegExp|undefined} the pattern to validate against, or undefined
+ * when the rule is empty
+ */
+export function checkRulePattern(rule) {
+ if (!rule || (typeof rule === "string" && !rule.trim())) {
+ return undefined;
+ }
+ const pattern = parseRegExpRule(rule);
+ if (pattern) {
+ return pattern;
+ }
+ if (!reportedInvalidRules.has(String(rule))) {
+ if (reportedInvalidRules.size >= MAX_REPORTED_INVALID_RULES) {
+ reportedInvalidRules.clear();
+ }
+ reportedInvalidRules.add(String(rule));
+ // eslint-disable-next-line no-console
+ console.error(
+ "Invalid plugin handle check rule, expected a /pattern/flags literal:",
+ rule,
+ );
+ }
+ return /(?!)/;
+}
diff --git a/src/utils/regExpRule.test.js b/src/utils/regExpRule.test.js
new file mode 100644
index 00000000..6f02bf6d
--- /dev/null
+++ b/src/utils/regExpRule.test.js
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { checkRulePattern, parseRegExpRule } from "./regExpRule";
+
+describe("parseRegExpRule", () => {
+ afterEach(() => {
+ delete global.pwned;
+ });
+
+ it("parses the rule formats shipped in ShenYu plugin handle data", () => {
+ const booleanRule = parseRegExpRule("/^(true|false)$/");
+ expect(booleanRule).toBeInstanceOf(RegExp);
+ expect(booleanRule.source).toBe("^(true|false)$");
+ expect(booleanRule.test("true")).toBe(true);
+ expect(booleanRule.test("yes")).toBe(false);
+
+ const bitRule = parseRegExpRule("/^[01]$/");
+ expect(bitRule.test("1")).toBe(true);
+ expect(bitRule.test("2")).toBe(false);
+ });
+
+ it("keeps valid flags", () => {
+ const rule = parseRegExpRule("/^abc$/i");
+ expect(rule.flags).toBe("i");
+ expect(rule.test("ABC")).toBe(true);
+ expect(parseRegExpRule("/^a.b$/ms").flags).toBe("ms");
+ });
+
+ it("matches the same values as the equivalent regular expression literal",
() => {
+ expect(parseRegExpRule(" /^\\d+$/ ").source).toBe(/^\d+$/.source);
+ expect(parseRegExpRule("/^a\\/b$/").test("a/b")).toBe(true);
+ expect(parseRegExpRule("/^[/]+$/").test("//")).toBe(true);
+ });
+
+ it.each(["/abc/x", "/abc/gg", "/abc/I", "/abc/ i"])(
+ "rejects invalid flags: %s",
+ (rule) => {
+ expect(parseRegExpRule(rule)).toBeUndefined();
+ },
+ );
+
+ it.each([
+ undefined,
+ null,
+ 1,
+ {},
+ "",
+ "//",
+ "/",
+ "abc",
+ "^\\d+$",
+ "/(/",
+ "/abc",
+ "/a/b/",
+ ])(
+ "returns undefined for a value that is not a valid literal: %p",
+ (rule) => {
+ expect(parseRegExpRule(rule)).toBeUndefined();
+ },
+ );
+
+ // JavaScript rejects these as regular expression literals (SyntaxError).
+ it.each([
+ ["LF", "/a\nb/"],
+ ["CR", "/a\rb/"],
+ ["LINE SEPARATOR", "/a\u2028b/"],
+ ["PARAGRAPH SEPARATOR", "/a\u2029b/"],
+ ["LF in a character class", "/[\n]/"],
+ ["escaped LF", "/a\\\nb/"],
+ ["escaped LINE SEPARATOR", "/a\\\u2028b/"],
+ ])("rejects a line terminator inside the literal: %s", (name, rule) => {
+ expect(parseRegExpRule(rule)).toBeUndefined();
+ });
+
+ it("ignores line terminators around the literal", () => {
+ expect(parseRegExpRule("\n/^a$/i\r\n").test("A")).toBe(true);
+ expect(parseRegExpRule("\u2028/^a$/\u2029").test("a")).toBe(true);
+ });
+
+ it("accepts an escaped line terminator written as an escape sequence", () =>
{
+ expect(parseRegExpRule("/^a\\nb$/").test("a\nb")).toBe(true);
+ expect(parseRegExpRule("/^a\\u2028b$/").test("a\u2028b")).toBe(true);
+ });
+
+ it.each([
+ "alert(1)",
+ "(()=>{globalThis.pwned=1})()",
+ "/x/,globalThis.pwned=1",
+ "/x/;globalThis.pwned=1",
+ "/x/.constructor.constructor('globalThis.pwned=1')()",
+ "globalThis.pwned=1,/x/",
+ ])("does not execute code stored in the rule: %s", (rule) => {
+ expect(parseRegExpRule(rule)).toBeUndefined();
+ expect(global.pwned).toBeUndefined();
+ });
+});
+
+describe("checkRulePattern", () => {
+ afterEach(() => {
+ jest.restoreAllMocks();
+ });
+
+ it.each([undefined, null, "", " ", "\t\r\n"])(
+ "adds no pattern for an empty rule: %p",
+ (rule) => {
+ const error = jest.spyOn(console, "error").mockImplementation(() => {});
+ expect(checkRulePattern(rule)).toBeUndefined();
+ expect(error).not.toHaveBeenCalled();
+ },
+ );
+
+ it("returns the parsed pattern for a valid rule", () => {
+ const error = jest.spyOn(console, "error").mockImplementation(() => {});
+ const pattern = checkRulePattern("/^(true|false)$/");
+ expect(pattern.test("true")).toBe(true);
+ expect(pattern.test("yes")).toBe(false);
+ expect(error).not.toHaveBeenCalled();
+ });
+
+ it.each(["^\\d+$", "abc", "/(/", "/a\u2028b/", "//", 1, {}])(
+ "fails validation for an invalid rule instead of skipping it: %p",
+ (rule) => {
+ const error = jest.spyOn(console, "error").mockImplementation(() => {});
+ const pattern = checkRulePattern(rule);
+ expect(pattern).toBeInstanceOf(RegExp);
+ ["", "a", "true", "123", "\n"].forEach((value) => {
+ expect(pattern.test(value)).toBe(false);
+ });
+ expect(error).toHaveBeenCalledTimes(1);
+ expect(error.mock.calls[0]).toContain(rule);
+ },
+ );
+
+ it("reports the same invalid rule only once", () => {
+ const error = jest.spyOn(console, "error").mockImplementation(() => {});
+ checkRulePattern("not-a-literal-once");
+ checkRulePattern("not-a-literal-once");
+ expect(checkRulePattern("not-a-literal-once").test("x")).toBe(false);
+ expect(error).toHaveBeenCalledTimes(1);
+ });
+
+ it("does not execute code stored in an invalid rule", () => {
+ jest.spyOn(console, "error").mockImplementation(() => {});
+ checkRulePattern("(()=>{globalThis.pwned=1})()");
+ expect(global.pwned).toBeUndefined();
+ });
+});
