[Fixes #13936] Support for XLSX File Uploads in GeoNode #13937
Conversation
Summary of ChangesHello @Gpetrak, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances GeoNode's data upload capabilities by introducing native support for XLSX and XLS file formats. It provides a robust and secure mechanism for ingesting tabular geographic data from Excel spreadsheets, ensuring data integrity through strict validation of headers and coordinates, and integrating seamlessly with the existing upload orchestration. The feature is also made configurable via an environment variable, offering flexibility for deployment. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| except Exception as e: | ||
| logger.exception("XLSX Pre-processing failed") | ||
| raise InvalidInputFileException(detail=f"Failed to securely parse Excel: {str(e)}") |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 15 hours ago
In general, to fix this kind of issue you should avoid including raw exception text, stack traces, or other internal details in messages that may be returned to end users. Instead, log the detailed exception server-side and provide a generic, non-sensitive error message to the user.
For this specific code, the best targeted fix is to change the InvalidInputFileException raised in the except block of pre_processing so that it no longer embeds str(e) in the detail. We can keep the logging line logger.exception("XLSX Pre-processing failed") so that developers still have full diagnostic information. The user-facing message can be something like "Failed to securely parse Excel file." or similar, without string interpolation from e.
Concretely, in geonode/upload/handlers/xlsx/handler.py, within the pre_processing method’s except Exception as e: block (around lines 222–224), replace:
except Exception as e:
logger.exception("XLSX Pre-processing failed")
raise InvalidInputFileException(detail=f"Failed to securely parse Excel: {str(e)}")with:
except Exception as e:
logger.exception("XLSX Pre-processing failed")
raise InvalidInputFileException(
detail="Failed to securely parse Excel file. Please verify the file format and contents."
)This preserves behavior (raising the same exception type in the same place) while removing the exposure of the underlying exception text.
No new imports or additional helper methods are required.
| @@ -221,7 +221,9 @@ | ||
|
|
||
| except Exception as e: | ||
| logger.exception("XLSX Pre-processing failed") | ||
| raise InvalidInputFileException(detail=f"Failed to securely parse Excel: {str(e)}") | ||
| raise InvalidInputFileException( | ||
| detail="Failed to securely parse Excel file. Please verify the file format and contents." | ||
| ) | ||
|
|
||
| # update the file path in the payload | ||
| _data["files"]["base_file"] = output_file |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for uploading XLSX and XLS files by converting them to CSV during a pre-processing step and then utilizing the existing CSV handler pipeline. While the implementation includes some security considerations, a critical command injection vulnerability was identified in the ogr2ogr command construction and execution flow. This vulnerability could allow an authenticated attacker to achieve remote code execution by uploading a specially crafted XLSX file, and remediation is required to ensure all user-supplied data is properly sanitized before being used in shell commands. Furthermore, a critical issue was found in the is_valid method that incorrectly attempts to validate an XLSX file using a CSV driver, which would block all uploads of this type. There are also several medium-severity recommendations to improve error handling by using more specific exception types instead of generic ones, which will enhance maintainability and provide clearer feedback to users.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #13937 +/- ##
========================================
Coverage 74.19% 74.20%
========================================
Files 944 950 +6
Lines 56468 56803 +335
Branches 7651 7716 +65
========================================
+ Hits 41899 42153 +254
- Misses 12885 12951 +66
- Partials 1684 1699 +15 🚀 New features to boost your workflow:
|
mattiagiupponi
requested review from
sijandh35
and removed request for
giohappy
2 participants
This PR was created accordiding to this issue: #13936
Checklist
For all pull requests:
The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):
Submitting the PR does not require you to check all items, but by the time it gets merged, they should be either satisfied or inapplicable.