fix: 9 - add sanitization of cli inputs in launch_explorer.py - #776
Conversation
tgilon
left a comment
There was a problem hiding this comment.
Thank you @daniel-rdt for this addition! I have a few comments but would like to ask @KristijanFaust-OET if he has any recommendations. This PR improves input validation, feel free to suggest improvements.
KristijanFaust-OET
left a comment
There was a problem hiding this comment.
I left some comments which are more questions at the moment than suggestions. Besides that, do we have any unit testing which can be extended with this validation cases?
Co-authored-by: Thomas Gilon <thomas.gilon@openenergytransition.org> Co-authored-by: Daniel Rüdt <117752024+daniel-rdt@users.noreply.github.com>
# Conflicts: # doc/release_notes.rst
|
Thank you @tgilon and @KristijanFaust-OET for the review and suggestions. I made the simplifications and improvements as suggested above. Let me know if this works better for you. |
# Conflicts: # doc/release_notes.md
tgilon
left a comment
There was a problem hiding this comment.
The code looks good now. Could you please update the doc accordingly? I will then approve! Thank you @daniel-rdt
Co-authored-by: Thomas Gilon <thomas.gilon@openenergytransition.org>
Changes proposed in this Pull Request
This PR sanitizes the handling of CLI inputs passed to
launch_explorer.pywhere untrustedsys.argvvalues used to flow unchecked intologging.FileHandler(file write) andpypsa.Network(file read).All three are applied before any filesystem access; invalid input raises ValueError and aborts. The Snakemake inputs are untouched, since its values come from trusted workflow config.
Workflow
sanitize_path— allow-lists characters, rejects absolute paths and..traversal, and requires the resolved path to stay within the working directory.sanitize_port— accepts only integers in 1024–65535 to account for privileged ports and protocol limits.sanitize_input_files— validates each path, requiring an existing.ncfile.Generative AI was used to assist with code implementation of this PR. All content has been reviewed, verified, and is the responsibility of the author.
Checklist
pixi.toml(usingpixi add <dependency-name>).config/config.default.yaml.doc/configtables/*.csv.config/test/*.yaml.pixi run -e open-tyndp tyndp-cyears-test).doc/*.rstfiles.doc/release_notes.rstis added.doc/index.rst.