fix: avoid pickle meta-path source probing

[mldangelo-oai]

Summary

• remove custom sys.meta_path and path-entry finder execution from pickle call-graph source probing
• fail closed as source_unavailable when attacker-selected module source cannot be safely inspected
• preserve direct source, frozen stdlib, and on-disk extension-module classification
• add malicious finder side-effect regressions plus local, stdlib, frozen, and extension-module guards

Validation

• merged current origin/main (372a72ab)
• focused API and call-graph suite: 676 passed, 1 skipped
• ruff format --check modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
• ruff check modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
• mypy modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/ (454 source files)
• repository pytest reached 99% with no failures twice, then reproduced an idle xdist teardown stall locally; fresh GitHub CI is running on the pushed head
• git diff --check

[mldangelo-oai]

@codex review

[github-actions]

Workflow run and artifacts

Performance Benchmarks

Compared 12 shared benchmarks with a regression threshold of 15%.
Status: 2 regressions, 0 improved, 10 stable, 0 new, 0 missing.
Aggregate shared-benchmark median: 1.259s -> 1.242s (-1.4%).

Top regressions:

• tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload +32.9% (1.59ms -> 2.11ms, direct-malicious-upload, malicious_reduce, size=52 B, files=1)
• tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload +32.2% (1.66ms -> 2.19ms, padded-multi-stream-upload, multi_stream_padded, size=4.1 KiB, files=1)

Workload	Benchmark	Target	Size	Files	Baseline	Current	Change	Status
direct-malicious-upload	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload	malicious_reduce	52 B	1	1.59ms	2.11ms	+32.9%	regression
padded-multi-stream-upload	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload	multi_stream_padded	4.1 KiB	1	1.66ms	2.19ms	+32.2%	regression
mixed-model-repository	tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository	release-candidate	547.3 KiB	32	411.42ms	399.03ms	-3.0%	stable
suspicious-pickle-intake	tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake	suspicious-intake	183.8 KiB	4	128.67ms	131.74ms	+2.4%	stable
nested-payload-review	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex]	nested_hex	130 B	1	493.5us	482.7us	-2.2%	stable
single-checkpoint-preflight	tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load	single_checkpoint.pkl	183.0 KiB	1	67.33ms	65.87ms	-2.2%	stable
duplicate-heavy-registry	tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot	registry-snapshot	915.2 KiB	13	362.40ms	357.05ms	-1.5%	stable
warm-cache-rescan	tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan	release-candidate	547.3 KiB	32	71.86ms	70.92ms	-1.3%	stable
nested-payload-review	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_raw]	nested_raw	78 B	1	478.5us	475.3us	-0.7%	stable
clean-training-checkpoint	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint	safe_large	278.2 KiB	1	104.85ms	104.13ms	-0.7%	stable
nested-payload-review	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64]	nested_base64	98 B	1	496.6us	493.5us	-0.6%	stable
chunked-upload-stream	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream	chunked_stream	278.2 KiB	1	107.80ms	107.24ms	-0.5%	stable

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0633495cf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ianw-oai]

Reviewed; this looks acceptable.

[mldangelo-oai]

QA/fix pass pushed in 618bf709.

Key hardening:

• Preserve real import precedence without invoking custom meta/path finders or path hooks.
• Fail closed when a later-added custom finder/hook or cached custom path importer could shadow a filesystem candidate.
• Respect bytecode-before-extension shadowing instead of classifying the later extension as safe.
• Trust only exact builtin module specs (_io.nonexistent is no longer treated as builtin).
• Preserve loaded/frozen stdlib analysis while avoiding custom loader execution.

Validation:

• test_call_graph_import_statements.py: 303 passed, 4 skipped
• targeted modified API regression: 1 passed
• scoped Ruff format/check and mypy: clean

The prior review threads were already resolved. A broader pre-final API run was not used as validation because the main-branch merge left the local compiled Rust extension stale relative to its source, producing unrelated Unicode/nested-scan failures; per the PR loop policy I did not rebuild or run the full suite.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 618bf709e5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[mldangelo-oai]

Reviewed and pushed 8c83361.

Findings and fixes:

• Closed the pre-import meta-path false-negative by removing broad snapshot trust and conservatively recognizing only known bootstrap finders that cannot handle the requested module.
• Closed spoofed path-hook metadata and pre-existing custom importer bypasses with identity/type-based checks.
• Preserved zipimport precedence so an earlier ZIP source cannot be hidden by a later extension module; retained the inverse benign precedence case.
• Fixed three shadowed-torch regressions that depended on test ordering and could silently exercise the already-loaded real torch module.
• Replied to and resolved all three open review threads.

Validation: all seven focused bypass tests passed; the complete call-graph module passed with 310 passed and 1 skipped under xdist; the associated API module passed in the combined targeted run; Ruff format/check and mypy passed for the affected files. CI is running on the pushed head.

[chatgpt-codex-connector]

💡 Codex Review

modelaudit/packages/modelaudit-picklescan/src/modelaudit_picklescan/call_graph.py

Line 1082 in 8c83361

search_path = list(locations)

Check cached finders on package search paths

For dotted imports, the resolver reuses a package's submodule_search_locations but _has_untrusted_path_hook() only inspected entries currently in sys.path. If sys.path_importer_cache already contains a custom finder for the package directory (which is normally not itself in sys.path), Python will use that cached finder for pkg.sub, while this code constructs a fresh FileFinder for the same location and can analyze a benign sub.py as clean instead of failing closed for the custom import path.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Reject preexisting custom path hooks

If an application installs a custom sys.path_hooks entry before importing modelaudit_picklescan, this identity check permanently treats that hook as standard. When a later sys.path entry handled by that hook also contains a benign .py file, _has_untrusted_path_hook() returns false and the resolver reads the benign file with a fresh FileFinder, even though Python's import machinery would use the custom hook for the pickle-selected module; this can turn an unanalyzable reference into a clean call-graph result instead of failing closed.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Reject cached FileFinder subclasses

When sys.path_importer_cache already contains a custom finder that subclasses FileFinder, isinstance(..., FileFinder) treats it as trusted, but real imports will dispatch to that cached finder while this resolver ignores it and constructs a new plain FileFinder for the same path. In a path entry ahead of a benign same-named .py, that lets the scan analyze the benign file and return no source_unavailable notice even though the actual import would be controlled by the cached custom finder.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Fail closed for preloaded module specs

When the scan process already has a module in sys.modules, this path trusts that module's __spec__.origin as the source to analyze. A plugin or custom loader can preload the pickle-selected module and set origin to a benign .py file; _resolve_module_source() then parses that file and can report the reference clean, while pickle.loads resolves to the already-loaded in-memory module rather than the benign source. Loaded specs from non-standard loaders need the same fail-closed treatment as custom finders.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Invalidate shared caches when import hooks change

_resolve_module_source() is cached and now depends on sys.meta_path/sys.path_hooks, but the shared-cache stability check still only tracks sys.path and source fingerprints. If a caller uses the public shared_source_sensitive_caches() context across scans, a module resolved cleanly before a custom finder is inserted remains cached as analyzable after the hook change, so the later scan can miss the required source_unavailable fail-closed notice until caches are manually cleared.

Useful? React with 👍 / 👎.