Skip to content

Commit

Permalink
feat(rules): implement flake8-bandit S201 (flask_debug_true) (#…
Browse files Browse the repository at this point in the history
…7503)

Part of #1646.

## Summary

Implement `S201`
([`flask_debug_true`](https://bandit.readthedocs.io/en/latest/plugins/b201_flask_debug_true.html))
rule from `bandit`.

I am fairly new to Rust and Ruff's codebase, so there might be better
ways to implement the rule or write the code.

## Test Plan

Snapshot test from
https://github.com/PyCQA/bandit/blob/1.7.5/examples/flask_debug.py, with
a few additions in the "unrelated" part to test a bit more cases.
  • Loading branch information
mkniewallner committed Sep 19, 2023
1 parent 40f6456 commit c6ba7df
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 0 deletions.
22 changes: 22 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_bandit/S201.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from flask import Flask

app = Flask(__name__)

@app.route('/')
def main():
raise

# OK
app.run(debug=True)

# Errors
app.run()
app.run(debug=False)

# Unrelated
run()
run(debug=True)
run(debug)
foo.run(debug=True)
app = 1
app.run(debug=True)
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::LoggingConfigInsecureListen) {
flake8_bandit::rules::logging_config_insecure_listen(checker, call);
}
if checker.enabled(Rule::FlaskDebugTrue) {
flake8_bandit::rules::flask_debug_true(checker, call);
}
if checker.any_enabled(&[
Rule::SubprocessWithoutShellEqualsTrue,
Rule::SubprocessPopenWithShellEqualsTrue,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Bandit, "110") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::TryExceptPass),
(Flake8Bandit, "112") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::TryExceptContinue),
(Flake8Bandit, "113") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::RequestWithoutTimeout),
(Flake8Bandit, "201") => (RuleGroup::Preview, rules::flake8_bandit::rules::FlaskDebugTrue),
(Flake8Bandit, "301") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::SuspiciousPickleUsage),
(Flake8Bandit, "302") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::SuspiciousMarshalUsage),
(Flake8Bandit, "303") => (RuleGroup::Unspecified, rules::flake8_bandit::rules::SuspiciousInsecureHashUsage),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flake8_bandit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ mod tests {
#[test_case(Rule::BadFilePermissions, Path::new("S103.py"))]
#[test_case(Rule::CallWithShellEqualsTrue, Path::new("S604.py"))]
#[test_case(Rule::ExecBuiltin, Path::new("S102.py"))]
#[test_case(Rule::FlaskDebugTrue, Path::new("S201.py"))]
#[test_case(Rule::HardcodedBindAllInterfaces, Path::new("S104.py"))]
#[test_case(Rule::HardcodedPasswordDefault, Path::new("S107.py"))]
#[test_case(Rule::HardcodedPasswordFuncArg, Path::new("S106.py"))]
Expand Down
92 changes: 92 additions & 0 deletions crates/ruff/src/rules/flake8_bandit/rules/flask_debug_true.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_true;
use ruff_python_ast::{Expr, ExprAttribute, ExprCall, Stmt, StmtAssign};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for uses of `debug=True` in Flask.
///
/// ## Why is this bad?
/// Enabling debug mode shows an interactive debugger in the browser if an
/// error occurs, and allows running arbitrary Python code from the browser.
/// This could leak sensitive information, or allow an attacker to run
/// arbitrary code.
///
/// ## Example
/// ```python
/// import flask
///
/// app = Flask()
///
/// app.run(debug=True)
/// ```
///
/// Use instead:
/// ```python
/// import flask
///
/// app = Flask()
///
/// app.run(debug=os.environ["ENV"] == "dev")
/// ```
///
/// ## References
/// - [Flask documentation: Debug Mode](https://flask.palletsprojects.com/en/latest/quickstart/#debug-mode)
#[violation]
pub struct FlaskDebugTrue;

impl Violation for FlaskDebugTrue {
#[derive_message_formats]
fn message(&self) -> String {
format!("Use of `debug=True` in Flask app detected")
}
}

/// S201
pub(crate) fn flask_debug_true(checker: &mut Checker, call: &ExprCall) {
let Expr::Attribute(ExprAttribute { attr, value, .. }) = call.func.as_ref() else {
return;
};

if attr.as_str() != "run" {
return;
}

let Some(debug_argument) = call.arguments.find_keyword("debug") else {
return;
};

if !is_const_true(&debug_argument.value) {
return;
}

let Expr::Name(name) = value.as_ref() else {
return;
};

checker
.semantic()
.resolve_name(name)
.map_or((), |binding_id| {
if let Some(Stmt::Assign(StmtAssign { value, .. })) = checker
.semantic()
.binding(binding_id)
.statement(checker.semantic())
{
if let Expr::Call(ExprCall { func, .. }) = value.as_ref() {
if checker
.semantic()
.resolve_call_path(func)
.is_some_and(|call_path| matches!(call_path.as_slice(), ["flask", "Flask"]))
{
checker
.diagnostics
.push(Diagnostic::new(FlaskDebugTrue, debug_argument.range()));
}
}
}
});
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_bandit/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pub(crate) use assert_used::*;
pub(crate) use bad_file_permissions::*;
pub(crate) use exec_used::*;
pub(crate) use flask_debug_true::*;
pub(crate) use hardcoded_bind_all_interfaces::*;
pub(crate) use hardcoded_password_default::*;
pub(crate) use hardcoded_password_func_arg::*;
Expand All @@ -24,6 +25,7 @@ pub(crate) use unsafe_yaml_load::*;
mod assert_used;
mod bad_file_permissions;
mod exec_used;
mod flask_debug_true;
mod hardcoded_bind_all_interfaces;
mod hardcoded_password_default;
mod hardcoded_password_func_arg;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
source: crates/ruff/src/rules/flake8_bandit/mod.rs
---
S201.py:10:9: S201 Use of `debug=True` in Flask app detected
|
9 | # OK
10 | app.run(debug=True)
| ^^^^^^^^^^ S201
11 |
12 | # Errors
|


3 changes: 3 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c6ba7df

Please sign in to comment.