search

fractal-analytics-platform / fractal-server

Fractal backend
https://fractal-analytics-platform.github.io/fractal-server/
BSD 3-Clause "New" or "Revised" License
10 stars 3 forks source link

Validate command argument for subprocess/remote-shell execution #1647

Closed tcompa closed 18 hours ago

tcompa commented 2 months ago

A couple of examples are the FractalSSH.remove_folder method https://github.com/fractal-analytics-platform/fractal-server/blob/05474dbf6b18848b893b994c50ed9d2e6777bba8/fractal_server/ssh/_fabric.py#L291-L308 or the following TaskCollectCustomV2 validator: https://github.com/fractal-analytics-platform/fractal-server/blob/fd3503ccdcffd429391fffb4f728ae4fca0b9998/fractal_server/app/schemas/v2/task_collection.py#L146-L155

Rather than re-defining this validation layer in multiple places, we should have a single validation function similar to these examples, and apply it every time we call e.g. subprocess.run or fabric.Connection.run.

tcompa commented 1 month ago

Ref https://www.oreilly.com/library/view/learning-the-bash/1565923472/ch01s09.html

tcompa commented 1 month ago

The new function validate_cmd is to be placed in https://github.com/fractal-analytics-platform/fractal-server/blob/main/fractal_server/string_tools.py, and used in front of any subprocess.run or FractalSSH.run_command call.

tcompa commented 1 month ago

If we move https://github.com/fractal-analytics-platform/fractal-server/blob/main/fractal_server/app/runner/run_subprocess.py up one level, we could include validate_cmd in this wrapper and then try to always use the wrapper rather than the raw function.

tcompa commented 2 weeks ago