dense-analysis / ale

Check syntax in Vim/Neovim asynchronously and fix files, with Language Server Protocol (LSP) support
BSD 2-Clause "Simplified" License
13.57k stars 1.44k forks source link

[python] bandit receives the file to analyse in a temporary directory instead of passing the actual file #4437

Open massimiliano-della-rovere opened 1 year ago

massimiliano-della-rovere commented 1 year ago

Information

VIM version

VIM - Vi IMproved 9.0 (2022 Jun 28, compilato May 10 2022 08:40:37) Patch included: 1-749

Operating System: Ubuntu 22.04.1 LTS

What went wrong

The way bandit is called is incompatible with bandit's configuration syntax and assumptions. Note this is generally true for every linting program allowing a configuration involving a directory name.

Reproducing the bug

  1. I created a python project with a pyproject.toml unified Python project settings file and a tests directory.
  2. Define the directory where the python virtual env is installed as
  3. Define the directory containing the pyproject.toml and the tests directory as
  4. In the pyproject.toml file there is the following configuration for bandit:
    [tool.bandit]
    assert_used.skips = ["tests/test_*.py"] 
  5. When ALE calls bandit for analysing the python file, the syntax used is: <venv>/bin/bandit --format custom --msg-template "{line}:{test_id}:{severity}:{msg}" - < /tmp/random_directory_id/test_commands.py instead of <venv>/bin/bandit --format custom --msg-template "{line}:{test_id}:{severity}:{msg}" -c <project_root>/pyproject.toml <project_root>/tests/test_commands.py
  6. The result is that any parameter in the [bandit.tool] that specifies a path (i.e. skip, exclude, etc) is deceived by the /tmp/random_direcory/ vs the proper /tests/
  7. Furthermore the invokation of bandit lacks the -c <project_root>/pyproject.toml option that allow to customize the bandit behaviour.

:ALEInfo

[...]
  Command History:
(executable check - success) /home/mader/DygmaRaise/.venv/bin/bandit
(finished - exit code 1) ['/bin/bash', '-c', '''/home/mader/DygmaRaise/.venv/bin/bandit'' --format custom --msg-template "{line}:{test_id}:{severity}:{msg}"  - < ''/tmp/vduvRFr/4/test_commands.py''']
<<<OUTPUT STARTS>>>
14:B101:LOW:Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
15:B101:LOW:Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
<<<OUTPUT ENDS>>>
[...]
hsanson commented 1 year ago

Looking at bandit's linter implementation in ALE I can see two things:

  1. It uses stdin redirection to pass the file to bandit binary (e.g. - < /random/file.py). This is usually better to enforce per-project configuration files. I don't think this is the problem.
  2. Instead of pyproject.toml it tries to find a .bandit configuration file and if found it add this argument to the command: --ini /path/to/.bandit. Maybe something changed in recent versions of bandit and now requires pyproject.toml instead of .bandit??

The linter implementation would need to either replace the code to find .bandit with code to find pyproject.toml and set proper argument to the command when invoking bandit.

Code in question is below:

function! ale_linters#python#bandit#GetCommand(buffer) abort
    let l:executable = ale_linters#python#bandit#GetExecutable(a:buffer)
    let l:flags = ' --format custom'
    \   . ' --msg-template "{line}:{test_id}:{severity}:{msg}" '

    if ale#Var(a:buffer, 'python_bandit_use_config')
        let l:config_path = ale#path#FindNearestFile(a:buffer, '.bandit')

        if !empty(l:config_path)
            let l:flags = ' --ini ' . ale#Escape(l:config_path) . l:flags
        endif
    endif

    let l:exec_args = l:executable =~? 'pipenv\|poetry$'
    \   ? ' run bandit'
    \   : ''

    return ale#Escape(l:executable) . l:exec_args
    \   . l:flags
    \   . ale#Pad(ale#Var(a:buffer, 'python_bandit_options'))
    \   . ' -'
endfunction