cannot seek stdin on pipe

[JuanHuaXu]

Describe the bug
There are couple bugs merged into one for stdin parsing

https://github.com/PyCQA/bandit/blob/master/bandit/core/manager.py#L249
self._parse_file('<stdin>', sys.stdin, new_files_list)

Note we are artificially changing the file name from '-' to '<stdin>', which doesn't exist in the new_file_list

https://github.com/PyCQA/bandit/blob/master/bandit/core/manager.py#L278
fdata.seek(0)

Note OS doesn't allow seek on pipes/stream stdins, this causes us to fail into general catchall exception clause.

https://github.com/PyCQA/bandit/blob/master/bandit/core/manager.py#L303
new_files_list.remove(fname)

remember back in line 249 we artificially renamed the filename? Here is where it came back and bite us. Since '<stdin>' doesn't exist in the new_files_list as an item, it hard failed without try catch and dumps a stack trace.

To Reproduce
cat blah.py | bandit --debug -

Expected behavior
it should either parse the contents of the stdin stream or not having this functionality for pipes

Bandit version

master

Additional context
Add any other context about the problem here.

[tylerwince]

Just so the trace is here in the ticket also, I was able to reproduce running cat assert.py | bandit --debug -

[main]  DEBUG   logging initialized
[main]  INFO    profile include tests: None
[main]  INFO    profile exclude tests: None
[main]  INFO    cli include tests: None
[main]  INFO    cli exclude tests: None
[test_set]      DEBUG   added function any_other_function_with_shell_equals_true (B604) targeting Call
[test_set]      DEBUG   added function assert_used (B101) targeting Assert
[test_set]      DEBUG   added function django_extra_used (B610) targeting Call
[test_set]      DEBUG   added function django_mark_safe (B703) targeting Call
[test_set]      DEBUG   added function django_rawsql_used (B611) targeting Call
[test_set]      DEBUG   added function exec_used (B102) targeting Call
[test_set]      DEBUG   added function flask_debug_true (B201) targeting Call
[test_set]      DEBUG   added function hardcoded_bind_all_interfaces (B104) targeting Str
[test_set]      DEBUG   added function hardcoded_password_default (B107) targeting FunctionDef
[test_set]      DEBUG   added function hardcoded_password_funcarg (B106) targeting Call
[test_set]      DEBUG   added function hardcoded_password_string (B105) targeting Str
[test_set]      DEBUG   added function hardcoded_sql_expressions (B608) targeting Str
[test_set]      DEBUG   added function hardcoded_tmp_directory (B108) targeting Str
[test_set]      DEBUG   added function hashlib_new_insecure_functions (B324) targeting Call
[test_set]      DEBUG   added function jinja2_autoescape_false (B701) targeting Call
[test_set]      DEBUG   added function linux_commands_wildcard_injection (B609) targeting Call
[test_set]      DEBUG   added function paramiko_calls (B601) targeting Call
[test_set]      DEBUG   added function request_with_no_cert_validation (B501) targeting Call
[test_set]      DEBUG   added function set_bad_file_permissions (B103) targeting Call
[test_set]      DEBUG   added function ssh_no_host_key_verification (B507) targeting Call
[test_set]      DEBUG   added function ssl_with_bad_defaults (B503) targeting FunctionDef
[test_set]      DEBUG   added function ssl_with_bad_version (B502) targeting Call
[test_set]      DEBUG   added function ssl_with_no_version (B504) targeting Call
[test_set]      DEBUG   added function start_process_with_a_shell (B605) targeting Call
[test_set]      DEBUG   added function start_process_with_no_shell (B606) targeting Call
[test_set]      DEBUG   added function start_process_with_partial_path (B607) targeting Call
[test_set]      DEBUG   added function subprocess_popen_with_shell_equals_true (B602) targeting Call
[test_set]      DEBUG   added function subprocess_without_shell_equals_true (B603) targeting Call
[test_set]      DEBUG   added function try_except_continue (B112) targeting ExceptHandler
[test_set]      DEBUG   added function try_except_pass (B110) targeting ExceptHandler
[test_set]      DEBUG   added function use_of_mako_templates (B702) targeting Call
[test_set]      DEBUG   added function weak_cryptographic_key (B505) targeting Call
[test_set]      DEBUG   added function yaml_load (B506) targeting Call
[test_set]      DEBUG   added function blacklist (B001) targeting Call
[test_set]      DEBUG   added function blacklist (B001) targeting Import
[test_set]      DEBUG   added function blacklist (B001) targeting ImportFrom
[main]  INFO    running on Python 3.8.0
[manager]       DEBUG   working on file : -
[manager]       ERROR   Exception occurred when executing tests against <stdin>. Run "bandit --debug <stdin>" to see the full traceback.
Traceback (most recent call last):
  File "/home/tylerwince/.pyenv/versions/3.8-dev/lib/python3.8/site-packages/bandit/core/manager.py", line 278, in _parse_file
    fdata.seek(0)
