Skip to content

SEC harden get_tree loaded types - #547

Merged
adrinjalali merged 5 commits into
skops-dev:mainfrom
adrinjalali:worktree-gh222-allowed-types
Sep 27, 2026
Merged

adrinjalali merged 5 commits into
skops-dev:mainfrom
adrinjalali:worktree-gh222-allowed-types

Conversation

@adrinjalali

Copy link
Copy Markdown
Member

Fixes #222

Wherever it makes sense, make sure the get_tree checks the loaded type is actually what's expected.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unexpected Node initializers execute before the new type guard rejects them, leaving avoidable security and resource-exhaustion exposure.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Adds child-node type restrictions to reduce unsafe deserialization paths.

Changes:

  • Adds allowed_types validation to get_tree.
  • Applies validation across general, NumPy, sklearn, and legacy loaders.
  • Adds malformed-file regression tests and changelog documentation.
File Description
skops/​io/​_audit.py Implements allowed node-type validation.
skops/​io/​_general.py Restricts expected general child nodes.
skops/​io/​_numpy.py Restricts NumPy child nodes.
skops/​io/​_sklearn.py Restricts sklearn child nodes.
skops/​io/​old/​_numpy_v1.py Hardens the protocol-1 loader.
skops/​io/​tests/​test_audit.py Tests current-protocol validation.
skops/​io/​tests/​test_persist_old.py Tests legacy validation.
docs/​changes.rst Documents deserialization hardening.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread skops/io/_audit.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Security-sensitive deserialization changes spanning current and legacy protocols warrant final human validation.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@adrinjalali
adrinjalali merged commit 4263053 into skops-dev:main Sep 27, 2026
30 checks passed
@adrinjalali
adrinjalali deleted the worktree-gh222-allowed-types branch September 27, 2026 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Limit get_tree to a predefined set of types

2 participants