[flake8-bandit] Move unsafe-markup-use from RUF035 to S704

crates/ruff/tests/snapshots/show_settings__display_default_settings.snap

@@ -225,6 +225,8 @@ linter.flake8_bandit.hardcoded_tmp_directory = [
 	/dev/shm,
 ]
 linter.flake8_bandit.check_typed_exception = false
+linter.flake8_bandit.extend_markup_names = []
+linter.flake8_bandit.allowed_markup_calls = []
 linter.flake8_bugbear.extend_immutable_calls = []
 linter.flake8_builtins.builtins_allowed_modules = []
 linter.flake8_builtins.builtins_ignorelist = []
@@ -368,8 +370,6 @@ linter.pylint.max_public_methods = 20
 linter.pylint.max_locals = 15
 linter.pyupgrade.keep_runtime_typing = false
 linter.ruff.parenthesize_tuple_in_subscript = false
-linter.ruff.extend_markup_names = []
-linter.ruff.allowed_markup_calls = []
 
 # Formatter Settings
 formatter.exclude = []

crates/ruff_linter/resources/test/fixtures/ruff/RUF035.py → crates/ruff_linter/resources/test/fixtures/flake8_bandit/S704.py

@@ -2,17 +2,17 @@
 from markupsafe import Markup, escape
 
 content = "<script>alert('Hello, world!')</script>"
-Markup(f"unsafe {content}")  # RUF035
-flask.Markup("unsafe {}".format(content))  # RUF035
+Markup(f"unsafe {content}")  # S704
+flask.Markup("unsafe {}".format(content))  # S704
 Markup("safe {}").format(content)
 flask.Markup(b"safe {}", encoding='utf-8').format(content)
 escape(content)
-Markup(content)  # RUF035
-flask.Markup("unsafe %s" % content)  # RUF035
+Markup(content)  # S704
+flask.Markup("unsafe %s" % content)  # S704
 Markup(object="safe")
 Markup(object="unsafe {}".format(content))  # Not currently detected
 
 # NOTE: We may be able to get rid of these false positives with red-knot
 #       if it includes comprehensive constant expression detection/evaluation.
-Markup("*" * 8)  # RUF035 (false positive)
-flask.Markup("hello {}".format("world"))  # RUF035 (false positive)
+Markup("*" * 8)  # S704 (false positive)
+flask.Markup("hello {}".format("world"))  # S704 (false positive)

crates/ruff_linter/resources/test/fixtures/ruff/RUF035_extend_markup_names.py → crates/ruff_linter/resources/test/fixtures/flake8_bandit/S704_extend_markup_names.py

@@ -2,5 +2,5 @@
 from webhelpers.html import literal
 
 content = "<script>alert('Hello, world!')</script>"
-Markup(f"unsafe {content}")  # RUF035
-literal(f"unsafe {content}")  # RUF035
+Markup(f"unsafe {content}")  # S704
+literal(f"unsafe {content}")  # S704

crates/ruff_linter/resources/test/fixtures/ruff/RUF035_skip_early_out.py → crates/ruff_linter/resources/test/fixtures/flake8_bandit/S704_skip_early_out.py

@@ -4,4 +4,4 @@
 #       additional markup names to be skipped if we don't import either
 #       markupsafe or flask first.
 content = "<script>alert('Hello, world!')</script>"
-literal(f"unsafe {content}")  # RUF035
+literal(f"unsafe {content}")  # S704

crates/ruff_linter/resources/test/fixtures/ruff/RUF035_whitelisted_markup_calls.py → crates/ruff_linter/resources/test/fixtures/flake8_bandit/S704_whitelisted_markup_calls.py

@@ -6,4 +6,4 @@
 
 # indirect assignments are currently not supported
 cleaned = clean(content)
-Markup(cleaned)  # RUF035
+Markup(cleaned)  # S704

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -1129,7 +1129,7 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
                 refurb::rules::int_on_sliced_str(checker, call);
             }
             if checker.enabled(Rule::UnsafeMarkupUse) {
-                ruff::rules::unsafe_markup_call(checker, call);
+                flake8_bandit::rules::unsafe_markup_call(checker, call);
             }
             if checker.enabled(Rule::MapIntVersionParsing) {
                 ruff::rules::map_int_version_parsing(checker, call);

crates/ruff_linter/src/codes.rs

@@ -690,6 +690,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8Bandit, "612") => (RuleGroup::Stable, rules::flake8_bandit::rules::LoggingConfigInsecureListen),
         (Flake8Bandit, "701") => (RuleGroup::Stable, rules::flake8_bandit::rules::Jinja2AutoescapeFalse),
         (Flake8Bandit, "702") => (RuleGroup::Stable, rules::flake8_bandit::rules::MakoTemplates),
