Skip to content
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

Multiple suppressions on the same rule_id only removes one instance #104

Open
mattmook opened this issue Jan 3, 2025 · 0 comments
Open

Comments

@mattmook
Copy link

mattmook commented Jan 3, 2025

For a working example, see: https://github.com/appmattus/certificatetransparency/tree/mobsf
There are three files which trigger android_kotlin_logging, each suppressed with a comment, however, only one suppression works. Raised issues https://github.com/appmattus/certificatetransparency/security/code-scanning?query=pr%3A127+is%3Aopen

Looking at the code there's looks like there's a bug in post_ignore_files, where we remove matches from the original list of files rather than the filtered tmp_files:

i.e.

                if self.suppress_pm_comments(file, rule_id):
                    # remove all matches of the file for the rule
                    tmp_files = self.remove_matches(file, files)

should be

                if self.suppress_pm_comments(file, rule_id):
                    # remove all matches of the file for the rule
                    tmp_files = self.remove_matches(file, tmp_files)

Adding an additional file in tests/assets/src/dot_mobsf with the same existing comment suppression shows this failure in unit tests. For example I duplicated scan_but_ignore.kt as scan_but_ignore2.kt, which results in a failed test.

With the above change to post_ignore_files the test then passes as expected.

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

No branches or pull requests

1 participant