8000 [Security Issue] Simplify Filename Sanitization in `/api/download` Endpoint by shyun020 · Pull Request #1503 · ComposioHQ/composio · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Security Issue] Simplify Filename Sanitization in /api/download Endpoint #1503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: development
Choose a base branch
from
Open

[Security Issue] Simplify Filename Sanitization in /api/download Endpoint #1503

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1f2367f
Update api.py
shyun020 Mar 28, 2025
fb3df58
Merge branch 'development' into master
angrybayblade Apr 2, 2025
5fb1d76
Merge branch 'development' into master
angrybayblade Apr 2, 2025
7734559
Merge branch 'development' into master
shyun020 Apr 2, 2025
d960247
Merge branch 'development' into master
shyun020 Apr 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10000 19 changes: 17 additions & 2 deletions python/composio/server/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from functools import update_wrapper
from hashlib import md5
from pathlib import Path
import unicodedata

import typing_extensions as te
from fastapi import FastAPI, HTTPException, Request, Response
Expand All @@ -30,7 +31,6 @@
from composio.tools.local import load_local_tools
from composio.utils.logging import get as get_logger


ResponseType = t.TypeVar("ResponseType")

R = t.TypeVar("R")
Expand Down Expand Up @@ -281,7 +281,11 @@ def _download_file_or_dir(file: t.Optional[str] = None):
raise HTTPException(
status_code=400, detail="File path is required as query parameter"
)
path = Path(file)
# Apply filename sanitization to prevent path injection
safe_file = secure_filename(file)
if not safe_file:
raise HTTPException(status_code=400, detail="Invalid file path after sanitization")
path = Path(safe_file)
if not path.exists():
return Response(
content=APIResponse[None](
Expand Down Expand Up @@ -313,3 +317,14 @@ def _archive(directory: Path, output: Path) -> Path:
),
)
return output


def secure_filename(filename: str) -> str:
filename = unicodedata.normalize("NFKD", filename)
filename = filename.encode("ascii", "ignore").decode("ascii")
for sep in (os.sep, os.path.altsep):
if sep:
Comment on lines +325 to +326
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The os module is used in secure_filename() but not imported at the top of the file, which will cause a NameError when the function is called.

filename = filename.replace(sep, " ")
filename = "_".join(filename.split())
filename = filename.strip("._")
return filename
Loading
0