Add lint rule for calling chmod with non-octal integers (#18541)

Co-authored-by: Micha Reiser <micha@reiser.io>
This commit is contained in:
Frazer McLean 2025-06-19 11:30:29 +02:00 committed by GitHub
parent dcf0a8d4d7
commit f67ff33177
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 621 additions and 0 deletions

View file

@ -0,0 +1,53 @@
import dbm.gnu
import dbm.ndbm
import os
from pathlib import Path
os.chmod("foo", 444) # Error
os.chmod("foo", 0o444) # OK
os.chmod("foo", 7777) # Error
os.chmod("foo", 10000) # Error
os.chmod("foo", 99999) # Error
os.umask(777) # Error
os.umask(0o777) # OK
os.fchmod(0, 400) # Error
os.fchmod(0, 0o400) # OK
os.lchmod("foo", 755) # Error
os.lchmod("foo", 0o755) # OK
os.mkdir("foo", 600) # Error
os.mkdir("foo", 0o600) # OK
os.makedirs("foo", 644) # Error
os.makedirs("foo", 0o644) # OK
os.mkfifo("foo", 640) # Error
os.mkfifo("foo", 0o640) # OK
os.mknod("foo", 660) # Error
os.mknod("foo", 0o660) # OK
os.open("foo", os.O_CREAT, 644) # Error
os.open("foo", os.O_CREAT, 0o644) # OK
Path("bar").chmod(755) # Error
Path("bar").chmod(0o755) # OK
path = Path("bar")
path.chmod(755) # Error
path.chmod(0o755) # OK
dbm.open("db", "r", 600) # Error
dbm.open("db", "r", 0o600) # OK
dbm.gnu.open("db", "r", 600) # Error
dbm.gnu.open("db", "r", 0o600) # OK
dbm.ndbm.open("db", "r", 600) # Error
dbm.ndbm.open("db", "r", 0o600) # OK
os.fchmod(0, 256) # 0o400
os.fchmod(0, 493) # 0o755

View file

@ -1205,6 +1205,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
if checker.enabled(Rule::FromisoformatReplaceZ) {
refurb::rules::fromisoformat_replace_z(checker, call);
}
if checker.enabled(Rule::NonOctalPermissions) {
ruff::rules::non_octal_permissions(checker, call);
}
}
Expr::Dict(dict) => {
if checker.any_enabled(&[

View file

@ -1028,6 +1028,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "059") => (RuleGroup::Preview, rules::ruff::rules::UnusedUnpackedVariable),
(Ruff, "060") => (RuleGroup::Preview, rules::ruff::rules::InEmptyCollection),
(Ruff, "061") => (RuleGroup::Preview, rules::ruff::rules::LegacyFormPytestRaises),
(Ruff, "064") => (RuleGroup::Preview, rules::ruff::rules::NonOctalPermissions),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),
(Ruff, "102") => (RuleGroup::Preview, rules::ruff::rules::InvalidRuleCode),

View file

@ -109,6 +109,7 @@ mod tests {
#[test_case(Rule::LegacyFormPytestRaises, Path::new("RUF061_raises.py"))]
#[test_case(Rule::LegacyFormPytestRaises, Path::new("RUF061_warns.py"))]
#[test_case(Rule::LegacyFormPytestRaises, Path::new("RUF061_deprecated_call.py"))]
#[test_case(Rule::NonOctalPermissions, Path::new("RUF064.py"))]
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101_0.py"))]
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101_1.py"))]
#[test_case(Rule::InvalidRuleCode, Path::new("RUF102.py"))]

View file

@ -29,6 +29,7 @@ pub(crate) use mutable_dataclass_default::*;
pub(crate) use mutable_fromkeys_value::*;
pub(crate) use needless_else::*;
pub(crate) use never_union::*;
pub(crate) use non_octal_permissions::*;
pub(crate) use none_not_at_end_of_union::*;
pub(crate) use parenthesize_chained_operators::*;
pub(crate) use post_init_default::*;
@ -91,6 +92,7 @@ mod mutable_dataclass_default;
mod mutable_fromkeys_value;
mod needless_else;
mod never_union;
mod non_octal_permissions;
mod none_not_at_end_of_union;
mod parenthesize_chained_operators;
mod post_init_default;

View file

@ -0,0 +1,213 @@
use ruff_diagnostics::{Edit, Fix};
use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_ast::name::QualifiedName;
use ruff_python_ast::{self as ast, Expr, ExprCall};
use ruff_python_semantic::{SemanticModel, analyze};
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
use crate::{FixAvailability, Violation};
/// ## What it does
/// Checks for standard library functions which take a numeric `mode` argument
/// where a non-octal integer literal is passed.
///
/// ## Why is this bad?
///
/// Numeric modes are made up of one to four octal digits. Converting a non-octal
/// integer to octal may not be the mode the author intended.
///
/// ## Example
///
/// ```python
/// os.chmod("foo", 644)
/// ```
///
/// Use instead:
///
/// ```python
/// os.chmod("foo", 0o644)
/// ```
///
/// ## Fix safety
///
/// There are two categories of fix, the first of which is where it looks like
/// the author intended to use an octal literal but the `0o` prefix is missing:
///
/// ```python
/// os.chmod("foo", 400)
/// os.chmod("foo", 644)
/// ```
///
/// This class of fix changes runtime behaviour. In the first case, `400`
/// corresponds to `0o620` (`u=rw,g=w,o=`). As this mode is not deemed likely,
/// it is changed to `0o400` (`u=r,go=`). Similarly, `644` corresponds to
/// `0o1204` (`u=ws,g=,o=r`) and is changed to `0o644` (`u=rw,go=r`).
///
/// The second category is decimal literals which are recognised as likely valid
/// but in decimal form:
///
/// ```python
/// os.chmod("foo", 256)
/// os.chmod("foo", 493)
/// ```
///
/// `256` corresponds to `0o400` (`u=r,go=`) and `493` corresponds to `0o755`
/// (`u=rwx,go=rx`). Both of these fixes keep runtime behavior unchanged. If the
/// original code really intended to use `0o256` (`u=w,g=rx,o=rw`) instead of
/// `256`, this fix should not be accepted.
///
/// ## Fix availability
///
/// A fix is only available if the integer literal matches a set of common modes.
#[derive(ViolationMetadata)]
pub(crate) struct NonOctalPermissions;
impl Violation for NonOctalPermissions {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
#[derive_message_formats]
fn message(&self) -> String {
"Non-octal mode".to_string()
}
fn fix_title(&self) -> Option<String> {
Some("Replace with octal literal".to_string())
}
}
/// RUF064
pub(crate) fn non_octal_permissions(checker: &Checker, call: &ExprCall) {
let mode_arg = find_func_mode_arg(call, checker.semantic())
.or_else(|| find_method_mode_arg(call, checker.semantic()));
let Some(mode_arg) = mode_arg else {
return;
};
let Expr::NumberLiteral(ast::ExprNumberLiteral {
value: ast::Number::Int(int),
..
}) = mode_arg
else {
return;
};
let mode_literal = &checker.locator().contents()[mode_arg.range()];
if mode_literal.starts_with("0o") || mode_literal.starts_with("0O") || mode_literal == "0" {
return;
}
let mut diagnostic = checker.report_diagnostic(NonOctalPermissions, mode_arg.range());
// Don't suggest a fix for 0x or 0b literals.
if mode_literal.starts_with('0') {
return;
}
let Some(suggested) = int.as_u16().and_then(suggest_fix) else {
return;
};
let edit = Edit::range_replacement(format!("{suggested:#o}"), mode_arg.range());
diagnostic.set_fix(Fix::unsafe_edit(edit));
}
fn find_func_mode_arg<'a>(call: &'a ExprCall, semantic: &SemanticModel) -> Option<&'a Expr> {
let qualified_name = semantic.resolve_qualified_name(&call.func)?;
match qualified_name.segments() {
["os", "umask"] => call.arguments.find_argument_value("mode", 0),
[
"os",
"chmod" | "fchmod" | "lchmod" | "mkdir" | "makedirs" | "mkfifo" | "mknod",
] => call.arguments.find_argument_value("mode", 1),
["os", "open"] => call.arguments.find_argument_value("mode", 2),
["dbm", "open"] | ["dbm", "gnu" | "ndbm", "open"] => {
call.arguments.find_argument_value("mode", 2)
}
_ => None,
}
}
fn find_method_mode_arg<'a>(call: &'a ExprCall, semantic: &SemanticModel) -> Option<&'a Expr> {
let (type_name, attr_name) = resolve_method_call(&call.func, semantic)?;
match (type_name.segments(), attr_name) {
(
["pathlib", "Path" | "PosixPath" | "WindowsPath"],
"chmod" | "lchmod" | "mkdir" | "touch",
) => call.arguments.find_argument_value("mode", 0),
_ => None,
}
}
fn resolve_method_call<'a>(
func: &'a Expr,
semantic: &'a SemanticModel,
) -> Option<(QualifiedName<'a>, &'a str)> {
let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func else {
return None;
};
// First: is this an inlined call like `pathlib.Path.chmod`?
// ```python
// from pathlib import Path
// Path("foo").chmod(0o644)
// ```
if let Expr::Call(call) = value.as_ref() {
let qualified_name = semantic.resolve_qualified_name(&call.func)?;
return Some((qualified_name, attr));
}
// Second, is this a call like `pathlib.Path.chmod` via a variable?
// ```python
// from pathlib import Path
// path = Path("foo")
// path.chmod()
// ```
let Expr::Name(name) = value.as_ref() else {
return None;
};
let binding_id = semantic.resolve_name(name)?;
let binding = semantic.binding(binding_id);
let Some(Expr::Call(call)) = analyze::typing::find_binding_value(binding, semantic) else {
return None;
};
let qualified_name = semantic.resolve_qualified_name(&call.func)?;
Some((qualified_name, attr))
}
/// Try to determine whether the integer literal
fn suggest_fix(mode: u16) -> Option<u16> {
// These suggestions are in the form of
// <missing `0o` prefix> | <mode as decimal> => <octal>
// If <as decimal> could theoretically be a valid octal literal, the
// comment explains why it's deemed unlikely to be intentional.
match mode {
400 | 256 => Some(0o400), // -w-r-xrw-, group/other > user unlikely
440 | 288 => Some(0o440),
444 | 292 => Some(0o444),
600 | 384 => Some(0o600),
640 | 416 => Some(0o640), // r----xrw-, other > user unlikely
644 | 420 => Some(0o644), // r---w----, group write but not read unlikely
660 | 432 => Some(0o660), // r---wx-w-, write but not read unlikely
664 | 436 => Some(0o664), // r---wxrw-, other > user unlikely
666 | 438 => Some(0o666),
700 | 448 => Some(0o700),
744 | 484 => Some(0o744),
750 | 488 => Some(0o750),
755 | 493 => Some(0o755),
770 | 504 => Some(0o770), // r-x---r--, other > group unlikely
775 | 509 => Some(0o775),
776 | 510 => Some(0o776), // r-x--x---, seems unlikely
777 | 511 => Some(0o777), // r-x--x--x, seems unlikely
_ => None,
}
}

