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

Add support for Python 3.9 #650

Merged
merged 6 commits into from Feb 11, 2021
Merged

Add support for Python 3.9 #650

merged 6 commits into from Feb 11, 2021

Conversation

ericwb
Copy link
Member

@ericwb ericwb commented Dec 6, 2020

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

@ericwb ericwb requested a review from lukehinds December 6, 2020 02:53
@ericwb ericwb added this to the Release 1.6.3 milestone Dec 6, 2020
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 ericwb requested a review from a team December 6, 2020 03:02
@sigmavirus24
Copy link
Member

@ericwb it seems like you forked from an old version of master and as if GitHub was configured to want you to update this first.

@@ -75,12 +75,22 @@ def hardcoded_password_string(context):

"""
node = context.node
print(node._bandit_parent)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left over debugging?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, yes

@ericwb ericwb modified the milestones: Release 1.6.3, Near Future Dec 6, 2020
if isinstance(assign, ast.Assign) and isinstance(assign.value,
ast.Str):
return _report(assign.value.s)

elif (isinstance(node._bandit_parent, ast.Index)
Copy link

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

@ericwb ericwb requested a review from ghugo as a code owner December 17, 2020 06:50
def someFunction(user, password="Admin"):
print("Hi " + user)

def someFunction2(password):
# Possible hardcoded password: 'root'
# Severity: Low Confidence: Medium

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?

Copy link
Member

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::
Copy link
Member

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?

Copy link
Member

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

Copy link
Member

@lukehinds lukehinds left a 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.

@ericwb ericwb merged commit 6765a57 into PyCQA:master Feb 11, 2021
@ericwb ericwb deleted the py39 branch February 11, 2021 16:44
mikespallino pushed a commit to mikespallino/bandit that referenced this pull request Aug 25, 2021
* 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>
mikespallino pushed a commit to mikespallino/bandit that referenced this pull request Jan 7, 2022
* 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>
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.

None yet

5 participants