[flake8-bandit] Move unsafe-markup-use from RUF035 to S704 (#15957)

## Summary

`RUF035` has been backported into bandit as `S704` in this
[PR](https://github.com/PyCQA/bandit/pull/1225)

This moves the rule and its corresponding setting to the `flake8-bandit`
category

## Test Plan

`cargo nextest run`

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
This commit is contained in:
David Salvisberg 2025-03-11 13:19:18 +01:00 committed by Micha Reiser
parent 798fa47c2e
commit c0b1413ecd
26 changed files with 436 additions and 261 deletions

View file

@ -226,6 +226,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 = []
@ -369,8 +371,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 = []

View file

@ -2,17 +2,17 @@ import flask
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)

View file

@ -2,5 +2,5 @@ from markupsafe import Markup
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

View file

@ -4,4 +4,4 @@ from webhelpers.html import literal
# 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

View file

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

View file

@ -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);

View file

@ -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),

View file

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

View file

@ -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)
},

View file

@ -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;

View file

@ -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)
}

View file

@ -14,6 +14,8 @@ 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 {
@ -21,6 +23,8 @@ impl Default for Settings {
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(())

View file

@ -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
|

View file

@ -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
|

View file

@ -0,0 +1,58 @@
---
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
---
S704.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 | flask.Markup("unsafe {}".format(content)) # S704
7 | Markup("safe {}").format(content)
|
S704.py:6:1: S704 Unsafe use of `flask.Markup` detected
|
4 | content = "<script>alert('Hello, world!')</script>"
5 | Markup(f"unsafe {content}") # S704
6 | flask.Markup("unsafe {}".format(content)) # S704
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S704
7 | Markup("safe {}").format(content)
8 | flask.Markup(b"safe {}", encoding='utf-8').format(content)
|
S704.py:10:1: S704 Unsafe use of `markupsafe.Markup` detected
|
8 | flask.Markup(b"safe {}", encoding='utf-8').format(content)
9 | escape(content)
10 | Markup(content) # S704
| ^^^^^^^^^^^^^^^ S704
11 | flask.Markup("unsafe %s" % content) # S704
12 | Markup(object="safe")
|
S704.py:11:1: S704 Unsafe use of `flask.Markup` detected
|
9 | escape(content)
10 | Markup(content) # S704
11 | flask.Markup("unsafe %s" % content) # S704
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S704
12 | Markup(object="safe")
13 | Markup(object="unsafe {}".format(content)) # Not currently detected
|
S704.py:17:1: S704 Unsafe use of `markupsafe.Markup` detected
|
15 | # NOTE: We may be able to get rid of these false positives with red-knot
16 | # if it includes comprehensive constant expression detection/evaluation.
17 | Markup("*" * 8) # S704 (false positive)
| ^^^^^^^^^^^^^^^ S704
18 | flask.Markup("hello {}".format("world")) # S704 (false positive)
|
S704.py:18:1: S704 Unsafe use of `flask.Markup` detected
|
16 | # if it includes comprehensive constant expression detection/evaluation.
17 | Markup("*" * 8) # S704 (false positive)
18 | flask.Markup("hello {}".format("world")) # S704 (false positive)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ S704
|

View file

@ -0,0 +1,10 @@
---
source: crates/ruff_linter/src/rules/flake8_bandit/mod.rs
---
S704_whitelisted_markup_calls.py:9:1: S704 Unsafe use of `markupsafe.Markup` detected
|
7 | # indirect assignments are currently not supported
8 | cleaned = clean(content)
9 | Markup(cleaned) # S704
| ^^^^^^^^^^^^^^^ S704
|

View file

@ -113,8 +113,6 @@ mod tests {
&LinterSettings {
ruff: super::settings::Settings {
parenthesize_tuple_in_subscript: true,
extend_markup_names: vec![],
allowed_markup_calls: vec![],
},
..LinterSettings::for_rule(Rule::IncorrectlyParenthesizedTupleInSubscript)
},
@ -130,8 +128,6 @@ mod tests {
&LinterSettings {
ruff: super::settings::Settings {
parenthesize_tuple_in_subscript: false,
extend_markup_names: vec![],
allowed_markup_calls: vec![],
},
unresolved_target_version: PythonVersion::PY310,
..LinterSettings::for_rule(Rule::IncorrectlyParenthesizedTupleInSubscript)
@ -423,7 +419,6 @@ mod tests {
Ok(())
}
#[test_case(Rule::UnsafeMarkupUse, Path::new("RUF035.py"))]
#[test_case(Rule::MapIntVersionParsing, Path::new("RUF048.py"))]
#[test_case(Rule::MapIntVersionParsing, Path::new("RUF048_1.py"))]
#[test_case(Rule::UnrawRePattern, Path::new("RUF039.py"))]
@ -457,53 +452,6 @@ mod tests {
Ok(())
}
#[test_case(Rule::UnsafeMarkupUse, Path::new("RUF035_extend_markup_names.py"))]
#[test_case(Rule::UnsafeMarkupUse, Path::new("RUF035_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("ruff").join(path).as_path(),
&LinterSettings {
ruff: super::settings::Settings {
parenthesize_tuple_in_subscript: true,
extend_markup_names: vec!["webhelpers.html.literal".to_string()],
allowed_markup_calls: vec![],
},
preview: PreviewMode::Enabled,
..LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
#[test_case(Rule::UnsafeMarkupUse, Path::new("RUF035_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("ruff").join(path).as_path(),
&LinterSettings {
ruff: super::settings::Settings {
parenthesize_tuple_in_subscript: true,
extend_markup_names: vec![],
allowed_markup_calls: vec!["bleach.clean".to_string()],
},
preview: PreviewMode::Enabled,
..LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
#[test_case(Rule::UsedDummyVariable, Path::new("RUF052.py"), r"^_+", 1)]
#[test_case(Rule::UsedDummyVariable, Path::new("RUF052.py"), r"", 2)]
fn custom_regexp_preset(

View file

@ -1,13 +1,10 @@
use ruff_python_ast::{Expr, ExprCall};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_diagnostics::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};
/// ## Removed
/// This rule was implemented in `bandit` and has been remapped to
/// [S704](unsafe-markup-use.md)
///
/// ## What it does
/// Checks for non-literal strings being passed to [`markupsafe.Markup`][markupsafe-markup].
///
@ -18,13 +15,13 @@ use crate::{checkers::ast::Checker, settings::LinterSettings};
///
/// Instead you should interpolate the `Markup` object.
///
/// Using [`lint.ruff.extend-markup-names`] additional objects can be
/// 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.ruff.allowed-markup-calls`] to specify exceptions.
/// You can use [`lint.flake8-bandit.allowed-markup-calls`] to specify exceptions.
///
/// ## Example
/// Given:
@ -65,92 +62,24 @@ use crate::{checkers::ast::Checker, settings::LinterSettings};
/// html = Markup("<br>").join(lines) # Safe
/// ```
/// ## Options
/// - `lint.ruff.extend-markup-names`
/// - `lint.ruff.allowed-markup-calls`
/// - `lint.flake8-bandit.extend-markup-names`
/// - `lint.flake8-bandit.allowed-markup-calls`
///
/// ## References
/// - [MarkupSafe](https://pypi.org/project/MarkupSafe/)
/// - [`markupsafe.Markup`](https://markupsafe.palletsprojects.com/en/stable/escaping/#markupsafe.Markup)
/// - [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 {
pub(crate) struct RuffUnsafeMarkupUse {
name: String,
}
impl Violation for UnsafeMarkupUse {
impl Violation for RuffUnsafeMarkupUse {
#[derive_message_formats]
fn message(&self) -> String {
let UnsafeMarkupUse { name } = self;
let RuffUnsafeMarkupUse { name } = self;
format!("Unsafe use of `{name}` detected")
}
}
/// RUF035
pub(crate) fn unsafe_markup_call(checker: &Checker, call: &ExprCall) {
if checker.settings.ruff.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
.ruff
.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
.ruff
.allowed_markup_calls
.iter()
.map(|target| QualifiedName::from_dotted_name(target))
.any(|target| qualified_name == target)
}

View file

@ -7,8 +7,6 @@ use std::fmt;
#[derive(Debug, Clone, CacheKey, Default)]
pub struct Settings {
pub parenthesize_tuple_in_subscript: bool,
pub extend_markup_names: Vec<String>,
pub allowed_markup_calls: Vec<String>,
}
impl fmt::Display for Settings {
@ -18,8 +16,6 @@ impl fmt::Display for Settings {
namespace = "linter.ruff",
fields = [
self.parenthesize_tuple_in_subscript,
self.extend_markup_names | array,
self.allowed_markup_calls | array,
]
}
Ok(())

View file

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

View file

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

View file

@ -1,59 +0,0 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
snapshot_kind: text
---
RUF035.py:5:1: RUF035 Unsafe use of `markupsafe.Markup` detected
|
4 | content = "<script>alert('Hello, world!')</script>"
5 | Markup(f"unsafe {content}") # RUF035
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF035
6 | flask.Markup("unsafe {}".format(content)) # RUF035
7 | Markup("safe {}").format(content)
|
RUF035.py:6:1: RUF035 Unsafe use of `flask.Markup` detected
|
4 | content = "<script>alert('Hello, world!')</script>"
5 | Markup(f"unsafe {content}") # RUF035
6 | flask.Markup("unsafe {}".format(content)) # RUF035
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF035
7 | Markup("safe {}").format(content)
8 | flask.Markup(b"safe {}", encoding='utf-8').format(content)
|
RUF035.py:10:1: RUF035 Unsafe use of `markupsafe.Markup` detected
|
8 | flask.Markup(b"safe {}", encoding='utf-8').format(content)
9 | escape(content)
10 | Markup(content) # RUF035
| ^^^^^^^^^^^^^^^ RUF035
11 | flask.Markup("unsafe %s" % content) # RUF035
12 | Markup(object="safe")
|
RUF035.py:11:1: RUF035 Unsafe use of `flask.Markup` detected
|
9 | escape(content)
10 | Markup(content) # RUF035
11 | flask.Markup("unsafe %s" % content) # RUF035
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF035
12 | Markup(object="safe")
13 | Markup(object="unsafe {}".format(content)) # Not currently detected
|
RUF035.py:17:1: RUF035 Unsafe use of `markupsafe.Markup` detected
|
15 | # NOTE: We may be able to get rid of these false positives with red-knot
16 | # if it includes comprehensive constant expression detection/evaluation.
17 | Markup("*" * 8) # RUF035 (false positive)
| ^^^^^^^^^^^^^^^ RUF035
18 | flask.Markup("hello {}".format("world")) # RUF035 (false positive)
|
RUF035.py:18:1: RUF035 Unsafe use of `flask.Markup` detected
|
16 | # if it includes comprehensive constant expression detection/evaluation.
17 | Markup("*" * 8) # RUF035 (false positive)
18 | flask.Markup("hello {}".format("world")) # RUF035 (false positive)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF035
|

View file

@ -1,10 +0,0 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
RUF035_whitelisted_markup_calls.py:9:1: RUF035 Unsafe use of `markupsafe.Markup` detected
|
7 | # indirect assignments are currently not supported
8 | cleaned = clean(content)
9 | Markup(cleaned) # RUF035
| ^^^^^^^^^^^^^^^ RUF035
|

View file

@ -334,7 +334,7 @@ impl Configuration {
.unwrap_or_default(),
flake8_bandit: lint
.flake8_bandit
.map(Flake8BanditOptions::into_settings)
.map(|flake8_bandit| flake8_bandit.into_settings(lint.ruff.as_ref()))
.unwrap_or_default(),
flake8_boolean_trap: lint
.flake8_boolean_trap

View file

@ -1070,10 +1070,57 @@ pub struct Flake8BanditOptions {
example = "check-typed-exception = true"
)]
pub check_typed_exception: Option<bool>,
/// A list of additional callable names that behave like
/// [`markupsafe.Markup`](https://markupsafe.palletsprojects.com/en/stable/escaping/#markupsafe.Markup).
///
/// Expects to receive a list of fully-qualified names (e.g., `webhelpers.html.literal`, rather than
/// `literal`).
#[option(
default = "[]",
value_type = "list[str]",
example = "extend-markup-names = [\"webhelpers.html.literal\", \"my_package.Markup\"]"
)]
pub extend_markup_names: Option<Vec<String>>,
/// A list of callable names, whose result may be safely passed into
/// [`markupsafe.Markup`](https://markupsafe.palletsprojects.com/en/stable/escaping/#markupsafe.Markup).
///
/// Expects to receive a list of fully-qualified names (e.g., `bleach.clean`, rather than `clean`).
///
/// This setting helps you avoid false positives in code like:
///
/// ```python
/// from bleach import clean
/// from markupsafe import Markup
///
/// cleaned_markup = Markup(clean(some_user_input))
/// ```
///
/// Where the use of [`bleach.clean`](https://bleach.readthedocs.io/en/latest/clean.html)
/// usually ensures that there's no XSS vulnerability.
///
/// Although it is not recommended, you may also use this setting to whitelist other
/// kinds of calls, e.g. calls to i18n translation functions, where how safe that is
/// will depend on the implementation and how well the translations are audited.
///
/// Another common use-case is to wrap the output of functions that generate markup
/// like [`xml.etree.ElementTree.tostring`](https://docs.python.org/3/library/xml.etree.elementtree.html#xml.etree.ElementTree.tostring)
/// or template rendering engines where sanitization of potential user input is either
/// already baked in or has to happen before rendering.
#[option(
default = "[]",
value_type = "list[str]",
example = "allowed-markup-calls = [\"bleach.clean\", \"my_package.sanitize\"]"
)]
pub allowed_markup_calls: Option<Vec<String>>,
}
impl Flake8BanditOptions {
pub fn into_settings(self) -> ruff_linter::rules::flake8_bandit::settings::Settings {
pub fn into_settings(
self,
ruff_options: Option<&RuffOptions>,
) -> ruff_linter::rules::flake8_bandit::settings::Settings {
ruff_linter::rules::flake8_bandit::settings::Settings {
hardcoded_tmp_directory: self
.hardcoded_tmp_directory
@ -1082,6 +1129,20 @@ impl Flake8BanditOptions {
.chain(self.hardcoded_tmp_directory_extend.unwrap_or_default())
.collect(),
check_typed_exception: self.check_typed_exception.unwrap_or(false),
extend_markup_names: self
.extend_markup_names
.or_else(|| {
#[allow(deprecated)]
ruff_options.and_then(|options| options.extend_markup_names.clone())
})
.unwrap_or_default(),
allowed_markup_calls: self
.allowed_markup_calls
.or_else(|| {
#[allow(deprecated)]
ruff_options.and_then(|options| options.allowed_markup_calls.clone())
})
.unwrap_or_default(),
}
}
}
@ -3279,6 +3340,10 @@ pub struct RuffOptions {
value_type = "list[str]",
example = "extend-markup-names = [\"webhelpers.html.literal\", \"my_package.Markup\"]"
)]
#[deprecated(
since = "0.10.0",
note = "The `extend-markup-names` option has been moved to the `flake8-bandit` section of the configuration."
)]
pub extend_markup_names: Option<Vec<String>>,
/// A list of callable names, whose result may be safely passed into
@ -3311,6 +3376,10 @@ pub struct RuffOptions {
value_type = "list[str]",
example = "allowed-markup-calls = [\"bleach.clean\", \"my_package.sanitize\"]"
)]
#[deprecated(
since = "0.10.0",
note = "The `allowed-markup-names` option has been moved to the `flake8-bandit` section of the configuration."
)]
pub allowed_markup_calls: Option<Vec<String>>,
}
@ -3320,8 +3389,6 @@ impl RuffOptions {
parenthesize_tuple_in_subscript: self
.parenthesize_tuple_in_subscript
.unwrap_or_default(),
extend_markup_names: self.extend_markup_names.unwrap_or_default(),
allowed_markup_calls: self.allowed_markup_calls.unwrap_or_default(),
}
}
}

