firecrest icon indicating copy to clipboard operation
firecrest copied to clipboard

`/utilities/upload` can modify the requested filename (Very problematic 😅)

Open chrisjsewell opened this issue 2 years ago • 3 comments
trafficstars

During the upload operation, a function secure_filename is applied to the filename: https://github.com/eth-cscs/firecrest/blob/7f02d11b224e4faee7f4a3b35211acb9c1cc2c6a/src/utilities/utilities.py#L372 (this function: https://github.com/pallets/werkzeug/blob/417268cb0ff2ecf8da29f80d542b0b10c97bab01/src/werkzeug/utils.py#L194)

This is really problematic because, among other things, it will strip _ and . from the ends of the filename. In AiiDA, for example, we use _aiidasubmit.sh for all SLURM submission scripts, which now gets copied as aiidasubmit.sh, and thus everything fails because it cannot find the file 😭

Obviously, I can understand if you want to have security checks, but especially stripping _ seems very unnecessary?

I would probably expect at least that the upload simply failed, if the "secure" filename was different from the original filename, otherwise it leads to very unexpected (and difficult to debug) outcomes

chrisjsewell avatar Jun 28 '23 23:06 chrisjsewell

Note there was an issue opened on this _ stripping, but its not very informative 🤷 : https://github.com/pallets/werkzeug/issues/1398

chrisjsewell avatar Jun 28 '23 23:06 chrisjsewell

Note also, this secure_filename function is used in a few other places

chrisjsewell avatar Jun 29 '23 00:06 chrisjsewell

Just for the record: this wired :smile: issue is fixed now. I think we can close here.

khsrali avatar Jul 31 '24 09:07 khsrali