fix(banking): use custom renderer for translated strings and parser for rules (backport #56643) (#56647)

fix(banking): use custom renderer for translated strings and parser for rules (#56643)

fix(banking): use custom renderer for translated strings and parser for formula evaluation

(cherry picked from commit 8447f551e7)

Co-authored-by: Nikhil Kothari <nik.kothari22@live.com>

banking/package.json

@@ -42,6 +42,7 @@
     "react-router": "^8.1.0",
     "rehype-raw": "^7.0.0",
     "remark-gfm": "^4.0.1",
+    "safe-expr-eval": "^1.0.4",
     "sonner": "^2.0.7",
     "tailwind-merge": "^3.5.0",
     "tailwindcss": "^4.3.0",

banking/src/components/features/BankReconciliation/BankClearanceSummary.tsx

@@ -2,7 +2,6 @@ import { useAtomValue } from "jotai"
 import { MissingFiltersBanner } from "./MissingFiltersBanner"
 import { bankRecDateAtom, SelectedBank, selectedBankAccountAtom } from "./bankRecAtoms"
 import { useCurrentCompany } from "@/hooks/useCurrentCompany"
-import { Paragraph } from "@/components/ui/typography"
 import type { ColumnDef } from "@tanstack/react-table"
 import { useCallback, useMemo, useState } from "react"
 import { useFrappeGetCall, useFrappePostCall, useSWRConfig } from "frappe-react-sdk"
@@ -26,6 +25,7 @@ import { Form } from "@/components/ui/form"
 import { useForm } from "react-hook-form"
 import { DateField } from "@/components/ui/form-elements"
 import { Empty, EmptyMedia, EmptyHeader, EmptyTitle, EmptyDescription } from "@/components/ui/empty"
+import MarkdownRenderer from "@/components/ui/markdown"
 
 const BankClearanceSummary = () => {
     const bankAccount = useAtomValue(selectedBankAccountAtom)
@@ -203,14 +203,14 @@ const BankClearanceSummaryView = () => {
         [accountCurrency, bankAccount, companyID, mutate, onCopy],
     )
 
+    const content = _("Below is a list of all accounting entries posted against the bank account {0} between {1} and {2}.", [`<strong>${bankAccount?.account}</strong>`, `<strong>${formattedFromDate}</strong>`, `<strong>${formattedToDate}</strong>`])
+
     return <div className="space-y-4 py-2">
 
         <div>
-            <Paragraph className="text-sm">
+            <span className="text-p-sm">
-                <span dangerouslySetInnerHTML={{
+                <MarkdownRenderer content={content} />
-                    __html: _("Below is a list of all accounting entries posted against the bank account {0} between {1} and {2}.", [`<strong>${bankAccount?.account}</strong>`, `<strong>${formattedFromDate}</strong>`, `<strong>${formattedToDate}</strong>`])
+            </span>
-                }} />
-            </Paragraph>
         </div>
 
         {error && <ErrorBanner error={error} />}

banking/src/components/features/BankReconciliation/BankEntryModalContent.tsx

@@ -18,6 +18,7 @@ import { useMultiFileUploadProgress } from "@/hooks/useMultiFileUploadProgress"
 import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@/components/ui/table"
 import { Checkbox } from "@/components/ui/checkbox"
 import { ArrowDownRight, ArrowUpRight, Plus, Trash2 } from "lucide-react"
+import { evaluateAmountFormula } from "@/lib/amountFormula"
 import { flt, formatCurrency } from "@/lib/numbers"
 import { cn } from "@/lib/utils"
 import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"
@@ -215,38 +216,13 @@ const BankEntryForm = ({ selectedTransaction }: { selectedTransaction: Unreconci
                         })
                     } else {
 
-                        /**
+                        const transactionAmount = selectedTransaction.unallocated_amount ?? 0
-                         * The debit and credit amounts can also be expressions - like "transaction_amount * 0.5"
-                         * So we need to compute the value of the expression
-                         * We can use the eval function to do this. But we need to expose certain variables to the expression.
-                         * One of them is transaction_amount which is the unallocated amount of the selected transaction
-                         * @param expression - The expression to compute
-                         * @returns The computed value
-                         */
-                        const computeExpression = (expression: string) => {
-
-                            const script = `
-                                const transaction_amount = ${selectedTransaction.unallocated_amount ?? 0}
-                                ${expression};
-                            `
-
-                            let value = 0;
-
-                            try {
-                                value = window.eval(script);
-                            } catch (error: unknown) {
-                                console.error(error);
-                                value = 0;
-                            }
-
-                            return value;
-                        }
                         if (!acc?.debit && !acc?.credit) {
                             hasTotallyEmptyRowEarlier = true;
                         }
 
-                        const computedDebit = acc?.debit ? flt(computeExpression(acc.debit), 2) : 0
+                        const computedDebit = acc?.debit ? flt(evaluateAmountFormula(acc.debit, transactionAmount), 2) : 0
-                        const computedCredit = acc?.credit ? flt(computeExpression(acc.credit), 2) : 0
+                        const computedCredit = acc?.credit ? flt(evaluateAmountFormula(acc.credit, transactionAmount), 2) : 0
 
                         totalDebits = flt(totalDebits + computedDebit, 2)
                         totalCredits = flt(totalCredits + computedCredit, 2)

banking/src/components/features/BankReconciliation/BankReconciliationStatement.tsx

@@ -2,7 +2,6 @@ import { useAtomValue } from "jotai"
 import { MissingFiltersBanner } from "./MissingFiltersBanner"
 import { bankRecDateAtom, selectedBankAccountAtom } from "./bankRecAtoms"
 import { useCurrentCompany } from "@/hooks/useCurrentCompany"
-import { Paragraph } from "@/components/ui/typography"
 import { useCallback, useMemo } from "react"
 import type { ColumnDef } from "@tanstack/react-table"
 import { useFrappeGetCall } from "frappe-react-sdk"
@@ -19,6 +18,7 @@ import _ from "@/lib/translate"
 import { toast } from "sonner"
 import { useCopyToClipboard } from "usehooks-ts"
 import { Empty, EmptyDescription, EmptyHeader, EmptyMedia, EmptyTitle } from "@/components/ui/empty"
+import MarkdownRenderer from "@/components/ui/markdown"
 
 const BankReconciliationStatement = () => {
     const bankAccount = useAtomValue(selectedBankAccountAtom)
@@ -189,14 +189,14 @@ const BankReconciliationStatementView = () => {
         return data.message.result.filter((row: BankClearanceSummaryEntry) => Boolean(row.payment_entry))
     }, [data])
 
+    const content = _("Below is a list of all entries posted against the bank account {0} which have not been cleared till {1}.", [`<strong>${bankAccount?.account}</strong>`, `<strong>${formatDate(dates.toDate)}</strong>`])
+
     return <div className="space-y-4 py-2">
 
         <div>
-            <Paragraph className="text-sm">
+            <span className="text-p-sm">
-                <span dangerouslySetInnerHTML={{
+                <MarkdownRenderer content={content} />
-                    __html: _("Below is a list of all entries posted against the bank account {0} which have not been cleared till {1}.", [`<strong>${bankAccount?.account}</strong>`, `<strong>${formatDate(dates.toDate)}</strong>`])
+            </span>
-                }} />
-            </Paragraph>
         </div>
 
         {error && <ErrorBanner error={error} />}

banking/src/components/features/BankReconciliation/BankTransactionList.tsx

@@ -1,7 +1,6 @@
 import { useAtomValue, useSetAtom } from "jotai"
 import { MissingFiltersBanner } from "./MissingFiltersBanner"
 import { bankRecDateAtom, bankRecUnreconcileModalAtom, selectedBankAccountAtom } from "./bankRecAtoms"
-import { Paragraph } from "@/components/ui/typography"
 import { formatDate } from "@/lib/date"
 import { ListView, type ListViewColumnMeta } from "@/components/ui/list-view"
 import { formatCurrency, getCurrencyFormatInfo } from "@/lib/numbers"
@@ -23,6 +22,7 @@ import { useCallback, useMemo, useState } from "react"
 import { Link } from "react-router"
 import { Empty, EmptyTitle, EmptyHeader, EmptyMedia, EmptyDescription, EmptyContent } from "@/components/ui/empty"
 import { InputGroup, InputGroupAddon } from "@/components/ui/input-group"
+import MarkdownRenderer from "@/components/ui/markdown"
 
 const BankTransactions = () => {
     const selectedBank = useAtomValue(selectedBankAccountAtom)
@@ -243,14 +243,14 @@ const BankTransactionListView = () => {
 
     }, [data, search, amountFilter, typeFilter, status])
 
+    const content = _("Below is a list of all bank transactions imported in the system for the bank account {0} between {1} and {2}.", [`<strong>${bankAccount?.account_name}</strong>`, `<strong>${formattedFromDate}</strong>`, `<strong>${formattedToDate}</strong>`])
+
     return <div className="space-y-2 py-2">
 
         <div className="flex gap-2 justify-between items-center">
-            <Paragraph className="text-sm">
+            <span className="text-p-sm">
-                <span dangerouslySetInnerHTML={{
+                <MarkdownRenderer content={content} />
-                    __html: _("Below is a list of all bank transactions imported in the system for the bank account {0} between {1} and {2}.", [`<strong>${bankAccount?.account_name}</strong>`, `<strong>${formattedFromDate}</strong>`, `<strong>${formattedToDate}</strong>`])
+            </span>
-                }} />
-            </Paragraph>
 
             <Button size='md' variant='subtle' asChild>
                 <Link to="/statement-importer">

banking/src/components/features/BankReconciliation/IncorrectlyClearedEntries.tsx

@@ -2,7 +2,6 @@ import { useAtomValue } from "jotai"
 import { MissingFiltersBanner } from "./MissingFiltersBanner"
 import { bankRecDateAtom, selectedBankAccountAtom } from "./bankRecAtoms"
 import { useCurrentCompany } from "@/hooks/useCurrentCompany"
-import { Paragraph } from "@/components/ui/typography"
 import type { ColumnDef } from "@tanstack/react-table"
 import { useCallback, useMemo } from "react"
 import { useFrappeGetCall, useFrappePostCall } from "frappe-react-sdk"
@@ -18,6 +17,7 @@ import { PartyPopper } from "lucide-react"
 import ErrorBanner from "@/components/ui/error-banner"
 import _ from "@/lib/translate"
 import { Empty, EmptyTitle, EmptyDescription, EmptyMedia, EmptyHeader } from "@/components/ui/empty"
+import MarkdownRenderer from "@/components/ui/markdown"
 
 const IncorrectlyClearedEntries = () => {
     const companyID = useCurrentCompany()
@@ -177,22 +177,22 @@ const IncorrectlyClearedEntriesView = () => {
         [accountCurrency, onClearClick],
     )
 
+    const content = _("This report shows all entries in the system where the <strong>clearance date is before the posting date</strong> which is incorrect.")
+
+    const entriesContent = _("Entries below have a posting date after {0} but the clearance date is before {1}.", [`<strong>${formattedToDate}</strong>`, `<strong>${formattedToDate}</strong>`])
+
     return <div className="space-y-4 py-2">
 
         <div>
-            <Paragraph className="text-sm">
+            <span className="text-p-sm">
-                <span dangerouslySetInnerHTML={{
+                <MarkdownRenderer content={content} />
-                    __html: _("This report shows all entries in the system where the <strong>clearance date is before the posting date</strong> which is incorrect.")
-                }} />
                 <br />
                 {data && data.message.result.length > 0 && <span>
-                    <span dangerouslySetInnerHTML={{
+                    <MarkdownRenderer content={entriesContent} />
-                        __html: _("Entries below have a posting date after {0} but the clearance date is before {1}.", [`<strong>${formattedToDate}</strong>`, `<strong>${formattedToDate}</strong>`])
-                    }} />
                     <br />
                     {_("You can reset the clearing dates of these entries here.")}
                 </span>}
-            </Paragraph>
+            </span>
         </div>
 
         {error && <ErrorBanner error={error} />}

banking/src/components/features/BankReconciliation/Rules/RuleForm.tsx

@@ -11,6 +11,7 @@ import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@
 import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"
 import { H4, Paragraph } from "@/components/ui/typography"
 import { today } from "@/lib/date"
+import { evaluateAmountFormula } from "@/lib/amountFormula"
 import _ from "@/lib/translate"
 import { cn } from "@/lib/utils"
 import { BankTransactionRule } from "@/types/Accounts/BankTransactionRule"
@@ -445,11 +446,10 @@ const AmountFormulaRenderer = ({ value }: { value?: string }) => {
     // If it's a string and cannot be a number, then show it as a formula
 
     if (isNaN(Number(value))) {
-
         let calculatedValue = "";
 
         try {
-            calculatedValue = window.eval(`const transaction_amount = 200; ${value}`);
+            calculatedValue = String(evaluateAmountFormula(value ?? "", 200));
         } catch (error: unknown) {
             console.error(error);
             calculatedValue = "Error";

banking/src/lib/amountFormula.ts

@@ -0,0 +1,26 @@
+import { Parser } from 'safe-expr-eval'
+
+const parser = new Parser()
+
+const PLAIN_NUMBER_PATTERN = /^-?\d+(\.\d+)?$/
+
+export function evaluateAmountFormula(expression: string, transactionAmount: number): number {
+    const trimmed = expression.trim()
+    if (!trimmed) {
+        return 0
+    }
+
+    if (PLAIN_NUMBER_PATTERN.test(trimmed)) {
+        return Number(trimmed)
+    }
+
+    try {
+        const result = parser.parse(trimmed).evaluate({ transaction_amount: transactionAmount })
+        if (typeof result !== 'number' || !Number.isFinite(result)) {
+            return 0
+        }
+        return result
+    } catch {
+        return 0
+    }
+}

banking/yarn.lock

@@ -3406,6 +3406,11 @@ rolldown@~1.1.3:
     "@rolldown/binding-win32-arm64-msvc" "1.1.3"
     "@rolldown/binding-win32-x64-msvc" "1.1.3"
 
+safe-expr-eval@^1.0.4:
+  version "1.0.4"
+  resolved "https://registry.yarnpkg.com/safe-expr-eval/-/safe-expr-eval-1.0.4.tgz#3694739b3eb42b906417b94a5e6a74f3c21041b5"
+  integrity sha512-p6ILL9oYItu8Dqm7atFCq3JXW/S1AcZDXQMHRhkRIGpF96SIwEpzogjVNM87mCg+6rfz3Q//rwn1zH2EW2TQ/Q==
+
 scheduler@^0.27.0:
   version "0.27.0"
   resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.27.0.tgz#0c4ef82d67d1e5c1e359e8fc76d3a87f045fe5bd"

erpnext/accounts/doctype/bank_transaction_rule/bank_transaction_rule.py

@@ -9,6 +9,48 @@ from frappe.model.document import Document
 
 from erpnext.accounts.doctype.bank_transaction.bank_transaction import BankTransaction
 
+PLAIN_NUMBER_PATTERN = re.compile(r"^-?\d+(\.\d+)?$")
+# Tokens accepted by safe-expr-eval on the frontend (must stay in sync).
+ALLOWED_FORMULA_TOKEN = re.compile(r"\s+|transaction_amount|\d+(?:\.\d+)?|[+\-*/%^()]")
+PYTHON_ONLY_OPERATORS = ("**", "//")
+
+
+def _is_expr_eval_formula(formula: str) -> bool:
+	position = 0
+	while position < len(formula):
+		match = ALLOWED_FORMULA_TOKEN.match(formula, position)
+		if not match:
+			return False
+		position = match.end()
+
+	return formula.count("(") == formula.count(")")
+
+
+def validate_amount_formula(formula: str) -> None:
+	if not formula:
+		return
+
+	stripped = formula.strip()
+	if PLAIN_NUMBER_PATTERN.match(stripped):
+		return
+
+	if any(operator in stripped for operator in PYTHON_ONLY_OPERATORS):
+		frappe.throw(_("Invalid debit/credit formula: {0}").format(formula))
+
+	if not _is_expr_eval_formula(stripped):
+		frappe.throw(_("Invalid debit/credit formula: {0}").format(formula))
+
+	# expr-eval uses ^ for exponentiation; translate for a smoke-test evaluation only.
+	python_formula = stripped.replace("^", "**")
+
+	try:
+		result = frappe.safe_eval(python_formula, eval_globals=None, eval_locals={"transaction_amount": 1})
+	except Exception:
+		frappe.throw(_("Invalid debit/credit formula: {0}").format(formula))
+
+	if not isinstance(result, (int | float)):
+		frappe.throw(_("Invalid debit/credit formula: {0}").format(formula))
+
 
 class BankTransactionRule(Document):
 	# begin: auto-generated types
@@ -86,6 +128,11 @@ class BankTransactionRule(Document):
 							frappe.throw(
 								_("The last account row must not have any debit or credit amounts set.")
 							)
+					else:
+						if account.debit:
+							validate_amount_formula(account.debit)
+						if account.credit:
+							validate_amount_formula(account.credit)
 
 		# Validate regex
 		for rule in self.description_rules:

erpnext/accounts/doctype/bank_transaction_rule/test_bank_transaction_rule.py

@@ -231,3 +231,45 @@ class TestBankTransactionRule(ERPNextTestSuite, AccountsTestMixin):
 		doc = self._rule("bad_rx", [{"check": "Regex", "value": "["}])
 		with self.assertRaises(ValidationError):
 			doc.insert()
+
+	def _multiple_accounts_rule(self, prefix: str, accounts, **fields):
+		return self._rule(
+			prefix,
+			[{"check": "Contains", "value": "x"}],
+			classify_as="Bank Entry",
+			bank_entry_type="Multiple Accounts",
+			accounts=accounts,
+			**fields,
+		)
+
+	def test_validate_bank_entry_multiple_valid_amount_formulas(self):
+		doc = self._multiple_accounts_rule(
+			"be_formula",
+			accounts=[
+				{"account": self.bank, "debit": "200", "credit": ""},
+				{"account": self.cash, "debit": "", "credit": "transaction_amount * 0.25"},
+				{"account": self.cash, "debit": "", "credit": ""},
+			],
+		)
+		doc.insert()
+		self.assertTrue(doc.name)
+
+	def test_validate_bank_entry_multiple_invalid_amount_formulas(self):
+		malicious_formulas = [
+			"__import__('os')",
+			"eval('1+1')",
+			"open('/etc/passwd')",
+			"transaction_amount ** 2",
+			"transaction_amount // 2",
+		]
+		for formula in malicious_formulas:
+			with self.subTest(formula=formula):
+				doc = self._multiple_accounts_rule(
+					"be_bad_formula",
+					accounts=[
+						{"account": self.bank, "debit": formula, "credit": ""},
+						{"account": self.cash, "debit": "", "credit": ""},
+					],
+				)
+				with self.assertRaises(ValidationError):
+					doc.insert()