Skip to content

fix: add torch and numpy helper primitive coverage#706

Open
mldangelo wants to merge 1 commit intomainfrom
feat/pickle-helper-primitives
Open

fix: add torch and numpy helper primitive coverage#706
mldangelo wants to merge 1 commit intomainfrom
feat/pickle-helper-primitives

Conversation

@mldangelo
Copy link
Member

@mldangelo mldangelo commented Mar 13, 2026

Summary

  • add exact pickle dangerous-global coverage for validated torch and NumPy helper references
  • add regression coverage for import-only GLOBAL, REDUCE, STACK_GLOBAL, and mixed-signal payloads
  • keep safe nearby tensor and NumPy reconstruction helpers non-failing

Validation

  • uv run ruff format modelaudit/ tests/
  • uv run ruff check --fix modelaudit/ tests/
  • uv run ruff check modelaudit/ tests/
  • uv run ruff format --check modelaudit/ tests/
  • uv run mypy modelaudit/
  • uv run pytest -n auto -m "not slow and not integration" --maxfail=1

Summary by CodeRabbit

  • Bug Fixes

    • Improved detection of dangerous helper references across multiple libraries and modules
  • Tests

    • Added comprehensive test coverage for helper reference detection across multiple validation scenarios

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 72d5f4cf-5b70-45e1-a57f-5f28768ed8da

📥 Commits

Reviewing files that changed from the base of the PR and between 2df2d78 and 969ed31.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • modelaudit/detectors/suspicious_symbols.py
  • tests/scanners/test_pickle_scanner.py

Walkthrough

Adds new dangerous helper reference patterns to the pickle scanner's suspicious symbols dictionary, covering specific functions from PyTorch and NumPy packages, along with corresponding test cases to verify detection behavior.

Changes

Cohort / File(s) Summary
Documentation
CHANGELOG.md
Documents fixed coverage for dangerous helper references in validated Torch and NumPy packages.
Detection Patterns
modelaudit/detectors/suspicious_symbols.py
Extends SUSPICIOUS_GLOBALS with seven new import-level suspicion entries targeting specific functions in numpy.f2py.crackfortran, torch._dynamo.guards, torch.fx.experimental.symbolic_shapes, torch.utils packages, and related submodules.
Test Coverage
tests/scanners/test_pickle_scanner.py
Introduces HELPER_REFS constant and helper function _craft_global_only_pickle(), adds five new test methods validating critical detection for bare GLOBAL payloads, REDUCE payloads, memoized STACK_GLOBAL references, safe reconstruction helpers, and mixed signal detection.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Poem

🐰 Whiskers twitch with delight so bright,
New guards detect helpers that hide in the night,
Torch and NumPy safely confined,
The picker's eye leaves dangers behind!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: add torch and numpy helper primitive coverage' directly describes the main change: adding coverage for dangerous helper references from torch and numpy packages.
Docstring Coverage ✅ Passed Docstring coverage is 90.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/pickle-helper-primitives
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

1 participant