Skip to content

Commit dbb064c

Browse files
authored
fix: path traversal in logs endpoint (#18462)
## Summary & Motivation Its currently possible to read arbitrary files with the `/logs` endpoint in `dagster-webserver`. The only restriction is that the filename must contain a "dot". For example, this cURL invocation allows me to read my `.bash_history` file when running `dagster-webserver` locally: ``` curl -vvv -X GET 'http://localhost:3333/logs/%2e%2e/%2e%2e/%2e%2e/%2e%2e//bash_history' ``` The proposed fix calls `os.path.abspath` [1] on paths constructed in`get_captured_local_path`. If the normalized path does not start with the logs dir, a `ValueError` is raised. [1] https://docs.python.org/3/library/os.path.html#os.path.abspath ## How I Tested These Changes See added unit tests
1 parent 0f61971 commit dbb064c

File tree

2 files changed

+15
-1
lines changed

2 files changed

+15
-1
lines changed

python_modules/dagster-webserver/dagster_webserver_tests/webserver/test_app.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,3 +289,13 @@ def test_async(test_client: TestClient):
289289
result = response.json()
290290
assert result["data"]["test"]["one"] == "slept", result
291291
assert result["data"]["test"]["two"] == "slept concurrently", result
292+
293+
294+
def test_download_captured_logs_not_found(test_client: TestClient):
295+
response = test_client.get("/logs/does-not-exist/stdout")
296+
assert response.status_code == 404
297+
298+
299+
def test_download_captured_logs_invalid_path(test_client: TestClient):
300+
with pytest.raises(ValueError, match="Invalid path"):
301+
test_client.get("/logs/%2e%2e/secret/txt")

python_modules/dagster/dagster/_core/storage/local_compute_log_manager.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,11 @@ def get_captured_local_path(self, log_key: Sequence[str], extension: str, partia
226226
filename = f"{filename}.partial"
227227
if len(filename) > MAX_FILENAME_LENGTH:
228228
filename = "{}.{}".format(hashlib.md5(filebase.encode("utf-8")).hexdigest(), extension)
229-
return os.path.join(self._base_dir, *namespace, filename)
229+
location = os.path.join(self._base_dir, *namespace, filename)
230+
location = os.path.abspath(location)
231+
if not location.startswith(self._base_dir):
232+
raise ValueError("Invalid path")
233+
return location
230234

231235
def subscribe(
232236
self, log_key: Sequence[str], cursor: Optional[str] = None

0 commit comments

Comments
 (0)