+        (Flake8Bandit, "704") => (RuleGroup::Preview, rules::flake8_bandit::rules::UnsafeMarkupUse),
 
         // flake8-boolean-trap
         (Flake8BooleanTrap, "001") => (RuleGroup::Stable, rules::flake8_boolean_trap::rules::BooleanTypeHintPositionalArgument),
@@ -991,7 +992,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "032") => (RuleGroup::Stable, rules::ruff::rules::DecimalFromFloatLiteral),
         (Ruff, "033") => (RuleGroup::Stable, rules::ruff::rules::PostInitDefault),
         (Ruff, "034") => (RuleGroup::Stable, rules::ruff::rules::UselessIfElse),
-        (Ruff, "035") => (RuleGroup::Preview, rules::ruff::rules::UnsafeMarkupUse),
+        (Ruff, "035") => (RuleGroup::Removed, rules::ruff::rules::RuffUnsafeMarkupUse),
         (Ruff, "036") => (RuleGroup::Preview, rules::ruff::rules::NoneNotAtEndOfUnion),
         (Ruff, "037") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryEmptyIterableWithinDequeCall),
         (Ruff, "038") => (RuleGroup::Preview, rules::ruff::rules::RedundantBoolLiteral),

crates/ruff_linter/src/rule_redirects.rs

@@ -134,6 +134,7 @@ static REDIRECTS: LazyLock<HashMap<&'static str, &'static str>> = LazyLock::new(
         ("TCH005", "TC005"),
         ("TCH006", "TC010"),
         ("TCH010", "TC010"),
+        ("RUF035", "S704"),
     ])
 });
 

crates/ruff_linter/src/rules/flake8_bandit/mod.rs

