-
Notifications
You must be signed in to change notification settings - Fork 17
fix(tests): preserve fail-closed Joblib source stability #1811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ | |
|
|
||
| import modelaudit_picklescan.api as picklescan_api | ||
| import pytest | ||
| from modelaudit_picklescan.call_graph import _clear_source_sensitive_caches | ||
| from modelaudit_picklescan.call_graph import _CallGraphAnalysisLimitError, _clear_source_sensitive_caches | ||
|
|
||
| from modelaudit.cache.cache_policy import should_cache_scan_result | ||
| from modelaudit.core import determine_exit_code, scan_file, scan_model_directory_or_file | ||
|
|
@@ -1336,14 +1336,39 @@ def test_scan_accepts_lzma_compressed_numpy_array_payload(tmp_path: Path) -> Non | |
|
|
||
|
|
||
| @pytest.mark.parametrize("raw_seed", [b"builtins", b"os.system", b"subprocess", b"pickle.loads"]) | ||
| def test_scan_accepts_security_like_bytes_inside_numpy_array(tmp_path: Path, raw_seed: bytes) -> None: | ||
| @pytest.mark.parametrize("source_snapshot_changes", [False, True], ids=["stable-source", "changed-source"]) | ||
| def test_scan_accepts_security_like_bytes_inside_numpy_array( | ||
| tmp_path: Path, | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| raw_seed: bytes, | ||
| source_snapshot_changes: bool, | ||
| ) -> None: | ||
| if source_snapshot_changes: | ||
|
|
||
| def raise_source_stability_error(_report_generation: int | None) -> None: | ||
| raise _CallGraphAnalysisLimitError("source changed during shared call-graph analysis") | ||
|
|
||
| monkeypatch.setattr(picklescan_api, "_ensure_shared_source_snapshot_stable", raise_source_stability_error) | ||
|
|
||
| raw_data = raw_seed.ljust(32, b"\x00") | ||
| payload = _joblib_numpy_list_payload(raw_data=raw_data, shape=len(raw_data), dtype="u1") | ||
|
|
||
| result = _scan_payload(tmp_path, payload, "benign_security_like_array_bytes.joblib") | ||
|
|
||
| assert result.success is True | ||
| assert result.metadata["trusted_incomplete_tail"] is True | ||
| if result.success: | ||
| assert source_snapshot_changes is False | ||
| assert result.metadata["trusted_incomplete_tail"] is True | ||
| else: | ||
| assert result.metadata["scan_outcome"] == INCONCLUSIVE_SCAN_OUTCOME | ||
| assert result.metadata["analysis_incomplete"] is True | ||
| assert result.metadata["operational_error_reason"] == "call_graph_analysis_error" | ||
| assert "trusted_incomplete_tail" not in result.metadata | ||
| assert any( | ||
| issue.details.get("category") == "call_graph_analysis_error" | ||
| and issue.details.get("analysis") == "python_call_graph_source_stability" | ||
| and issue.details.get("analysis_incomplete") is True | ||
| for issue in result.issues | ||
|
Comment on lines
+1366
to
+1370
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In the deterministic AGENTS.md reference: AGENTS.md:L137-L137 Useful? React with 👍 / 👎. |
||
| ) | ||
| assert not any(issue.severity in {IssueSeverity.WARNING, IssueSeverity.CRITICAL} for issue in result.issues) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When AGENTS.md reference: AGENTS.md:L137-L137 Useful? React with 👍 / 👎. |
||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
source_snapshot_changesis false, this branch still accepts an inconclusive source-stability failure, so a regression that makes every benign security-like NumPy payload fail with that error would leave all eight cases green. Keep a deterministic stable-source control—such as forcing the stability check to succeed—and requireresult.success is Truethere, while retaining the separate injected-change cases for fail-closed coverage.AGENTS.md reference: AGENTS.md:L115-L117
Useful? React with 👍 / 👎.