Skip to content

Commit 7cb3550

Browse files
teemu-rytilahti-sonarsourceclaude
authored andcommitted
SONARPY-4248 Add support for bandit's # nosec directive (#1183)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> GitOrigin-RevId: 93c06d9ef7b24b76d1120bac3daeb3af937c5f24
1 parent 99f9f52 commit 7cb3550

10 files changed

Lines changed: 283 additions & 11 deletions

File tree

its/plugin/it-python-plugin-test/profiles/nosonar.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,10 @@
2222
<key>S1309</key>
2323
<priority>INFO</priority>
2424
</rule>
25+
<rule>
26+
<repositoryKey>python</repositoryKey>
27+
<key>S4423</key>
28+
<priority>INFO</priority>
29+
</rule>
2530
</rules>
2631
</profile>
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import ssl
2+
3+
# Bare nosec suppresses every rule on the line (Bandit semantics).
4+
ssl.SSLContext(ssl.PROTOCOL_TLSv1)
5+
ssl.SSLContext(ssl.PROTOCOL_TLSv1) # nosec
6+
7+
# Scoped nosec with a Bandit ID narrows suppression to that ID; B502 has no Sonar mapping, so S4423 still fires.
8+
ssl.SSLContext(ssl.PROTOCOL_TLSv1) # nosec B502 legacy peer
9+
10+
# Selective suppression by Sonar rule key.
11+
ssl.SSLContext(ssl.PROTOCOL_TLSv1) # nosec S4423
12+
13+
# Selective suppression narrows to the listed keys; OneStatementPerLine still fires.
14+
ssl.SSLContext(ssl.PROTOCOL_TLSv1); ssl.SSLContext(ssl.PROTOCOL_TLSv1_1) # nosec S4423
15+
16+
# Baseline: two S4423 + OneStatementPerLine all fire.
17+
ssl.SSLContext(ssl.PROTOCOL_TLSv1); ssl.SSLContext(ssl.PROTOCOL_TLSv1_1)
18+
19+
# Known FN: bare nosec silences a non-security rule on the same line.
20+
ssl.SSLContext(ssl.PROTOCOL_TLSv1); ssl.SSLContext(ssl.PROTOCOL_TLSv1_1) # nosec
21+
22+
# Trailing directive on the last statement.
23+
ssl.SSLContext(ssl.PROTOCOL_TLSv1) # nosec
24+
25+
# Selective suppression lets S4423 through
26+
ssl.SSLContext(ssl.PROTOCOL_TLSv1) # nosec S1234

