fix(security)!: harden artifact serialization and loading - #2339
Open
XianBW (XianBW) wants to merge 27 commits into
Open
XianBW (XianBW) wants to merge 27 commits into
XianBW (XianBW) wants to merge 27 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 |
Allow the exact MaskedArray and SparseDtype module paths emitted by the older dependency versions in CI. Preserve restricted loading and cover both old and new paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PIT normalization uses report dates and calendar-day offsets, so do not request an unused Baostock calendar. Cover annual and quarterly normalization without online calendar access. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Expose default-off artifact trust at online, delayed-training and meta-workflow entry points. Keep data loading restricted, preserve legacy default recorder calls and restored workflow defaults, and document migration and trust boundaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Exercise real training, cache and recorder restoration, saved-task replay, signals and portfolio backtests. Fix the cache trust, grouping, relative-path, LightGBM early-stopping and HIST CPU blockers exposed by full execution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use trusted consistently across public APIs and CLI options. Migrate pre-release saved consent only at serialization boundaries, reject invalid or conflicting settings, and preserve restricted data reads. Add a canonical upgrade guide with README, installation, changelog and runtime-error entry points. BREAKING CHANGE: executable artifact restoration requires explicit trusted=True. HIST mappings use JSON and high-frequency cache paths must remain within artifact_root. See docs/start/artifact_migration.rst for upgrading existing workflows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Target upgrades from pre-PR Qlib rather than artifacts generated by intermediate PR revisions. Retain restricted defaults for saved objects without consent, unify current APIs, and remove state/keyword migration branches and documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Download the same existing v2 archives used by successful jobs without probing nonexistent package-version assets. Allow five download attempts 60 seconds apart; preserve the full matrix, datasets, noninteractive setup and failure propagation. Cover the exact archives and retry policy in offline CI tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Show explicit consent for restored online components while preserving the original manager, schema-aware Arrow conversions, timezone-preserving ZoneInfo migration, and restricted round trips into a separate recorder. Explain shared-component scope, staging artifacts and trusted-source prerequisites. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
HIST metadata and default MLflow artifact loading could deserialize executable pickle content. The restricted unpickler also trusted entire pandas/NumPy namespaces, allowing I/O helpers such as
pandas.read_pickleto bypass its restrictions. This PR replaces HIST object-NPY metadata with validated JSON and makes artifact loading restricted by default.Breaking upgrade: public Python APIs and CLI entry points consistently use
trusted=False/--trusted=True. The upgrade guide covers required changes and unaffected workflows; its source isdocs/start/artifact_migration.rst. These changes are unreleased until included in a tagged package release.Issue coverage
fit/predictRestrictedUnpicklerpandas.read_pickle,numpy.load, arbitrary execution globals, and nested/two-stage payloads.MLflowRecorder.load_objecttrusted=True; custom unpicklers are explicit and emit a warning. The recorder facade forwards the trust option.HighFreqProviderpathsartifact_root, including split, temporary, daily, and per-instrument files. Reject parent and symlink escapes, and preserve caller configurations.Expression execution and file-module loading are handled separately in #2340.
Compatibility and trust boundaries
stock_indexmust be JSON; object.npymetadata is no longer supported.trusted=True. High-level online, delayed-training and meta-workflows use the same default-offtrusted=Falseoption, rather than silently granting trust internally. Their artifact sources and storage must remain trusted; this option does not make an untrusted shared MLflow store safe.trusted=Truefrom a trusted store, or conversion to supported dtypes/timezones. StaticDataLoader already rejected these formats on main.artifact_root. Set an explicit root for data stored elsewhere. Cached high-frequency datasets still use Python pickle and require trusted contents; path containment is not a filesystem sandbox against concurrent replacement.CI dataset download follow-up
v2archives directly: the same simple/full datasets used by successful jobs. Download setup gets five attempts, 60 seconds apart, with the existing 15-minute per-attempt timeout and failure propagation.Unified API and discoverable migration
trusted. Prediction/label loading, default refusal and the absence of global consent are unchanged. The four example CLIs expose--trustedin generated help.Trust-workflow validation: 263 scoped security/API/full-workflow cases, including four CLI-help cases, passed on both NumPy 1.24.4 / pandas 2.0.3 and NumPy 2.2.6 / pandas 2.3.3. Production lint, Python 3.8 syntax and the Sphinx warnings-as-errors build passed. The two original full-sized online prediction/label workflows also passed on the preceding revision. Earlier validation details are recorded below.
Explicit workflow trust follow-up
Commit
1039e70emoves recorder-artifact consent to caller-facing workflow entry points without replacing serialization formats or splitting this PR:RollingStrategypasses its selected policy throughOnlineToolR, the updater andRMDLoader. Direct updaters/loaders expose the same default-off option. Prediction and label artifacts stay restricted even when model/dataset loading is trusted.DelayTrainerR/DelayTrainerRMpass explicit consent to training completion and the delayed worker, with per-call overrides. Ordinary data-only task configurations remain usable without opting in.DDGDA,MetaDatasetDSandInternalData.setupexpose recorder consent. The subsequent full-workflow follow-up below also covers DDG-DA's necessary local handler/internal-data caches; lower-levelMetaDatasetDS/InternalData.setupconsent remains scoped to recorder tasks.R.load_object(name)calls preserve custom recorders' original default signature. Restored workflow objects without a saved trust field default to restricted loading. Custom implementations must enforce the policy themselves.Follow-up validation on Linux / Python 3.10:
78abf829with the follow-up using identical sample data and seeds: actual Alpha158/LightGBM training with a serialized TimeReweighter, IC generation, a 20-day portfolio backtest, model/dataset reload, and online prediction/label updates. All 6,000 initial predictions, 8,700 updated predictions and labels, and daily IC values matched exactly. The backtest table's maximum absolute numeric difference was 1.49e-8. The shortened training run checks execution/output compatibility, not model performance.The follow-up retains the existing trust assumptions of unrelated local pickle APIs and executable task/YAML configurations. Broader serialization-format changes remain deferred pending additional actual Qlib workflow coverage.
Full-workflow coverage follow-up
Added repeatable offline integration tests using actual Qlib binary-data providers, Alpha158/Alpha360 handlers, model training, MLflow serialization, signal analysis, strategy and backtest execution. No workflow stage is mocked, and fixtures use isolated temporary stores without market/model downloads. Existing slow CI discovers these tests on its Python/OS matrix.
task_train, one epoch and 12 optimizer updates, 48 prediction/label rows, signal analysis and a six-day portfolio backtest. Default model/dataset reads fail; trusted reload reproduces predictions exactly in both the same and a fresh Python process.Full execution exposed and fixed tightly coupled blockers that routing-only tests did not reveal:
trusted) to its required local handler/internal-data caches as well as recorder artifacts. Default loading stays restricted, actual booleans are required, and explicit unrestricted cache loads warn. There is no unsafe retry or global allowlist expansion.working_dirpaths and avoid mutating shared proxy-model/processor templates across runs.None.Validation on Linux / Python 3.10:
The new market fixtures are deterministic synthetic daily prices passed through real Qlib components, not paper-scale market-performance experiments. These tests establish full small-scale execution, meaningful training and artifact round-trip behavior; they do not establish model profitability or justify a repository-wide serialization-format migration. The migration guide and both model example READMEs document the scope and reproduction commands.
Earlier validation
Verified locally on Linux / Python 3.11, NumPy 2.2.6, pandas 2.2.3, and PyTorch 2.8:
python -m pytest tests/security -q: 140 passed.git diff --checkpassed.Executed workflow comparisons
Compared main
be725493with both PRs combined, using the same data and seeds:get_pre_datasets()now succeeds without losing configuration paths.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.