Skip to content

fix(security): constrain config-driven code execution - #2340

Open
XianBW (XianBW) wants to merge 15 commits into
mainfrom
security/constrain-config-execution
Open

XianBW (XianBW) wants to merge 15 commits into
mainfrom
security/constrain-config-execution

Conversation

@XianBW

@XianBW XianBW (XianBW) commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

Feature expressions and configured model/graph names previously reached Python eval(). File-module configuration also executed arbitrary .py paths 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

Area Result
ExpressionProvider and DiskExpressionCache Replace eval() 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 as reset() are not callable from expressions.
Qlib-side expressions used by Qlib-Server The library's expression and disk-cache execution sinks are removed. A server deployment must upgrade this dependency and be redeployed; server authentication and deployment validation are outside this repository.
TRA model / performance graphs Replace eval() with explicit registries for RNN/Transformer and the four supported graph types. Unknown names fail before execution.
Configuration-driven .py imports Disable by default. Resolve file paths and require containment under trusted directories, including symlink/parent checks. Reject a scalar root string so its characters cannot accidentally authorize the filesystem root.

Artifact serialization, restricted pickle loading, and high-frequency file paths are handled separately in #2339.

Compatibility and trust boundaries

  • Normal registered/custom operators and Alpha158/Alpha360 feature definitions remain supported, including numeric parameter arithmetic such as 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 raises ExpressionSyntaxError.
  • Package imports remain available. YAML configurations and custom operator code must still be trusted; this is not a sandbox for arbitrary configurations or a resource-exhaustion defense.
  • Configurations loading local .py files must declare a sequence of trusted directories, for example:
qlib_init:
  trusted_module_roots: [./custom_modules]

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.
  • Both security suites plus original operator/custom-operator, utility, workflow/MLflow, and CI configuration tests: 224 passed, using the existing local simple dataset.
  • Combined with fix(security): harden artifact serialization and loading #2339: 224 passed, including both security suites, original operators, utilities, workflow/MLflow, and CI configuration tests.
  • Runtime tests exercise both expression-provider and disk-cache rejection before side effects, real expression values, Alpha158/Alpha360 definitions, custom operators, both TRA models, all graph names, and file-module configuration paths.
  • Black for changed Python files, Flake8 for affected Qlib code, Pylint for the newly modified production code, Python 3.8 syntax checks, YAML parsing, and git diff --check passed.

Executed workflow comparisons

Compared main be725493 with both PRs combined, using identical inputs and seeds:

  • All 518 Alpha158/Alpha360 features across 138 instrument/day rows matched exactly. Runtime comparisons exposed rejected numeric parameter arithmetic; this is now supported with numeric-only evaluation and bounded intermediate values.
  • LightGBM training/backtesting/online updates and all seven generated performance figures passed; predictions, labels, and figure JSON matched main exactly.
  • Both TRA RNN and Transformer completed a training epoch and inference on real Alpha158 data, with 120 predictions each matching main exactly.
  • File-based custom operators ran through two fresh loky workers. A YAML/qrun task using a file-based LightGBM model trained and generated the same 460 predictions.
  • A file-based model trained and saved by main was loaded in a fresh process under the combined PRs after its file was explicitly imported from a trusted root. Its 460 predictions matched exactly. This exposed and fixed the legacy-module-name regression; no alias is installed before path authorization or over an existing unrelated package.

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.

@XianBW XianBW (XianBW) changed the title security: constrain config-driven code execution fix(security): constrain config-driven code execution Sep 2, 2026
@XianBW

Copy link
Copy Markdown
Collaborator Author

Local verification update:

  • 18 security regression tests passed
  • security tests plus handler serialization and full train/backtest pipeline: 22 passed
  • repository-wide Black, Flake8, and Pylint passed
  • all eight modified example YAML files parsed successfully
  • removed the optional Plotly import from the registry regression test so test collection is independent of the analysis extra

The branch also contains minimal CI compatibility fixes for current Node/commitlint, Plotly, MLflow, and Windows/Python 3.8 OSQP behavior.

@XianBW

Copy link
Copy Markdown
Collaborator Author

Final verification: all 76 GitHub checks pass on commit ee6b4518/latest branch state, including title, CLA, pip compatibility, source builds, notebook execution, full pytest, and slow pytest matrices.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant