-
-
Notifications
You must be signed in to change notification settings - Fork 615
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
Add support for Python 3.9 #650
Conversation
Add GitHub Action unit testing of Python 3.9 and add to our supported list of Python versions. This patch also fixes some Py39 related issues. Namely: * A hardcoded password such as d["password"] = "blerg" in examples/hardcoded-passwords.py goes undetected. This is due to a change in behavior of the Py3.9 AST. * The README does match the output of bandit -h. Specially the targets is now [targets ...] instead of [targets [targets ...]]. This was introduced with Python fix: https://bugs.python.org/issue38438. As a result, the README no longer contains output of -h and the unit test to make sure they match is gone. Signed-off-by: Eric Brown <browne@vmware.com>
@ericwb it seems like you forked from an old version of |
@@ -75,12 +75,22 @@ def hardcoded_password_string(context): | |||
|
|||
""" | |||
node = context.node | |||
print(node._bandit_parent) |
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.
Left over debugging?
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.
Oops, yes
if isinstance(assign, ast.Assign) and isinstance(assign.value, | ||
ast.Str): | ||
return _report(assign.value.s) | ||
|
||
elif (isinstance(node._bandit_parent, ast.Index) |
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.
Could add a comment that this next block can be removed when Python 3.8 is dropped
def someFunction(user, password="Admin"): | ||
print("Hi " + user) | ||
|
||
def someFunction2(password): | ||
# Possible hardcoded password: 'root' | ||
# Severity: Low Confidence: Medium |
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.
the way comments are done here is inconsistent (indent & before/after)
maybe these should be docstrings?
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.
I don't think it matters that much, so long as there's context around what each one is attempting to warn about. With the exception of the first function here, functions without a real body (e.g., using pass
) have the comment outside the function definition and the rest have the comments before (especially since they're call sites). Personally, given the context of this file, I don't care enough. If Eric wants to change that, it's fine that way too. I don't think this is significant enough to warrant him making a second pass though
@@ -96,167 +96,10 @@ run Bandit with standard input:: | |||
|
|||
cat examples/imports.py | bandit - | |||
|
|||
Usage:: |
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.
Totally tangential, I think Doug Hellman wrote a plugin to sphinx (or maybe it was Jeff Forcier) that puts the execution output of something in the docs where desired. We could take advantage of that here maybe?
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.
We don't need it, though
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.
I don't see any substantial reason not to merge this now, especially with 3.9 nos becoming the default version in the various linux dists.
* Add support for Python 3.9 Add GitHub Action unit testing of Python 3.9 and add to our supported list of Python versions. This patch also fixes some Py39 related issues. Namely: * A hardcoded password such as d["password"] = "blerg" in examples/hardcoded-passwords.py goes undetected. This is due to a change in behavior of the Py3.9 AST. * The README does match the output of bandit -h. Specially the targets is now [targets ...] instead of [targets [targets ...]]. This was introduced with Python fix: https://bugs.python.org/issue38438. As a result, the README no longer contains output of -h and the unit test to make sure they match is gone. Signed-off-by: Eric Brown <browne@vmware.com> * Update general_hardcoded_password.py Co-authored-by: Luke Hinds <7058938+lukehinds@users.noreply.github.com>
* Add support for Python 3.9 Add GitHub Action unit testing of Python 3.9 and add to our supported list of Python versions. This patch also fixes some Py39 related issues. Namely: * A hardcoded password such as d["password"] = "blerg" in examples/hardcoded-passwords.py goes undetected. This is due to a change in behavior of the Py3.9 AST. * The README does match the output of bandit -h. Specially the targets is now [targets ...] instead of [targets [targets ...]]. This was introduced with Python fix: https://bugs.python.org/issue38438. As a result, the README no longer contains output of -h and the unit test to make sure they match is gone. Signed-off-by: Eric Brown <browne@vmware.com> * Update general_hardcoded_password.py Co-authored-by: Luke Hinds <7058938+lukehinds@users.noreply.github.com>
Add GitHub Action unit testing of Python 3.9 and add to our supported
list of Python versions.
Signed-off-by: Eric Brown browne@vmware.com