Skip to content

Commit 3333312

Browse files
thomas-serre-sonarsourcesonartech
thomas-serre-sonarsource
authored and
sonartech
committed
SONARPY-3694 CORSMiddleware should be added last in the middleware chain (#820)
GitOrigin-RevId: 2be4ef23b3215f8e308edbec0273c762d87d23d0
1 parent 5945c4d commit 3333312

7 files changed

Lines changed: 575 additions & 0 deletions

File tree

‎python-checks/src/main/java/org/sonar/python/checks/CorsMiddlewareOrderingCheck.java‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import java.util.Optional;
20+
import javax.annotation.Nullable;
21+
import org.sonar.check.Rule;
22+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
23+
import org.sonar.plugins.python.api.SubscriptionContext;
24+
import org.sonar.plugins.python.api.symbols.v2.SymbolV2;
25+
import org.sonar.plugins.python.api.symbols.v2.UsageV2;
26+
import org.sonar.plugins.python.api.tree.CallExpression;
27+
import org.sonar.plugins.python.api.tree.Expression;
28+
import org.sonar.plugins.python.api.tree.Name;
29+
import org.sonar.plugins.python.api.tree.QualifiedExpression;
30+
import org.sonar.plugins.python.api.tree.RegularArgument;
31+
import org.sonar.plugins.python.api.tree.Tree;
32+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
33+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
34+
import org.sonar.python.checks.utils.Expressions;
35+
import org.sonar.python.tree.TreeUtils;
36+
37+
@Rule(key = "S8414")
38+
public class CorsMiddlewareOrderingCheck extends PythonSubscriptionCheck {
39+
40+
private static final String MESSAGE = "Add CORSMiddleware last in the middleware chain.";
41+
42+
// Stubs are missing entirely for fastapi/starlette.middleware.cors so we match by FQN for add_middleware
43+
private static final TypeMatcher ADD_MIDDLEWARE_MATCHER = TypeMatchers.any(
44+
TypeMatchers.withFQN("fastapi.applications.FastAPI.add_middleware"),
45+
TypeMatchers.withFQN("starlette.applications.Starlette.add_middleware")
46+
);
47+
48+
private static final TypeMatcher CORS_MIDDLEWARE_MATCHER = TypeMatchers.any(
49+
TypeMatchers.withFQN("fastapi.middleware.cors.CORSMiddleware"),
50+
TypeMatchers.withFQN("starlette.middleware.cors.CORSMiddleware")
51+
);
52+
53+
@Override
54+
public void initialize(Context context) {
55+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, CorsMiddlewareOrderingCheck::checkAddMiddlewareCall);
56+
}
57+
58+
private static void checkAddMiddlewareCall(SubscriptionContext ctx) {
59+
CallExpression callExpr = (CallExpression) ctx.syntaxNode();
60+
61+
if (!ADD_MIDDLEWARE_MATCHER.isTrueFor(callExpr.callee(), ctx)) {
62+
return;
63+
}
64+
65+
if (!isCorsMiddleware(ctx, callExpr)) {
66+
return;
67+
}
68+
69+
SymbolV2 receiverSymbol = extractReceiverSymbol(callExpr).orElse(null);
70+
if (receiverSymbol == null) {
71+
return;
72+
}
73+
74+
Tree currentEnclosingScope = getScope(callExpr);
75+
if (currentEnclosingScope == null) {
76+
return;
77+
}
78+
79+
int currentLine = callExpr.firstToken().line();
80+
81+
if (hasSubsequentMiddlewareAddition(ctx, receiverSymbol, currentLine, currentEnclosingScope)) {
82+
ctx.addIssue(callExpr.callee(), MESSAGE);
83+
}
84+
}
85+
86+
private static boolean isCorsMiddleware(SubscriptionContext ctx, CallExpression callExpr) {
87+
return TreeUtils.nthArgumentOrKeywordOptional(0, "middleware_class", callExpr.arguments())
88+
.map(RegularArgument::expression)
89+
.map(expr -> {
90+
if (CORS_MIDDLEWARE_MATCHER.isTrueFor(expr, ctx)) {
91+
return true;
92+
}
93+
if (expr instanceof Name name) {
94+
Expression assignedValue = Expressions.singleAssignedValue(name);
95+
return assignedValue != null && CORS_MIDDLEWARE_MATCHER.isTrueFor(assignedValue, ctx);
96+
}
97+
return false;
98+
})
99+
.orElse(false);
100+
}
101+
102+
private static boolean isNameContainingCors(@Nullable Expression expr){
103+
if(expr == null){
104+
return false;
105+
}
106+
if(!(expr instanceof Name name)) {
107+
return false;
108+
}
109+
String simpleName = name.name();
110+
return simpleName != null && (simpleName.contains("CORS") || simpleName.contains("Cors"));
111+
}
112+
113+
private static boolean hasSubsequentMiddlewareAddition(
114+
SubscriptionContext ctx,
115+
SymbolV2 receiverSymbol,
116+
int currentLine,
117+
Tree currentEnclosingScope) {
118+
119+
return receiverSymbol.usages().stream()
120+
.filter(usage -> usage.kind() == UsageV2.Kind.OTHER)
121+
.filter(usage -> usage.tree().firstToken().line() > currentLine)
122+
.filter(usage -> isSameScope(usage.tree(), currentEnclosingScope))
123+
.anyMatch(usage -> isReceiverOfAddMiddleware(usage, ctx));
124+
}
125+
126+
private static boolean isSameScope(Tree usageTree, Tree currentEnclosingScope) {
127+
Tree usageEnclosingScope = getScope(usageTree);
128+
return usageEnclosingScope == currentEnclosingScope;
129+
}
130+
131+
private static Tree getScope(Tree tree) {
132+
return TreeUtils.firstAncestorOfKind(tree, Tree.Kind.FUNCDEF, Tree.Kind.LAMBDA, Tree.Kind.FILE_INPUT);
133+
}
134+
135+
private static boolean isReceiverOfAddMiddleware(UsageV2 usage, SubscriptionContext ctx) {
136+
Tree tree = usage.tree();
137+
138+
Tree parent = tree.parent();
139+
if (!(parent instanceof QualifiedExpression qualifiedExpr)) {
140+
return false;
141+
}
142+
143+
Tree qualiifiedParent = qualifiedExpr.parent();
144+
if (!(qualiifiedParent instanceof CallExpression parentCall)) {
145+
return false;
146+
}
147+
148+
return ADD_MIDDLEWARE_MATCHER.isTrueFor(parentCall.callee(), ctx);
149+
}
150+
151+
private static Optional<SymbolV2> extractReceiverSymbol(CallExpression callExpr) {
152+
return Optional.ofNullable(callExpr.callee())
153+
.flatMap(TreeUtils.toOptionalInstanceOfMapper(QualifiedExpression.class))
154+
.map(QualifiedExpression::qualifier)
155+
.flatMap(TreeUtils.toOptionalInstanceOfMapper(Name.class))
156+
.map(Name::symbolV2);
157+
}
158+
}

