Skip to content

#694 Bandit fails when using importlib with named arguments#701

Merged
lukehinds merged 4 commits into
PyCQA:masterfrom
maciejstromich:694
Apr 5, 2021
Merged

#694 Bandit fails when using importlib with named arguments#701
lukehinds merged 4 commits into
PyCQA:masterfrom
maciejstromich:694

Conversation

@maciejstromich
Copy link
Copy Markdown
Contributor

@maciejstromich maciejstromich commented Mar 30, 2021

This PR fixes the issue when importlib is being used with named arguments outlined in the #694.

Following cases are found:

  1. importlib.import_module("foo", "bar.baz") results in call_args = ["foo", "bar.baz"] and call_keywords = {}
  2. importlib.import_module("foo", package="bar.baz") results in call_args = ["foo"] and call_keywords = {"package": "bar.baz"}
  3. importlib.import_module(name="foo", package="bar.baz") results in call_args = [] and call_keywords = {"name": "foo", "package": "bar.baz"}

The simplest approach is to use context.call_args_count and if the call_args_count is larger than 0, call call_args[0] which will choose foo. If call_args_count is equal to 0 then we need to choose name from the call_keywords.

I haven't found previous tests covering the blacklisting importlib functionality so if anyone more fluent in the codebase could point me in the direction of where this should be tested I can add the test cases for this functionality.

@sigmavirus24
Copy link
Copy Markdown
Member

I think we run tests against the examples directory where you could add to https://github.com/PyCQA/bandit/blob/master/examples/imports-with-importlib.py

@maciejstromich
Copy link
Copy Markdown
Contributor Author

@sigmavirus24 ah, that makes sense now. I've added two new examples to the import-with-imposelib.py. Thanks for pointing that out

@maciejstromich
Copy link
Copy Markdown
Contributor Author

@sigmavirus24 I've found one more interesting case. When these new tests are executed against master they are not failing the build because bandit throws an exception and continues with execution so I've updated the results to follow incremented values and changed the modules in the tests to trigger additional issues. Also I've updated tests to include just name named argument

@lukehinds lukehinds merged commit 193c355 into PyCQA:master Apr 5, 2021
@maciejstromich maciejstromich deleted the 694 branch April 5, 2021 20:28
mikespallino pushed a commit to mikespallino/bandit that referenced this pull request Jan 7, 2022
…QA#701)

* PyCQA#694 Bandit fails when using importlib with named arguments

* add missing tests

* improvement in the tests

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.

3 participants