From e780f759d05529c68f7fdfdf50491b1ffca6c984 Mon Sep 17 00:00:00 2001 From: Mark Backman Date: Mon, 4 May 2026 16:20:27 -0400 Subject: [PATCH 1/5] fix: validate download path containment in runner Resolve and contain the user-supplied filename before serving it from the runner's /files endpoint. Also raise a 404 (instead of returning None) when the downloads folder is unset, and use the resolved basename for Content-Disposition. --- src/pipecat/runner/run.py | 21 ++++++-- tests/test_runner_downloads.py | 95 ++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 5 deletions(-) create mode 100644 tests/test_runner_downloads.py diff --git a/src/pipecat/runner/run.py b/src/pipecat/runner/run.py index 4deda3b7d..f3965b79c 100644 --- a/src/pipecat/runner/run.py +++ b/src/pipecat/runner/run.py @@ -209,6 +209,17 @@ def _configure_server_app(args: argparse.Namespace): logger.warning(f"Unknown transport type: {args.transport}") +def _resolve_download_path(folder: str, filename: str) -> Path: + """Resolve a download path and ensure it stays within the downloads folder.""" + allowed_base = Path(folder).resolve() + file_path = (allowed_base / filename).resolve() + + if not file_path.is_relative_to(allowed_base): + raise HTTPException(status_code=403, detail="Access denied") + + return file_path + + def _setup_webrtc_routes(app: FastAPI, args: argparse.Namespace): """Set up WebRTC-specific routes.""" try: @@ -250,16 +261,16 @@ def _setup_webrtc_routes(app: FastAPI, args: argparse.Namespace): async def download_file(filename: str): """Handle file downloads.""" if not args.folder: - logger.warning(f"Attempting to dowload {filename}, but downloads folder not setup.") - return + logger.warning(f"Attempting to download {filename}, but downloads folder not setup.") + raise HTTPException(404) - file_path = Path(args.folder) / filename - if not os.path.exists(file_path): + file_path = _resolve_download_path(args.folder, filename) + if not file_path.exists(): raise HTTPException(404) media_type, _ = mimetypes.guess_type(file_path) - return FileResponse(path=file_path, media_type=media_type, filename=filename) + return FileResponse(path=file_path, media_type=media_type, filename=file_path.name) # Initialize the SmallWebRTC request handler small_webrtc_handler: SmallWebRTCRequestHandler = SmallWebRTCRequestHandler( diff --git a/tests/test_runner_downloads.py b/tests/test_runner_downloads.py new file mode 100644 index 000000000..4dc4fc25d --- /dev/null +++ b/tests/test_runner_downloads.py @@ -0,0 +1,95 @@ +# +# Copyright (c) 2024-2026, Daily +# +# SPDX-License-Identifier: BSD 2-Clause License +# + +import os +import tempfile +import unittest +from pathlib import Path +from types import SimpleNamespace + +from fastapi import FastAPI, HTTPException +from fastapi.testclient import TestClient + +from pipecat.runner.run import _resolve_download_path, _setup_webrtc_routes + + +class TestRunnerDownloads(unittest.TestCase): + def test_resolve_download_path_allows_files_inside_folder(self): + with tempfile.TemporaryDirectory() as tmpdir: + downloads = Path(tmpdir) / "downloads" + nested = downloads / "nested" + nested.mkdir(parents=True) + file_path = nested / "recording.txt" + file_path.write_text("session transcript") + + resolved = _resolve_download_path(str(downloads), "nested/recording.txt") + + self.assertEqual(resolved, file_path.resolve()) + + def test_resolve_download_path_blocks_parent_traversal(self): + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + downloads = root / "downloads" + downloads.mkdir() + (root / "secret.txt").write_text("secret") + + with self.assertRaises(HTTPException) as context: + _resolve_download_path(str(downloads), "../secret.txt") + + self.assertEqual(context.exception.status_code, 403) + + def test_download_file_returns_404_when_folder_not_configured(self): + app = FastAPI() + args = SimpleNamespace(folder=None, esp32=False, host="127.0.0.1") + _setup_webrtc_routes(app, args) + + response = TestClient(app).get("/files/recording.txt") + + self.assertEqual(response.status_code, 404) + + def test_resolve_download_path_blocks_decoded_encoded_slashes(self): + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + downloads = root / "media" + downloads.mkdir() + outside = root / "outside" + outside.mkdir() + (outside / "secret.txt").write_text("secret") + + with self.assertRaises(HTTPException) as context: + _resolve_download_path(str(downloads), "../outside/secret.txt") + + self.assertEqual(context.exception.status_code, 403) + + def test_resolve_download_path_blocks_absolute_paths(self): + with tempfile.TemporaryDirectory() as tmpdir: + downloads = Path(tmpdir) / "downloads" + downloads.mkdir() + + with self.assertRaises(HTTPException) as context: + _resolve_download_path(str(downloads), "/etc/passwd") + + self.assertEqual(context.exception.status_code, 403) + + @unittest.skipUnless(hasattr(os, "symlink"), "os.symlink is not available") + def test_resolve_download_path_blocks_symlink_escape(self): + with tempfile.TemporaryDirectory() as tmpdir: + root = Path(tmpdir) + downloads = root / "downloads" + outside = root / "outside" + downloads.mkdir() + outside.mkdir() + (outside / "secret.txt").write_text("secret") + + try: + os.symlink(outside, downloads / "linked") + except OSError as e: + self.skipTest(f"Unable to create symlink: {e}") + + with self.assertRaises(HTTPException) as context: + _resolve_download_path(str(downloads), "linked/secret.txt") + + self.assertEqual(context.exception.status_code, 403) From 5b087d6aebaf44c5e7f6f1b3eaab09cc61e3d36f Mon Sep 17 00:00:00 2001 From: Mark Backman Date: Mon, 4 May 2026 16:22:26 -0400 Subject: [PATCH 2/5] docs: add changelog for #4417 --- changelog/4417.security.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/4417.security.md diff --git a/changelog/4417.security.md b/changelog/4417.security.md new file mode 100644 index 000000000..a51c369df --- /dev/null +++ b/changelog/4417.security.md @@ -0,0 +1 @@ +- Fixed a path traversal issue in the development runner's `/files/{filename:path}` download endpoint. Previously, when the runner was started with `--folder`, a request like `/files/..%2F..%2Fetc%2Fpasswd` could escape the configured folder because `%2F`-encoded separators bypassed Starlette's path normalisation. The endpoint now resolves the joined path and rejects any filename that escapes the allowed base with a 403, and also returns 404 (instead of an implicit `null` 200) when `--folder` is unset. From f67e3ef0b2dbb251c572aa5177c8bf71b5b8ce9c Mon Sep 17 00:00:00 2001 From: Mark Backman Date: Mon, 4 May 2026 16:29:58 -0400 Subject: [PATCH 3/5] ci: install runner and webrtc extras for the test job --- .github/workflows/tests.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index b22d502c4..eb5414a30 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -46,8 +46,10 @@ jobs: --extra langchain \ --extra livekit \ --extra piper \ + --extra runner \ --extra sagemaker \ --extra tracing \ + --extra webrtc \ --extra websocket - name: Test with pytest From 89f10dd9a1a29f904ba6e27daf0822fe77d9629f Mon Sep 17 00:00:00 2001 From: Mark Backman Date: Mon, 4 May 2026 16:42:05 -0400 Subject: [PATCH 4/5] test: drop webrtc-dependent test, remove webrtc extra from CI --- .github/workflows/tests.yaml | 1 - tests/test_runner_downloads.py | 15 ++------------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index eb5414a30..04ff89543 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -49,7 +49,6 @@ jobs: --extra runner \ --extra sagemaker \ --extra tracing \ - --extra webrtc \ --extra websocket - name: Test with pytest diff --git a/tests/test_runner_downloads.py b/tests/test_runner_downloads.py index 4dc4fc25d..5b984dda0 100644 --- a/tests/test_runner_downloads.py +++ b/tests/test_runner_downloads.py @@ -8,12 +8,10 @@ import os import tempfile import unittest from pathlib import Path -from types import SimpleNamespace -from fastapi import FastAPI, HTTPException -from fastapi.testclient import TestClient +from fastapi import HTTPException -from pipecat.runner.run import _resolve_download_path, _setup_webrtc_routes +from pipecat.runner.run import _resolve_download_path class TestRunnerDownloads(unittest.TestCase): @@ -41,15 +39,6 @@ class TestRunnerDownloads(unittest.TestCase): self.assertEqual(context.exception.status_code, 403) - def test_download_file_returns_404_when_folder_not_configured(self): - app = FastAPI() - args = SimpleNamespace(folder=None, esp32=False, host="127.0.0.1") - _setup_webrtc_routes(app, args) - - response = TestClient(app).get("/files/recording.txt") - - self.assertEqual(response.status_code, 404) - def test_resolve_download_path_blocks_decoded_encoded_slashes(self): with tempfile.TemporaryDirectory() as tmpdir: root = Path(tmpdir) From e864d5778a9607cab6b1cc4f8fe4d36fdbc40b0a Mon Sep 17 00:00:00 2001 From: Mark Backman Date: Mon, 4 May 2026 16:44:47 -0400 Subject: [PATCH 5/5] ci: install runner extra for the coverage job --- .github/workflows/coverage.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/coverage.yaml b/.github/workflows/coverage.yaml index 26d03861b..5883afc8a 100644 --- a/.github/workflows/coverage.yaml +++ b/.github/workflows/coverage.yaml @@ -42,6 +42,7 @@ jobs: --extra langchain \ --extra livekit \ --extra piper \ + --extra runner \ --extra sagemaker \ --extra tracing \ --extra websocket