fix(security): constrain config-driven code execution - #2340
Open
XianBW (XianBW) wants to merge 15 commits into
Open
XianBW (XianBW) wants to merge 15 commits into
XianBW (XianBW) wants to merge 15 commits into
Conversation
Collaborator
Author
|
Local verification update:
The branch also contains minimal CI compatibility fixes for current Node/commitlint, Plotly, MLflow, and Windows/Python 3.8 OSQP behavior. |
Collaborator
Author
|
Final verification: all 76 GitHub checks pass on commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Feature expressions and configured model/graph names previously reached Python
eval(). File-module configuration also executed arbitrary.pypaths without an explicit directory boundary. This PR restricts expressions to Qlib's supported syntax and registered operators, uses explicit model/graph registries, and requires trusted directories for file-module imports.Issue coverage
ExpressionProviderandDiskExpressionCacheeval()with an AST interpreter. Reject arbitrary calls, attributes, comprehensions, lambdas, expanded arguments, and private operators. Dispatch only through registered expression operators; registry-management methods such asreset()are not callable from expressions.eval()with explicit registries for RNN/Transformer and the four supported graph types. Unknown names fail before execution..pyimportsArtifact serialization, restricted pickle loading, and high-frequency file paths are handled separately in #2339.
Compatibility and trust boundaries
Mean($close, 2 + 3). Constant arithmetic accepts real numbers with bounded integer/exponent sizes; string/list expansion remains rejected. Expressions are a restricted language, not general Python; unsupported syntax raisesExpressionSyntaxError..pyfiles must declare a sequence of trusted directories, for example:The affected LightGBM multi-frequency and TRA examples declare their intended roots. After an explicitly trusted file import, legacy module aliases also allow old trusted model pickles to load without overwriting existing packages. Direct callers can pass
allowed_module_roots; an explicit empty sequence disables file loading even if process-wide roots are configured.Validation
Verified locally on Linux / Python 3.11:
python -m pytest tests/security -q: 58 passed.git diff --checkpassed.Executed workflow comparisons
Compared main
be725493with both PRs combined, using identical inputs and seeds:Reproduction environment: Python 3.11.11, NumPy 2.2.6, pandas 2.2.3, SciPy 1.15.3, LightGBM 4.7.0, MLflow 2.21.3, PyTorch 2.8.0. Shortened model runs verify execution and output compatibility.
Both PR branches incorporate current
main(be72549), preserving the current CI setup and action pins. Fresh cross-platform GitHub Actions runs have been triggered; the current runs, not earlier green commits, determine CI readiness. This PR is not yet merged or released.