fix(tests): preserve fail-closed Joblib source stability - #1811
fix(tests): preserve fail-closed Joblib source stability#1811mldangelo-oai wants to merge 1 commit into
Conversation
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b227b84529
ℹ️ 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".
| 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 |
There was a problem hiding this comment.
In the deterministic changed-source cases, this assertion verifies only the issue details, so the test would still pass if the adapter emitted an empty or misleading operational message. Assert the expected source-stability diagnostic on the matching issue as part of the fail-closed contract.
AGENTS.md reference: AGENTS.md:L137-L137
Useful? React with 👍 / 👎.
| and issue.details.get("analysis_incomplete") is True | ||
| for issue in result.issues | ||
| ) | ||
| assert not any(issue.severity in {IssueSeverity.WARNING, IssueSeverity.CRITICAL} for issue in result.issues) |
There was a problem hiding this comment.
When source_snapshot_changes is true, an unsuccessful result is expected, but this final assertion inspects only result.issues; a false-positive warning or critical failed entry in result.checks would therefore pass unnoticed. Check both collections, including CheckStatus.FAILED, so the benign payload regression verifies finding precedence across the complete scan result.
AGENTS.md reference: AGENTS.md:L137-L137
Useful? React with 👍 / 👎.
| if result.success: | ||
| assert source_snapshot_changes is False | ||
| assert result.metadata["trusted_incomplete_tail"] is True |
There was a problem hiding this comment.
When source_snapshot_changes is 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 require result.success is True there, while retaining the separate injected-change cases for fail-closed coverage.
AGENTS.md reference: AGENTS.md:L115-L117
Useful? React with 👍 / 👎.
1 participant
Summary
test_scan_accepts_security_like_bytes_inside_numpy_arraywithout changing production scanner behavior or weakening fail-closed handling.call_graph_analysis_error/python_call_graph_source_stability, does not trust the incomplete tail, and still contains no warning or critical findings.Regression evidence
Validation
mainare already fixed by fix(deps): audit all CI extras and upgrade vulnerable packages #1808, whose Dependency Audit check is green.