‎python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ public Stream<Class<?>> getChecks() {
177177
ConstantValueDictComprehensionCheck.class,
178178
ControlFlowInTaskGroupCheck.class,
179179
CorsCheck.class,
180+
CorsMiddlewareOrderingCheck.class,
180181
CsrfDisabledCheck.class,
181182
DataEncryptionCheck.class,
182183
DbNoPasswordCheck.class,

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8414.html‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<p>This is an issue when <code>CORSMiddleware</code> is added to a FastAPI application and then followed by additional middleware using
2+
<code>add_middleware()</code>.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>In FastAPI, middleware is executed in reverse order of how it’s added to the application. When you call <code>app.add_middleware()</code>, each
5+
middleware wraps the previous one, forming a stack.</p>
6+
<p>Think of it like wrapping a gift: the first layer you add becomes the innermost layer, and the last layer you add becomes the outermost layer. When
7+
processing a request, FastAPI starts from the outermost layer and works its way in.</p>
8+
<p>CORS (Cross-Origin Resource Sharing) middleware needs to add specific HTTP headers to responses to allow browsers to make cross-origin requests.
9+
For these headers to be present on all responses - including those modified by other middleware - CORSMiddleware must be the outermost layer.</p>
10+
<p>If you add other middleware after CORSMiddleware, those middleware will wrap around it and process requests/responses outside of the CORS layer.
11+
This means:</p>
12+
<ul>
13+
<li> CORS headers may not be added to responses modified by the outer middleware </li>
14+
<li> Preflight requests may not be handled correctly </li>
15+
<li> Cross-origin requests from browsers will fail </li>
16+
</ul>
17+
<h3>What is the potential impact?</h3>
18+
<p>When CORSMiddleware is not positioned correctly, cross-origin requests from web browsers will fail. This breaks the functionality of web
19+
applications that need to make API calls from a different domain.</p>
20+
<p>Users will see errors in their browser console, and features that depend on cross-origin requests will not work. This can affect:</p>
21+
<ul>
22+
<li> Single-page applications (SPAs) hosted on different domains </li>
23+
<li> Mobile apps using web views </li>
24+
<li> Third-party integrations </li>
25+
<li> Development environments where frontend and backend run on different ports </li>
26+
</ul>
27+
<h2>How to fix it</h2>
28+
<p>Move the <code>CORSMiddleware</code> configuration to be the last <code>add_middleware()</code> call in your application setup. Add all other
29+
middleware before adding CORSMiddleware.</p>
30+
<h3>Code examples</h3>
31+
<h4>Noncompliant code example</h4>
32+
<pre data-diff-id="1" data-diff-type="noncompliant">
33+
from fastapi import FastAPI
34+
from fastapi.middleware.cors import CORSMiddleware
35+
from fastapi.middleware.gzip import GZipMiddleware
36+
37+
app = FastAPI()
38+
39+
app.add_middleware(
40+
CORSMiddleware,
41+
allow_origins=["*"],
42+
allow_credentials=True,
43+
allow_methods=["*"],
44+
allow_headers=["*"],
45+
)
46+
app.add_middleware(GZipMiddleware) # Noncompliant
47+
</pre>
48+
<h4>Compliant solution</h4>
49+
<pre data-diff-id="1" data-diff-type="compliant">
50+
from fastapi import FastAPI
51+
from fastapi.middleware.cors import CORSMiddleware
52+
from fastapi.middleware.gzip import GZipMiddleware
53+
54+
app = FastAPI()
55+
56+
app.add_middleware(GZipMiddleware)
57+
app.add_middleware(
58+
CORSMiddleware,
59+
allow_origins=["*"],
60+
allow_credentials=True,
61+
allow_methods=["*"],
62+
allow_headers=["*"],
63+
)
64+
</pre>
65+
<h2>Resources</h2>
66+
<h3>Documentation</h3>
67+
<ul>
68+
<li> FastAPI CORS Documentation - <a href="https://fastapi.tiangolo.com/tutorial/cors/">Official FastAPI documentation on CORS middleware
69+
configuration</a> </li>
70+
<li> FastAPI Middleware Documentation - <a href="https://fastapi.tiangolo.com/tutorial/middleware/">Official FastAPI documentation explaining
71+
middleware execution order</a> </li>
72+
<li> Starlette Middleware Documentation - <a href="https://www.starlette.io/middleware/">Starlette middleware documentation (FastAPI is built on
73+
Starlette)</a> </li>
74+
</ul>
75+

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8414.json‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
{
2+
"title": "CORSMiddleware should be added last in the middleware chain",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5 min"
8+
},
9+
"tags": [
10+
"fastapi",
11+
"cors",
12+
"middleware",
13+
"configuration"
14+
],
15+
"defaultSeverity": "Blocker",
16+
"ruleSpecification": "RSPEC-8414",
17+
"sqKey": "S8414",
18+
"scope": "Main",
19+
"quickfix": "unknown",
20+
"code": {
21+
"impacts": {
22+
"RELIABILITY": "BLOCKER",
23+
"MAINTAINABILITY": "BLOCKER"
24+
},
25+
"attribute": "LOGICAL"
26+
}
27+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@
326326
"S8411",
327327
"S8412",
328328
"S8413",
329+
"S8414",
329330
"S8415"
330331
]
331332
}

‎python-checks/src/test/java/org/sonar/python/checks/CorsMiddlewareOrderingCheckTest.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.python.checks.utils.PythonCheckVerifier;
21+
22+
class CorsMiddlewareOrderingCheckTest {
23+
@Test
24+
void test() {
25+
PythonCheckVerifier.verify(
26+
"src/test/resources/checks/corsMiddlewareOrdering.py",
27+
new CorsMiddlewareOrderingCheck());
28+
}
29+
}

0 commit comments

Comments
 (0)