Skip to content

llmguard: replace raw eval() with a safe AST-based evaluator#2180

Merged
crivetimihai merged 1 commit intoIBM:mainfrom
RinZ27:refactor/llmguard-safe-policy-eval
Jan 23, 2026
Merged

llmguard: replace raw eval() with a safe AST-based evaluator#2180
crivetimihai merged 1 commit intoIBM:mainfrom
RinZ27:refactor/llmguard-safe-policy-eval

Conversation

@RinZ27
Copy link
Copy Markdown
Contributor

@RinZ27 RinZ27 commented Jan 19, 2026 •
edited
Loading

Hardened the policy evaluation by ditching raw eval(). Instead of relying on a whitelist and then compiling, I wrote a dedicated AST walker that only knows how to handle the specific logic we actually use. It keeps things strictly contained while preserving all the boolean and comparison features we need.

  • Removed eval(compile(tree, ...)) entirely.
  • Added a _safe_eval method to compute results based on a strict set of supported operations.
  • Maintained support for existing policy logic.

@RinZ27 RinZ27 requested a review from crivetimihai as a code owner January 19, 2026 09:11
@RinZ27 RinZ27 force-pushed the refactor/llmguard-safe-policy-eval branch 2 times, most recently from 4561499 to c80ed46 Compare January 19, 2026 09:19
@crivetimihai crivetimihai added this to the Release 1.0.0-RC1 milestone Jan 20, 2026
@RinZ27 @crivetimihai
llmguard: replace raw eval() with a safe AST-based evaluator
077aad4 
Signed-off-by: RinCodeForge927 <dangnhatrin90@gmail.com>
Signed-off-by: RinZ27 <222222878+RinZ27@users.noreply.github.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
@crivetimihai crivetimihai force-pushed the refactor/llmguard-safe-policy-eval branch from c80ed46 to 077aad4 Compare January 23, 2026 19:15
@crivetimihai crivetimihai self-assigned this Jan 23, 2026
@crivetimihai
Copy link
Copy Markdown
Member

crivetimihai commented Jan 23, 2026

Review and Updates

Rebased onto main and made the following changes:

Fixes Applied

  1. Short-circuit evaluation - Fixed a security regression where eager evaluation of and/or operands could turn a deny into an allow when a missing filter caused an exception. Now uses lazy evaluation to preserve Python's short-circuit semantics:

    • False and MissingFilter → returns False (deny) without evaluating MissingFilter
    • True or MissingFilter → returns True (allow) without evaluating MissingFilter

  2. Removed deprecated ast.Num - Removed handling for ast.Num which is deprecated and removed in Python 3.14. Not needed since Python 3.11+ uses ast.Constant for all literals.

Tests Added

Added tests/test_policy.py with 24 unit tests covering:

  • Short-circuit semantics (4 tests)
  • Boolean operations (8 tests)
  • Constant handling (2 tests)
  • Security/rejection of dangerous operations (6 tests)
  • Edge cases (4 tests)

Behavioral Note

The new evaluator accepts boolean constants (True/False) in policies. This is a minor behavioral change from the old code which rejected them on Python 3.11+ (returning "Invalid expression"). The new behavior is more intuitive: policy: "False" now correctly denies rather than allowing due to a truthy error string.

@crivetimihai crivetimihai merged commit 73b9a2c into IBM:main Jan 23, 2026
52 checks passed
@RinZ27 RinZ27 deleted the refactor/llmguard-safe-policy-eval branch January 24, 2026 09:42
@crivetimihai crivetimihai mentioned this pull request Jan 24, 2026
Merged
2 tasks
kcostell06 pushed a commit to kcostell06/mcp-context-forge that referenced this pull request Feb 24, 2026
@RinZ27
llmguard: replace raw eval() with a safe AST-based evaluator (IBM#2180)
2936788 
Signed-off-by: RinCodeForge927 <dangnhatrin90@gmail.com>
Signed-off-by: RinZ27 <222222878+RinZ27@users.noreply.github.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crivetimihai crivetimihai crivetimihai approved these changes

Assignees

@crivetimihai crivetimihai

Labels

None yet

Projects

None yet

Milestone

Release 1.0.0-BETA-2

Development

Successfully merging this pull request may close these issues.

2 participants

@RinZ27 @crivetimihai