bandit check seems to link arguments not function call

[Borda]

It seems that Ruff reports wrong lines for this bandit issue

out = subprocess.check_output(  # <-- yesqa expected
    ["git", "rev-parse", "HEAD"],  # <-- ruff reported
    stderr=DEVNULL,
)

in fact, it creates deadlock as yesqa removes noqas from line 2 as it expects them with line 1, but then ruff reports line 2

2: S603 `subprocess` call: check for execution of untrusted input
2: S607 Starting a process with a partial executable path

adding a dummy line to the argument, and it seems to be baked with the first argument as this

out = subprocess.check_output(  # <-- yesqa expected
	print(), # <-- ruff reported
    ["git", "rev-parse", "HEAD"],
    stderr=DEVNULL,
)

still reports line 2 but the next one

out = subprocess.check_output(  # <-- yesqa expected
	# any line
    ["git", "rev-parse", "HEAD"],  # <-- ruff reported
    stderr=DEVNULL,
)

reports line 3

--

using pre-commit with

  - repo: https://github.com/asottile/yesqa
    rev: v1.5.0
    hooks:
      - id: yesqa
        additional_dependencies:
          - flake8-bandit

  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.2.1
    hooks:
      - id: ruff
        args: ["--fix"]

[charliermarsh]

Interesting, will take a look.

[dhruvmanila]

It seems like bandit highlights the subprocess.check_output call in the diagnostic and we highlight the argument. And, yesqa uses the location bandit provides to check if NoQA comment is valid or not.

$ bat src/S603.py 
         File: src/S603.py
     1   import subprocess
     2   
     3   out = subprocess.check_output(
     4       ["git", "rev-parse", "HEAD"],
     5   )

$ flake8 src/S603.py
src/S603.py:1:1: S404 Consider possible security implications associated with the subprocess module.
src/S603.py:3:1: S607 Starting a process with a partial executable path
src/S603.py:3:1: S603 subprocess call - check for execution of untrusted input.

A simple fix would be to use the same location.

[charliermarsh]

Note that in general, using yesqa alongside Ruff isn't necessarily a supported workflow (for this reason: we don't guarantee that our rules trigger on the exact same locations, since we sometimes refine the ranges).

[dhruvmanila]

You can use our own implementation of unused-noqa if yesqa isn't strictly required in your workflow.

[charliermarsh]

@dhruvmanila - I'm not totally sure which (if any) of these rules should have their ranges adjusted -- would need to look...