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

plugins/semgrep: add csmock semgrep plugin #149

Merged
merged 2 commits into from
Apr 2, 2024
Merged

Conversation

rhyw
Copy link
Contributor

@rhyw rhyw commented Jan 8, 2024

@rhyw rhyw marked this pull request as draft January 8, 2024 07:09
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
rhyw added a commit to rhyw/csmock that referenced this pull request Jan 8, 2024
@rhyw rhyw force-pushed the semgrep branch 2 times, most recently from 5102f33 to 0ef1ef1 Compare January 8, 2024 09:40
rhyw added a commit to rhyw/csmock that referenced this pull request Jan 8, 2024
rhyw added a commit to rhyw/csmock that referenced this pull request Jan 11, 2024
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
@rhyw rhyw self-assigned this Jan 11, 2024
rhyw added a commit to rhyw/csmock that referenced this pull request Jan 11, 2024
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
py/plugins/semgrep.py Fixed Show fixed Hide fixed
make-srpm.sh Outdated Show resolved Hide resolved
make-srpm.sh Outdated Show resolved Hide resolved
make-srpm.sh Outdated Show resolved Hide resolved
py/CMakeLists.txt Outdated Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Fixed Show fixed Hide fixed
@rhyw
Copy link
Contributor Author

rhyw commented Mar 4, 2024

Any more updates needed please?

py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Outdated Show resolved Hide resolved
@kdudka
Copy link
Member

kdudka commented Mar 18, 2024

@rhyw Did you look at the warnings produced by Differential PyLint? For example, this one looks valid:

Error: PYLINT_WARNING:
py/plugins/semgrep.py:83:4: R1710[inconsistent-return-statements]: Plugin.handle_args: Either all return statements in a function should return an expression, or none of them should.

As far as I can see, no other csmock plug-in returns any value from handle_args(). I find it confusing that the semgrep plug-in (in some cases) does.

py/plugins/semgrep.py Outdated Show resolved Hide resolved
"""
import os

from csmock.common.util import sanitize_opts_arg

Check warning

Code scanning / vcs-diff-lint

Unable to import 'csmock.common.util'

Unable to import 'csmock.common.util'
parser.error("'--semgrep-rules-repo' is required to run semgrep scan")

# sanitize options passed to --semgrep-scan-opts to avoid shell injection
self.semgrep_scan_opts = sanitize_opts_arg(parser, args, "--semgrep-scan-opts")

Check warning

Code scanning / vcs-diff-lint

Plugin.handle_args: Attribute 'semgrep_scan_opts' defined outside __init__

Plugin.handle_args: Attribute 'semgrep_scan_opts' defined outside __init__
@lzaoral
Copy link
Member

lzaoral commented Mar 21, 2024

@rhyw It is possible to execute the semgrep plugin without any need for additional resources? If yes, could you please add a test description for it? You can take a look at the one for the GCC plugin as the inspiration: 219e34e

edit: typo

@kdudka
Copy link
Member

kdudka commented Mar 21, 2024

@lzaoral If we want to extend test coverage, I would focus on plug-ins with self.stable = True in PluginProps. All other plug-ins are experimental. This plug-in downloads the full software stack of semgrep over network, which is going to be slow and prone to fail for gazillion of reasons unrelated to the changes of csmock code.

py/plugins/semgrep.py Outdated Show resolved Hide resolved
py/plugins/semgrep.py Show resolved Hide resolved
py/plugins/semgrep.py Show resolved Hide resolved
py/plugins/semgrep.py Show resolved Hide resolved
py/plugins/semgrep.py Show resolved Hide resolved
py/plugins/semgrep.py Show resolved Hide resolved
@kdudka
Copy link
Member

kdudka commented Mar 27, 2024

@rhyw Looks good. Please squash the fixup commits. Namely 4014c4e needs to be squashed before merging so that we do not unnecessarily introduce and fix a security issue in the git history.

@rhyw
Copy link
Contributor Author

rhyw commented Mar 28, 2024

@rhyw Looks good. Please squash the fixup commits. Namely 4014c4e needs to be squashed before merging so that we do not unnecessarily introduce and fix a security issue in the git history.

Thanks, commits squashed

@kdudka
Copy link
Member

kdudka commented Mar 28, 2024

@rhyw Thanks! I have squashed my fixup commits, too. We do not want them in the main branch.

@rhyw rhyw merged commit feba72d into csutils:main Apr 2, 2024
42 checks passed
rhyw added a commit that referenced this pull request Apr 2, 2024
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

Successfully merging this pull request may close these issues.

5 participants