Skip to content

Conversation

MeGaGiGaGon
Copy link
Contributor

Summary

Part of #18972

Both in one PR since they are in the same file.

S604

This PR makes call-with-shell-equals-true (S604)'s example error out-of-the-box

Old example

import subprocess

user_input = input("Enter a command: ")
subprocess.run(user_input, shell=True)

New example

import my_custom_subprocess

user_input = input("Enter a command: ")
my_custom_subprocess.run(user_input, shell=True)

The old example doesn't raise S604 because it gets overwritten by subprocess-popen-with-shell-equals-true (S602) (which is a good idea to prevent two lints saying the same thing from being raised)

S609

This PR makes unix-command-wildcard-injection (S609)'s example error out-of-the-box

Old example

import subprocess

subprocess.Popen(["chmod", "777", "*.py"])

New example

import subprocess

subprocess.Popen(["chmod", "777", "*.py"], shell=True)

I'm not familiar enough with subprocess to know why shell=True is required to make S609 raise here, but it works.

Test Plan

N/A, no functionality/tests affected

Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Collaborator

@dylwil3 dylwil3 left a comment

Choose a reason for hiding this comment

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

makes sense, thanks!

@dylwil3 dylwil3 added the documentation Improvements or additions to documentation label Jun 30, 2025
@dylwil3 dylwil3 merged commit 4963835 into astral-sh:main Jun 30, 2025
36 checks passed
@MeGaGiGaGon MeGaGiGaGon deleted the patch-5 branch June 30, 2025 21:22
iyakushev pushed a commit to iyakushev/ruff that referenced this pull request Jul 1, 2025
astral-sh#19049)

<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
  requests.)
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

Part of astral-sh#18972

Both in one PR since they are in the same file.

S604
---

This PR makes [call-with-shell-equals-true
(S604)](https://docs.astral.sh/ruff/rules/call-with-shell-equals-true/#call-with-shell-equals-true-s604)'s
example error out-of-the-box

[Old example](https://play.ruff.rs/a054fb79-7653-47f7-9ab5-3d8b7540c810)
```py
import subprocess

user_input = input("Enter a command: ")
subprocess.run(user_input, shell=True)
```

[New example](https://play.ruff.rs/6fea81b4-e745-4b85-8bea-faaabea5c86d)
```py
import my_custom_subprocess

user_input = input("Enter a command: ")
my_custom_subprocess.run(user_input, shell=True)
```

The old example doesn't raise `S604` because it gets overwritten by
[subprocess-popen-with-shell-equals-true
(S602)](https://docs.astral.sh/ruff/rules/subprocess-popen-with-shell-equals-true/#subprocess-popen-with-shell-equals-true-s602)
(which is a good idea to prevent two lints saying the same thing from
being raised)

S609
---

This PR makes [unix-command-wildcard-injection
(S609)](https://docs.astral.sh/ruff/rules/unix-command-wildcard-injection/#unix-command-wildcard-injection-s609)'s
example error out-of-the-box

[Old example](https://play.ruff.rs/849860fa-0d12-4916-bdbc-64a0fa14cd9b)
```py
import subprocess

subprocess.Popen(["chmod", "777", "*.py"])
```

[New example](https://play.ruff.rs/77a54d7c-cf78-4158-bcf8-96dd698cf366)
```py
import subprocess

subprocess.Popen(["chmod", "777", "*.py"], shell=True)
```

I'm not familiar enough with `subprocess` to know why `shell=True` is
required to make `S609` raise here, but it works.

## Test Plan

<!-- How was it tested? -->

N/A, no functionality/tests affected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants