Add stronger validation for storage path on upload complete endpoints #2047
Conversation
server/util/file.py
Outdated
| def validate_and_get_standard_file_upload_request_params( | ||
| request: Request, | ||
| validate_storage_path: Callable[[str], None], | ||
| validate_file_type: Callable[[str], None], |
There was a problem hiding this comment.
As a reader, it's a little complicated to grok the call sites of this function. It seems like they are mostly doing the same thing, so I wonder if this function could have the following signature:
def validate_and_get_standard_file_upload_request_params(
request: Request,
expected_file_directory_path: str,
expected_file_name_prefix: str,
expected_file_extension: str | List[str]
expected_file_type: str | List[str]
)
Also, I'm realizing that file types and file extensions are in a predefined mapping, so maybe we could pull that out into a constant and just pass in one of them.
carolinemodic
force-pushed
the
caro/add_validation_storage_path
branch
2 times, most recently
from
75c72c2 to
fde2835
Compare
server/api/batch_inventory.py
Outdated
| validate_zip_mimetype(file_type) | ||
| else: | ||
| expected_file_types = [FileType.ZIP] | ||
| elif batch_inventory_data.system_type != CvrFileType.DOMINION: |
There was a problem hiding this comment.
It's a little confusing that the dominion case is set as a default at the top and then checked as the last case in the if/else chain. Now that this is just a mapping from CvrFileType to List[FileType], maybe we could just use a dict?
| @@ -316,7 +316,7 @@ def test_batch_inventory_happy_path( | |||
| json.loads(rv.data), | |||
| { | |||
| "file": { | |||
| "name": asserts_startswith("batchTallies"), | |||
| "name": asserts_startswith("batch_tallies"), | |||
There was a problem hiding this comment.
Nit: should we be consistent with file prefixes using either hyphen or underscore word separators? I noticed there's a mix
carolinemodic
force-pushed
the
caro/add_validation_storage_path
branch
from
fde2835 to
2031ffc
Compare
#2039
Validates that the storage path passed to upload-complete is the expected path before saving.