Skip to content

Commit 81bbb27

Browse files
Seppli11sonartech
Seppli11
authored and
sonartech
committed
SONARPY-3653 Create rule S8389: FastAPI file upload endpoints should use "Form()" with Pydantic validators instead of "Body()" or "Depends()" (#789)
GitOrigin-RevId: 2c2d38fa9d48c7e0e5404598157b7e5d186995d6
1 parent 101a889 commit 81bbb27

8 files changed

Lines changed: 640 additions & 0 deletions

File tree

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,6 @@ python-frontend/typeshed_serializer/.tox
5050
.sonar-code-context
5151
!.sonar-code-context/settings.json
5252
.cursor/rules/sonar_code_context.mdc
53+
54+
# claude code
55+
.claude/*.local.*

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

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
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.List;
20+
import org.sonar.check.Rule;
21+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
22+
import org.sonar.plugins.python.api.SubscriptionContext;
23+
import org.sonar.plugins.python.api.tree.CallExpression;
24+
import org.sonar.plugins.python.api.tree.Decorator;
25+
import org.sonar.plugins.python.api.tree.Expression;
26+
import org.sonar.plugins.python.api.tree.FunctionDef;
27+
import org.sonar.plugins.python.api.tree.Parameter;
28+
import org.sonar.plugins.python.api.tree.RegularArgument;
29+
import org.sonar.plugins.python.api.tree.SubscriptionExpression;
30+
import org.sonar.plugins.python.api.tree.Tree;
31+
import org.sonar.plugins.python.api.tree.TypeAnnotation;
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.tree.TreeUtils;
35+
36+
@Rule(key = "S8389")
37+
public class FastAPIFileUploadFormCheck extends PythonSubscriptionCheck {
38+
39+
private static final String MESSAGE_BODY_WITH_FILE =
40+
"Use \"Form()\" instead of \"Body()\" when handling file uploads; " +
41+
"\"Body()\" expects JSON, which is incompatible with multipart/form-data.";
42+
43+
private static final String MESSAGE_DEPENDS_WITH_FILE =
44+
"Use \"Form()\" with Pydantic validators instead of \"Depends()\" for " +
45+
"file upload endpoints; query parameters may expose sensitive data in URLs.";
46+
47+
private static final TypeMatcher IS_FASTAPI_ROUTE = TypeMatchers.any(
48+
TypeMatchers.isType("fastapi.routing.APIRouter.post"),
49+
TypeMatchers.isType("fastapi.routing.APIRouter.put"),
50+
TypeMatchers.isType("fastapi.routing.APIRouter.patch"),
51+
TypeMatchers.isType("fastapi.routing.APIRouter.delete"),
52+
TypeMatchers.isType("fastapi.applications.FastAPI.post"),
53+
TypeMatchers.isType("fastapi.applications.FastAPI.put"),
54+
TypeMatchers.isType("fastapi.applications.FastAPI.patch"),
55+
TypeMatchers.isType("fastapi.applications.FastAPI.delete")
56+
);
57+
58+
private static final TypeMatcher IS_FILE_PARAM = TypeMatchers.isType("fastapi.param_functions.File");
59+
private static final TypeMatcher IS_BODY_PARAM = TypeMatchers.isType("fastapi.param_functions.Body");
60+
private static final TypeMatcher IS_DEPENDS_PARAM = TypeMatchers.isType("fastapi.param_functions.Depends");
61+
62+
private static final TypeMatcher IS_UPLOAD_FILE = TypeMatchers.any(
63+
TypeMatchers.isOrExtendsType("fastapi.datastructures.UploadFile"),
64+
TypeMatchers.isOrExtendsType("starlette.datastructures.UploadFile")
65+
);
66+
67+
private static final TypeMatcher IS_PYDANTIC_MODEL = TypeMatchers.isOrExtendsType("pydantic.BaseModel");
68+
69+
@Override
70+
public void initialize(Context context) {
71+
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, FastAPIFileUploadFormCheck::checkFunctionDef);
72+
}
73+
74+
private static void checkFunctionDef(SubscriptionContext ctx) {
75+
FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
76+
77+
if (!hasFastAPIRouteDecorator(functionDef, ctx)) {
78+
return;
79+
}
80+
81+
List<Parameter> parameters = functionDef.parameters().nonTuple();
82+
83+
boolean hasFileParameter = parameters.stream()
84+
.anyMatch(param -> hasFileParam(param, ctx));
85+
86+
if (!hasFileParameter) {
87+
return;
88+
}
89+
90+
parameters.stream()
91+
.filter(param -> hasBodyParam(param, ctx))
92+
.forEach(param -> ctx.addIssue(param, MESSAGE_BODY_WITH_FILE));
93+
94+
parameters.stream()
95+
.filter(param -> hasDependsWithPydanticModel(param, ctx))
96+
.forEach(param -> ctx.addIssue(param, MESSAGE_DEPENDS_WITH_FILE));
97+
}
98+
99+
private static boolean hasFastAPIRouteDecorator(FunctionDef functionDef, SubscriptionContext ctx) {
100+
return functionDef.decorators().stream()
101+
.map(Decorator::expression)
102+
.flatMap(TreeUtils.toStreamInstanceOfMapper(CallExpression.class))
103+
.anyMatch(callExpr -> IS_FASTAPI_ROUTE.isTrueFor(callExpr.callee(), ctx));
104+
}
105+
106+
private static boolean hasFileParam(Parameter param, SubscriptionContext ctx) {
107+
Expression defaultValue = param.defaultValue();
108+
if (defaultValue instanceof CallExpression callExpr && IS_FILE_PARAM.isTrueFor(callExpr.callee(), ctx)) {
109+
return true;
110+
}
111+
112+
TypeAnnotation annotation = param.typeAnnotation();
113+
if (annotation != null) {
114+
Expression annotationExpr = annotation.expression();
115+
116+
if (IS_UPLOAD_FILE.isTrueFor(annotationExpr, ctx)) {
117+
return true;
118+
}
119+
120+
if (annotationExpr instanceof SubscriptionExpression subscriptionExpr) {
121+
return subscriptionExpr.subscripts().expressions().stream()
122+
.anyMatch(expr -> IS_UPLOAD_FILE.isTrueFor(expr, ctx));
123+
}
124+
}
125+
126+
return false;
127+
}
128+
129+
private static boolean hasBodyParam(Parameter param, SubscriptionContext ctx) {
130+
Expression defaultValue = param.defaultValue();
131+
if (defaultValue instanceof CallExpression callExpr) {
132+
return IS_BODY_PARAM.isTrueFor(callExpr.callee(), ctx);
133+
}
134+
return false;
135+
}
136+
137+
private static boolean hasDependsWithPydanticModel(Parameter param, SubscriptionContext ctx) {
138+
Expression defaultValue = param.defaultValue();
139+
if (!(defaultValue instanceof CallExpression dependsCall)) {
140+
return false;
141+
}
142+
143+
if (!IS_DEPENDS_PARAM.isTrueFor(dependsCall.callee(), ctx)) {
144+
return false;
145+
}
146+
147+
if (!dependsCall.arguments().isEmpty()) {
148+
var firstArg = dependsCall.arguments().get(0);
149+
if (firstArg instanceof RegularArgument regularArg) {
150+
Expression argExpr = regularArg.expression();
151+
return IS_PYDANTIC_MODEL.isTrueFor(argExpr, ctx);
152+
}
153+
}
154+
155+
TypeAnnotation annotation = param.typeAnnotation();
156+
if (annotation != null) {
157+
Expression annotationExpr = annotation.expression();
158+
return IS_PYDANTIC_MODEL.isTrueFor(annotationExpr, ctx);
159+
}
160+
161+
return false;
162+
}
163+
}

‎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
@@ -219,6 +219,7 @@ public Stream<Class<?>> getChecks() {
219219
ExitHasBadArgumentsCheck.class,
220220
ExpandingArchiveCheck.class,
221221
FastAPIBindToAllNetworkInterfacesCheck.class,
222+
FastAPIFileUploadFormCheck.class,
222223
FastHashingOrPlainTextCheck.class,
223224
FieldDuplicatesClassNameCheck.class,
224225
FieldNameCheck.class,

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

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
<p>This is an issue when a FastAPI endpoint accepts file uploads (using <code>File()</code>) and also receives structured data through
2+
<code>Body()</code> parameters or query parameters (via <code>Depends()</code> without proper form handling). This pattern causes either validation
3+
errors at runtime or exposes sensitive information in URLs and logs.</p>
4+
<h2>Why is this an issue?</h2>
5+
<p>When building web APIs with FastAPI, developers often need to create endpoints that accept both file uploads and structured data. However, the way
6+
this data is transmitted requires careful consideration due to HTTP protocol constraints and security implications.</p>
7+
<h2>Understanding HTTP Content Types</h2>
8+
<p>HTTP requests can encode data in different ways, specified by the <code>Content-Type</code> header:</p>
9+
<ul>
10+
<li> <code>application/json</code> - Used for JSON data in the request body </li>
11+
<li> <code>multipart/form-data</code> - Required for file uploads, encodes both files and form fields </li>
12+
<li> Query parameters - Appended to the URL after a <code>?</code> character </li>
13+
</ul>
14+
<p>When a FastAPI endpoint includes <code>File()</code> parameters, the client must send the request using <code>multipart/form-data</code> encoding.
15+
This creates a fundamental incompatibility: you cannot mix <code>Body()</code> parameters (which expect <code>application/json</code>) with
16+
<code>File()</code> parameters in the same endpoint.</p>
17+
<h2>The Technical Problem</h2>
18+
<p>If you declare both <code>Body()</code> and <code>File()</code> parameters in the same endpoint, FastAPI cannot properly parse the request. The
19+
framework expects JSON in the body when it sees <code>Body()</code> parameters, but receives form-encoded data instead when files are included. This
20+
results in validation errors like "value is not a valid dict" or similar type mismatches.</p>
21+
<h2>The Security Problem</h2>
22+
<p>Some developers work around the technical constraint by passing structured data through query parameters using <code>Depends()</code> with a
23+
Pydantic model. While this avoids the encoding conflict, it creates a serious security vulnerability.</p>
24+
<p>Query parameters appear in the URL itself, which means they are:</p>
25+
<ul>
26+
<li> Visible in browser address bars </li>
27+
<li> Stored in browser history </li>
28+
<li> Logged in web server access logs </li>
29+
<li> Potentially cached by proxies and CDNs </li>
30+
<li> Visible in network monitoring tools </li>
31+
</ul>
32+
<p>If the structured data contains sensitive information (user credentials, personal data, tokens, etc.), this exposure creates a significant security
33+
risk. An attacker with access to server logs, browser history, or network traffic can extract this sensitive information.</p>
34+
<h2>Why Form Data is the Solution</h2>
35+
<p>Form data transmitted via <code>multipart/form-data</code> encoding:</p>
36+
<ul>
37+
<li> Is compatible with file uploads </li>
38+
<li> Is sent in the request body, not the URL </li>
39+
<li> Does not appear in server logs (only the URL path is logged) </li>
40+
<li> Can be properly validated and parsed by FastAPI </li>
41+
</ul>
42+
<p>The challenge is that form data is transmitted as strings, not as structured JSON objects. This is where Pydantic’s custom validators become
43+
essential - they allow you to parse JSON strings from form fields while maintaining type safety and validation.</p>
44+
<h3>What is the potential impact?</h3>
45+
<h3>Exposure of Sensitive Data</h3>
46+
<p>When structured data containing sensitive information is passed through query parameters, it becomes visible in multiple locations:</p>
47+
<ul>
48+
<li> Server access logs permanently record the full URL including all query parameters </li>
49+
<li> Browser history stores the complete URL, accessible to anyone with access to the device </li>
50+
<li> Network intermediaries (proxies, load balancers) may log or cache the URLs </li>
51+
<li> Referrer headers may leak the URL to third-party sites </li>
52+
</ul>
53+
<p>This exposure can lead to unauthorized access to user accounts, personal information disclosure, or compliance violations (GDPR, HIPAA,
54+
PCI-DSS).</p>
55+
<h3>Application Failures</h3>
56+
<p>When <code>Body()</code> and <code>File()</code> parameters are mixed incorrectly, the application will fail at runtime with validation errors.
57+
Users will be unable to complete file upload operations, resulting in:</p>
58+
<ul>
59+
<li> Poor user experience </li>
60+
<li> Failed business processes </li>
61+
<li> Support burden from confused users </li>
62+
<li> Potential data loss if users abandon the operation </li>
63+
</ul>
64+
<h2>How to fix it in FastAPI</h2>
65+
<p>Replace <code>Body()</code> parameters with <code>Form()</code> parameters and add a Pydantic validator to parse JSON strings. This ensures
66+
compatibility with file uploads while maintaining type safety.</p>
67+
<h3>Code examples</h3>
68+
<h4>Noncompliant code example</h4>
69+
<pre data-diff-id="1" data-diff-type="noncompliant">
70+
@router.post("/upload")
71+
async def create_policy(
72+
countryId: str = Body(...), # Noncompliant
73+
policyDetails: List[dict] = Body(...), # Noncompliant
74+
files: List[UploadFile] = File(...)
75+
):
76+
return {"status": "ok"}
77+
</pre>
78+
<h4>Compliant solution</h4>
79+
<pre data-diff-id="1" data-diff-type="compliant">
80+
class PolicyData(BaseModel):
81+
countryId: str
82+
policyDetails: List[dict]
83+
84+
@model_validator(mode='before')
85+
@classmethod
86+
def validate_to_json(cls, value):
87+
if isinstance(value, str):
88+
return cls(**json.loads(value))
89+
return value
90+
91+
@router.post("/upload")
92+
async def create_policy(
93+
data: PolicyData = Form(...),
94+
files: List[UploadFile] = File(...)
95+
):
96+
return {"status": "ok"}
97+
</pre>
98+
<p>Replace <code>Depends()</code> with <code>Form()</code> when passing structured data alongside file uploads. Use a custom validator to parse the
99+
JSON string from the form field.</p>
100+
<h4>Noncompliant code example</h4>
101+
<pre data-diff-id="2" data-diff-type="noncompliant">
102+
@app.post("/submit")
103+
def submit(
104+
base: Base = Depends(), # Noncompliant - data exposed in query parameters
105+
files: List[UploadFile] = File(...)
106+
):
107+
return {"JSON Payload": base, "Filenames": [file.filename for file in files]}
108+
</pre>
109+
<h4>Compliant solution</h4>
110+
<pre data-diff-id="2" data-diff-type="compliant">
111+
class Base(BaseModel):
112+
countryId: str
113+
sensitiveData: str
114+
115+
@model_validator(mode='before')
116+
@classmethod
117+
def validate_to_json(cls, value):
118+
if isinstance(value, str):
119+
return cls(**json.loads(value))
120+
return value
121+
122+
@app.post("/submit")
123+
def submit(
124+
base: Base = Form(...),
125+
files: List[UploadFile] = File(...)
126+
):
127+
return {"JSON Payload": base, "Filenames": [file.filename for file in files]}
128+
</pre>
129+
<p>If you need to accept complex nested structures, create a dependency function that reads from <code>Form()</code> and performs validation with
130+
proper error handling.</p>
131+
<h4>Noncompliant code example</h4>
132+
<pre data-diff-id="3" data-diff-type="noncompliant">
133+
@app.post("/data")
134+
async def upload_data(
135+
config: DataConfiguration = Depends(), # Noncompliant
136+
csvFile: UploadFile = File(...)
137+
):
138+
pass
139+
</pre>
140+
<h4>Compliant solution</h4>
141+
<pre data-diff-id="3" data-diff-type="compliant">
142+
from fastapi import HTTPException, status
143+
from fastapi.encoders import jsonable_encoder
144+
from pydantic import ValidationError
145+
146+
def parse_config(data: str = Form(...)) -&gt; DataConfiguration:
147+
try:
148+
return DataConfiguration.model_validate_json(data)
149+
except ValidationError as e:
150+
raise HTTPException(
151+
detail=jsonable_encoder(e.errors()),
152+
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
153+
)
154+
155+
@app.post("/data")
156+
async def upload_data(
157+
config: DataConfiguration = Depends(parse_config),
158+
csvFile: UploadFile = File(...)
159+
):
160+
pass
161+
</pre>
162+
<h2>Resources</h2>
163+
<h3>Documentation</h3>
164+
<ul>
165+
<li> FastAPI - Request Forms and Files - <a href="https://fastapi.tiangolo.com/tutorial/request-forms-and-files/">Official FastAPI documentation on
166+
handling forms and files together</a> </li>
167+
<li> FastAPI - Request Body - <a href="https://fastapi.tiangolo.com/tutorial/body/">Official documentation explaining Body parameters and JSON
168+
encoding</a> </li>
169+
<li> Pydantic - Validators - <a href="https://docs.pydantic.dev/latest/concepts/validators/">Documentation on Pydantic validators for custom data
170+
parsing</a> </li>
171+
<li> OWASP - Logging Cheat Sheet - <a href="https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html">Guidelines on what data should
172+
not be logged, including query parameters with sensitive data</a> </li>
173+
</ul>
174+
<h3>Standards</h3>
175+
<ul>
176+
<li> OWASP Top 10 2021 A01 - <a href="https://owasp.org/Top10/A01_2021-Broken_Access_Control/">Broken Access Control - exposing sensitive data in
177+
URLs can lead to unauthorized access</a> </li>
178+
<li> OWASP Top 10 2021 A09 - <a href="https://owasp.org/Top10/A09_2021-Security_Logging_and_Monitoring_Failures/">Security Logging and Monitoring
179+
Failures - sensitive data in logs creates security risks</a> </li>
180+
<li> CWE 359 - <a href="https://cwe.mitre.org/data/definitions/359.html">Exposure of Private Personal Information to an Unauthorized Actor</a> </li>
181+
<li> CWE 598 - <a href="https://cwe.mitre.org/data/definitions/598.html">Use of GET Request Method With Sensitive Query Strings</a> </li>
182+
</ul>
183+

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
{
2+
"title": "FastAPI file upload endpoints should use \"Form()\" with Pydantic validators instead of \"Body()\" or \"Depends()\"",
3+
"type": "VULNERABILITY",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "10 min"
8+
},
9+
"tags": [
10+
"fastapi",
11+
"security",
12+
"file-upload",
13+
"pydantic"
14+
],
15+
"defaultSeverity": "Blocker",
16+
"ruleSpecification": "RSPEC-8389",
17+
"sqKey": "S8389",
18+
"scope": "Main",
19+
"quickfix": "unknown",
20+
"code": {
21+
"impacts": {
22+
"SECURITY": "BLOCKER",
23+
"RELIABILITY": "BLOCKER",
24+
"MAINTAINABILITY": "HIGH"
25+
},
26+
"attribute": "CONVENTIONAL"
27+
}
28+
}

‎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
@@ -309,6 +309,7 @@
309309
"S7945",
310310
"S8375",
311311
"S8392",
312+
"S8389",
312313
"S8396"
313314
]
314315
}

0 commit comments

Comments
 (0)