OSError: [Errno 29] Illegal seek

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/tylerwince/.pyenv/versions/3.8-dev/bin/bandit", line 10, in <module>
    sys.exit(main())
  File "/home/tylerwince/.pyenv/versions/3.8-dev/lib/python3.8/site-packages/bandit/cli/main.py", line 391, in main
    b_mgr.run_tests()
  File "/home/tylerwince/.pyenv/versions/3.8-dev/lib/python3.8/site-packages/bandit/core/manager.py", line 249, in run_tests
    self._parse_file('<stdin>', sys.stdin, new_files_list)
  File "/home/tylerwince/.pyenv/versions/3.8-dev/lib/python3.8/site-packages/bandit/core/manager.py", line 303, in _parse_file
    new_files_list.remove(fname)
ValueError: list.remove(x): x not in list

[tylerwince]

Proposed fix is sitting in PR #496 -- @JuanHuaXu if you have any comments please feel free to make them and please test out this branch to ensure it fixes the behavior you are seeing.

[JuanHuaXu]

Thanks for the quick fix, however this still doesn't fix the secondary issue where file name is reassigned for the file list. as soon as there is another general exception in that try catch block, it will throw unhandled stack trace again and halt the whole program. new_files_list variable need to be looked at and adjusted accordingly.

[JuanHuaXu]

@tylerwince created list handling PR against your fork, please review

[JuanHuaXu]

After running the patched code, looks like there is a minor bug associated with the way _execute_ast_visitor is implemented. It is handled by the try catch block in utils.py

What we see:
[node_visitor] INFO Unable to find qualified name for module: <stdin>

Cause:
Hard swap file name to '<stdin>'

Detail:
https://github.com/PyCQA/bandit/blob/master/bandit/core/manager.py#L289
_execute_ast_visitor is called for fname = '<stdin>'

https://github.com/PyCQA/bandit/blob/master/bandit/core/manager.py#L316
which in term get passed to BanditNodeVisitor

https://github.com/PyCQA/bandit/blob/master/bandit/core/node_visitor.py#L50
which in term gets treated like a module file

https://github.com/PyCQA/bandit/blob/master/bandit/core/utils.py#L148
Of course just the file name without path is unsuitable, since it is stdin, the file technically doesn't even exist on the filesystem. Thus throws exception.

Over all, the pipe stdin implementation probably could use more design polish.

[tylerwince]

I feel like this may be desired behavior has we are catching anything that doesn't have a file on disk and this is one of those cases. Unless @ericwb feels like it should be redesigned, I feel like this is a good solution to the problem once your change on my branch is merged @JuanHuaXu.

[JuanHuaXu]

You are right, it is desired behavior. I'm only picking it up because SublimeLinter is picking up the exception as an error for bandit. Eric needs to address it either in the bandit end or the SublimeLinter end. Either way what we did is good enough as far as bandit is concerned standing alone

[tylerwince]

@ericwb Can we get this slated for 1.6.3?

[loscil06]

Still having this problem as of 1.7.0, is there any temporary workaround I can do?