Skip to content

Security hardening: replace unsafe eval() with AST-based evaluator in configure_optimizers#1306

Open
abhayrajjais01 wants to merge 1 commit intoweecology:mainfrom
abhayrajjais01:fix/safe-eval-lr-lambda
Open

Security hardening: replace unsafe eval() with AST-based evaluator in configure_optimizers#1306
abhayrajjais01 wants to merge 1 commit intoweecology:mainfrom
abhayrajjais01:fix/safe-eval-lr-lambda

Conversation

@abhayrajjais01
Copy link
Contributor

@abhayrajjais01 abhayrajjais01 commented Feb 14, 2026

Summary

Replace the unsafe eval(params.lr_lambda) call in configure_optimizers() with a restricted AST-based evaluator (_safe_eval_lr_lambda) that only permits numeric constants, the epoch variable, parentheses, unary +/- and arithmetic operators (+, -, *, /, //, %, **).

This prevents arbitrary code execution via malicious config overrides (CWE-95) while preserving full flexibility for valid scheduler expressions like 0.95 ** epoch.

Closes #1305

Changes

  • src/deepforest/main.py: Added import ast, replaced eval(params.lr_lambda) with self._safe_eval_lr_lambda(params.lr_lambda, epoch), and added the _safe_eval_lr_lambda static method.
  • tests/test_main.py: Added test_configure_optimizers_rejects_unsafe_lr_lambda regression test that verifies malicious expressions raise ValueError.

Testing

  • Regression test passes locally (1 passed in 0.57s)
  • Existing test_configure_optimizers tests unaffected (valid expressions like 0.95**epoch continue to work)

Used Ai to review the changes

@abhayrajjais01 abhayrajjais01 force-pushed the fix/safe-eval-lr-lambda branch 2 times, most recently from 849a7c8 to 3a0056e Compare February 17, 2026 11:04
@abhayrajjais01
Copy link
Contributor Author

@henrykironde i have fixed the conflict, let me know if any changes required

@abhayrajjais01 abhayrajjais01 force-pushed the fix/safe-eval-lr-lambda branch 2 times, most recently from 1bcfe8e to 9d9e2a2 Compare March 6, 2026 21:40
@abhayrajjais01 abhayrajjais01 force-pushed the fix/safe-eval-lr-lambda branch from 9d9e2a2 to 286c556 Compare March 6, 2026 21:40
@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 73.80952% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.14%. Comparing base (efb872f) to head (286c556).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/deepforest/main.py 73.80% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1306      +/-   ##
==========================================
- Coverage   87.34%   87.14%   -0.20%     
==========================================
  Files          24       24              
  Lines        2978     3019      +41     
==========================================
+ Hits         2601     2631      +30     
- Misses        377      388      +11     
Flag Coverage Δ
unittests 87.14% <73.80%> (-0.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security hardening: replace unsafe eval-based scheduler expression parsing with a restricted arithmetic AST evaluator (CWE-95)

2 participants