24
ruff.schema.json generated
View file

@ -947,6 +947,16 @@
"description": "Options for the `flake8-bandit` plugin.",
"type": "object",
"properties": {
"allowed-markup-calls": {
"description": "A list of callable names, whose result may be safely passed into [`markupsafe.Markup`](https://markupsafe.palletsprojects.com/en/stable/escaping/#markupsafe.Markup).\n\nExpects to receive a list of fully-qualified names (e.g., `bleach.clean`, rather than `clean`).\n\nThis setting helps you avoid false positives in code like:\n\n```python from bleach import clean from markupsafe import Markup\n\ncleaned_markup = Markup(clean(some_user_input)) ```\n\nWhere the use of [`bleach.clean`](https://bleach.readthedocs.io/en/latest/clean.html) usually ensures that there's no XSS vulnerability.\n\nAlthough it is not recommended, you may also use this setting to whitelist other kinds of calls, e.g. calls to i18n translation functions, where how safe that is will depend on the implementation and how well the translations are audited.\n\nAnother common use-case is to wrap the output of functions that generate markup like [`xml.etree.ElementTree.tostring`](https://docs.python.org/3/library/xml.etree.elementtree.html#xml.etree.ElementTree.tostring) or template rendering engines where sanitization of potential user input is either already baked in or has to happen before rendering.",
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
},
"check-typed-exception": {
"description": "Whether to disallow `try`-`except`-`pass` (`S110`) for specific exception types. By default, `try`-`except`-`pass` is only disallowed for `Exception` and `BaseException`.",
"type": [
@ -954,6 +964,16 @@
"null"
]
},
"extend-markup-names": {
"description": "A list of additional callable names that behave like [`markupsafe.Markup`](https://markupsafe.palletsprojects.com/en/stable/escaping/#markupsafe.Markup).\n\nExpects to receive a list of fully-qualified names (e.g., `webhelpers.html.literal`, rather than `literal`).",
"type": [
"array",
"null"
],
"items": {
"type": "string"
}
},
"hardcoded-tmp-directory": {
"description": "A list of directories to consider temporary (see `S108`).",
"type": [
@ -2847,6 +2867,7 @@
"properties": {
"allowed-markup-calls": {
"description": "A list of callable names, whose result may be safely passed into [`markupsafe.Markup`](https://markupsafe.palletsprojects.com/en/stable/escaping/#markupsafe.Markup).\n\nExpects to receive a list of fully-qualified names (e.g., `bleach.clean`, rather than `clean`).\n\nThis setting helps you avoid false positives in code like:\n\n```python from bleach import clean from markupsafe import Markup\n\ncleaned_markup = Markup(clean(some_user_input)) ```\n\nWhere the use of [`bleach.clean`](https://bleach.readthedocs.io/en/latest/clean.html) usually ensures that there's no XSS vulnerability.\n\nAlthough it is not recommended, you may also use this setting to whitelist other kinds of calls, e.g. calls to i18n translation functions, where how safe that is will depend on the implementation and how well the translations are audited.\n\nAnother common use-case is to wrap the output of functions that generate markup like [`xml.etree.ElementTree.tostring`](https://docs.python.org/3/library/xml.etree.elementtree.html#xml.etree.ElementTree.tostring) or template rendering engines where sanitization of potential user input is either already baked in or has to happen before rendering.",
"deprecated": true,
"type": [
"array",
"null"
@ -2857,6 +2878,7 @@
},
"extend-markup-names": {
"description": "A list of additional callable names that behave like [`markupsafe.Markup`](https://markupsafe.palletsprojects.com/en/stable/escaping/#markupsafe.Markup).\n\nExpects to receive a list of fully-qualified names (e.g., `webhelpers.html.literal`, rather than `literal`).",
"deprecated": true,
"type": [
"array",
"null"
@ -3949,7 +3971,6 @@
"RUF032",
"RUF033",
"RUF034",
"RUF035",
"RUF036",
"RUF037",
"RUF038",
@ -4071,6 +4092,7 @@
"S70",
"S701",
"S702",
"S704",
"SIM",
"SIM1",
"SIM10",