@@ -103,6 +103,7 @@ mod tests {
     #[test_case(Rule::SuspiciousURLOpenUsage, Path::new("S310.py"))]
     #[test_case(Rule::SuspiciousNonCryptographicRandomUsage, Path::new("S311.py"))]
     #[test_case(Rule::SuspiciousTelnetUsage, Path::new("S312.py"))]
+    #[test_case(Rule::UnsafeMarkupUse, Path::new("S704.py"))]
     fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!(
             "preview__{}_{}",
@@ -120,6 +121,51 @@ mod tests {
         Ok(())
     }
 
+    #[test_case(Rule::UnsafeMarkupUse, Path::new("S704_extend_markup_names.py"))]
+    #[test_case(Rule::UnsafeMarkupUse, Path::new("S704_skip_early_out.py"))]
+    fn extend_allowed_callable(rule_code: Rule, path: &Path) -> Result<()> {
+        let snapshot = format!(
+            "extend_allow_callables__{}_{}",
+            rule_code.noqa_code(),
+            path.to_string_lossy()
+        );
+        let diagnostics = test_path(
+            Path::new("flake8_bandit").join(path).as_path(),
+            &LinterSettings {
+                flake8_bandit: super::settings::Settings {
+                    extend_markup_names: vec!["webhelpers.html.literal".to_string()],
+                    ..Default::default()
+                },
+                preview: PreviewMode::Enabled,
+                ..LinterSettings::for_rule(rule_code)
+            },
+        )?;
+        assert_messages!(snapshot, diagnostics);
+        Ok(())
+    }
+
+    #[test_case(Rule::UnsafeMarkupUse, Path::new("S704_whitelisted_markup_calls.py"))]
+    fn whitelisted_markup_calls(rule_code: Rule, path: &Path) -> Result<()> {
+        let snapshot = format!(
+            "whitelisted_markup_calls__{}_{}",
+            rule_code.noqa_code(),
+            path.to_string_lossy()
+        );
+        let diagnostics = test_path(
+            Path::new("flake8_bandit").join(path).as_path(),
+            &LinterSettings {
+                flake8_bandit: super::settings::Settings {
+                    allowed_markup_calls: vec!["bleach.clean".to_string()],
+                    ..Default::default()
+                },
+                preview: PreviewMode::Enabled,
+                ..LinterSettings::for_rule(rule_code)
+            },
+        )?;
+        assert_messages!(snapshot, diagnostics);
+        Ok(())
+    }
+
     #[test]
     fn check_hardcoded_tmp_additional_dirs() -> Result<()> {
         let diagnostics = test_path(
@@ -132,7 +178,7 @@ mod tests {
                         "/dev/shm".to_string(),
                         "/foo".to_string(),
                     ],
-                    check_typed_exception: false,
+                    ..Default::default()
                 },
                 ..LinterSettings::for_rule(Rule::HardcodedTempFile)
             },

crates/ruff_linter/src/rules/flake8_bandit/rules/mod.rs

@@ -29,6 +29,7 @@ pub(crate) use suspicious_imports::*;
 pub(crate) use tarfile_unsafe_members::*;
 pub(crate) use try_except_continue::*;
 pub(crate) use try_except_pass::*;
+pub(crate) use unsafe_markup_use::*;
 pub(crate) use unsafe_yaml_load::*;
 pub(crate) use weak_cryptographic_key::*;
 
@@ -63,5 +64,6 @@ mod suspicious_imports;
 mod tarfile_unsafe_members;
 mod try_except_continue;
 mod try_except_pass;
+mod unsafe_markup_use;
 mod unsafe_yaml_load;
 mod weak_cryptographic_key;

crates/ruff_linter/src/rules/flake8_bandit/rules/unsafe_markup_use.rs

@@ -0,0 +1,160 @@
+use ruff_python_ast::{Expr, ExprCall};
+
+use ruff_diagnostics::{Diagnostic, Violation};
+use ruff_macros::{derive_message_formats, ViolationMetadata};
+use ruff_python_ast::name::QualifiedName;
+use ruff_python_semantic::{Modules, SemanticModel};
+use ruff_text_size::Ranged;
+
+use crate::{checkers::ast::Checker, settings::LinterSettings};
+
+/// ## What it does
+/// Checks for non-literal strings being passed to [`markupsafe.Markup`][markupsafe-markup].
+///
+/// ## Why is this bad?
+/// [`markupsafe.Markup`] does not perform any escaping, so passing dynamic
+/// content, like f-strings, variables or interpolated strings will potentially
+/// lead to XSS vulnerabilities.
+///
+/// Instead you should interpolate the `Markup` object.
+///
+/// Using [`lint.flake8-bandit.extend-markup-names`] additional objects can be
+/// treated like `Markup`.
+///
+/// This rule was originally inspired by [flake8-markupsafe] but doesn't carve
+/// out any exceptions for i18n related calls by default.
+///
+/// You can use [`lint.flake8-bandit.allowed-markup-calls`] to specify exceptions.
+///
+/// ## Example
+/// Given:
+/// ```python
+/// from markupsafe import Markup
+///
+/// content = "<script>alert('Hello, world!')</script>"
+/// html = Markup(f"<b>{content}</b>")  # XSS
+/// ```
+///
+/// Use instead:
+/// ```python
+/// from markupsafe import Markup
+///
+/// content = "<script>alert('Hello, world!')</script>"
+/// html = Markup("<b>{}</b>").format(content)  # Safe
+/// ```
+///
+/// Given:
+/// ```python
+/// from markupsafe import Markup
+///
+/// lines = [
+///     Markup("<b>heading</b>"),
+///     "<script>alert('XSS attempt')</script>",
+/// ]
+/// html = Markup("<br>".join(lines))  # XSS
+/// ```
+///
+/// Use instead:
+/// ```python
+/// from markupsafe import Markup
+///
+/// lines = [
+///     Markup("<b>heading</b>"),
+///     "<script>alert('XSS attempt')</script>",
+/// ]
+/// html = Markup("<br>").join(lines)  # Safe
+/// ```
+/// ## Options
+/// - `lint.flake8-bandit.extend-markup-names`
+/// - `lint.flake8-bandit.allowed-markup-calls`
+///
+/// ## References
+/// - [MarkupSafe on PyPI](https://pypi.org/project/MarkupSafe/)
+/// - [`markupsafe.Markup` API documentation](https://markupsafe.palletsprojects.com/en/stable/escaping/#markupsafe.Markup)
+///
+/// [markupsafe-markup]: https://markupsafe.palletsprojects.com/en/stable/escaping/#markupsafe.Markup
+/// [flake8-markupsafe]: https://github.com/vmagamedov/flake8-markupsafe
+#[derive(ViolationMetadata)]
+pub(crate) struct UnsafeMarkupUse {
+    name: String,
+}
+
+impl Violation for UnsafeMarkupUse {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let UnsafeMarkupUse { name } = self;
+        format!("Unsafe use of `{name}` detected")
+    }
+}
+
+/// S704
+pub(crate) fn unsafe_markup_call(checker: &Checker, call: &ExprCall) {
+    if checker
+        .settings
+        .flake8_bandit
+        .extend_markup_names
+        .is_empty()
+        && !(checker.semantic().seen_module(Modules::MARKUPSAFE)
+            || checker.semantic().seen_module(Modules::FLASK))
+    {
+        return;
+    }
+
+    if !is_unsafe_call(call, checker.semantic(), checker.settings) {
+        return;
+    }
+
+    let Some(qualified_name) = checker.semantic().resolve_qualified_name(&call.func) else {
+        return;
+    };
+
+    if !is_markup_call(&qualified_name, checker.settings) {
+        return;
+    }
+
+    checker.report_diagnostic(Diagnostic::new(
+        UnsafeMarkupUse {
+            name: qualified_name.to_string(),
+        },
+        call.range(),
+    ));
+}
+
+fn is_markup_call(qualified_name: &QualifiedName, settings: &LinterSettings) -> bool {
+    matches!(
+        qualified_name.segments(),
+        ["markupsafe" | "flask", "Markup"]
+    ) || settings
+        .flake8_bandit
+        .extend_markup_names
+        .iter()
+        .map(|target| QualifiedName::from_dotted_name(target))
+        .any(|target| *qualified_name == target)
+}
+
+fn is_unsafe_call(call: &ExprCall, semantic: &SemanticModel, settings: &LinterSettings) -> bool {
+    // technically this could be circumvented by using a keyword argument
+    // but without type-inference we can't really know which keyword argument
+    // corresponds to the first positional argument and either way it is
+    // unlikely that someone will actually use a keyword argument here
+    // TODO: Eventually we may want to allow dynamic values, as long as they
+    //       have a __html__ attribute, since that is part of the API
+    matches!(&*call.arguments.args, [first] if !first.is_string_literal_expr() && !first.is_bytes_literal_expr() && !is_whitelisted_call(first, semantic, settings))
+}
+
+fn is_whitelisted_call(expr: &Expr, semantic: &SemanticModel, settings: &LinterSettings) -> bool {
+    let Expr::Call(ExprCall { func, .. }) = expr else {
+        return false;
+    };
+
+    let Some(qualified_name) = semantic.resolve_qualified_name(func) else {
+        return false;
+    };
+
+    settings
+        .flake8_bandit
+        .allowed_markup_calls
+        .iter()
+        .map(|target| QualifiedName::from_dotted_name(target))
+        .any(|target| qualified_name == target)
+}

crates/ruff_linter/src/rules/flake8_bandit/settings.rs

@@ -14,13 +14,17 @@ pub fn default_tmp_dirs() -> Vec<String> {
 pub struct Settings {
     pub hardcoded_tmp_directory: Vec<String>,
     pub check_typed_exception: bool,
+    pub extend_markup_names: Vec<String>,
+    pub allowed_markup_calls: Vec<String>,
 }
 
 impl Default for Settings {
     fn default() -> Self {
         Self {
             hardcoded_tmp_directory: default_tmp_dirs(),
             check_typed_exception: false,
+            extend_markup_names: vec![],
+            allowed_markup_calls: vec![],
         }
     }
 }
@@ -32,7 +36,9 @@ impl Display for Settings {
             namespace = "linter.flake8_bandit",
             fields = [
                 self.hardcoded_tmp_directory | array,
-                self.check_typed_exception
+                self.check_typed_exception,
+                self.extend_markup_names | array,
+                self.allowed_markup_calls | array,
             ]
         }
         Ok(())

crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__extend_allow_callables__S704_S704_extend_markup_names.py.snap

@@ -0,0 +1,18 @@
+---
+source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
+---
+S704_extend_markup_names.py:5:1: S704 Unsafe use of `markupsafe.Markup` detected
+  |
+4 | content = "<script>alert('Hello, world!')</script>"
+5 | Markup(f"unsafe {content}")  # S704
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ S704
+6 | literal(f"unsafe {content}")  # S704
+  |
+
+S704_extend_markup_names.py:6:1: S704 Unsafe use of `webhelpers.html.literal` detected
+  |
+4 | content = "<script>alert('Hello, world!')</script>"
+5 | Markup(f"unsafe {content}")  # S704
+6 | literal(f"unsafe {content}")  # S704
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S704
+  |

crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__extend_allow_callables__S704_S704_skip_early_out.py.snap

@@ -0,0 +1,10 @@
+---
+source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
+---
+S704_skip_early_out.py:7:1: S704 Unsafe use of `webhelpers.html.literal` detected
+  |
+5 | #       markupsafe or flask first.
+6 | content = "<script>alert('Hello, world!')</script>"
+7 | literal(f"unsafe {content}")  # S704
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S704
+  |