its/plugin/it-python-plugin-test/src/test/java/com/sonar/python/it/plugin/NoSonarTest.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public class NoSonarTest {
3131
private static final String NO_SONAR_PROJECT_KEY = "nosonar";
3232
private static final String EXTERNAL_ISSUE_PROJECT_KEY = "external-issues";
3333
private static final String NOQA_PROJECT_KEY = "noqa";
34+
private static final String NO_SEC_PROJECT_KEY = "nosec";
3435

3536
private static final String PROFILE_NAME = "nosonar";
3637

@@ -132,6 +133,48 @@ void test_noqa() {
132133
.containsIssue(22, "python:S1309").doesNotContainIssue(22, "python:PrintStatementUsage");
133134
}
134135

136+
@Test
137+
void test_nosec() {
138+
analyzeProject(createScanner(NO_SEC_PROJECT_KEY, "projects/nosonar/nosec-project"));
139+
140+
IssueListAssert.assertThat(issues(NO_SEC_PROJECT_KEY))
141+
.hasSize(14)
142+
// bare nosec suppresses everything on the line; S1309 still fires (whitelisted)
143+
.containsIssue(4, "python:S4423")
144+
.doesNotContainIssue(5, "python:S4423")
145+
.containsIssue(5, "python:S1309")
146+
147+
// scoped bandit ID has no Sonar mapping, so S4423 still fires
148+
.containsIssue(8, "python:S4423")
149+
.containsIssue(8, "python:S1309")
150+
151+
// selective suppression by Sonar rule key
152+
.doesNotContainIssue(11, "python:S4423")
153+
.containsIssue(11, "python:S1309")
154+
155+
// selective suppression narrows to listed keys; OneStatementPerLine still fires
156+
.doesNotContainIssue(14, "python:S4423")
157+
.containsIssue(14, "python:OneStatementPerLine")
158+
.containsIssue(14, "python:S1309")
159+
160+
// baseline: 2 x S4423 + OneStatementPerLine all fire
161+
.containsIssue(17, "python:S4423")
162+
.containsIssue(17, "python:OneStatementPerLine")
163+
164+
// FN: bare nosec silences a non-security rule on the same line
165+
.doesNotContainIssue(20, "python:S4423")
166+
.doesNotContainIssue(20, "python:OneStatementPerLine")
167+
.containsIssue(20, "python:S1309")
168+
169+
// nosec at end of file
170+
.doesNotContainIssue(23, "python:S4423")
171+
.containsIssue(23, "python:S1309")
172+
173+
// scoped nosec with unrelated key does not suppress S4423
174+
.containsIssue(26, "python:S4423")
175+
.containsIssue(26, "python:S1309");
176+
}
177+
135178
private void analyzeProject(SonarScanner scanner) {
136179
String projectKey = scanner.getProperty("sonar.projectKey");
137180
ORCHESTRATOR.getServer().provisionProject(projectKey, projectKey);

python-checks/src/main/java/org/sonar/python/checks/NoQaCommentCheck.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,25 @@
2626
@Rule(key = "S1309")
2727
public class NoQaCommentCheck extends PythonSubscriptionCheck {
2828

29-
private static final String MESSAGE = "Is #noqa used to exclude false-positive or to hide real quality flaw?";
29+
private static final String NOQA_MESSAGE = "Is 'noqa' used to exclude false-positive or to hide real quality flaw?";
30+
private static final String NOSEC_MESSAGE = "Is 'nosec' used to exclude false-positive or to hide real security issue?";
3031

3132
@Override
3233
public void initialize(Context context) {
3334
context.registerSyntaxNodeConsumer(Tree.Kind.TOKEN, ctx -> {
3435
Token token = (Token) ctx.syntaxNode();
3536
for (Trivia trivia : token.trivia()) {
36-
String commentLine = trivia.token().value();
37-
if (NoSonarInfoParser.isValidNoQa(commentLine)) {
38-
ctx.addIssue(trivia.token(), MESSAGE);
37+
for (String comment : NoSonarInfoParser.splitInlineComments(trivia.token().value())) {
38+
String message = null;
39+
if (NoSonarInfoParser.isValidNoQa(comment)) {
40+
message = NOQA_MESSAGE;
41+
} else if (NoSonarInfoParser.isValidNoSec(comment)) {
42+
message = NOSEC_MESSAGE;
43+
}
44+
if (message != null) {
45+
ctx.addIssue(trivia.token(), message);
46+
break;
47+
}
3948
}
4049
}
4150
});

python-checks/src/test/resources/checks/noqaComment.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
def divide(numerator, denominator):
2-
# Noncompliant@+1 {{Is #noqa used to exclude false-positive or to hide real quality flaw?}}
2+
# Noncompliant@+1 {{Is 'noqa' used to exclude false-positive or to hide real quality flaw?}}
33
return numerator / denominator # noqa denominator value might be 0
44
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
55

@@ -30,3 +30,26 @@ def divide(numerator, denominator):
3030
# this is not no qa
3131
a = "noqa-text"
3232
text = "noqa is mentioned here"
33+
34+
# --- nosec cases ---
35+
# S1309 is raised for both noqa and nosec suppression directives, so they share this fixture.
36+
37+
# Noncompliant@+1 {{Is 'nosec' used to exclude false-positive or to hide real security issue?}}
38+
import ssl # nosec
39+
# ^^^^^^^
40+
41+
# Noncompliant@+1
42+
import ssl # nosec B101
43+
44+
# Noncompliant@+1
45+
import ssl # nosec S4423, S5332 reason text
46+
47+
# Noncompliant@+1
48+
# NOSEC
49+
50+
# a nosec just mixed in should not be detected
51+
52+
# no sec with space should not be detected
53+
54+
# Noncompliant@+1 {{Is 'nosec' used to exclude false-positive or to hide real security issue?}}
55+
# some comment followed by # nosec should be detected

python-checks/src/test/resources/checks/nosonarCommentFormat.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,18 @@
4040
# NOSONAR(NoSonar)
4141
# Noncompliant@+1
4242
# NOSONAR(python:S1234)
43+
44+
# nosec
45+
# nosec B101
46+
# nosec B101, B102
47+
# nosec B101, B102 reason text
48+
# nosec: S1234, S5678
49+
# nosec because of foo, bar, and baz
50+
# Noncompliant@+1
51+
# nosec B101,
52+
# Noncompliant@+1
53+
# nosec ,B101
54+
# Noncompliant@+1
55+
# nosec B101,,B102
56+
# Noncompliant@+1
57+
# nosec B101 reason, B102

python-commons/src/test/java/org/sonar/plugins/python/nosonar/NoSonarIssueFilterTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,18 @@ private static Stream<Arguments> provideFilterParameters() {
7171
Arguments.of(Map.of(), "my_rule", 1, false, false)
7272
);
7373
}
74+
75+
// Known FN of `# nosec` handling: a bare `# nosec` suppresses every rule on the line, even non-security ones.
76+
@ParameterizedTest
77+
@MethodSource("provideNoSecFalseNegativeParameters")
78+
void noSecFalseNegativeTest(Map<Integer, NoSonarLineInfo> noSonarInfos, String ruleKey, int line, boolean filterChainAcceptResult, boolean expectedResult) {
79+
test(noSonarInfos, ruleKey, line, filterChainAcceptResult, expectedResult);
80+
}
81+
82+
private static Stream<Arguments> provideNoSecFalseNegativeParameters() {
83+
return Stream.of(
84+
// FN: bare `# nosec` silences a non-security rule (e.g. S1481 unused local).
85+
Arguments.of(Map.of(1, new NoSonarLineInfo(Set.of(), "")), "S1481", 1, true, false)
86+
);
87+
}
7488
}

python-commons/src/test/java/org/sonar/plugins/python/nosonar/NoSonarLineInfoCollectorTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,33 @@ private static Stream<Arguments> provideCollectorParameters() {
185185
Set.of(1, 2, 3, 4, 5),
186186
"",
187187
""
188+
),
189+
Arguments.of("""
190+
import hashlib
191+
hashlib.md5(b"x").hexdigest() # nosec
192+
""",
193+
Map.of(2, new NoSonarLineInfo(Set.of(), "")),
194+
Set.of(2),
195+
"",
196+
""
197+
),
198+
Arguments.of("""
199+
import hashlib
200+
hashlib.md5(b"x").hexdigest() # nosec B303 legacy hash
201+
""",
202+
Map.of(2, new NoSonarLineInfo(Set.of("B303"))),
203+
Set.of(),
204+
"B303",
205+
"B303:"
206+
),
207+
Arguments.of("""
208+
import ssl
209+
ssl.SSLContext(ssl.PROTOCOL_TLSv1) # nosec S4423
210+
""",
211+
Map.of(2, new NoSonarLineInfo(Set.of("S4423"))),
212+
Set.of(),
213+
"S4423",
214+
"S4423:"
188215
)
189216
);
190217
}

python-frontend/src/main/java/org/sonar/plugins/python/api/nosonar/NoSonarInfoParser.java

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,20 +33,24 @@ public class NoSonarInfoParser {
3333
private static final String NOQA_PATTERN_REGEX = "^#\\s*noqa(?::\\s*(.+))?(?:[\\s;:].*)?";
3434
private static final String NOSONAR_PREFIX_REGEX = "^#\\s*NOSONAR(\\W.*)?";
3535
private static final String NOSONAR_PATTERN_REGEX = "^#\\s*NOSONAR(?:\\s*\\(([^)]*)\\))?($|\\s.*)";
36+
private static final String NOSEC_PATTERN_REGEX = "(?i)^#\\s*nosec\\b[:\\s]*(.*)";
37+
private static final String NOSEC_RULE_KEY_REGEX = "^[A-Za-z]\\d+$";
3638
private static final String RULE_KEY_PATTERN_REGEX = "^[a-zA-Z0-9]+$";
3739

3840
private final Pattern noSonarPattern;
3941
private final Pattern noQaPattern;
42+
private final Pattern noSecPattern;
4043

4144
public NoSonarInfoParser() {
4245
noSonarPattern = Pattern.compile(NOSONAR_PATTERN_REGEX);
4346
noQaPattern = Pattern.compile(NOQA_PATTERN_REGEX);
47+
noSecPattern = Pattern.compile(NOSEC_PATTERN_REGEX);
4448
}
4549

4650
public boolean isInvalidIssueSuppressionComment(String commentsLine) {
4751
return splitInlineComments(commentsLine)
4852
.stream()
49-
.anyMatch(comment -> isInvalidNoSonarComment(comment) || isInvalidNoQaComment(comment));
53+
.anyMatch(comment -> isInvalidNoSonarComment(comment) || isInvalidNoQaComment(comment) || isInvalidNoSecComment(comment));
5054
}
5155

5256
private boolean isInvalidNoSonarComment(String comment) {
@@ -86,6 +90,20 @@ private boolean isInvalidNoQaComment(String comment) {
8690
return !rules.isEmpty() && rules.stream().anyMatch(r -> r.isBlank() || r.contains(" "));
8791
}
8892

93+
private boolean isInvalidNoSecComment(String comment) {
94+
if (!isValidNoSec(comment)) {
95+
return false;
96+
}
97+
var args = getParamsString(noSecPattern, comment);
98+
if (!args.contains(",")) {
99+
return false;
100+
}
101+
var tokens = parseSuppressionRules(args);
102+
// Only flag when the args look like a rule list (>=1 rule-shape token); free-form reasons with commas stay valid.
103+
var ruleKeys = tokens.stream().filter(t -> t.matches(NOSEC_RULE_KEY_REGEX)).toList();
104+
return !ruleKeys.isEmpty() && ruleKeys.size() != tokens.size();
105+
}
106+
89107
private static boolean isValidNoSonar(String noSonarCommentLine) {
90108
return noSonarCommentLine.matches(NOSONAR_PATTERN_REGEX);
91109
}
@@ -94,6 +112,10 @@ public static boolean isValidNoQa(String noSonarCommentLine) {
94112
return noSonarCommentLine.matches(NOQA_PATTERN_REGEX);
95113
}
96114

115+
public static boolean isValidNoSec(String commentLine) {
116+
return commentLine.matches(NOSEC_PATTERN_REGEX);
117+
}
118+
97119
public Optional<NoSonarLineInfo> parse(String commentLine) {
98120
var rules = new HashSet<String>();
99121
StringBuilder concatenatedCommentBuilder = new StringBuilder();
@@ -132,13 +154,20 @@ private NoSonarLineInfo parseComment(String commentLine) {
132154
.filter(Predicate.not(String::isEmpty))
133155
.forEach(rules::add);
134156
comment = parseNoQaComment(commentLine);
157+
} else if (isValidNoSec(commentLine)) {
158+
var noSecRules = parseNoSecRules(commentLine);
159+
if (noSecRules.isEmpty()) {
160+
comment = parseNoSecComment(commentLine);
161+
} else {
162+
rules.addAll(noSecRules);
163+
}
135164
} else {
136165
return null;
137166
}
138167
return new NoSonarLineInfo(rules, comment);
139168
}
140169

141-
private static List<String> splitInlineComments(String commentsLine) {
170+
public static List<String> splitInlineComments(String commentsLine) {
142171
return Stream.of(commentsLine.split("#"))
143172
.filter(Predicate.not(String::isBlank))
144173
.map(s -> "#" + s)
@@ -155,7 +184,21 @@ private String parseNoSonarComment(String noSonarCommentLine) {
155184
}
156185

157186
private List<String> parseNoQaRules(String noSonarCommentLine) {
158-
var paramsString = getParamsString(noQaPattern, noSonarCommentLine);
187+
return parseSuppressionRules(getParamsString(noQaPattern, noSonarCommentLine));
188+
}
189+
190+
private String parseNoQaComment(String noSonarCommentLine) {
191+
return getTruncatedCommentString(noQaPattern, noSonarCommentLine).strip();
192+
}
193+
194+
private List<String> parseNoSecRules(String noSecCommentLine) {
195+
return parseSuppressionRules(getParamsString(noSecPattern, noSecCommentLine))
196+
.stream()
197+
.filter(t -> t.matches(NOSEC_RULE_KEY_REGEX))
198+
.toList();
199+
}
200+
201+
private static List<String> parseSuppressionRules(String paramsString) {
159202
var paramsList = parseParamsString(paramsString).collect(Collectors.toList());
160203
if (!paramsList.isEmpty()) {
161204
// to get the last suppressed rule ID we need to split it to cut the trailing comment text.
@@ -170,8 +213,10 @@ private List<String> parseNoQaRules(String noSonarCommentLine) {
170213
return paramsList;
171214
}
172215

173-
private String parseNoQaComment(String noSonarCommentLine) {
174-
return getTruncatedCommentString(noQaPattern, noSonarCommentLine).strip();
216+
private String parseNoSecComment(String noSecCommentLine) {
217+
var raw = getPatternGroup(1, noSecPattern, noSecCommentLine);
218+
var truncated = raw.length() > MAX_COMMENT_LENGTH ? raw.substring(0, MAX_COMMENT_LENGTH) : raw;
219+
return truncated.strip();
175220
}
176221

177222
private static String getParamsString(Pattern pattern, String noSonarCommentLine) {

0 commit comments

Comments
 (0)