-
Notifications
You must be signed in to change notification settings - Fork 30
refactor: remove depends_on fields from heuristics and handle skips in problog #1133
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?
Conversation
15a0c52 to
24d224e
Compare
| mock_defaults.__getitem__.side_effect = lambda section: ( | ||
| MagicMock(get=lambda _: None) if section == "heuristic.pypi" else None | ||
| ) | ||
| defaults["heuristic.pypi"].clear() |
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.
What is the scenario where this case happen? For instance, what would be the values in the user provided defaults.ini (if any).
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.
Our defaults.ini usually looks like this:
disabled_default_rulesets = exfiltration
disabled_rules =
custom_semgrep_rules_path =
disabled_custom_rulesets =
The reason I used .clear() here is to make sure that the rules that are being tested aren't disabled. Particularly here, by default our exfiltration ruleset is disabled, but it is tested in this unit test, so the test would fail if I didn't remove that setting.
Signed-off-by: Carl Flottmann <[email protected]>
…nges, modified docstrings.
Signed-off-by: Carl Flottmann <[email protected]>
0c5b3b7 to
08bcd50
Compare
Summary
The purpose of this PR is to address #1052, and implement appropriate skip and error handling in all existing heuristics. This PR removes the
depends_onfield from heuristics and makes it a responsibility of the heuristic code to determine if it should be skipped or not. All skip handling is then done my the ProbLog logic.Description of changes
The
BaseHeuristicAnalyzerno longer has thedepends_onfield, and this has propagated to all other analyzers. The description for aSKIPresult has been updated in line with issue #1052, andHeuristicAnalyzerValueErrors have replaced someSKIPresults previously used when malformed data was encountered. Unit tests have been updated accordingly.Some refactoring to the
test_pypi_sourcecode_analyzer.pyunit tests have also been done in this PR to address #1108, adding in temporary.iniconfig files to the unit tests as opposed to the existing mock objects.Related issues
Closes #1052 and #1108.
Blockers
Currently blocked by #1147, as this is treated as a
HeuristicAnalyzerValueErrorin this PR and causes integration tests to fail.Checklist
verifiedlabel should appear next to all of your commits on GitHub.