Conversation
mudit-eng
changed the title
| if not name: | ||
| raise Exception("Please provide a model name for the deployment") | ||
| if '/' in name or '\\' in name or '..' in name: # Path traversal protection | ||
| raise Exception("Path traversal is not allowed in model's name: {}".format(name)) |
There was a problem hiding this comment.
A more commonly used is f"Path traversal is not allowed in model's name: {name}"
There was a problem hiding this comment.
I will change it. I just followed the convention in this file where .format is used.
There was a problem hiding this comment.
I see. In this file it's fine. I've seen the other way more commonly used in our code.
| filecmp.cmp(config_path, "./models/onnx_model_with_files/config.pbtxt") | ||
| ) | ||
|
|
||
| def test_path_traversal_model_name(self): |
There was a problem hiding this comment.
| def test_path_traversal_model_name(self): | |
| def test_invalid_path_traversal_model_name(self): |
| model_uri = "models:/onnx_model_with_files/1" | ||
|
|
||
| model_name_normal = "onnx_model_123" | ||
| self.client_.create_deployment(model_name_normal, model_uri, flavor="onnx") |
There was a problem hiding this comment.
Why you creating then deleting a deployment?
There was a problem hiding this comment.
Earlier, my unit test was failing. I wanted to rule out a potential resource leak, so I added a delete step after a successful create. I don't think it is needed.
| def _validate_model_name(self, name): | ||
| if not name: | ||
| raise Exception("Please provide a model name for the deployment") | ||
| if '/' in name or '\\' in name or '..' in name: # Path traversal protection |
There was a problem hiding this comment.
This is for path traversal in Windows. Since we don't support Windows, I will remove it.
There was a problem hiding this comment.
What's double backsplash \\ on windows?
There was a problem hiding this comment.
This translates to backslash traversal. Something like this: C:\Windows\System32
There was a problem hiding this comment.
Got it. The first backslash is the escape character.
2 participants
This code change sanitizes the model name in the MLflow-Triton deployment plugin to prevent path traversal.
Added a unit test and verified locally.
Gitlab test: https://gitlab-master.nvidia.com/dl/dgx/tritonserver/-/pipelines/44321248
TRI-665