Claude/fix counter reset bug jw2 o3#5
Open
daler91 wants to merge 69 commits into
Open
Conversation
Added `pytest` to `requirements.txt` to include test dependencies and allow running `pytest tests/` successfully. Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
🎯 What: Replaced lxml.etree.parse with defusedxml.lxml.parse in xml-validator.py to prevent XML External Entity (XXE) vulnerabilities. Added defusedxml and lxml to requirements.txt.⚠️ Risk: If left unfixed, the application could be vulnerable to XXE attacks when parsing malicious XML or XSD files, potentially leading to unauthorized data disclosure or denial of service. 🛡️ Solution: defusedxml acts as a drop-in replacement that strictly disables external entity resolution by default, successfully mitigating the XXE attack vector while maintaining compatibility with lxml.etree.XMLSchema and validation. Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…2842002 🔒 Fix XXE vulnerability in xml-validator
…matting-2048886441696389176 🧪 Add Error Path Tests for Date Formatting in src/data_cleaning.py
…99421084779733224 🧪 Add unit tests for Data Validation module
…2181123944 🧪 Add tests for BaseConverter to verify ABC behavior
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…13268053583850 🧪 Add test for untestable exception block in xml-validator.py
…41166032 chore: Add pytest to requirements.txt
🎯 What: Fixed an XML External Entity (XXE) vulnerability in `src/xml-validator.py` caused by using `lxml.etree.parse` without disabling external entity resolution.⚠️ Risk: When parsing user-provided XML files, `lxml` default configuration resolves external entities. This allows attackers to define malicious external entities (e.g., local files via `file://`) that get included in the parsed XML, leading to arbitrary file disclosure (Local File Inclusion), Server-Side Request Forgery (SSRF), or Denial of Service (Billion Laughs attack). 🛡️ Solution: Created a secure `etree.XMLParser` with `resolve_entities=False` and passed it to the `etree.parse` calls for both the XML document and the XSD schema document. This prevents the parser from resolving external entities, neutralizing the XXE threat. Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…ers-1942838300665811679 Refactor Address and Phone logic in CounselingConverter
…cs-4508032247991148993 🧪 Add test for _calculate_demographics in TrainingConverter
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…s-12243694426949791705 🧪 Add comprehensive tests for clean_percentage
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…-4221162114748354920 🧪 Add tests for clean_numeric in data_cleaning.py
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
🔒 Fix XXE vulnerability in xml-validator.py
…930982119168753 🧹 [code health improvement] Remove unused sys import in xml-validator.py
…-10558645891496680352 🧹 [remove unused datetime import]
…4193786634700570222
…15564282990125268104 🧹 [code health improvement] Remove unused import clean_percentage
…rcentage-4193786634700570222 🧹 [code health improvement] Remove unused import `clean_percentage` in `src/data_validation.py`
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…58699535 🧹 [code health improvement] Remove unused import validate_against_xsd
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…1811653190035510 🧹 [refactor] Extract helper functions from main in fix-sba-xml.py
…section to reduce complexity Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…-record-9040510237882545055 🧹 [code health improvement] Refactor build_training_counselor_record_section to reduce complexity
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…9308996156551803 Fix element ordering and generation to strictly match SBA XML schemas
Extracted separate sections into well-named helper methods to improve maintainability and readability without altering the external functionality. Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…eport-6605107257311533779 🧹 [code health improvement] Refactor `generate_html_report` for improved readability
🎯 What: Extracted argument parsing and single file processing logic from the main function in xml-validator.py into separate parse_arguments and process_single_file helper functions. 💡 Why: The main function was too long and complex, handling argument parsing, logging setup, and routing logic for both directory and single-file processing. This refactoring improves maintainability and readability by separating concerns. ✅ Verification: Ran the existing test suite (python3 -m unittest discover tests) which all passed successfully. Also verified the script's help command functions as expected. Pylint score improved significantly (+5.82). ✨ Result: The main function is now much more concise and easier to follow, delegating tasks to well-named helper functions. Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…idator-14601174951492042830 🧹 [code health improvement] Refactor main function in xml-validator.py
🎯 What: Refactored `build_client_intake_section` in `src/training_client_xml.py` by extracting its large logical blocks into smaller, well-named helper functions (e.g., `_build_race_info`, `_build_demographics_info`, etc.). 💡 Why: The original function was overly complex and too long. Breaking it into specific helper functions significantly improves the readability, maintainability, and testing capabilities of the codebase. ✅ Verification: Ensured that the newly extracted functions accept the necessary parameters (`client_intake`, `row`) and effectively mutate the parent XML tree without changing the underlying generation logic. All 53 existing unit tests pass successfully, confirming that the functionality remains identical. Added inline comments to preserve XSD requirement context. ✨ Result: The codebase is much easier to read, navigate, and manage as the `build_client_intake_section` is now a clean sequence of helper function calls. Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…66194256 🧹 [Code Health] Refactor build_client_intake_section
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
…5991067533 🔒 [security fix] Fix XXE vulnerabilities in XML validator
Co-authored-by: rdale-dev <203160809+rdale-dev@users.noreply.github.com>
🧹 [code health improvement] Standardize import logic for logging_util
The method was resetting total_records and successful_records to 0 on every call, causing the summary to always report 0 records processed. https://claude.ai/code/session_011Y5dtDv9MC8qhjbiqDdirV
Double braces {{var}} in Python f-strings produce literal {var} text.
Changed to single braces {var} for all data interpolations so actual
values are rendered. CSS double braces are kept as they correctly
produce the literal braces needed for style rules.
https://claude.ai/code/session_011Y5dtDv9MC8qhjbiqDdirV
|
Warning Gemini encountered an error creating the summary. You can try again by commenting |
In _calculate_demographics(), count_matches() and inline lookups called _get_column_value() to discover column names, which returns the cell value (e.g. "Female") instead of the column label (e.g. "Gender"). This caused all demographic counts to return 0. Extracted a resolve_column() helper that looks up the config's COLUMN_MAPPING directly to find the actual DataFrame column name. https://claude.ai/code/session_011Y5dtDv9MC8qhjbiqDdirV
ElementTree automatically escapes special characters when assigning to .text. Calling escape_xml() before create_element() caused double-escaping (e.g. & became &amp;), corrupting output XML. https://claude.ai/code/session_011Y5dtDv9MC8qhjbiqDdirV
Changed bare imports to relative imports (data_cleaning, config, xml_utils, logging_util). Also fixed importing DEFAULT_LOCATION_CODE and DEFAULT_LANGUAGE as module-level names from config when they are actually class attributes on GeneralConfig. https://claude.ai/code/session_011Y5dtDv9MC8qhjbiqDdirV
Makes clean_percentage() consistent with other cleaning functions (clean_numeric, format_date, etc.) that return safe defaults on bad input. Also fixed an int vs float bug where max(0, ...) could return an int, breaking the .is_integer() call. https://claude.ai/code/session_011Y5dtDv9MC8qhjbiqDdirV
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
2 participants
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.
No description provided.