ruff cleanup #489
ruff cleanup #489
Conversation
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return import_rows_batch( | ||
| tally, | ||
| open(UPLOADED_FILES_PATH + file_map[file_name], "r"), | ||
| open(UPLOADED_FILES_PATH + file_map[file_name]), |
Check warning
Code scanning / CodeQL
File is not always closed Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
The best way to fix this problem is to ensure that the file is always closed after it is used, even if an exception occurs. The idiomatic way to do this in Python is to use a with statement when opening the file. In this case, the file is opened in process_batch_step and passed to import_rows_batch, which expects a file object as file_to_parse. Therefore, we should open the file using a with statement in process_batch_step, and pass the file object to import_rows_batch within the context of the with block. This ensures the file is closed as soon as the block is exited, regardless of whether an exception is raised.
What to change:
- In
process_batch_step, replace the direct call toopen(...)in the argument list with awith open(...) as f:block, and callimport_rows_batchinside the block, passingfas the argument. - Return the result of
import_rows_batchfrom within thewithblock.
No new imports or method definitions are needed.
| @@ -216,12 +216,13 @@ | ||
| """ | ||
| file_name, _, process_function = STEP_TO_ARGS[current_step] | ||
|
|
||
| return import_rows_batch( | ||
| tally, | ||
| open(UPLOADED_FILES_PATH + file_map[file_name]), | ||
| process_function, | ||
| current_step, | ||
| ) | ||
| with open(UPLOADED_FILES_PATH + file_map[file_name]) as f: | ||
| return import_rows_batch( | ||
| tally, | ||
| f, | ||
| process_function, | ||
| current_step, | ||
| ) | ||
|
|
||
|
|
||
| class DashboardView( |
| return import_rows_batch( | ||
| tally, | ||
| open(UPLOADED_FILES_PATH + file_map[file_name], "r"), | ||
| open(UPLOADED_FILES_PATH + file_map[file_name]), |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix this vulnerability, we need to ensure that any file path constructed from user input is validated before being used to access the file system. The best approach here is to normalize the constructed path using os.path.normpath and then check that the resulting path is still within the intended upload directory (UPLOADED_FILES_PATH). This prevents path traversal attacks using .. or absolute paths. The fix should be applied in the process_batch_step function, specifically before opening the file at line 221. We will also need to import the os module if it is not already imported.
| @@ -3,6 +3,7 @@ | ||
| import time | ||
|
|
||
| import duckdb | ||
| import os | ||
| from celery.result import AsyncResult | ||
| from django.conf import settings | ||
| from django.contrib import messages | ||
| @@ -216,9 +217,16 @@ | ||
| """ | ||
| file_name, _, process_function = STEP_TO_ARGS[current_step] | ||
|
|
||
| # Validate and normalize the file path to prevent path traversal | ||
| requested_file = file_map[file_name] | ||
| full_path = os.path.normpath(os.path.join(UPLOADED_FILES_PATH, requested_file)) | ||
| # Ensure the normalized path is within the upload directory | ||
| if not full_path.startswith(os.path.abspath(UPLOADED_FILES_PATH)): | ||
| raise Exception("Invalid file path") | ||
|
|
||
| return import_rows_batch( | ||
| tally, | ||
| open(UPLOADED_FILES_PATH + file_map[file_name]), | ||
| open(full_path), | ||
| process_function, | ||
| current_step, | ||
| ) |
| new_path = path_with_timestamp(path) | ||
| file_path =\ | ||
| default_storage.save(new_path, File(open(csv_file.name, mode='r'))) | ||
| default_storage.save(new_path, File(open(csv_file.name))) |
Check warning
Code scanning / CodeQL
File is not always closed Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix the problem, we should ensure that the file opened with open(csv_file.name) is always closed, even if an exception occurs. The best way to do this is to use a with statement when opening the file. This ensures that the file is closed automatically when the block is exited, regardless of whether an exception is raised. Specifically, in the save_csv_file_and_symlink function, replace File(open(csv_file.name)) with a with open(csv_file.name) as f: block, and pass File(f) to default_storage.save. The rest of the function can remain unchanged. No new imports are needed.
| @@ -34,8 +34,8 @@ | ||
|
|
||
| def save_csv_file_and_symlink(csv_file, path): | ||
| new_path = path_with_timestamp(path) | ||
| file_path =\ | ||
| default_storage.save(new_path, File(open(csv_file.name))) | ||
| with open(csv_file.name, 'rb') as f: | ||
| file_path = default_storage.save(new_path, File(f)) | ||
| new_path = default_storage.path(file_path) | ||
|
|
||
| try: |
1 participant
No description provided.