Skip to content

Potential fix for code scanning alert no. 5: Uncontrolled data used in path expression#17

Draft
johnsamuelwrites wants to merge 1 commit intomasterfrom
alert-autofix-5
Draft

Potential fix for code scanning alert no. 5: Uncontrolled data used in path expression#17
johnsamuelwrites wants to merge 1 commit intomasterfrom
alert-autofix-5

Conversation

@johnsamuelwrites
Copy link
Copy Markdown
Owner

Potential fix for https://github.com/johnsamuelwrites/ShExStatements/security/code-scanning/5

In general, fix uncontrolled path usage by (a) not trusting the client‑supplied filename, (b) normalizing and constraining the path to a dedicated upload directory, and/or (c) replacing the provided name with a safe server‑generated name (or using secure_filename). Here, we do not actually need the original client filename for anything other than extension detection; the content is passed via stream. The safest minimal fix is therefore to (1) ignore the user’s filename for filesystem paths, (2) generate a unique temporary filename in a safe temp directory, and (3) use that path both for open() and for remove().

Concretely:

  • In generate_shex_from_spreadsheet, import os (or tempfile) and create a temp filename using tempfile.NamedTemporaryFile(delete=False) or tempfile.mkstemp() for the cases where stream is not None. Write stream into this safe path, and use that path for load_workbook / load and for remove(). Do not concatenate "tmp" + filepath or otherwise reuse the user‑provided name in a filesystem path. The original filepath parameter is still fine to use with splitext() to determine file_extension, but not as a path.
  • Only modify code in shexstatements/shexfromspreadsheet.py within the shown method. shexstatements/application.py can stay unchanged because it just passes through the client filename; the dangerous part is using that string as a path in generate_shex_from_spreadsheet.

This preserves existing behavior (read spreadsheet from the uploaded stream, delete the temporary file afterward) while ensuring that all actual filesystem paths are generated by the server and not influenced by user input.


Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…n path expression

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant