fix(security): replace new Function() with safe math expression parser

[cyphercodes]

Summary

Fixes #725 by replacing the dangerous new Function() constructor in evaluateMathExpression() with a safe recursive descent parser.

Problem

The evaluateMathExpression function in apps/web/src/utils/math.ts used new Function() to evaluate math expressions. While the current regex validation is reasonably tight, using the Function constructor is inherently dangerous and a recognized anti-pattern. If the validation logic is ever relaxed or refactored, this becomes a vector for arbitrary code execution.

Solution

Replaced the Function constructor with a proper recursive descent parser that:

• Only supports numbers, +, -, *, /, parentheses, and whitespace
• Has no eval-like functionality that could execute arbitrary code
• Maintains backward compatibility with existing expressions
• Handles operator precedence and parentheses correctly
• Returns null for any invalid or potentially dangerous input

Changes

• Rewrote evaluateMathExpression() to use a safe parser
• Added tokenizer and parser functions
• Added comprehensive test suite with 19 tests covering:
  • Basic arithmetic operations
  • Operator precedence
  • Parentheses and nesting
  • Decimal numbers
  • Negative numbers
  • Edge cases and error handling
  • Security/injection prevention

Testing

All tests pass:

• 19 new tests for math expression evaluation
• All existing tests continue to pass
• Code passes linting with Biome

Security Impact

This change eliminates a potential code injection vulnerability while maintaining full backward compatibility with existing functionality.

Summary by CodeRabbit

• Bug Fixes

  • Improved security and robustness of math expression evaluation with stricter input validation and consistent handling of invalid or unsafe inputs.

• Refactor

  • Reworked internal evaluation logic for safer, more accurate computation and error detection.

• Tests

  • Added comprehensive test suite covering operators, precedence, parentheses, decimals, edge cases, and security scenarios.

[vercel]

@cyphercodes is attempting to deploy a commit to the OpenCut OSS Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a64b89c1-b7a4-43b2-b8a8-405f8b1aed26

📥 Commits

Reviewing files that changed from the base of the PR and between 0c7cc11 and c599d37.

📒 Files selected for processing (1)

• apps/web/src/utils/math.ts

✅ Files skipped from review due to trivial changes (1)

• apps/web/src/utils/math.ts

---

📝 Walkthrough

Walkthrough

Replaces the use of new Function() in evaluateMathExpression with a recursive-descent tokenizer/parser and adds a comprehensive Bun test suite verifying correctness, edge cases, and rejection of code-injection attempts. No exported signatures were changed.

Changes

Cohort / File(s)	Summary
Math Expression Parser Implementation apps/web/src/utils/math.ts	Reimplemented evaluateMathExpression to use an internal recursive-descent tokenizer/parser (parseExpression) instead of new Function. Supports +, -, *, /, parentheses, unary minus, decimal parsing, operator precedence, complete-token consumption, finite-number checks, division-by-zero detection, and returns null on any parse/eval error.
Comprehensive Test Suite apps/web/src/utils/__tests__/math.test.ts	Added new Bun test suite (~120 lines) covering arithmetic operations, precedence, nested parentheses, decimals, negatives, whitespace variations, malformed inputs (empty, invalid chars, unbalanced parentheses, incomplete expressions, division by zero), and security-focused cases asserting rejection of injection/prototype-access patterns.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I hopped through tokens, nibbling unsafe strings,

No Function left to sprout mischievous wings,
Parentheses hugged, precedence in place,
Safe sums and divisions — a tidy, calm space.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main security fix: replacing Function() with a safe parser.
Description check	✅ Passed	The description provides comprehensive details on the problem, solution, changes, testing, and security impact, fully addressing the PR context.
Linked Issues check	✅ Passed	The PR successfully addresses all requirements from issue #725: eliminates Function constructor, implements safe recursive descent parser supporting only +, -, *, /, numbers, parentheses, and whitespace, preserves functionality, and returns null for invalid inputs.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #725: rewriting the evaluateMathExpression function, adding tokenizer/parser logic, and adding comprehensive tests for the new implementation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

apps/web/src/utils/math.ts (1)

150-167: Use descriptive tokenizer variable names

Consider renaming i, str, char, and numStr to full names (tokenIndex, trimmedInput, currentCharacter, numberLiteral) for readability and consistency.

As per coding guidelines: "Never abbreviate variable and parameter names. Use event not e, element not el."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/utils/math.ts` around lines 150 - 167, Rename the terse
tokenizer variables in the while-loop to descriptive names for readability:
change i to tokenIndex, str to trimmedInput, char to currentCharacter, and
numStr to numberLiteral throughout the tokenization block (the loop that trims
input and iterates characters to build numeric literals). Update all references
inside that scope (including the inner while that accumulates digits and dots)
so logic and behavior remain identical.

apps/web/src/utils/__tests__/math.test.ts (1)

50-55: Add malformed-decimal regression cases

Please add cases like "1..2+3" and "12.3.4" asserting null, so invalid numeric literals are explicitly locked down.

Also applies to: 71-100

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/utils/__tests__/math.test.ts` around lines 50 - 55, Add
regression tests to the decimal-number suite covering malformed numeric literals
so evaluateMathExpression returns null for invalid decimals: add cases like
expect(evaluateMathExpression({ input: "1..2+3" })).toBeNull() and
expect(evaluateMathExpression({ input: "12.3.4" })).toBeNull(); place them
alongside the existing decimal tests for "3.5+2.5" and "10.5 * 2" (also mirror
similar additions in the other test block referenced around lines 71-100) to
ensure invalid numeric literals are explicitly asserted as null.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web/src/utils/math.ts`:
- Around line 163-171: The code currently uses Number.parseFloat on numStr which
accepts malformed tokens like "1..2"; update the numeric validation before
conversion: after building numStr in the numeric parsing loop (variable numStr
inside the tokenization logic in utils/math.ts), validate it with a strict regex
such as /^\d+(\.\d+)?$/ (ensuring at most one decimal point and at least one
digit before the dot), throw the existing Error(`Invalid number: ${numStr}`) if
it doesn't match, and then convert using Number(numStr) (or parseFloat only
after the regex check) to produce value.

---

Nitpick comments:
In `@apps/web/src/utils/__tests__/math.test.ts`:
- Around line 50-55: Add regression tests to the decimal-number suite covering
malformed numeric literals so evaluateMathExpression returns null for invalid
decimals: add cases like expect(evaluateMathExpression({ input: "1..2+3"
})).toBeNull() and expect(evaluateMathExpression({ input: "12.3.4"
})).toBeNull(); place them alongside the existing decimal tests for "3.5+2.5"
and "10.5 * 2" (also mirror similar additions in the other test block referenced
around lines 71-100) to ensure invalid numeric literals are explicitly asserted
as null.

In `@apps/web/src/utils/math.ts`:
- Around line 150-167: Rename the terse tokenizer variables in the while-loop to
descriptive names for readability: change i to tokenIndex, str to trimmedInput,
char to currentCharacter, and numStr to numberLiteral throughout the
tokenization block (the loop that trims input and iterates characters to build
numeric literals). Update all references inside that scope (including the inner
while that accumulates digits and dots) so logic and behavior remain identical.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8a2fe5e8-deb9-4a67-b741-d23e5b89a368

📥 Commits

Reviewing files that changed from the base of the PR and between f2eed05 and 0c7cc11.

📒 Files selected for processing (2)

• apps/web/src/utils/__tests__/math.test.ts
• apps/web/src/utils/math.ts