View file

@ -0,0 +1,347 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
RUF064.py:6:17: RUF064 [*] Non-octal mode
|
4 | from pathlib import Path
5 |
6 | os.chmod("foo", 444) # Error
| ^^^ RUF064
7 | os.chmod("foo", 0o444) # OK
8 | os.chmod("foo", 7777) # Error
|
= help: Replace with octal literal
Unsafe fix
3 3 | import os
4 4 | from pathlib import Path
5 5 |
6 |-os.chmod("foo", 444) # Error
6 |+os.chmod("foo", 0o444) # Error
7 7 | os.chmod("foo", 0o444) # OK
8 8 | os.chmod("foo", 7777) # Error
9 9 | os.chmod("foo", 10000) # Error
RUF064.py:8:17: RUF064 Non-octal mode
|
6 | os.chmod("foo", 444) # Error
7 | os.chmod("foo", 0o444) # OK
8 | os.chmod("foo", 7777) # Error
| ^^^^ RUF064
9 | os.chmod("foo", 10000) # Error
10 | os.chmod("foo", 99999) # Error
|
= help: Replace with octal literal
RUF064.py:9:17: RUF064 Non-octal mode
|
7 | os.chmod("foo", 0o444) # OK
8 | os.chmod("foo", 7777) # Error
9 | os.chmod("foo", 10000) # Error
| ^^^^^ RUF064
10 | os.chmod("foo", 99999) # Error
|
= help: Replace with octal literal
RUF064.py:10:17: RUF064 Non-octal mode
|
8 | os.chmod("foo", 7777) # Error
9 | os.chmod("foo", 10000) # Error
10 | os.chmod("foo", 99999) # Error
| ^^^^^ RUF064
11 |
12 | os.umask(777) # Error
|
= help: Replace with octal literal
RUF064.py:12:10: RUF064 [*] Non-octal mode
|
10 | os.chmod("foo", 99999) # Error
11 |
12 | os.umask(777) # Error
| ^^^ RUF064
13 | os.umask(0o777) # OK
|
= help: Replace with octal literal
Unsafe fix
9 9 | os.chmod("foo", 10000) # Error
10 10 | os.chmod("foo", 99999) # Error
11 11 |
12 |-os.umask(777) # Error
12 |+os.umask(0o777) # Error
13 13 | os.umask(0o777) # OK
14 14 |
15 15 | os.fchmod(0, 400) # Error
RUF064.py:15:14: RUF064 [*] Non-octal mode
|
13 | os.umask(0o777) # OK
14 |
15 | os.fchmod(0, 400) # Error
| ^^^ RUF064
16 | os.fchmod(0, 0o400) # OK
|
= help: Replace with octal literal
Unsafe fix
12 12 | os.umask(777) # Error
13 13 | os.umask(0o777) # OK
14 14 |
15 |-os.fchmod(0, 400) # Error
15 |+os.fchmod(0, 0o400) # Error
16 16 | os.fchmod(0, 0o400) # OK
17 17 |
18 18 | os.lchmod("foo", 755) # Error
RUF064.py:18:18: RUF064 [*] Non-octal mode
|
16 | os.fchmod(0, 0o400) # OK
17 |
18 | os.lchmod("foo", 755) # Error
| ^^^ RUF064
19 | os.lchmod("foo", 0o755) # OK
|
= help: Replace with octal literal
Unsafe fix
15 15 | os.fchmod(0, 400) # Error
16 16 | os.fchmod(0, 0o400) # OK
17 17 |
18 |-os.lchmod("foo", 755) # Error
18 |+os.lchmod("foo", 0o755) # Error
19 19 | os.lchmod("foo", 0o755) # OK
20 20 |
21 21 | os.mkdir("foo", 600) # Error
RUF064.py:21:17: RUF064 [*] Non-octal mode
|
19 | os.lchmod("foo", 0o755) # OK
20 |
21 | os.mkdir("foo", 600) # Error
| ^^^ RUF064
22 | os.mkdir("foo", 0o600) # OK
|
= help: Replace with octal literal
Unsafe fix
18 18 | os.lchmod("foo", 755) # Error
19 19 | os.lchmod("foo", 0o755) # OK
20 20 |
21 |-os.mkdir("foo", 600) # Error
21 |+os.mkdir("foo", 0o600) # Error
22 22 | os.mkdir("foo", 0o600) # OK
23 23 |
24 24 | os.makedirs("foo", 644) # Error
RUF064.py:24:20: RUF064 [*] Non-octal mode
|
22 | os.mkdir("foo", 0o600) # OK
23 |
24 | os.makedirs("foo", 644) # Error
| ^^^ RUF064
25 | os.makedirs("foo", 0o644) # OK
|
= help: Replace with octal literal
Unsafe fix
21 21 | os.mkdir("foo", 600) # Error
22 22 | os.mkdir("foo", 0o600) # OK
23 23 |
24 |-os.makedirs("foo", 644) # Error
24 |+os.makedirs("foo", 0o644) # Error
25 25 | os.makedirs("foo", 0o644) # OK
26 26 |
27 27 | os.mkfifo("foo", 640) # Error
RUF064.py:27:18: RUF064 [*] Non-octal mode
|
25 | os.makedirs("foo", 0o644) # OK
26 |
27 | os.mkfifo("foo", 640) # Error
| ^^^ RUF064
28 | os.mkfifo("foo", 0o640) # OK
|
= help: Replace with octal literal
Unsafe fix
24 24 | os.makedirs("foo", 644) # Error
25 25 | os.makedirs("foo", 0o644) # OK
26 26 |
27 |-os.mkfifo("foo", 640) # Error
27 |+os.mkfifo("foo", 0o640) # Error
28 28 | os.mkfifo("foo", 0o640) # OK
29 29 |
30 30 | os.mknod("foo", 660) # Error
RUF064.py:30:17: RUF064 [*] Non-octal mode
|
28 | os.mkfifo("foo", 0o640) # OK
29 |
30 | os.mknod("foo", 660) # Error
| ^^^ RUF064
31 | os.mknod("foo", 0o660) # OK
|
= help: Replace with octal literal
Unsafe fix
27 27 | os.mkfifo("foo", 640) # Error
28 28 | os.mkfifo("foo", 0o640) # OK
29 29 |
30 |-os.mknod("foo", 660) # Error
30 |+os.mknod("foo", 0o660) # Error
31 31 | os.mknod("foo", 0o660) # OK
32 32 |
33 33 | os.open("foo", os.O_CREAT, 644) # Error
RUF064.py:33:28: RUF064 [*] Non-octal mode
|
31 | os.mknod("foo", 0o660) # OK
32 |
33 | os.open("foo", os.O_CREAT, 644) # Error
| ^^^ RUF064
34 | os.open("foo", os.O_CREAT, 0o644) # OK
|
= help: Replace with octal literal
Unsafe fix
30 30 | os.mknod("foo", 660) # Error
31 31 | os.mknod("foo", 0o660) # OK
32 32 |
33 |-os.open("foo", os.O_CREAT, 644) # Error
33 |+os.open("foo", os.O_CREAT, 0o644) # Error
34 34 | os.open("foo", os.O_CREAT, 0o644) # OK
35 35 |
36 36 | Path("bar").chmod(755) # Error
RUF064.py:36:19: RUF064 [*] Non-octal mode
|
34 | os.open("foo", os.O_CREAT, 0o644) # OK
35 |
36 | Path("bar").chmod(755) # Error
| ^^^ RUF064
37 | Path("bar").chmod(0o755) # OK
|
= help: Replace with octal literal
Unsafe fix
33 33 | os.open("foo", os.O_CREAT, 644) # Error
34 34 | os.open("foo", os.O_CREAT, 0o644) # OK
35 35 |
36 |-Path("bar").chmod(755) # Error
36 |+Path("bar").chmod(0o755) # Error
37 37 | Path("bar").chmod(0o755) # OK
38 38 |
39 39 | path = Path("bar")
RUF064.py:40:12: RUF064 [*] Non-octal mode
|
39 | path = Path("bar")
40 | path.chmod(755) # Error
| ^^^ RUF064
41 | path.chmod(0o755) # OK
|
= help: Replace with octal literal
Unsafe fix
37 37 | Path("bar").chmod(0o755) # OK
38 38 |
39 39 | path = Path("bar")
40 |-path.chmod(755) # Error
40 |+path.chmod(0o755) # Error
41 41 | path.chmod(0o755) # OK
42 42 |
43 43 | dbm.open("db", "r", 600) # Error
RUF064.py:43:21: RUF064 [*] Non-octal mode
|
41 | path.chmod(0o755) # OK
42 |
43 | dbm.open("db", "r", 600) # Error
| ^^^ RUF064
44 | dbm.open("db", "r", 0o600) # OK
|
= help: Replace with octal literal
Unsafe fix
40 40 | path.chmod(755) # Error
41 41 | path.chmod(0o755) # OK
42 42 |
43 |-dbm.open("db", "r", 600) # Error
43 |+dbm.open("db", "r", 0o600) # Error
44 44 | dbm.open("db", "r", 0o600) # OK
45 45 |
46 46 | dbm.gnu.open("db", "r", 600) # Error
RUF064.py:46:25: RUF064 [*] Non-octal mode
|
44 | dbm.open("db", "r", 0o600) # OK
45 |
46 | dbm.gnu.open("db", "r", 600) # Error
| ^^^ RUF064
47 | dbm.gnu.open("db", "r", 0o600) # OK
|
= help: Replace with octal literal
Unsafe fix
43 43 | dbm.open("db", "r", 600) # Error
44 44 | dbm.open("db", "r", 0o600) # OK
45 45 |
46 |-dbm.gnu.open("db", "r", 600) # Error
46 |+dbm.gnu.open("db", "r", 0o600) # Error
47 47 | dbm.gnu.open("db", "r", 0o600) # OK
48 48 |
49 49 | dbm.ndbm.open("db", "r", 600) # Error
RUF064.py:49:26: RUF064 [*] Non-octal mode
|
47 | dbm.gnu.open("db", "r", 0o600) # OK
48 |
49 | dbm.ndbm.open("db", "r", 600) # Error
| ^^^ RUF064
50 | dbm.ndbm.open("db", "r", 0o600) # OK
|
= help: Replace with octal literal
Unsafe fix
46 46 | dbm.gnu.open("db", "r", 600) # Error
47 47 | dbm.gnu.open("db", "r", 0o600) # OK
48 48 |
49 |-dbm.ndbm.open("db", "r", 600) # Error
49 |+dbm.ndbm.open("db", "r", 0o600) # Error
50 50 | dbm.ndbm.open("db", "r", 0o600) # OK
51 51 |
52 52 | os.fchmod(0, 256) # 0o400
RUF064.py:52:14: RUF064 [*] Non-octal mode
|
50 | dbm.ndbm.open("db", "r", 0o600) # OK
51 |
52 | os.fchmod(0, 256) # 0o400
| ^^^ RUF064
53 | os.fchmod(0, 493) # 0o755
|
= help: Replace with octal literal
Unsafe fix
49 49 | dbm.ndbm.open("db", "r", 600) # Error
50 50 | dbm.ndbm.open("db", "r", 0o600) # OK
51 51 |
52 |-os.fchmod(0, 256) # 0o400
52 |+os.fchmod(0, 0o400) # 0o400
53 53 | os.fchmod(0, 493) # 0o755
RUF064.py:53:14: RUF064 [*] Non-octal mode
|
52 | os.fchmod(0, 256) # 0o400
53 | os.fchmod(0, 493) # 0o755
| ^^^ RUF064
|
= help: Replace with octal literal
Unsafe fix
50 50 | dbm.ndbm.open("db", "r", 0o600) # OK
51 51 |
52 52 | os.fchmod(0, 256) # 0o400
53 |-os.fchmod(0, 493) # 0o755
53 |+os.fchmod(0, 0o755) # 0o755

1
ruff.schema.json generated
View file

@ -4040,6 +4040,7 @@
"RUF06",
"RUF060",
"RUF061",
"RUF064",
"RUF1",
"RUF10",
"RUF100",