fix: security hardening and code quality improvements#1
Merged
Conversation
- Path traversal: validate filename and realpath containment in CSV download - Thread safety: add threading.Lock around global mutable state - Connection leaks: add finally blocks to all DB methods in fabric_sql_writer and state_manager - Session leaks: add close() to all collector classes; call after collect() - Code injection: validate table_name and write_mode in PySpark payload generation - JWT handling: validate token structure before decoding payload - Input validation: add keyword count/length limits and Flask MAX_CONTENT_LENGTH - Replace ast.literal_eval with json.loads for CSV keyword parsing - Replace stderr debug prints with proper logger calls in fabric_writer - Remove unused xml.etree.ElementTree import and redundant inline json imports - Add exc_info=True to exception handlers missing stack traces - Remove overly-aggressive regex that stripped legitimate text in utils.py - Warn when FLASK_SECRET_KEY is not set instead of silently using default
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive security hardening and code quality improvements across the Python backend.
Security Fixes
realpathcontainmenttable_nameandwrite_modebefore embedding in PySpark codejson.loadsfor CSV keyword parsingFLASK_SECRET_KEYis not setMAX_CONTENT_LENGTH(16 MB)Reliability Fixes
finally: conn.close()to 8 methods acrossfabric_sql_writerandstate_managerclose()to all 6 collector classes and call it aftercollect()threading.Lockaround global mutable state (collection_status,last_collected_feedback)Code Quality
stderrdebug prints with properloggercalls infabric_writerxml.etree.ElementTreeimportimport jsonto module top-level infabric_sql_writerexc_info=Trueto exception handlers missing stack tracesutils.pyFiles Changed
src/app.py— thread lock, path traversal fix, input validation, collector close callssrc/collectors.py—close()methods, unused import removal, error loggingsrc/fabric_writer.py— input validation, debug print cleanupsrc/fabric_sql_writer.py— connection leak fixes, import cleanupsrc/state_manager.py— JWT validation, connection leak fixessrc/utils.py— regex fix