Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement flake8-bandit rule S108 #1644

Merged
merged 10 commits into from Jan 5, 2023
38 changes: 38 additions & 0 deletions README.md
Expand Up @@ -770,6 +770,7 @@ For more, see [flake8-bandit](https://pypi.org/project/flake8-bandit/4.1.1/) on
| S105 | HardcodedPasswordString | Possible hardcoded password: `"..."` | |
| S106 | HardcodedPasswordFuncArg | Possible hardcoded password: `"..."` | |
| S107 | HardcodedPasswordDefault | Possible hardcoded password: `"..."` | |
| S108 | HardcodedTempFile | Probable insecure usage of temp file/directory: `"..."` | |

### flake8-blind-except (BLE)

Expand Down Expand Up @@ -2362,6 +2363,43 @@ suppress-none-returning = true

---

### `flake8-bandit`

#### [`hardcoded-tmp-directory`](#hardcoded-tmp-directory)

A list of directories to consider temporary.

**Default value**: `["/tmp", "/var/tmp", "/dev/shm"]`

**Type**: `Vec<String>`

**Example usage**:

```toml
[tool.ruff.flake8-bandit]
hardcoded-tmp-directory = ["/foo/bar"]
```

---

#### [`hardcoded-tmp-directory-extend`](#hardcoded-tmp-directory-extend)

A list of directories to consider temporary, in addition to those
specified by `hardcoded-tmp-directory`.

**Default value**: `[]`

**Type**: `Vec<String>`

**Example usage**:

```toml
[tool.ruff.flake8-bandit]
extend-hardcoded-tmp-directory = ["/foo/bar"]
```

---

### `flake8-bugbear`

#### [`extend-immutable-calls`](#extend-immutable-calls)
Expand Down
7 changes: 7 additions & 0 deletions flake8_to_ruff/src/converter.rs
Expand Up @@ -393,6 +393,7 @@ mod tests {
task_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bandit: None,
flake8_bugbear: None,
flake8_errmsg: None,
flake8_pytest_style: None,
Expand Down Expand Up @@ -455,6 +456,7 @@ mod tests {
task_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bandit: None,
flake8_bugbear: None,
flake8_errmsg: None,
flake8_pytest_style: None,
Expand Down Expand Up @@ -517,6 +519,7 @@ mod tests {
task_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bandit: None,
flake8_bugbear: None,
flake8_errmsg: None,
flake8_pytest_style: None,
Expand Down Expand Up @@ -579,6 +582,7 @@ mod tests {
task_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bandit: None,
flake8_bugbear: None,
flake8_errmsg: None,
flake8_pytest_style: None,
Expand Down Expand Up @@ -641,6 +645,7 @@ mod tests {
task_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bandit: None,
flake8_bugbear: None,
flake8_errmsg: None,
flake8_pytest_style: None,
Expand Down Expand Up @@ -712,6 +717,7 @@ mod tests {
task_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bandit: None,
flake8_bugbear: None,
flake8_errmsg: None,
flake8_pytest_style: None,
Expand Down Expand Up @@ -777,6 +783,7 @@ mod tests {
task_tags: None,
update_check: None,
flake8_annotations: None,
flake8_bandit: None,
flake8_bugbear: None,
flake8_errmsg: None,
flake8_pytest_style: None,
Expand Down
16 changes: 16 additions & 0 deletions resources/test/fixtures/flake8_bandit/S108.py
@@ -0,0 +1,16 @@
# ok
with open("/abc/tmp", "w") as f:
f.write("def")

with open("/tmp/abc", "w") as f:
f.write("def")

with open("/var/tmp/123", "w") as f:
f.write("def")

with open("/dev/shm/unit/test", "w") as f:
f.write("def")

# not ok by config
with open("/foo/bar", "w") as f:
f.write("def")
39 changes: 39 additions & 0 deletions ruff.schema.json
Expand Up @@ -121,6 +121,17 @@
}
]
},
"flake8-bandit": {
"description": "Options for the `flake8-bandit` plugin.",
"anyOf": [
{
"$ref": "#/definitions/Flake8BanditOptions"
},
{
"type": "null"
}
]
},
"flake8-bugbear": {
"description": "Options for the `flake8-bugbear` plugin.",
"anyOf": [
Expand Down Expand Up @@ -915,9 +926,11 @@
"S105",
"S106",
"S107",
"S108",
"SIM",
"SIM1",
"SIM10",
"SIM102",
"SIM105",
"SIM11",
"SIM118",
Expand Down Expand Up @@ -1082,6 +1095,32 @@
},
"additionalProperties": false
},
"Flake8BanditOptions": {
"type": "object",
"properties": {
"hardcoded-tmp-directory": {
"description": "A list of directories to consider temporary.",
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
},
"hardcoded-tmp-directory-extend": {
"description": "A list of directories to consider temporary, in addition to those specified by `hardcoded-tmp-directory`.",
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
}
},
"additionalProperties": false
},
"Flake8BugbearOptions": {
"type": "object",
"properties": {
Expand Down
9 changes: 9 additions & 0 deletions src/checkers/ast.rs
Expand Up @@ -2536,6 +2536,15 @@ where
self.add_check(check);
}
}
if self.settings.enabled.contains(&CheckCode::S108) {
if let Some(check) = flake8_bandit::checks::hardcoded_tmp_directory(
expr,
value,
&self.settings.flake8_bandit.hardcoded_tmp_directory,
) {
self.add_check(check);
}
}
if self.settings.enabled.contains(&CheckCode::UP025) {
pyupgrade::plugins::rewrite_unicode_literal(self, expr, kind);
}
Expand Down
16 changes: 16 additions & 0 deletions src/flake8_bandit/checks/hardcoded_tmp_directory.rs
@@ -0,0 +1,16 @@
use rustpython_ast::Expr;

use crate::ast::types::Range;
use crate::registry::{Check, CheckKind};

/// S108
pub fn hardcoded_tmp_directory(expr: &Expr, value: &str, prefixes: &[String]) -> Option<Check> {
if prefixes.iter().any(|prefix| value.starts_with(prefix)) {
Some(Check::new(
CheckKind::HardcodedTempFile(value.to_string()),
Range::from_located(expr),
))
} else {
None
}
}
2 changes: 2 additions & 0 deletions src/flake8_bandit/checks/mod.rs
Expand Up @@ -7,6 +7,7 @@ pub use hardcoded_password_func_arg::hardcoded_password_func_arg;
pub use hardcoded_password_string::{
assign_hardcoded_password_string, compare_to_hardcoded_password_string,
};
pub use hardcoded_tmp_directory::hardcoded_tmp_directory;

mod assert_used;
mod bad_file_permissions;
Expand All @@ -15,3 +16,4 @@ mod hardcoded_bind_all_interfaces;
mod hardcoded_password_default;
mod hardcoded_password_func_arg;
mod hardcoded_password_string;
mod hardcoded_tmp_directory;
26 changes: 24 additions & 2 deletions src/flake8_bandit/mod.rs
@@ -1,5 +1,6 @@
pub mod checks;
mod helpers;
pub mod settings;

#[cfg(test)]
mod tests {
Expand All @@ -10,7 +11,7 @@ mod tests {

use crate::linter::test_path;
use crate::registry::CheckCode;
use crate::settings;
use crate::{flake8_bandit, Settings};

#[test_case(CheckCode::S101, Path::new("S101.py"); "S101")]
#[test_case(CheckCode::S102, Path::new("S102.py"); "S102")]
Expand All @@ -19,15 +20,36 @@ mod tests {
#[test_case(CheckCode::S105, Path::new("S105.py"); "S105")]
#[test_case(CheckCode::S106, Path::new("S106.py"); "S106")]
#[test_case(CheckCode::S107, Path::new("S107.py"); "S107")]
#[test_case(CheckCode::S108, Path::new("S108.py"); "S108")]
fn checks(check_code: CheckCode, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy());
let checks = test_path(
Path::new("./resources/test/fixtures/flake8_bandit")
.join(path)
.as_path(),
&settings::Settings::for_rule(check_code),
&Settings::for_rule(check_code),
)?;
insta::assert_yaml_snapshot!(snapshot, checks);
Ok(())
}

#[test]
fn check_hardcoded_tmp_additional_dirs() -> Result<()> {
let checks = test_path(
Path::new("./resources/test/fixtures/flake8_bandit/S108.py"),
&Settings {
flake8_bandit: flake8_bandit::settings::Settings {
hardcoded_tmp_directory: vec![
"/tmp".to_string(),
"/var/tmp".to_string(),
"/dev/shm".to_string(),
"/foo".to_string(),
],
},
..Settings::for_rule(CheckCode::S108)
},
)?;
insta::assert_yaml_snapshot!("S108_extend", checks);
Ok(())
}
}
77 changes: 77 additions & 0 deletions src/flake8_bandit/settings.rs
@@ -0,0 +1,77 @@
//! Settings for the `flake8-bandit` plugin.

use ruff_macros::ConfigurationOptions;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

fn default_tmp_dirs() -> Vec<String> {
["/tmp", "/var/tmp", "/dev/shm"]
.map(std::string::ToString::to_string)
.to_vec()
}

#[derive(
Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions, JsonSchema,
)]
#[serde(
deny_unknown_fields,
rename_all = "kebab-case",
rename = "Flake8BanditOptions"
)]
pub struct Options {
#[option(
default = "[\"/tmp\", \"/var/tmp\", \"/dev/shm\"]",
value_type = "Vec<String>",
example = "hardcoded-tmp-directory = [\"/foo/bar\"]"
)]
/// A list of directories to consider temporary.
pub hardcoded_tmp_directory: Option<Vec<String>>,
#[option(
default = "[]",
value_type = "Vec<String>",
example = "extend-hardcoded-tmp-directory = [\"/foo/bar\"]"
)]
/// A list of directories to consider temporary, in addition to those
/// specified by `hardcoded-tmp-directory`.
pub hardcoded_tmp_directory_extend: Option<Vec<String>>,
}

#[derive(Debug, Hash)]
pub struct Settings {
pub hardcoded_tmp_directory: Vec<String>,
Copy link
Member

Choose a reason for hiding this comment

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

You could consider just having hardcoded_tmp_directory (removing hardcoded_tmp_directory_extend from Settings), and then concatenating the two in From<Options>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that makes sense.

}

impl From<Options> for Settings {
fn from(options: Options) -> Self {
Self {
hardcoded_tmp_directory: options
.hardcoded_tmp_directory
.unwrap_or_else(default_tmp_dirs)
.into_iter()
.chain(
options
.hardcoded_tmp_directory_extend
.unwrap_or_default()
.into_iter(),
)
.collect(),
}
}
}

impl From<Settings> for Options {
fn from(settings: Settings) -> Self {
Self {
hardcoded_tmp_directory: Some(settings.hardcoded_tmp_directory),
hardcoded_tmp_directory_extend: None,
}
}
}

impl Default for Settings {
fn default() -> Self {
Self {
hardcoded_tmp_directory: default_tmp_dirs(),
}
}
}